Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability

[David Gibson]

On Tue, Dec 12, 2017 at 05:28:33PM +1100, Alexey Kardashevskiy wrote:
> On 12/12/17 16:54, David Gibson wrote:
> > On Tue, Dec 12, 2017 at 03:06:33PM +1100, Alexey Kardashevskiy wrote:
> >> On 12/12/17 14:40, David Gibson wrote:
> >>> On Tue, Dec 12, 2017 at 02:28:33PM +1100, Alexey Kardashevskiy wrote:
> >>>> On 12/12/17 11:20, David Gibson wrote:
> >>>>> On Mon, Dec 11, 2017 at 08:20:30PM +1100, Alexey Kardashevskiy wrote:
> >>>>>> On 11/12/17 18:08, David Gibson wrote:
[snip]
> >>> No, if the feature is on, _allow() makes sure it can be implemented
> >>> with the cpu/accel, _enforce() does nothing.
> >>>
> >>> If the feature os off, _allow() does nothing, _enforce makes a best
> >>> effort to prevent it from working.
> >>
> >> Ah. _allow() can only enable, _enforce() can only disable. ok.
> >>
> >> This is what I meant elsewhere - the "enforce" word sounds to me (one bad
> >> engligh speaker :) ) more like "enable" (forcefully) than "disable".
> >> "forbid" seems better but I do not insist.
> > 
> > Right, but I wanted to avoid "forbid" because I was already using it
> > for forbidden_caps which has different semantics.  forbidden_caps is
> > those caps the _user_ says shouldn't be there 
> 
> Sounds like a plain "disabled" to me :)

Not really, because there can be features that aren't turned on in the
defaults (either machine or cpu).  Those will also be "disabled" even
though they haven't been selected as such by the user.

> > and are therefore
> > excluded from the "effective caps" regardless of other defaults.
> > "enforce" is about taking the effective caps and matching them to
> > actual hardware capabilities.  i.e. they're operating at different
> > levels.  Maybe this makes it clearer:
> > 
> > USER                            QEMU STATE                    HARDWARE
> > 
> >                   caps_reset()                 allow/enforce
> > forced_caps       ----------->  effective_caps ------------>  HFSCR
> > forbidden_caps         ^                             ^        KVM caps
> >                        |                             |        ...
> >          (defaults) ---/                             |
> >                                    (hw abilities) ---/
> > 
> >>>> It just looks nicer to have a single place which enables or disables a
> >>>> capability. Dunno...
> >>>
> >>> I could put both the allowing and enforcing in one function, say
> >>> spapr_caps_apply().  But then I'd need an alternative mechanism for
> >>> reporting fatal vs. non-fatal errors.  Maybe worth it, I'll have a
> >>> look.
> >>
> >> And now it is assumed that all failures in _allow() are non-fatal?
> > 
> > No, failures in _allow() are always fatal.  Failures in _enforce() aren't.
> 
> 
> Ah. Ok. So if we cannot disable TM in KVM, we print a message and continue?
> And we only fail if the user specifically asked for something and we could
> not enable it? What about preselecting 2.06-compat - should not _enforce()
> fail fatally if it fails to disable TM?

No.  Note that if a feature is disabled we will never advertise it to
the guest.  enforce is just about making it actually generate 0x700s
if the guest tries to use it.

I don't believe that should be fatal, since in some cases it's awkward
or impossible to actually generate 700s.  For example, right now host
kernels don't have a way to turn off HTM.  We don't want new qemus to
generate fatal errors until we have a kernel that allows this.

I a guest goes ahead and attempts to use a feature it was told not to,
it doesn't get to complain about it being weirdly broken and blowing
up on migration.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature