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

[Alexander Graf]

On 08.05.2012, at 22:14, Eduardo Habkost wrote:

> 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.

The kernel can check if in-kernel irqchip has it enabled and otherwise mask it 
out, no?

> - x2apic: ditto.

Same here. For user space irqchip the kernel side doesn't care. If in-kernel 
APIC is enabled, check for its capabilities.

> 
> 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.

Then XSAVE awareness should be manually enabled by user space. That's what we 
have ENABLE_CAP for :). Do ENABLE_CAP(XSAVE) -> get XSAVE as a bit in 
GET_SUPPORTED_CPUID.

> 
> 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, [...]"
>                      ^^^^^^^^^^^^^^^^^^^^^^

Yeah, so the intent is that kvm is aware of all the bits user space would care 
about.

> 
>> 
>> 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

not instead. In addition. Define a KVM_CAP_ and do an ENABLE_CAP on that one to 
have it exposed.

> 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.

Life is simple, eh? :)


Alex