Re: Semantics of "-cpu host" (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest)

[Eduardo Habkost <ehabkost@redhat.com>]

On Tue, May 08, 2012 at 02:58:11AM +0200, Alexander Graf wrote:
> On 07.05.2012, at 20:21, Eduardo Habkost wrote:
> 
> > 
> > Andre? Are you able to help to answer the question below?
> > 
> > I would like to clarify what's the expected behavior of "-cpu host" to
> > be able to continue working on it. I believe the code will need to be
> > fixed on either case, but first we need to figure out what are the
> > expectations/requirements, to know _which_ changes will be needed.
> > 
> > 
> > On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote:
> >> (CCing Andre Przywara, in case he can help to clarify what's the
> >> expected meaning of "-cpu host")
> >> 
> > [...]
> >> I am not sure I understand what you are proposing. Let me explain the
> >> use case I am thinking about:
> >> 
> >> - Feature FOO is of type (A) (e.g. just a new instruction set that
> >>  doesn't require additional userspace support)
> >> - User has a Qemu vesion that doesn't know anything about feature FOO
> >> - User gets a new CPU that supports feature FOO
> >> - User gets a new kernel that supports feature FOO (i.e. has FOO in
> >>  GET_SUPPORTED_CPUID)
> >> - User does _not_ upgrade Qemu.
> >> - User expects to get feature FOO enabled if using "-cpu host", without
> >>  upgrading Qemu.
> >> 
> >> The problem here is: to support the above use-case, userspace need a
> >> probing mechanism that can differentiate _new_ (previously unknown)
> >> features that are in group (A) (safe to blindly enable) from features
> >> that are in group (B) (that can't be enabled without an userspace
> >> upgrade).
> >> 
> >> In short, it becomes a problem if we consider the following case:
> >> 
> >> - Feature BAR is of type (B) (it can't be enabled without extra
> >>  userspace support)
> >> - User has a Qemu version that doesn't know anything about feature BAR
> >> - User gets a new CPU that supports feature BAR
> >> - User gets a new kernel that supports feature BAR (i.e. has BAR in
> >>  GET_SUPPORTED_CPUID)
> >> - User does _not_ upgrade Qemu.
> >> - User simply shouldn't get feature BAR enabled, even if using "-cpu
> >>  host", otherwise Qemu would break.
> >> 
> >> If userspace always limited itself to features it knows about, it would
> >> be really easy to implement the feature without any new probing
> >> mechanism from the kernel. But that's not how I think users expect "-cpu
> >> host" to work. Maybe I am wrong, I don't know. I am CCing Andre, who
> >> introduced the "-cpu host" feature, in case he can explain what's the
> >> expected semantics on the cases above.
> 
> Can you think of any feature that'd go into category B?

- TSC-deadline: can't be enabled unless userspace takes care to enable
  the in-kernel irqchip.
- x2apic: ditto.

I am not sure about XSAVE: an old qemu version would call kvm_put_fpu()
instead of kvm_put_xsave() on kvm_arch_put_registers(), but I don't know
if this would have unexpected side-effects or not.

I wouldn't be surprised if we find many other cases, as even the
GET_SUPPORTED_CPUID documentation is explicit about that: "Userspace can
use the information returned by this ioctl [GET_SUPPORTED_CPUID] to
construct cpuid information (for KVM_SET_CPUID2) that is consistent with
hardware, kernel, and userspace capabilities, [...]"
                      ^^^^^^^^^^^^^^^^^^^^^^

> 
> All features I'm aware of work fine (without migration, but that one
> is moot for -cpu host anyway) as long as the host kvm implementation
> is fine with it (GET_SUPPORTED_CPUID). So they'd be category A.

So, you would argue that GET_SUPPORTED_CPUID should include only
features of type A? That's the opposite of what we have today.

Maybe we could change the semantics to type-A-only if we define "type A"
as:

- Don't require any extra userspace support except for:
  - Migration support.
  - Enabling the in-kernel irqchip.

If we agree on that semantics, "-cpu host" could safely enable all the
fetures returned by GET_SUPPORTED_CPUID blindly, after making sure that
migration support is disabled and the in-kernel irqchip is enabled. Then
all type-B features will require defining a KVM_CAP_* capability instead
of using GET_SUPPORTED_CPUID. It's the opposite of the direction I was
proposing earlier in this thread, but it is starting to look like a
better idea (otherwise "-cpu host" would never be reliable).

If we agree on that semantics, it won't require any code change on the
current code, just a documentation update.

-- 
Eduardo