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

[Jan Kiszka]

On 2012-04-23 16:48, Eduardo Habkost wrote:
> Trying to summarize the points above:
> 
> Groups (A) and (B) are:
> 
> A) a feature that KVM supports and emulate by itself and can be enabled
>    by userspace blindly, without requiring any additional userspace
>    code to work.
> B) a feature that KVM supports but need support from userspace to work.
> 
> We have to differentiate those two groups somehow, otherwise "-cpu host"
> will always risk being unstable (in case we can't identify group (B) and
> end up enabling a feature that will break) or useless (if group (A) is
> considered always empty).
> 
> (If you think this two-group model is not sufficient, please let me know.)
> 
> Note that I am discussing two things above:
> 
> - Whether GET_SUPPORTED_CPUID should expose only features from group
>   (A), or group (B) too.
>   - One problem here is that today GET_SUPPORTED_CPUIDS have many
>     examples of (B) features inside it. Should we stop doing that?

We have exactly two for the category that I was concerned about: TSC
deadline and X2APIC. The latter is already exposed unconditionally, even
if the kernel does not provide emulation. So, you are right, TSC
deadline is not a new scenario.

>     - TSC-deadline is the first case where we are _not_ doing that. It
>       is the first CPU feature in KVM that can be enabled by userspace
>       (as long as userspace does the proper setup), but it's not
>       included on GET_SUPPORTED_CPUIDs.
>   - Even the current documentation implies that group (B) is included:
> 
>     "This ioctl returns x86 cpuid features which are supported by both
>     the hardware and kvm.  Userspace can use the information returned by
>     this ioctl to construct cpuid information (for KVM_SET_CPUID2) that
>     is consistent with hardware, kernel, and userspace capabilities, and
>                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^
>     with user requirements (for example, the user may wish to constrain
>     cpuid to emulate older hardware, or for feature consistency across a
>     cluster)."
> 
>     In the specific case of TSC-deadline, I consider "Qemu knowing that
>     TSC-deadline can be enabled only if in-kernel irqchip is enabled" as
>     an "userpace capability".
> 
> - How to precisely define the groups (A) and (B)?
>   - "requires additional code only if migration is required" qualifies
>     as (B) or (A)?
>   - "requires in-kernel irqchip to be enabled to work" qualifies as (B),
>     doesn't it?

My problem is that features like X2APIC and TSC deadline are exposed by
the kernel without "contributing" to them (if in-kernel irqchip is off).
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.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux