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

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

[Eduardo Habkost]

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

-- 
Eduardo

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

[Alexander Graf]

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?

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.


Alex

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

[Eduardo Habkost]

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

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

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

[Gleb Natapov]

On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
> 
> 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?
> 
How kernel should know that userspace does not emulate it?

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

Well, technically both of those features can't be implemented in
userspace right now since MSRs are terminated in the kernel, but I
wouldn't make it into ABI.


--
			Gleb.

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

[Alexander Graf]

On 09.05.2012, at 10:14, Gleb Natapov <gleb@redhat.com> wrote:

> On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
>> 
>> 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?
>> 
> How kernel should know that userspace does not emulate it?

You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right?

> 
>>> - x2apic: ditto.
>> 
>> Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities.
>> 
> Same here.
> 
> Well, technically both of those features can't be implemented in
> userspace right now since MSRs are terminated in the kernel, but I

Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case.

> wouldn't make it into ABI.
> 
> 
> --
>            Gleb.

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

[Gleb Natapov]

On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote:
> 
> 
> On 09.05.2012, at 10:14, Gleb Natapov <gleb@redhat.com> wrote:
> 
> > On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
> >> 
> >> 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?
> >> 
> > How kernel should know that userspace does not emulate it?
> 
> You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right?
> 
> > 
> >>> - x2apic: ditto.
> >> 
> >> Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities.
> >> 
> > Same here.
> > 
> > Well, technically both of those features can't be implemented in
> > userspace right now since MSRs are terminated in the kernel, but I
> 
> Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case.
> 
You mean terminating MSRs in kernel does not sound like the greatest
design? I do not disagree. That is why IMO kernel can't filter out
TSC-deadline and x2apic like you suggest.

> > wouldn't make it into ABI.
> > 
> > 
> > --
> >            Gleb.

--
			Gleb.

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

[Alexander Graf]

On 09.05.2012, at 10:51, Gleb Natapov wrote:

> On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote:
>> 
>> 
>> On 09.05.2012, at 10:14, Gleb Natapov <gleb@redhat.com> wrote:
>> 
>>> On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
>>>> 
>>>> 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?
>>>> 
>>> How kernel should know that userspace does not emulate it?
>> 
>> You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right?
>> 
>>> 
>>>>> - x2apic: ditto.
>>>> 
>>>> Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities.
>>>> 
>>> Same here.
>>> 
>>> Well, technically both of those features can't be implemented in
>>> userspace right now since MSRs are terminated in the kernel, but I
>> 
>> Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case.
>> 
> You mean terminating MSRs in kernel does not sound like the greatest
> design? I do not disagree. That is why IMO kernel can't filter out
> TSC-deadline and x2apic like you suggest.

I still don't see why it can't.

Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet.
Now, we implement TSC-deadline in the kernel. We still filter TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide an interface to user space that says "call me to enable TSC-deadline CPUID, but only if you're using the in-kernel apic"
New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic.
Old user space doesn't call that ioctl.

So at the end all bits in GET_SUPPORTED_CPUID are consistent with what user space is capable of.


Alex

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

[Gleb Natapov]

On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote:
> 
> On 09.05.2012, at 10:51, Gleb Natapov wrote:
> 
> > On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote:
> >> 
> >> 
> >> On 09.05.2012, at 10:14, Gleb Natapov <gleb@redhat.com> wrote:
> >> 
> >>> On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
> >>>> 
> >>>> 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?
> >>>> 
> >>> How kernel should know that userspace does not emulate it?
> >> 
> >> You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right?
> >> 
> >>> 
> >>>>> - x2apic: ditto.
> >>>> 
> >>>> Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities.
> >>>> 
> >>> Same here.
> >>> 
> >>> Well, technically both of those features can't be implemented in
> >>> userspace right now since MSRs are terminated in the kernel, but I
> >> 
> >> Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case.
> >> 
> > You mean terminating MSRs in kernel does not sound like the greatest
> > design? I do not disagree. That is why IMO kernel can't filter out
> > TSC-deadline and x2apic like you suggest.
> 
> I still don't see why it can't.
> 
> Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet.
> Now, we implement TSC-deadline in the kernel. We still filter TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide an interface to user space that says "call me to enable TSC-deadline CPUID, but only if you're using the in-kernel apic"
> New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic.
> Old user space doesn't call that ioctl.
First of all we already have TSC-deadline in GET_SUPORTED_CPUID without
any additional ioctls. And second I do not see why we need additional
iostls here.

Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and
x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not
from KVM_SET_CPUID? For those two features it may make sense indeed. Not
sure there won't be others that are not dependent on irq chip presence.
You propose to add additional ioctls to enable them if they appear?

> 
> So at the end all bits in GET_SUPPORTED_CPUID are consistent with what user space is capable of.
> 
GET_SUPPORTED_CPUID should not be necessary consistent with what user
space is capable of. Userspace may emulate features that are not in
GET_SUPPORTED_CPUID.

--
			Gleb.

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

[Alexander Graf]

On 05/09/2012 11:38 AM, Gleb Natapov wrote:
> On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote:
>> On 09.05.2012, at 10:51, Gleb Natapov wrote:
>>
>>> On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote:
>>>>
>>>> On 09.05.2012, at 10:14, Gleb Natapov<gleb@redhat.com>  wrote:
>>>>
>>>>> On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
>>>>>> 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?
>>>>>>
>>>>> How kernel should know that userspace does not emulate it?
>>>> You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right?
>>>>
>>>>>>> - x2apic: ditto.
>>>>>> Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities.
>>>>>>
>>>>> Same here.
>>>>>
>>>>> Well, technically both of those features can't be implemented in
>>>>> userspace right now since MSRs are terminated in the kernel, but I
>>>> Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case.
>>>>
>>> You mean terminating MSRs in kernel does not sound like the greatest
>>> design? I do not disagree. That is why IMO kernel can't filter out
>>> TSC-deadline and x2apic like you suggest.
>> I still don't see why it can't.
>>
>> Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet.
>> Now, we implement TSC-deadline in the kernel. We still filter TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide an interface to user space that says "call me to enable TSC-deadline CPUID, but only if you're using the in-kernel apic"
>> New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic.
>> Old user space doesn't call that ioctl.
> First of all we already have TSC-deadline in GET_SUPORTED_CPUID without
> any additional ioctls. And second I do not see why we need additional
> iostls here.

Yeah, some times our ABI is already broken :(. What a shame...

> Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and
> x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not
> from KVM_SET_CPUID? For those two features it may make sense indeed. Not
> sure there won't be others that are not dependent on irq chip presence.
> You propose to add additional ioctls to enable them if they appear?

That's the only backwards compatible way to design this without putting 
a plethora of knowledge into user space I can see, yes.

>
>> So at the end all bits in GET_SUPPORTED_CPUID are consistent with what user space is capable of.
>>
> GET_SUPPORTED_CPUID should not be necessary consistent with what user
> space is capable of. Userspace may emulate features that are not in
> GET_SUPPORTED_CPUID.

If it does, it can OR them in. GET_SUPPORTED_CPUID should return "these 
are the bits that your currently configured kvm is capable of".


Alex

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

[Eduardo Habkost]

On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote:
> On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote:
> > 
> > On 09.05.2012, at 10:51, Gleb Natapov wrote:
> > 
> > > On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote:
> > >> 
> > >> 
> > >> On 09.05.2012, at 10:14, Gleb Natapov <gleb@redhat.com> wrote:
> > >> 
> > >>> On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
> > >>>> 
> > >>>> 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?
> > >>>> 
> > >>> How kernel should know that userspace does not emulate it?
> > >> 
> > >> You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right?
> > >> 
> > >>> 
> > >>>>> - x2apic: ditto.
> > >>>> 
> > >>>> Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities.
> > >>>> 
> > >>> Same here.
> > >>> 
> > >>> Well, technically both of those features can't be implemented in
> > >>> userspace right now since MSRs are terminated in the kernel, but I
> > >> 
> > >> Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case.
> > >> 
> > > You mean terminating MSRs in kernel does not sound like the greatest
> > > design? I do not disagree. That is why IMO kernel can't filter out
> > > TSC-deadline and x2apic like you suggest.
> > 
> > I still don't see why it can't.
> > 
> > Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet.
> > Now, we implement TSC-deadline in the kernel. We still filter
> > TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide
> > an interface to user space that says "call me to enable TSC-deadline
> > CPUID, but only if you're using the in-kernel apic"

We have that interface already, it is called KVM_SET_CPUID.  :-)

> > New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic.
> > Old user space doesn't call that ioctl.
> First of all we already have TSC-deadline in GET_SUPORTED_CPUID without
> any additional ioctls. And second I do not see why we need additional
> iostls here.

We don't have TSC-deadline set today (and that's what started this
thread), but we have x2apic. So what you say is true for x2apic, anyway.

> Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and
> x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not
> from KVM_SET_CPUID? For those two features it may make sense indeed.

It makes sense to me.

It looks like my assumptions were wrong. They were:

- GET_SUPPORTED_CPUID simply can't know if the in-kernel irqchip is
  going to be enabled or not.
- GET_SUPPORTED_CPUID output has to be a function of the kernel code
  capabilitie and host CPU, and not depend on any input from userspace.

Are those assumptions incorrect? If we break them, we may try what
Alexander is proposing. It would be much more flexible than the options
I was considering.

I didn't know ENABLE_CAP existed. Even if GET_SUPPORTED_CPUID can't
check for the in-kernel irqchip setup for some reason, ENABLE_CAP could
be used by userpace to tell the kernel "I will enable the in-kernel
irqchip, so feel free to return features that depend on it on
GET_SUPPORTED_CPUID".

In other words, we would return only the type-A features on
GET_SUPPORTED_CPUID (i.e. safe to be blindly enabled by -cpu host as
long as migration is not required), but if we use ENABLE_CAP we can make
group A safely grow, as long as userspace first tells the kernel what it
supports.

Anybody is against doing that? Otherwise I plan to work on this.
Probably I will start by making GET_SUPPORTED_CPUID not return x2apic
unless userspace tells the kernel (using ENABLE_CAP) that it will enable
the in-kernel irqchip. Then we can do the same with TSC-deadline.

Note that all this work is to allow the kernel to let userspace blindly
enable features it _doesn't know yet_. If we limit ourselves to features
userspace already knows about, we could simply remove x2apic and
TSC-deadline from GET_SUPPORTED_CPUID completely, not use ENABLE_CAP for
that, and let userspace set x2apic or TSC-deadline on KVM_SET_CPUID only
if it knows it is safe (either because it checked for the corresponding
KVM_CAP_* capability is present and it will enable the in-kernel
irqchip, or because it will emulated it in userspace).

In case anybody is against the proposal above: note that the current
documented GET_SUPPORTED_CPUID semantics (unconditionally returning bits
that depend on specific userspace behavior/capabilities) is simply
unusable by "-cpu host". If the above proposal gets rejected, my Plan B
is to update the GET_SUPPORTED_CPUID documentation to note that it
returns only type-A features (features that userspace can safely enable
even if it doesn't know what it does), remove x2apic from
GET_SUPPORTED_CPUID forever, and use KVM_CAP_* for discovery of all
type-B features (features that depend on specific userspace
capabilities/behavior).


> Not
> sure there won't be others that are not dependent on irq chip presence.
> You propose to add additional ioctls to enable them if they appear?

I am sure there will be new features in the future that don't depend on
any userspace support, so they would be enabled on GET_SUPPORTED_CPUID
unconditionally.

But if we have new features that depend on specific userspace
capabilities/behavior (i.e. enabling the irqchip, or something else), we
could also add them as long as we check if that capability/behavior was
enabled using ENABLE_CAP.


> > 
> > So at the end all bits in GET_SUPPORTED_CPUID are consistent with what user space is capable of.
> > 
> GET_SUPPORTED_CPUID should not be necessary consistent with what user
> space is capable of. Userspace may emulate features that are not in
> GET_SUPPORTED_CPUID.

True. We don't need to make the interface too complex just to make
GET_SUPPORTED_CPUID match exactly what userspace is going to enable. If
userspace wants to enable a feature because it can emulate it by its
own, it can just enable it using SET_CPUID.

-- 
Eduardo

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

[Alexander Graf]

On 09.05.2012, at 21:38, Eduardo Habkost wrote:

> On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote:
>> On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote:
>>> 
>>> On 09.05.2012, at 10:51, Gleb Natapov wrote:
>>> 
>>>> On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote:
>>>>> 
>>>>> 
>>>>> On 09.05.2012, at 10:14, Gleb Natapov <gleb@redhat.com> wrote:
>>>>> 
>>>>>> On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
>>>>>>> 
>>>>>>> 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?
>>>>>>> 
>>>>>> How kernel should know that userspace does not emulate it?
>>>>> 
>>>>> You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right?
>>>>> 
>>>>>> 
>>>>>>>> - x2apic: ditto.
>>>>>>> 
>>>>>>> Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities.
>>>>>>> 
>>>>>> Same here.
>>>>>> 
>>>>>> Well, technically both of those features can't be implemented in
>>>>>> userspace right now since MSRs are terminated in the kernel, but I
>>>>> 
>>>>> Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case.
>>>>> 
>>>> You mean terminating MSRs in kernel does not sound like the greatest
>>>> design? I do not disagree. That is why IMO kernel can't filter out
>>>> TSC-deadline and x2apic like you suggest.
>>> 
>>> I still don't see why it can't.
>>> 
>>> Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet.
>>> Now, we implement TSC-deadline in the kernel. We still filter
>>> TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide
>>> an interface to user space that says "call me to enable TSC-deadline
>>> CPUID, but only if you're using the in-kernel apic"
> 
> We have that interface already, it is called KVM_SET_CPUID.  :-)
> 
>>> New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic.
>>> Old user space doesn't call that ioctl.
>> First of all we already have TSC-deadline in GET_SUPORTED_CPUID without
>> any additional ioctls. And second I do not see why we need additional
>> iostls here.
> 
> We don't have TSC-deadline set today (and that's what started this
> thread), but we have x2apic. So what you say is true for x2apic, anyway.
> 
>> Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and
>> x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not
>> from KVM_SET_CPUID? For those two features it may make sense indeed.
> 
> It makes sense to me.
> 
> It looks like my assumptions were wrong. They were:
> 
> - GET_SUPPORTED_CPUID simply can't know if the in-kernel irqchip is
>  going to be enabled or not.
> - GET_SUPPORTED_CPUID output has to be a function of the kernel code
>  capabilitie and host CPU, and not depend on any input from userspace.
> 
> Are those assumptions incorrect? If we break them, we may try what
> Alexander is proposing. It would be much more flexible than the options
> I was considering.
> 
> I didn't know ENABLE_CAP existed. Even if GET_SUPPORTED_CPUID can't
> check for the in-kernel irqchip setup for some reason, ENABLE_CAP could
> be used by userpace to tell the kernel "I will enable the in-kernel
> irqchip, so feel free to return features that depend on it on
> GET_SUPPORTED_CPUID".
> 
> In other words, we would return only the type-A features on
> GET_SUPPORTED_CPUID (i.e. safe to be blindly enabled by -cpu host as
> long as migration is not required), but if we use ENABLE_CAP we can make
> group A safely grow, as long as userspace first tells the kernel what it
> supports.
> 
> Anybody is against doing that? Otherwise I plan to work on this.

I think it makes a lot of sense to go this route. But get your blessing from Avi first - ultimately he owns the API :).


Alex

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

[Gleb Natapov]

On Wed, May 09, 2012 at 04:38:02PM -0300, Eduardo Habkost wrote:
> On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote:
> > On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote:
> > > 
> > > On 09.05.2012, at 10:51, Gleb Natapov wrote:
> > > 
> > > > On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote:
> > > >> 
> > > >> 
> > > >> On 09.05.2012, at 10:14, Gleb Natapov <gleb@redhat.com> wrote:
> > > >> 
> > > >>> On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
> > > >>>> 
> > > >>>> 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?
> > > >>>> 
> > > >>> How kernel should know that userspace does not emulate it?
> > > >> 
> > > >> You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right?
> > > >> 
> > > >>> 
> > > >>>>> - x2apic: ditto.
> > > >>>> 
> > > >>>> Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities.
> > > >>>> 
> > > >>> Same here.
> > > >>> 
> > > >>> Well, technically both of those features can't be implemented in
> > > >>> userspace right now since MSRs are terminated in the kernel, but I
> > > >> 
> > > >> Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case.
> > > >> 
> > > > You mean terminating MSRs in kernel does not sound like the greatest
> > > > design? I do not disagree. That is why IMO kernel can't filter out
> > > > TSC-deadline and x2apic like you suggest.
> > > 
> > > I still don't see why it can't.
> > > 
> > > Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet.
> > > Now, we implement TSC-deadline in the kernel. We still filter
> > > TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide
> > > an interface to user space that says "call me to enable TSC-deadline
> > > CPUID, but only if you're using the in-kernel apic"
> 
> We have that interface already, it is called KVM_SET_CPUID.  :-)
> 
> > > New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic.
> > > Old user space doesn't call that ioctl.
> > First of all we already have TSC-deadline in GET_SUPORTED_CPUID without
> > any additional ioctls. And second I do not see why we need additional
> > iostls here.
> 
> We don't have TSC-deadline set today (and that's what started this
> thread), but we have x2apic. So what you say is true for x2apic, anyway.
> 
Hmm, strange. But s/TSC-deadline/x2apic/ and the point stands :)

> > Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and
> > x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not
> > from KVM_SET_CPUID? For those two features it may make sense indeed.
> 
> It makes sense to me.
> 
> It looks like my assumptions were wrong. They were:
> 
> - GET_SUPPORTED_CPUID simply can't know if the in-kernel irqchip is
>   going to be enabled or not.
I think this is currently true. Before we changing that we have to make
sure that no existing userspace calls GET_SUPPORTED_CPUID before
creating irqchip. Not sure we can check all existing userspaces.

> - GET_SUPPORTED_CPUID output has to be a function of the kernel code
>   capabilitie and host CPU, and not depend on any input from userspace.
> 
> Are those assumptions incorrect? If we break them, we may try what
> Alexander is proposing. It would be much more flexible than the options
> I was considering.
> 
> I didn't know ENABLE_CAP existed. Even if GET_SUPPORTED_CPUID can't
What is ENABLE_CAP?

> check for the in-kernel irqchip setup for some reason, ENABLE_CAP could
> be used by userpace to tell the kernel "I will enable the in-kernel
> irqchip, so feel free to return features that depend on it on
> GET_SUPPORTED_CPUID".
> 
> In other words, we would return only the type-A features on
> GET_SUPPORTED_CPUID (i.e. safe to be blindly enabled by -cpu host as
> long as migration is not required), but if we use ENABLE_CAP we can make
> group A safely grow, as long as userspace first tells the kernel what it
> supports.
> 
> Anybody is against doing that? Otherwise I plan to work on this.
> Probably I will start by making GET_SUPPORTED_CPUID not return x2apic
> unless userspace tells the kernel (using ENABLE_CAP) that it will enable
> the in-kernel irqchip. Then we can do the same with TSC-deadline.
> 
> Note that all this work is to allow the kernel to let userspace blindly
> enable features it _doesn't know yet_. If we limit ourselves to features
> userspace already knows about, we could simply remove x2apic and
> TSC-deadline from GET_SUPPORTED_CPUID completely, not use ENABLE_CAP for
> that, and let userspace set x2apic or TSC-deadline on KVM_SET_CPUID only
> if it knows it is safe (either because it checked for the corresponding
> KVM_CAP_* capability is present and it will enable the in-kernel
> irqchip, or because it will emulated it in userspace).
> 
There is not KVM_CAP_* for x2apic as far as I see.

> In case anybody is against the proposal above: note that the current
> documented GET_SUPPORTED_CPUID semantics (unconditionally returning bits
> that depend on specific userspace behavior/capabilities) is simply
> unusable by "-cpu host". If the above proposal gets rejected, my Plan B
> is to update the GET_SUPPORTED_CPUID documentation to note that it
> returns only type-A features (features that userspace can safely enable
> even if it doesn't know what it does), remove x2apic from
> GET_SUPPORTED_CPUID forever, and use KVM_CAP_* for discovery of all
> type-B features (features that depend on specific userspace
> capabilities/behavior).
> 
This will break existing userspaces, no?

> 
> > Not
> > sure there won't be others that are not dependent on irq chip presence.
> > You propose to add additional ioctls to enable them if they appear?
> 
> I am sure there will be new features in the future that don't depend on
> any userspace support, so they would be enabled on GET_SUPPORTED_CPUID
> unconditionally.
> 
Those not the kind of features I am worry about though. I worry about
the features that can be emulated either by kernel or by userspace and
userspace may choose where it wants the feature to be emulated.

> But if we have new features that depend on specific userspace
> capabilities/behavior (i.e. enabling the irqchip, or something else), we
> could also add them as long as we check if that capability/behavior was
> enabled using ENABLE_CAP.
> 
> 
> > > 
> > > So at the end all bits in GET_SUPPORTED_CPUID are consistent with what user space is capable of.
> > > 
> > GET_SUPPORTED_CPUID should not be necessary consistent with what user
> > space is capable of. Userspace may emulate features that are not in
> > GET_SUPPORTED_CPUID.
> 
> True. We don't need to make the interface too complex just to make
> GET_SUPPORTED_CPUID match exactly what userspace is going to enable. If
> userspace wants to enable a feature because it can emulate it by its
> own, it can just enable it using SET_CPUID.
> 
> -- 
> Eduardo

--
			Gleb.

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

[Alexander Graf]

On 05/10/2012 02:53 PM, Gleb Natapov wrote:
> On Wed, May 09, 2012 at 04:38:02PM -0300, Eduardo Habkost wrote:
>> On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote:
>>> On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote:
>>>> On 09.05.2012, at 10:51, Gleb Natapov wrote:
>>>>
>>>>> On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote:
>>>>>>
>>>>>> On 09.05.2012, at 10:14, Gleb Natapov<gleb@redhat.com>  wrote:
>>>>>>
>>>>>>> On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
>>>>>>>> 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?
>>>>>>>>
>>>>>>> How kernel should know that userspace does not emulate it?
>>>>>> You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right?
>>>>>>
>>>>>>>>> - x2apic: ditto.
>>>>>>>> Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities.
>>>>>>>>
>>>>>>> Same here.
>>>>>>>
>>>>>>> Well, technically both of those features can't be implemented in
>>>>>>> userspace right now since MSRs are terminated in the kernel, but I
>>>>>> Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case.
>>>>>>
>>>>> You mean terminating MSRs in kernel does not sound like the greatest
>>>>> design? I do not disagree. That is why IMO kernel can't filter out
>>>>> TSC-deadline and x2apic like you suggest.
>>>> I still don't see why it can't.
>>>>
>>>> Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet.
>>>> Now, we implement TSC-deadline in the kernel. We still filter
>>>> TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide
>>>> an interface to user space that says "call me to enable TSC-deadline
>>>> CPUID, but only if you're using the in-kernel apic"
>> We have that interface already, it is called KVM_SET_CPUID.  :-)
>>
>>>> New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic.
>>>> Old user space doesn't call that ioctl.
>>> First of all we already have TSC-deadline in GET_SUPORTED_CPUID without
>>> any additional ioctls. And second I do not see why we need additional
>>> iostls here.
>> We don't have TSC-deadline set today (and that's what started this
>> thread), but we have x2apic. So what you say is true for x2apic, anyway.
>>
> Hmm, strange. But s/TSC-deadline/x2apic/ and the point stands :)
>
>>> Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and
>>> x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not
>>> from KVM_SET_CPUID? For those two features it may make sense indeed.
>> It makes sense to me.
>>
>> It looks like my assumptions were wrong. They were:
>>
>> - GET_SUPPORTED_CPUID simply can't know if the in-kernel irqchip is
>>    going to be enabled or not.
> I think this is currently true. Before we changing that we have to make
> sure that no existing userspace calls GET_SUPPORTED_CPUID before
> creating irqchip. Not sure we can check all existing userspaces.
>
>> - GET_SUPPORTED_CPUID output has to be a function of the kernel code
>>    capabilitie and host CPU, and not depend on any input from userspace.
>>
>> Are those assumptions incorrect? If we break them, we may try what
>> Alexander is proposing. It would be much more flexible than the options
>> I was considering.
>>
>> I didn't know ENABLE_CAP existed. Even if GET_SUPPORTED_CPUID can't
> What is ENABLE_CAP?

It's an ioctl that you pass in a CAP. It's used on non-x86 already to 
easily turn on features inside kvm :).

>
>> check for the in-kernel irqchip setup for some reason, ENABLE_CAP could
>> be used by userpace to tell the kernel "I will enable the in-kernel
>> irqchip, so feel free to return features that depend on it on
>> GET_SUPPORTED_CPUID".
>>
>> In other words, we would return only the type-A features on
>> GET_SUPPORTED_CPUID (i.e. safe to be blindly enabled by -cpu host as
>> long as migration is not required), but if we use ENABLE_CAP we can make
>> group A safely grow, as long as userspace first tells the kernel what it
>> supports.
>>
>> Anybody is against doing that? Otherwise I plan to work on this.
>> Probably I will start by making GET_SUPPORTED_CPUID not return x2apic
>> unless userspace tells the kernel (using ENABLE_CAP) that it will enable
>> the in-kernel irqchip. Then we can do the same with TSC-deadline.
>>
>> Note that all this work is to allow the kernel to let userspace blindly
>> enable features it _doesn't know yet_. If we limit ourselves to features
>> userspace already knows about, we could simply remove x2apic and
>> TSC-deadline from GET_SUPPORTED_CPUID completely, not use ENABLE_CAP for
>> that, and let userspace set x2apic or TSC-deadline on KVM_SET_CPUID only
>> if it knows it is safe (either because it checked for the corresponding
>> KVM_CAP_* capability is present and it will enable the in-kernel
>> irqchip, or because it will emulated it in userspace).
>>
> There is not KVM_CAP_* for x2apic as far as I see.
>
>> In case anybody is against the proposal above: note that the current
>> documented GET_SUPPORTED_CPUID semantics (unconditionally returning bits
>> that depend on specific userspace behavior/capabilities) is simply
>> unusable by "-cpu host". If the above proposal gets rejected, my Plan B
>> is to update the GET_SUPPORTED_CPUID documentation to note that it
>> returns only type-A features (features that userspace can safely enable
>> even if it doesn't know what it does), remove x2apic from
>> GET_SUPPORTED_CPUID forever, and use KVM_CAP_* for discovery of all
>> type-B features (features that depend on specific userspace
>> capabilities/behavior).
>>
> This will break existing userspaces, no?

It would mean that new user space on old kernels don't get x2apic or 
tsc-deadline in cpuid, yes.

>
>>> Not
>>> sure there won't be others that are not dependent on irq chip presence.
>>> You propose to add additional ioctls to enable them if they appear?
>> I am sure there will be new features in the future that don't depend on
>> any userspace support, so they would be enabled on GET_SUPPORTED_CPUID
>> unconditionally.
>>
> Those not the kind of features I am worry about though. I worry about
> the features that can be emulated either by kernel or by userspace and
> userspace may choose where it wants the feature to be emulated.

Like? Usually CPUID features reflect new instructions. We don't notify 
user space on unknown instructions, right?


Alex

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

[Gleb Natapov]

On Thu, May 10, 2012 at 03:21:41PM +0200, Alexander Graf wrote:
> On 05/10/2012 02:53 PM, Gleb Natapov wrote:
> >On Wed, May 09, 2012 at 04:38:02PM -0300, Eduardo Habkost wrote:
> >>On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote:
> >>>On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote:
> >>>>On 09.05.2012, at 10:51, Gleb Natapov wrote:
> >>>>
> >>>>>On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote:
> >>>>>>
> >>>>>>On 09.05.2012, at 10:14, Gleb Natapov<gleb@redhat.com>  wrote:
> >>>>>>
> >>>>>>>On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
> >>>>>>>>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?
> >>>>>>>>
> >>>>>>>How kernel should know that userspace does not emulate it?
> >>>>>>You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right?
> >>>>>>
> >>>>>>>>>- x2apic: ditto.
> >>>>>>>>Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities.
> >>>>>>>>
> >>>>>>>Same here.
> >>>>>>>
> >>>>>>>Well, technically both of those features can't be implemented in
> >>>>>>>userspace right now since MSRs are terminated in the kernel, but I
> >>>>>>Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case.
> >>>>>>
> >>>>>You mean terminating MSRs in kernel does not sound like the greatest
> >>>>>design? I do not disagree. That is why IMO kernel can't filter out
> >>>>>TSC-deadline and x2apic like you suggest.
> >>>>I still don't see why it can't.
> >>>>
> >>>>Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet.
> >>>>Now, we implement TSC-deadline in the kernel. We still filter
> >>>>TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide
> >>>>an interface to user space that says "call me to enable TSC-deadline
> >>>>CPUID, but only if you're using the in-kernel apic"
> >>We have that interface already, it is called KVM_SET_CPUID.  :-)
> >>
> >>>>New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic.
> >>>>Old user space doesn't call that ioctl.
> >>>First of all we already have TSC-deadline in GET_SUPORTED_CPUID without
> >>>any additional ioctls. And second I do not see why we need additional
> >>>iostls here.
> >>We don't have TSC-deadline set today (and that's what started this
> >>thread), but we have x2apic. So what you say is true for x2apic, anyway.
> >>
> >Hmm, strange. But s/TSC-deadline/x2apic/ and the point stands :)
> >
> >>>Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and
> >>>x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not
> >>>from KVM_SET_CPUID? For those two features it may make sense indeed.
> >>It makes sense to me.
> >>
> >>It looks like my assumptions were wrong. They were:
> >>
> >>- GET_SUPPORTED_CPUID simply can't know if the in-kernel irqchip is
> >>   going to be enabled or not.
> >I think this is currently true. Before we changing that we have to make
> >sure that no existing userspace calls GET_SUPPORTED_CPUID before
> >creating irqchip. Not sure we can check all existing userspaces.
> >
> >>- GET_SUPPORTED_CPUID output has to be a function of the kernel code
> >>   capabilitie and host CPU, and not depend on any input from userspace.
> >>
> >>Are those assumptions incorrect? If we break them, we may try what
> >>Alexander is proposing. It would be much more flexible than the options
> >>I was considering.
> >>
> >>I didn't know ENABLE_CAP existed. Even if GET_SUPPORTED_CPUID can't
> >What is ENABLE_CAP?
> 
> It's an ioctl that you pass in a CAP. It's used on non-x86 already
> to easily turn on features inside kvm :).
> 
No wonder grep in arch/x86 told me nothing :) So for x86 this ioctl as
good as non-existent for now.

> >
> >>check for the in-kernel irqchip setup for some reason, ENABLE_CAP could
> >>be used by userpace to tell the kernel "I will enable the in-kernel
> >>irqchip, so feel free to return features that depend on it on
> >>GET_SUPPORTED_CPUID".
> >>
> >>In other words, we would return only the type-A features on
> >>GET_SUPPORTED_CPUID (i.e. safe to be blindly enabled by -cpu host as
> >>long as migration is not required), but if we use ENABLE_CAP we can make
> >>group A safely grow, as long as userspace first tells the kernel what it
> >>supports.
> >>
> >>Anybody is against doing that? Otherwise I plan to work on this.
> >>Probably I will start by making GET_SUPPORTED_CPUID not return x2apic
> >>unless userspace tells the kernel (using ENABLE_CAP) that it will enable
> >>the in-kernel irqchip. Then we can do the same with TSC-deadline.
> >>
> >>Note that all this work is to allow the kernel to let userspace blindly
> >>enable features it _doesn't know yet_. If we limit ourselves to features
> >>userspace already knows about, we could simply remove x2apic and
> >>TSC-deadline from GET_SUPPORTED_CPUID completely, not use ENABLE_CAP for
> >>that, and let userspace set x2apic or TSC-deadline on KVM_SET_CPUID only
> >>if it knows it is safe (either because it checked for the corresponding
> >>KVM_CAP_* capability is present and it will enable the in-kernel
> >>irqchip, or because it will emulated it in userspace).
> >>
> >There is not KVM_CAP_* for x2apic as far as I see.
> >
> >>In case anybody is against the proposal above: note that the current
> >>documented GET_SUPPORTED_CPUID semantics (unconditionally returning bits
> >>that depend on specific userspace behavior/capabilities) is simply
> >>unusable by "-cpu host". If the above proposal gets rejected, my Plan B
> >>is to update the GET_SUPPORTED_CPUID documentation to note that it
> >>returns only type-A features (features that userspace can safely enable
> >>even if it doesn't know what it does), remove x2apic from
> >>GET_SUPPORTED_CPUID forever, and use KVM_CAP_* for discovery of all
> >>type-B features (features that depend on specific userspace
> >>capabilities/behavior).
> >>
> >This will break existing userspaces, no?
> 
> It would mean that new user space on old kernels don't get x2apic or
> tsc-deadline in cpuid, yes.
> 
tsc-deadline actually exposed via capability, but disappearance of
x2apic would be serious regression.

> >
> >>>Not
> >>>sure there won't be others that are not dependent on irq chip presence.
> >>>You propose to add additional ioctls to enable them if they appear?
> >>I am sure there will be new features in the future that don't depend on
> >>any userspace support, so they would be enabled on GET_SUPPORTED_CPUID
> >>unconditionally.
> >>
> >Those not the kind of features I am worry about though. I worry about
> >the features that can be emulated either by kernel or by userspace and
> >userspace may choose where it wants the feature to be emulated.
> 
> Like? Usually CPUID features reflect new instructions. We don't
> notify user space on unknown instructions, right?
> 
No we do not. I can't give you concrete example, but think about some
apic unrelated device that is accessible via MSR and can be emulated
either in kernel or in userspace. Hmm, maybe something like ACPI
(Thermal Monitor and Software Controlled Clock Facilities), or PSN 
(Processor Serial Number).

--
			Gleb.

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

[Eduardo Habkost]

On Thu, May 10, 2012 at 04:39:45PM +0300, Gleb Natapov wrote:
> On Thu, May 10, 2012 at 03:21:41PM +0200, Alexander Graf wrote:
> > On 05/10/2012 02:53 PM, Gleb Natapov wrote:
> > >On Wed, May 09, 2012 at 04:38:02PM -0300, Eduardo Habkost wrote:
> > >>On Wed, May 09, 2012 at 12:38:37PM +0300, Gleb Natapov wrote:
> > >>>On Wed, May 09, 2012 at 11:05:58AM +0200, Alexander Graf wrote:
> > >>>>On 09.05.2012, at 10:51, Gleb Natapov wrote:
> > >>>>
> > >>>>>On Wed, May 09, 2012 at 10:42:26AM +0200, Alexander Graf wrote:
> > >>>>>>
> > >>>>>>On 09.05.2012, at 10:14, Gleb Natapov<gleb@redhat.com>  wrote:
> > >>>>>>
> > >>>>>>>On Wed, May 09, 2012 at 12:07:04AM +0200, Alexander Graf wrote:
> > >>>>>>>>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?
> > >>>>>>>>
> > >>>>>>>How kernel should know that userspace does not emulate it?
> > >>>>>>You have to enable the in-kernel apic to use it, at which point the kernel knows it's in use, right?
> > >>>>>>
> > >>>>>>>>>- x2apic: ditto.
> > >>>>>>>>Same here. For user space irqchip the kernel side doesn't care. If in-kernel APIC is enabled, check for its capabilities.
> > >>>>>>>>
> > >>>>>>>Same here.
> > >>>>>>>
> > >>>>>>>Well, technically both of those features can't be implemented in
> > >>>>>>>userspace right now since MSRs are terminated in the kernel, but I
> > >>>>>>Doesn't sound like the greatest design - unless you deprecate the non-in-kernel apic case.
> > >>>>>>
> > >>>>>You mean terminating MSRs in kernel does not sound like the greatest
> > >>>>>design? I do not disagree. That is why IMO kernel can't filter out
> > >>>>>TSC-deadline and x2apic like you suggest.
> > >>>>I still don't see why it can't.
> > >>>>
> > >>>>Imagine we would filter TSC-deadline and x2apic by default in the kernel - they are not known to exist yet.
> > >>>>Now, we implement TSC-deadline in the kernel. We still filter
> > >>>>TSC-deadline out in GET_SUPORTED_CPUID in the kernel. But we provide
> > >>>>an interface to user space that says "call me to enable TSC-deadline
> > >>>>CPUID, but only if you're using the in-kernel apic"
> > >>We have that interface already, it is called KVM_SET_CPUID.  :-)
> > >>
> > >>>>New user space calls that ioctl when it's using the in-kernel apic, it doesn't when it's using the user space apic.
> > >>>>Old user space doesn't call that ioctl.
> > >>>First of all we already have TSC-deadline in GET_SUPORTED_CPUID without
> > >>>any additional ioctls. And second I do not see why we need additional
> > >>>iostls here.
> > >>We don't have TSC-deadline set today (and that's what started this
> > >>thread), but we have x2apic. So what you say is true for x2apic, anyway.
> > >>
> > >Hmm, strange. But s/TSC-deadline/x2apic/ and the point stands :)
> > >
> > >>>Hmm, so may be I misunderstood you. You propose to mask TSC-deadline and
> > >>>x2apic out from GET_SUPORTED_CPUID if irq chip is not in kernel, not
> > >>>from KVM_SET_CPUID? For those two features it may make sense indeed.
> > >>It makes sense to me.
> > >>
> > >>It looks like my assumptions were wrong. They were:
> > >>
> > >>- GET_SUPPORTED_CPUID simply can't know if the in-kernel irqchip is
> > >>   going to be enabled or not.
> > >I think this is currently true. Before we changing that we have to make
> > >sure that no existing userspace calls GET_SUPPORTED_CPUID before
> > >creating irqchip. Not sure we can check all existing userspaces.
> > >
> > >>- GET_SUPPORTED_CPUID output has to be a function of the kernel code
> > >>   capabilitie and host CPU, and not depend on any input from userspace.
> > >>
> > >>Are those assumptions incorrect? If we break them, we may try what
> > >>Alexander is proposing. It would be much more flexible than the options
> > >>I was considering.
> > >>
> > >>I didn't know ENABLE_CAP existed. Even if GET_SUPPORTED_CPUID can't
> > >What is ENABLE_CAP?
> > 
> > It's an ioctl that you pass in a CAP. It's used on non-x86 already
> > to easily turn on features inside kvm :).
> > 
> No wonder grep in arch/x86 told me nothing :) So for x86 this ioctl as
> good as non-existent for now.
> 
> > >
> > >>check for the in-kernel irqchip setup for some reason, ENABLE_CAP could
> > >>be used by userpace to tell the kernel "I will enable the in-kernel
> > >>irqchip, so feel free to return features that depend on it on
> > >>GET_SUPPORTED_CPUID".
> > >>
> > >>In other words, we would return only the type-A features on
> > >>GET_SUPPORTED_CPUID (i.e. safe to be blindly enabled by -cpu host as
> > >>long as migration is not required), but if we use ENABLE_CAP we can make
> > >>group A safely grow, as long as userspace first tells the kernel what it
> > >>supports.
> > >>
> > >>Anybody is against doing that? Otherwise I plan to work on this.
> > >>Probably I will start by making GET_SUPPORTED_CPUID not return x2apic
> > >>unless userspace tells the kernel (using ENABLE_CAP) that it will enable
> > >>the in-kernel irqchip. Then we can do the same with TSC-deadline.
> > >>
> > >>Note that all this work is to allow the kernel to let userspace blindly
> > >>enable features it _doesn't know yet_. If we limit ourselves to features
> > >>userspace already knows about, we could simply remove x2apic and
> > >>TSC-deadline from GET_SUPPORTED_CPUID completely, not use ENABLE_CAP for
> > >>that, and let userspace set x2apic or TSC-deadline on KVM_SET_CPUID only
> > >>if it knows it is safe (either because it checked for the corresponding
> > >>KVM_CAP_* capability is present and it will enable the in-kernel
> > >>irqchip, or because it will emulated it in userspace).
> > >>
> > >There is not KVM_CAP_* for x2apic as far as I see.
> > >
> > >>In case anybody is against the proposal above: note that the current
> > >>documented GET_SUPPORTED_CPUID semantics (unconditionally returning bits
> > >>that depend on specific userspace behavior/capabilities) is simply
> > >>unusable by "-cpu host". If the above proposal gets rejected, my Plan B
> > >>is to update the GET_SUPPORTED_CPUID documentation to note that it
> > >>returns only type-A features (features that userspace can safely enable
> > >>even if it doesn't know what it does), remove x2apic from
> > >>GET_SUPPORTED_CPUID forever, and use KVM_CAP_* for discovery of all
> > >>type-B features (features that depend on specific userspace
> > >>capabilities/behavior).
> > >>
> > >This will break existing userspaces, no?
> > 
> > It would mean that new user space on old kernels don't get x2apic or
> > tsc-deadline in cpuid, yes.
> > 
> tsc-deadline actually exposed via capability, but disappearance of
> x2apic would be serious regression.

To keep compatibility, we can let userspace tell the kernel that they
want the "safe" version of GET_SUPPORTED_CPUID (using SET_CAP?). If
userspace doesn't do that, x2apic would be kept enabled on
GET_SUPPORTED_CPUID for compatiblity.

The problem here is that the current behavior of GET_SUPPORTED_CPUID
(returning x2apic) is unusable by -cpu host, unless we make the
in-kernel irqchip mandatory for -cpu host (but that would be solving
just our immediate needs, and not any similar cases in the future).


> > >
> > >>>Not
> > >>>sure there won't be others that are not dependent on irq chip presence.
> > >>>You propose to add additional ioctls to enable them if they appear?
> > >>I am sure there will be new features in the future that don't depend on
> > >>any userspace support, so they would be enabled on GET_SUPPORTED_CPUID
> > >>unconditionally.
> > >>
> > >Those not the kind of features I am worry about though. I worry about
> > >the features that can be emulated either by kernel or by userspace and
> > >userspace may choose where it wants the feature to be emulated.
> > 
> > Like? Usually CPUID features reflect new instructions. We don't
> > notify user space on unknown instructions, right?
> > 
> No we do not. I can't give you concrete example, but think about some
> apic unrelated device that is accessible via MSR and can be emulated
> either in kernel or in userspace. Hmm, maybe something like ACPI
> (Thermal Monitor and Software Controlled Clock Facilities), or PSN 
> (Processor Serial Number).

Those cases are the easy ones (or maybe they're just the ones I don't
worry about, right now :).

- If the kernel can't emulate it, the feature would never appear on
  GET_SUPPORTED_CPUID, but userspace could enable it using SET_CPUID if
  userspace is going to emulate it.
  - In case userspace emulation don't depend on any extra kernel
    capability, userspace would simply set it on SET_CPUID.
    - Now, what happens here if a future kernel version becomes able to
      emulate it in-kernel?
  - In case userspace emulation depend on some kernel capability,
    userspace can use CHECK_EXTENSION before setting it on SET_CPUID.
- If the kernel can emulate it without depending on any userspace
  capability/behavior, the feature would appear on GET_SUPPORTED_CPUID.
  - If userspace wants to use the kernel emulation, it would just enable
    it on SET_CPUID.
  - If userspace wants to emulate it itself, it would set it on
    SET_CPUID, but it would need to ask the kernel to disable the
    in-kernel emulation (how?).

I am more worried about this case:

- If the kernel can emulate it, but depends on specific userspace
  capability/behavior to work. That feature can't appear on
  GET_SUPPORTED_CPUID by default, otherwise older qemu versions using
  "-cpu host" would try to enable it without doing what's necessary to
  make it work. This is the case for x2apic and TSC-deadline.

-- 
Eduardo

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

[Andre Przywara]

On 05/07/2012 08:21 PM, Eduardo Habkost wrote:
>
> Andre? Are you able to help to answer the question below?

Sorry for the delay, the easy answers first:

> I would like to clarify what's the expected behavior of "-cpu host" to
> be able to continue working on it.

The purpose of -cpu host is to let guests use ISA features that the host 
CPU provides. Those would not need any KVM/QEMU intervention, because 
they work out of the box. Things like AES or SSE4.2, which are used by 
Linux and glibc, for instance. Users would expect those to be usable, 
and actually we only need to set the bits and are done.

A second goal is to get rid of the awkward and artificial 
family/model/stepping settings and just let the guest see the actual CPU 
model. This has some strings attached, though, but other virtualization 
solution do it the same way and they can cope with it.

A third thing currently not really addressed are the more informational 
bits like cache size and organization and TLB layout. Those are 
properties of the (core) CPU (except shared L3, cache maybe) and apply 
to guests as well. I don't see any reason why this should not be exposed 
to the guest. From the top of my head I don't know any prominent user 
(just glibc reading the cache size, but not really using it), but I 
guess there are applications which benefit from it.

What clearly is not the intention of -cpu host is to provide access to 
every feature a certain CPU model provides. So things which require 
emulation or hypervisor intervention are not in the primary use case. 
That does not mean that they don't need to implemented, but that was not 
the purpose of -cpu host and they should be handled independently.

Regards,
Andre.

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


-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Eduardo, Jan

I will update tsc deadline timer patch (at qemu-kvm side) recently.
Have you made a final agreement of the issue 'KVM_CAP_TSC_DEADLINE_TIMER' vs. 'GET_SUPPORTED_CPUID'?

Thanks,
Jinsong


Eduardo Habkost wrote:
> (CCing Andre Przywara, in case he can help to clarify what's the
> expected meaning of "-cpu host")
> 
> On Tue, Apr 24, 2012 at 06:06:55PM +0200, Jan Kiszka wrote:
>> On 2012-04-23 22:02, Eduardo Habkost wrote:
>>> On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote:
>>>> However, that was how I interpreted this GET_SUPPORTED_CPUID. In
>>>> fact, 
>>>> it is used as "kernel or hardware does not _prevent_" already. And
>>>> in that sense, it's ok to enable even features that are not in
>>>> kernel/hardware hands. We should point out this fact in the
>>>> documentation. 
>>> 
>>> I see GET_SUPPORTED_CPUID as just a "what userspace can enable
>>> because the kernel and the hardware support it (= don't prevent
>>> it), as long as userspace has the required support" (meaning A+B).
>>> It's a bit like KVM_CHECK_EXTENSION, but with the nice feature that
>>> that the capabilities map directly to CPUID bits.
>>> 
>>> So, it's not clear to me: now you are OK with adding TSC_DEADLINE
>>> to GET_SUPPORTED_CPUID? 
>>> 
>>> But we still have the issue of "-cpu host" not knowing what can be
>>> safely enabled (without userspace feature-specific setup code), or
>>> not. Do you have any suggestion for that? Avi, do you have any
>>> suggestion? 
>> 
>> First of all, I bet this was already broken with the introduction of
>> x2apic. So TSC deadline won't make it worse. I guess we need to
>> address this in userspace, first by masking those features out,
>> later by actually emulating them.
> 
> 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.
> 
>> 
>>> 
>>> And I still don't know the answer to:
>>> 
>>>>> - How to precisely define the groups (A) and (B)?
>>>>>   - "requires additional code only if migration is required"
>>>>>     qualifies as (B) or (A)?
>>> 
>>> 
>>> Re: documentation, isn't the following paragraph (already present
>>> on api.txt) sufficient? 
>>> 
>>> "The entries returned are the host cpuid as returned by the cpuid
>>> instruction, with unknown or unsupported features masked out.  Some
>>> features (for example, x2apic), may not be present in the host cpu,
>>> but are exposed by kvm if it can emulate them efficiently."
>> 
>> That suggests such features are always emulated - which is not true.
>> They are either emulated, or nothing _prevents_ their emulation by
>> user space.
> 
> Well... it's a bit more complicated than that: the current semantics
> are a bit more than "doesn't prevent", as in theory every single
> feature can be emulated by userspace, without any help from the
> kernel. So, if "doesn't prevent" were the only criteria, the kernel
> would set every single feature bit on GET_SUPPORTED_CPUID, making it
> not very useful. 
> 
> At least in the case of x2apic, the kernel is using
> GET_SUPPORTED_CPUID to expose a _capability_ too: when x2apic is
> present on GET_SUPPORTED_CPUID, userspace knows that in addition to
> "not preventing" the feature from being enabled, the kernel is now
> able to emulate x2apic (if proper setup is made by userspace). A
> kernel that can't emulate x2apic (even if userspace was allowed to
> emulate it completely in userspace) would never have x2apic enabled on
> GET_SUPPORTED_CPUID.
> 
> Like I said previously, in the end GET_SUPPORTED_CPUID is just a
> capability querying mechanism like KVM_CHECK_EXTENSION (where each
> extension have a specific kernel-capability meaning), but with the
> nice feature of being automatically mapped to CPUID bits (with no
> need for additional KVM_CAP_* => CPUID translation in userspace).

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Eduardo Habkost]

On Thu, Jun 14, 2012 at 07:02:03PM +0000, Liu, Jinsong wrote:
> Eduardo, Jan
> 
> I will update tsc deadline timer patch (at qemu-kvm side) recently.
> Have you made a final agreement of the issue 'KVM_CAP_TSC_DEADLINE_TIMER' vs. 'GET_SUPPORTED_CPUID'?

I don't think there's a final agreement, but I was convinced later that
it's probably better to _not_ have TSC-deadline on GET_SUPPORTED_CPUID,
at least not by default.

Even if this is changed in the future, it's a good idea to make qemu
support the KVM_CAP_TSC_DEADLINE_TIMER method if running on older
kernels, anyway.


> Eduardo Habkost wrote:
> > (CCing Andre Przywara, in case he can help to clarify what's the
> > expected meaning of "-cpu host")
> > 
> > On Tue, Apr 24, 2012 at 06:06:55PM +0200, Jan Kiszka wrote:
> >> On 2012-04-23 22:02, Eduardo Habkost wrote:
> >>> On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote:
> >>>> However, that was how I interpreted this GET_SUPPORTED_CPUID. In
> >>>> fact, 
> >>>> it is used as "kernel or hardware does not _prevent_" already. And
> >>>> in that sense, it's ok to enable even features that are not in
> >>>> kernel/hardware hands. We should point out this fact in the
> >>>> documentation. 
> >>> 
> >>> I see GET_SUPPORTED_CPUID as just a "what userspace can enable
> >>> because the kernel and the hardware support it (= don't prevent
> >>> it), as long as userspace has the required support" (meaning A+B).
> >>> It's a bit like KVM_CHECK_EXTENSION, but with the nice feature that
> >>> that the capabilities map directly to CPUID bits.
> >>> 
> >>> So, it's not clear to me: now you are OK with adding TSC_DEADLINE
> >>> to GET_SUPPORTED_CPUID? 
> >>> 
> >>> But we still have the issue of "-cpu host" not knowing what can be
> >>> safely enabled (without userspace feature-specific setup code), or
> >>> not. Do you have any suggestion for that? Avi, do you have any
> >>> suggestion? 
> >> 
> >> First of all, I bet this was already broken with the introduction of
> >> x2apic. So TSC deadline won't make it worse. I guess we need to
> >> address this in userspace, first by masking those features out,
> >> later by actually emulating them.
> > 
> > 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.
> > 
> >> 
> >>> 
> >>> And I still don't know the answer to:
> >>> 
> >>>>> - How to precisely define the groups (A) and (B)?
> >>>>>   - "requires additional code only if migration is required"
> >>>>>     qualifies as (B) or (A)?
> >>> 
> >>> 
> >>> Re: documentation, isn't the following paragraph (already present
> >>> on api.txt) sufficient? 
> >>> 
> >>> "The entries returned are the host cpuid as returned by the cpuid
> >>> instruction, with unknown or unsupported features masked out.  Some
> >>> features (for example, x2apic), may not be present in the host cpu,
> >>> but are exposed by kvm if it can emulate them efficiently."
> >> 
> >> That suggests such features are always emulated - which is not true.
> >> They are either emulated, or nothing _prevents_ their emulation by
> >> user space.
> > 
> > Well... it's a bit more complicated than that: the current semantics
> > are a bit more than "doesn't prevent", as in theory every single
> > feature can be emulated by userspace, without any help from the
> > kernel. So, if "doesn't prevent" were the only criteria, the kernel
> > would set every single feature bit on GET_SUPPORTED_CPUID, making it
> > not very useful. 
> > 
> > At least in the case of x2apic, the kernel is using
> > GET_SUPPORTED_CPUID to expose a _capability_ too: when x2apic is
> > present on GET_SUPPORTED_CPUID, userspace knows that in addition to
> > "not preventing" the feature from being enabled, the kernel is now
> > able to emulate x2apic (if proper setup is made by userspace). A
> > kernel that can't emulate x2apic (even if userspace was allowed to
> > emulate it completely in userspace) would never have x2apic enabled on
> > GET_SUPPORTED_CPUID.
> > 
> > Like I said previously, in the end GET_SUPPORTED_CPUID is just a
> > capability querying mechanism like KVM_CHECK_EXTENSION (where each
> > extension have a specific kernel-capability meaning), but with the
> > nice feature of being automatically mapped to CPUID bits (with no
> > need for additional KVM_CAP_* => CPUID translation in userspace).
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Eduardo Habkost wrote:
> On Thu, Jun 14, 2012 at 07:02:03PM +0000, Liu, Jinsong wrote:
>> Eduardo, Jan
>> 
>> I will update tsc deadline timer patch (at qemu-kvm side) recently.
>> Have you made a final agreement of the issue
>> 'KVM_CAP_TSC_DEADLINE_TIMER' vs. 'GET_SUPPORTED_CPUID'? 
> 
> I don't think there's a final agreement, but I was convinced later
> that it's probably better to _not_ have TSC-deadline on
> GET_SUPPORTED_CPUID, at least not by default.
> 
> Even if this is changed in the future, it's a good idea to make qemu
> support the KVM_CAP_TSC_DEADLINE_TIMER method if running on older
> kernels, anyway.

OK, so I will coding based on current KVM_CAP_TSC_DEADLINE_TIMER method.

Thanks for clarifying!


> 
> 
>> Eduardo Habkost wrote:
>>> (CCing Andre Przywara, in case he can help to clarify what's the
>>> expected meaning of "-cpu host")
>>> 
>>> On Tue, Apr 24, 2012 at 06:06:55PM +0200, Jan Kiszka wrote:
>>>> On 2012-04-23 22:02, Eduardo Habkost wrote:
>>>>> On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote:
>>>>>> However, that was how I interpreted this GET_SUPPORTED_CPUID. In
>>>>>> fact, it is used as "kernel or hardware does not _prevent_"
>>>>>> already. And in that sense, it's ok to enable even features that
>>>>>> are not in kernel/hardware hands. We should point out this fact
>>>>>> in the documentation.
>>>>> 
>>>>> I see GET_SUPPORTED_CPUID as just a "what userspace can enable
>>>>> because the kernel and the hardware support it (= don't prevent
>>>>> it), as long as userspace has the required support" (meaning A+B).
>>>>> It's a bit like KVM_CHECK_EXTENSION, but with the nice feature
>>>>> that that the capabilities map directly to CPUID bits.
>>>>> 
>>>>> So, it's not clear to me: now you are OK with adding TSC_DEADLINE
>>>>> to GET_SUPPORTED_CPUID? 
>>>>> 
>>>>> But we still have the issue of "-cpu host" not knowing what can be
>>>>> safely enabled (without userspace feature-specific setup code), or
>>>>> not. Do you have any suggestion for that? Avi, do you have any
>>>>> suggestion?
>>>> 
>>>> First of all, I bet this was already broken with the introduction
>>>> of x2apic. So TSC deadline won't make it worse. I guess we need to
>>>> address this in userspace, first by masking those features out,
>>>> later by actually emulating them.
>>> 
>>> 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.
>>> 
>>>> 
>>>>> 
>>>>> And I still don't know the answer to:
>>>>> 
>>>>>>> - How to precisely define the groups (A) and (B)?
>>>>>>>   - "requires additional code only if migration is required"
>>>>>>>     qualifies as (B) or (A)?
>>>>> 
>>>>> 
>>>>> Re: documentation, isn't the following paragraph (already present
>>>>> on api.txt) sufficient? 
>>>>> 
>>>>> "The entries returned are the host cpuid as returned by the cpuid
>>>>> instruction, with unknown or unsupported features masked out. 
>>>>> Some features (for example, x2apic), may not be present in the
>>>>> host cpu, but are exposed by kvm if it can emulate them
>>>>> efficiently." 
>>>> 
>>>> That suggests such features are always emulated - which is not
>>>> true. They are either emulated, or nothing _prevents_ their
>>>> emulation by user space.
>>> 
>>> Well... it's a bit more complicated than that: the current semantics
>>> are a bit more than "doesn't prevent", as in theory every single
>>> feature can be emulated by userspace, without any help from the
>>> kernel. So, if "doesn't prevent" were the only criteria, the kernel
>>> would set every single feature bit on GET_SUPPORTED_CPUID, making
>>> it not very useful. 
>>> 
>>> At least in the case of x2apic, the kernel is using
>>> GET_SUPPORTED_CPUID to expose a _capability_ too: when x2apic is
>>> present on GET_SUPPORTED_CPUID, userspace knows that in addition to
>>> "not preventing" the feature from being enabled, the kernel is now
>>> able to emulate x2apic (if proper setup is made by userspace). A
>>> kernel that can't emulate x2apic (even if userspace was allowed to
>>> emulate it completely in userspace) would never have x2apic enabled
>>> on GET_SUPPORTED_CPUID. 
>>> 
>>> Like I said previously, in the end GET_SUPPORTED_CPUID is just a
>>> capability querying mechanism like KVM_CHECK_EXTENSION (where each
>>> extension have a specific kernel-capability meaning), but with the
>>> nice feature of being automatically mapped to CPUID bits (with no
>>> need for additional KVM_CAP_* => CPUID translation in userspace).