From: "Tian, Kevin" <kevin.tian@intel.com>
To: Like Xu <like.xu.linux@gmail.com>,
Paolo Bonzini <pbonzini@redhat.com>,
"Liu, Jing2" <jing2.liu@intel.com>
Cc: "Zeng, Guang" <guang.zeng@intel.com>, "Christopherson,,
Sean" <seanjc@google.com>,
"tglx@linutronix.de" <tglx@linutronix.de>,
"Wang, Wei W" <wei.w.wang@intel.com>,
"Zhong, Yang" <yang.zhong@intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
kvm list <kvm@vger.kernel.org>
Subject: RE: [PATCH v6 04/21] kvm: x86: Exclude unpermitted xfeatures at KVM_GET_SUPPORTED_CPUID
Date: Mon, 24 Jan 2022 07:18:20 +0000 [thread overview]
Message-ID: <BN9PR11MB5276A817FEC26CAFCCE11CEA8C5E9@BN9PR11MB5276.namprd11.prod.outlook.com> (raw)
In-Reply-To: <75fe2317-4159-adbe-30f8-5bbd2f5e9d11@gmail.com>
> From: Like Xu <like.xu.linux@gmail.com>
> Sent: Sunday, January 23, 2022 2:22 PM
>
> On 8/1/2022 2:54 am, Paolo Bonzini wrote:
> > From: Jing Liu <jing2.liu@intel.com>
> >
> > KVM_GET_SUPPORTED_CPUID should not include any dynamic xstates in
> > CPUID[0xD] if they have not been requested with prctl. Otherwise
> > a process which directly passes KVM_GET_SUPPORTED_CPUID to
> > KVM_SET_CPUID2 would now fail even if it doesn't intend to use a
> > dynamically enabled feature. Userspace must know that prctl is
> > required and allocate >4K xstate buffer before setting any dynamic
> > bit.
> >
> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Jing Liu <jing2.liu@intel.com>
> > Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> > Message-Id: <20220105123532.12586-5-yang.zhong@intel.com>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > Documentation/virt/kvm/api.rst | 4 ++++
> > arch/x86/kvm/cpuid.c | 9 ++++++---
> > 2 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/virt/kvm/api.rst
> b/Documentation/virt/kvm/api.rst
> > index 6b683dfea8f2..f4ea5e41a4d0 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -1687,6 +1687,10 @@ 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).
> >
> > +Dynamically-enabled feature bits need to be requested with
> > +``arch_prctl()`` before calling this ioctl. Feature bits that have not
> > +been requested are excluded from the result.
> > +
> > Note that certain capabilities, such as KVM_CAP_X86_DISABLE_EXITS, may
> > expose cpuid features (e.g. MONITOR) which are not supported by kvm in
> > its default configuration. If userspace enables such capabilities, it
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > index f3e6fda6b858..eb52dde5deec 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -815,11 +815,13 @@ static inline int __do_cpuid_func(struct
> kvm_cpuid_array *array, u32 function)
> > goto out;
> > }
> > break;
> > - case 0xd:
> > - entry->eax &= supported_xcr0;
> > + case 0xd: {
> > + u64 guest_perm = xstate_get_guest_group_perm();
> > +
> > + entry->eax &= supported_xcr0 & guest_perm;
> > entry->ebx = xstate_required_size(supported_xcr0, false);
>
> If we choose to exclude unpermitted xfeatures in the entry->eax, why do
> we choose to expose the size of unpermitted xfeatures in ebx and ecx?
>
> This seems to be an inconsistency, how about:
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 1bd4d560cbdd..193cbf56a5fa 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -888,12 +888,12 @@ static inline int __do_cpuid_func(struct
> kvm_cpuid_array
> *array, u32 function)
> }
> break;
> case 0xd: {
> - u64 guest_perm = xstate_get_guest_group_perm();
> + u64 supported_xcr0 = supported_xcr0 &
> xstate_get_guest_group_perm();
>
> - entry->eax &= supported_xcr0 & guest_perm;
> + entry->eax &= supported_xcr0;
> entry->ebx = xstate_required_size(supported_xcr0, false);
> entry->ecx = entry->ebx;
> - entry->edx &= (supported_xcr0 & guest_perm) >> 32;
> + entry->edx &= supported_xcr0 >> 32;
> if (!supported_xcr0)
> break;
>
> It also helps to fix the CPUID_D_1_EBX and later for (i = 2; i < 64; ++i);
>
> Is there anything I've missed ?
No, you are correct. Would you please send out a formal fix?
>
> > entry->ecx = entry->ebx;
> > - entry->edx &= supported_xcr0 >> 32;
> > + entry->edx &= (supported_xcr0 & guest_perm) >> 32;
> > if (!supported_xcr0)
> > break;
> >
> > @@ -866,6 +868,7 @@ static inline int __do_cpuid_func(struct
> kvm_cpuid_array *array, u32 function)
> > entry->edx = 0;
> > }
> > break;
> > + }
> > case 0x12:
> > /* Intel SGX */
> > if (!kvm_cpu_cap_has(X86_FEATURE_SGX)) {
next prev parent reply other threads:[~2022-01-24 7:22 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-07 18:54 [PATCH v6 00/21] AMX support for KVM Paolo Bonzini
2022-01-07 18:54 ` [PATCH v6 01/21] x86/fpu: Extend fpu_xstate_prctl() with guest permissions Paolo Bonzini
2022-01-07 18:54 ` [PATCH v6 02/21] x86/fpu: Prepare guest FPU for dynamically enabled FPU features Paolo Bonzini
2022-01-07 18:54 ` [PATCH v6 03/21] kvm: x86: Fix xstate_required_size() to follow XSTATE alignment rule Paolo Bonzini
2022-01-07 18:54 ` [PATCH v6 04/21] kvm: x86: Exclude unpermitted xfeatures at KVM_GET_SUPPORTED_CPUID Paolo Bonzini
2022-01-23 6:22 ` Like Xu
2022-01-24 7:18 ` Tian, Kevin [this message]
2022-01-07 18:54 ` [PATCH v6 05/21] x86/fpu: Make XFD initialization in __fpstate_reset() a function argument Paolo Bonzini
2022-01-07 19:43 ` Borislav Petkov
2022-01-10 5:15 ` Tian, Kevin
2022-01-10 8:52 ` Borislav Petkov
2022-01-10 14:18 ` Paolo Bonzini
2022-01-10 15:25 ` Borislav Petkov
2022-01-10 15:55 ` Paolo Bonzini
2022-01-10 18:18 ` Borislav Petkov
2022-01-11 1:45 ` Tian, Kevin
2022-01-07 18:54 ` [PATCH v6 06/21] x86/fpu: Add guest support to xfd_enable_feature() Paolo Bonzini
2022-01-07 18:54 ` [PATCH v6 07/21] x86/fpu: Provide fpu_enable_guest_xfd_features() for KVM Paolo Bonzini
2022-01-10 5:25 ` Tian, Kevin
2022-01-07 18:54 ` [PATCH v6 08/21] kvm: x86: Enable dynamic xfeatures at KVM_SET_CPUID2 Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 09/21] x86/fpu: Provide fpu_update_guest_xfd() for IA32_XFD emulation Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 10/21] kvm: x86: Add emulation for IA32_XFD Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 11/21] x86/fpu: Prepare xfd_err in struct fpu_guest Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 12/21] kvm: x86: Intercept #NM for saving IA32_XFD_ERR Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 13/21] kvm: x86: Emulate IA32_XFD_ERR for guest Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 14/21] kvm: x86: Disable RDMSR interception of IA32_XFD_ERR Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 15/21] kvm: x86: Add XCR0 support for Intel AMX Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 16/21] kvm: x86: Add CPUID " Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 17/21] x86/fpu: Add uabi_size to guest_fpu Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 18/21] kvm: x86: Add support for getting/setting expanded xstate buffer Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 19/21] kvm: selftests: Add support for KVM_CAP_XSAVE2 Paolo Bonzini
2022-01-17 16:51 ` Janis Schoetterl-Glausch
2022-01-18 2:06 ` Wang, Wei W
2022-01-07 18:55 ` [PATCH v6 20/21] x86/fpu: Provide fpu_sync_guest_vmexit_xfd_state() Paolo Bonzini
2022-01-07 18:55 ` [PATCH v6 21/21] kvm: x86: Disable interception for IA32_XFD on demand Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BN9PR11MB5276A817FEC26CAFCCE11CEA8C5E9@BN9PR11MB5276.namprd11.prod.outlook.com \
--to=kevin.tian@intel.com \
--cc=guang.zeng@intel.com \
--cc=jing2.liu@intel.com \
--cc=kvm@vger.kernel.org \
--cc=like.xu.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=tglx@linutronix.de \
--cc=wei.w.wang@intel.com \
--cc=yang.zhong@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).