From: Wanpeng Li <[email protected]>
Per KVM_GET_SUPPORTED_CPUID ioctl documentation:
This ioctl returns x86 cpuid features which are supported by both the
hardware and kvm in its default configuration.
A well-behaved userspace should not set the bit if it is not supported.
Suggested-by: Jim Mattson <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
arch/x86/kvm/cpuid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 06a278b..225d251 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -789,7 +789,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
entry->ebx = 0;
entry->ecx = 0;
- entry->edx = 0;
+ entry->edx = (1 << KVM_HINTS_REALTIME);
break;
case 0x80000000:
entry->eax = min(entry->eax, 0x8000001f);
--
2.7.4
On 22/10/20 03:34, Wanpeng Li wrote:
> From: Wanpeng Li <[email protected]>
>
> Per KVM_GET_SUPPORTED_CPUID ioctl documentation:
>
> This ioctl returns x86 cpuid features which are supported by both the
> hardware and kvm in its default configuration.
>
> A well-behaved userspace should not set the bit if it is not supported.
>
> Suggested-by: Jim Mattson <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
It's common for userspace to copy all supported CPUID bits to
KVM_SET_CPUID2, I don't think this is the right behavior for
KVM_HINTS_REALTIME.
(But maybe this was discussed already; if so, please point me to the
previous discussion).
Paolo
Paolo Bonzini <[email protected]> writes:
> On 22/10/20 03:34, Wanpeng Li wrote:
>> From: Wanpeng Li <[email protected]>
>>
>> Per KVM_GET_SUPPORTED_CPUID ioctl documentation:
>>
>> This ioctl returns x86 cpuid features which are supported by both the
>> hardware and kvm in its default configuration.
>>
>> A well-behaved userspace should not set the bit if it is not supported.
>>
>> Suggested-by: Jim Mattson <[email protected]>
>> Signed-off-by: Wanpeng Li <[email protected]>
>
> It's common for userspace to copy all supported CPUID bits to
> KVM_SET_CPUID2, I don't think this is the right behavior for
> KVM_HINTS_REALTIME.
>
> (But maybe this was discussed already; if so, please point me to the
> previous discussion).
>
Not for KVM_GET_SUPPORTED_CPUID but for KVM_GET_SUPPORTED_HV_CPUID: just
copying all the bits blindly is incorrect as e.g. SYNIC needs to be
enabled with KVM_CAP_HYPERV_SYNIC[2]. KVM PV features also have
pre-requisites, e.g. KVM_ASYNC_PF_DELIVERY_AS_INT requires an irqchip so
again copy/paste may not work.
--
Vitaly
On 22/10/20 15:31, Xiaoyao Li wrote:
>>
>> It's common for userspace to copy all supported CPUID bits to
>> KVM_SET_CPUID2, I don't think this is the right behavior for
>> KVM_HINTS_REALTIME.
>
> It reminds of X86_FEATURE_WAITPKG, which is added to supported CPUID
> recently as a fix but QEMU exposes it to guest only when "-overcommit
> cpu-pm"
WAITPKG is not included in KVM_GET_SUPPORTED_CPUID either. QEMU detects
it through the MSR_IA32_UMWAIT register.
Paolo
On 10/22/2020 10:06 PM, Paolo Bonzini wrote:
> On 22/10/20 15:31, Xiaoyao Li wrote:
>>>
>>> It's common for userspace to copy all supported CPUID bits to
>>> KVM_SET_CPUID2, I don't think this is the right behavior for
>>> KVM_HINTS_REALTIME.
>>
>> It reminds of X86_FEATURE_WAITPKG, which is added to supported CPUID
>> recently as a fix but QEMU exposes it to guest only when "-overcommit
>> cpu-pm"
>
> WAITPKG is not included in KVM_GET_SUPPORTED_CPUID either. QEMU detects
> it through the MSR_IA32_UMWAIT register.
Doesn't 0abcc8f65cc2 ("KVM: VMX: enable X86_FEATURE_WAITPKG in KVM
capabilities") add WAITPKG to KVM_GET_SUPPORTED_CPUID?
> Paolo
>
On 22/10/20 16:28, Xiaoyao Li wrote:
> On 10/22/2020 10:06 PM, Paolo Bonzini wrote:
>> On 22/10/20 15:31, Xiaoyao Li wrote:
>>>>
>>>> It's common for userspace to copy all supported CPUID bits to
>>>> KVM_SET_CPUID2, I don't think this is the right behavior for
>>>> KVM_HINTS_REALTIME.
>>>
>>> It reminds of X86_FEATURE_WAITPKG, which is added to supported CPUID
>>> recently as a fix but QEMU exposes it to guest only when "-overcommit
>>> cpu-pm"
>>
>> WAITPKG is not included in KVM_GET_SUPPORTED_CPUID either. QEMU detects
>> it through the MSR_IA32_UMWAIT register.
>
> Doesn't 0abcc8f65cc2 ("KVM: VMX: enable X86_FEATURE_WAITPKG in KVM
> capabilities") add WAITPKG to KVM_GET_SUPPORTED_CPUID?
You're right, I shouldn't have checked only cpuid.c. :)
Still I think WAITPKG is different, because it's only for userspace and
it's always possible for userspace to do "for(;;)" and burn CPU.
KVM_HINTS_REALTIME is more similar to MONITOR, which is not set.
Paolo
On Thu, Oct 22, 2020 at 6:02 AM Paolo Bonzini <[email protected]> wrote:
>
> On 22/10/20 03:34, Wanpeng Li wrote:
> > From: Wanpeng Li <[email protected]>
> >
> > Per KVM_GET_SUPPORTED_CPUID ioctl documentation:
> >
> > This ioctl returns x86 cpuid features which are supported by both the
> > hardware and kvm in its default configuration.
> >
> > A well-behaved userspace should not set the bit if it is not supported.
> >
> > Suggested-by: Jim Mattson <[email protected]>
> > Signed-off-by: Wanpeng Li <[email protected]>
>
> It's common for userspace to copy all supported CPUID bits to
> KVM_SET_CPUID2, I don't think this is the right behavior for
> KVM_HINTS_REALTIME.
That is not how the API is defined, but I'm sure you know that. :-)
> (But maybe this was discussed already; if so, please point me to the
> previous discussion).
>
> Paolo
On 22/10/20 18:35, Jim Mattson wrote:
> On Thu, Oct 22, 2020 at 6:02 AM Paolo Bonzini <[email protected]> wrote:
>>
>> On 22/10/20 03:34, Wanpeng Li wrote:
>>> From: Wanpeng Li <[email protected]>
>>>
>>> Per KVM_GET_SUPPORTED_CPUID ioctl documentation:
>>>
>>> This ioctl returns x86 cpuid features which are supported by both the
>>> hardware and kvm in its default configuration.
>>>
>>> A well-behaved userspace should not set the bit if it is not supported.
>>>
>>> Suggested-by: Jim Mattson <[email protected]>
>>> Signed-off-by: Wanpeng Li <[email protected]>
>>
>> It's common for userspace to copy all supported CPUID bits to
>> KVM_SET_CPUID2, I don't think this is the right behavior for
>> KVM_HINTS_REALTIME.
>
> That is not how the API is defined, but I'm sure you know that. :-)
Yes, though in my defense :) KVM_HINTS_REALTIME is not a property of the
kernel, it's a property of the environment that the guest runs in. This
was the original reason to separate it from other feature bits in the
same leaf.
Paolo
On Thu, Oct 22, 2020 at 9:37 AM Paolo Bonzini <[email protected]> wrote:
>
> On 22/10/20 18:35, Jim Mattson wrote:
> > On Thu, Oct 22, 2020 at 6:02 AM Paolo Bonzini <[email protected]> wrote:
> >>
> >> On 22/10/20 03:34, Wanpeng Li wrote:
> >>> From: Wanpeng Li <[email protected]>
> >>>
> >>> Per KVM_GET_SUPPORTED_CPUID ioctl documentation:
> >>>
> >>> This ioctl returns x86 cpuid features which are supported by both the
> >>> hardware and kvm in its default configuration.
> >>>
> >>> A well-behaved userspace should not set the bit if it is not supported.
> >>>
> >>> Suggested-by: Jim Mattson <[email protected]>
> >>> Signed-off-by: Wanpeng Li <[email protected]>
> >>
> >> It's common for userspace to copy all supported CPUID bits to
> >> KVM_SET_CPUID2, I don't think this is the right behavior for
> >> KVM_HINTS_REALTIME.
> >
> > That is not how the API is defined, but I'm sure you know that. :-)
>
> Yes, though in my defense :) KVM_HINTS_REALTIME is not a property of the
> kernel, it's a property of the environment that the guest runs in. This
> was the original reason to separate it from other feature bits in the
> same leaf.
>
> Paolo
>
We don't actually use KVM_GET_SUPPORTED_CPUID at all today. If it's
commonly being misinterpreted as you say, perhaps we should add a
KVM_GET_TRUE_SUPPORTED_CPUID ioctl. Or, perhaps we can just fix this
in the documentation?
On 10/22/2020 9:02 PM, Paolo Bonzini wrote:
> On 22/10/20 03:34, Wanpeng Li wrote:
>> From: Wanpeng Li <[email protected]>
>>
>> Per KVM_GET_SUPPORTED_CPUID ioctl documentation:
>>
>> This ioctl returns x86 cpuid features which are supported by both the
>> hardware and kvm in its default configuration.
>>
>> A well-behaved userspace should not set the bit if it is not supported.
>>
>> Suggested-by: Jim Mattson <[email protected]>
>> Signed-off-by: Wanpeng Li <[email protected]>
>
> It's common for userspace to copy all supported CPUID bits to
> KVM_SET_CPUID2, I don't think this is the right behavior for
> KVM_HINTS_REALTIME.
It reminds of X86_FEATURE_WAITPKG, which is added to supported CPUID
recently as a fix but QEMU exposes it to guest only when "-overcommit
cpu-pm"
> (But maybe this was discussed already; if so, please point me to the
> previous discussion).
>
> Paolo
>
On Thu, 22 Oct 2020 at 21:02, Paolo Bonzini <[email protected]> wrote:
>
> On 22/10/20 03:34, Wanpeng Li wrote:
> > From: Wanpeng Li <[email protected]>
> >
> > Per KVM_GET_SUPPORTED_CPUID ioctl documentation:
> >
> > This ioctl returns x86 cpuid features which are supported by both the
> > hardware and kvm in its default configuration.
> >
> > A well-behaved userspace should not set the bit if it is not supported.
> >
> > Suggested-by: Jim Mattson <[email protected]>
> > Signed-off-by: Wanpeng Li <[email protected]>
>
> It's common for userspace to copy all supported CPUID bits to
> KVM_SET_CPUID2, I don't think this is the right behavior for
> KVM_HINTS_REALTIME.
>
> (But maybe this was discussed already; if so, please point me to the
> previous discussion).
The discussion is here. :) https://www.spinics.net/lists/kvm/msg227265.html
Wanpeng
On 22/10/20 19:13, Jim Mattson wrote:
> We don't actually use KVM_GET_SUPPORTED_CPUID at all today. If it's
> commonly being misinterpreted as you say, perhaps we should add a
> KVM_GET_TRUE_SUPPORTED_CPUID ioctl. Or, perhaps we can just fix this
> in the documentation?
Yes, I think we should fix the documentation and document the best
practices around MSRs and CPUID bits. Mostly documenting what QEMU
does, perhaps without all the quirks it has to support old kernels that
messed things up even more.
Paolo
On Fri, Oct 23, 2020 at 2:07 AM Paolo Bonzini <[email protected]> wrote:
>
> On 22/10/20 19:13, Jim Mattson wrote:
> > We don't actually use KVM_GET_SUPPORTED_CPUID at all today. If it's
> > commonly being misinterpreted as you say, perhaps we should add a
> > KVM_GET_TRUE_SUPPORTED_CPUID ioctl. Or, perhaps we can just fix this
> > in the documentation?
>
> Yes, I think we should fix the documentation and document the best
> practices around MSRs and CPUID bits. Mostly documenting what QEMU
> does, perhaps without all the quirks it has to support old kernels that
> messed things up even more.
>
> Paolo
I'd really like to be able to call an ioctl that will help me
determine whether the host can support the guest CPUID information
that I already have (e.g. on the target of a live migration). At first
glance, KVM_GET_SUPPORTED_CPUID appeared to be that ioctl. Sadly, it
appears that no such ioctl exists.