| Message ID | 20230914063325.85503-12-weijiang.yang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable CET Virtualization | expand |
On Thu, 2023-09-14 at 02:33 -0400, Yang Weijiang wrote: > From: Sean Christopherson <seanjc@google.com> > > Add MSR_IA32_XSS to list of MSRs reported to userspace if supported_xss > is non-zero, i.e. KVM supports at least one XSS based feature. I can't believe that CET is the first supervisor feature that KVM supports... Ah, now I understand why: 1. XSAVES on AMD can't really be intercepted (other than clearing CR4.OSXSAVE bit, which isn't an option if you want to support AVX for example) On VMX however you can intercept XSAVES and even intercept it only when it touches specific bits of state that you don't want the guest to read/write freely. 2. Even if it was possible to intercept it, guests use XSAVES on every context switch if available and emulating it might be costly. 3. Emulating XSAVES is also not that easy to do correctly. However XSAVES touches various MSRs, thus letting the guest use it unintercepted means giving access to host MSRs, which might be wrong security wise in some cases. Thus I see that KVM hardcodes the IA32_XSS to 0, and that makes the XSAVES work exactly like XSAVE. And for some features which would benefit from XSAVES state components, KVM likely won't even be able to do so due to this limitation. (this is allowed thankfully by the CPUID), forcing the guests to use rdmsr/wrmsr instead. However it is possible to enable IA32_XSS bits in case the msrs XSAVES reads/writes can't do harm to the host, and then KVM can context switch these MSRs when the guest exits and that is what is done here with CET. If you think that a short summary of the above can help the future reader to understand why IA32_XSS support is added only now, it might be a good idea to add a few lines to the changelog of this patch. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > --- > arch/x86/kvm/x86.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index e0b55c043dab..1258d1d6dd52 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -1464,6 +1464,7 @@ static const u32 msrs_to_save_base[] = { > MSR_IA32_UMWAIT_CONTROL, > > MSR_IA32_XFD, MSR_IA32_XFD_ERR, > + MSR_IA32_XSS, > }; > > static const u32 msrs_to_save_pmu[] = { > @@ -7195,6 +7196,10 @@ static void kvm_probe_msr_to_save(u32 msr_index) > if (!(kvm_get_arch_capabilities() & ARCH_CAP_TSX_CTRL_MSR)) > return; > break; > + case MSR_IA32_XSS: > + if (!kvm_caps.supported_xss) > + return; > + break; > default: > break; > } Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky
On Tue, Oct 31, 2023, Maxim Levitsky wrote: > On Thu, 2023-09-14 at 02:33 -0400, Yang Weijiang wrote: > > From: Sean Christopherson <seanjc@google.com> > > > > Add MSR_IA32_XSS to list of MSRs reported to userspace if supported_xss > > is non-zero, i.e. KVM supports at least one XSS based feature. > > > I can't believe that CET is the first supervisor feature that KVM supports... > > Ah, now I understand why: > > 1. XSAVES on AMD can't really be intercepted (other than clearing CR4.OSXSAVE > bit, which isn't an option if you want to support AVX for example) On VMX > however you can intercept XSAVES and even intercept it only when it touches > specific bits of state that you don't want the guest to read/write freely. > > 2. Even if it was possible to intercept it, guests use XSAVES on every > context switch if available and emulating it might be costly. > > 3. Emulating XSAVES is also not that easy to do correctly. > > However XSAVES touches various MSRs, thus letting the guest use it > unintercepted means giving access to host MSRs, which might be wrong security > wise in some cases. > > Thus I see that KVM hardcodes the IA32_XSS to 0, and that makes the XSAVES > work exactly like XSAVE. > > And for some features which would benefit from XSAVES state components, > KVM likely won't even be able to do so due to this limitation. > (this is allowed thankfully by the CPUID), forcing the guests to use > rdmsr/wrmsr instead. Sort of? KVM doesn't (yet) virtualize PASID, HDC, HWP, or arch LBRs (wow, there's a lot of stuff getting thrown into XSTATE), so naturally those aren't supported in XSS. KVM does virtualize Processor Trace (PT), but PT is a bit of a special snowflake. E.g. the host kernel elects NOT to manage PT MSRs via XSTATE, but it would be possible for KVM to the guest to manage PT MSRs via XSTATE. I suspect the answer to PT is threefold: 1. Exposing a feature that isn't "supported" by the host kernel is scary. 2. No one has pushed for the support, e.g. Linux guests obviously don't complain about lack of XSS support for PT. 3. Toggling PT MSR passthrough on XSAVES/XRSTORS accesses would be more complex and less performant than KVM's current approach. Re: #3, KVM does passthrough PT MSRs, but only when the guest is actively using PT. PT is basically a super fancy PMU feature, and so KVM "needs" to load guest state as late as possible before VM-Entry, and load host state as early as possible after VM-Exit. I.e. the context switch happens on *every* entry/exit pair. By passing through PT MSRs only when needed, KVM avoids a rather large pile of RDMSRs and WRMSRs on every entry/exit, as the host values can be kept resident in hardware so long as the main enable bit is cleared in the guest's control MSR (which is context switch via a dedicated VMCS field). XSAVES isn't subject to MSR intercepts, but KVM could utilize VMX's XSS-exiting bitmap to effectively intercept reads and writes to PT MSRs. Except that as you note, KVM would either need to emulate XSAVES (oof) or save/load PT MSRs much more frequently. So it's kind of an emulation thing, but I honestly doubt that emulating XSAVES was ever seriously considered when KVM support for PT was added. CET is different than PT because the MSRs that need to be context switched at every entry/exit have dedicated VMCS fields. The IA32_PLx_SSP MSRs don't have VMCS fields, but they are consumed only in privelege level changes, i.e. can be safely deferred until guest "FPU" state is put. > However it is possible to enable IA32_XSS bits in case the msrs XSAVES > reads/writes can't do harm to the host, and then KVM can context switch these > MSRs when the guest exits and that is what is done here with CET. This isn't really true. It's not a safety or correctness issue so much as it's a performance issue. E.g. KVM could let the guest use XSS for any virtualized feature, but it would effectively require context switching related state that the host needs loaded "immediately" after VM-Exit. And for MSRs, that gets very expensive without dedicated VMCS fields. I mean, yeah, it's a correctness thing to not consume guest state in the host and vice versa, but that's not unique to XSS in any way.
On Wed, 2023-11-01 at 12:18 -0700, Sean Christopherson wrote: > On Tue, Oct 31, 2023, Maxim Levitsky wrote: > > On Thu, 2023-09-14 at 02:33 -0400, Yang Weijiang wrote: > > > From: Sean Christopherson <seanjc@google.com> > > > > > > Add MSR_IA32_XSS to list of MSRs reported to userspace if supported_xss > > > is non-zero, i.e. KVM supports at least one XSS based feature. > > > > I can't believe that CET is the first supervisor feature that KVM supports... > > > > Ah, now I understand why: > > > > 1. XSAVES on AMD can't really be intercepted (other than clearing CR4.OSXSAVE > > bit, which isn't an option if you want to support AVX for example) On VMX > > however you can intercept XSAVES and even intercept it only when it touches > > specific bits of state that you don't want the guest to read/write freely. > > > > 2. Even if it was possible to intercept it, guests use XSAVES on every > > context switch if available and emulating it might be costly. > > > > 3. Emulating XSAVES is also not that easy to do correctly. > > > > However XSAVES touches various MSRs, thus letting the guest use it > > unintercepted means giving access to host MSRs, which might be wrong security > > wise in some cases. > > > > Thus I see that KVM hardcodes the IA32_XSS to 0, and that makes the XSAVES > > work exactly like XSAVE. > > > > And for some features which would benefit from XSAVES state components, > > KVM likely won't even be able to do so due to this limitation. > > (this is allowed thankfully by the CPUID), forcing the guests to use > > rdmsr/wrmsr instead. > > Sort of? KVM doesn't (yet) virtualize PASID, HDC, HWP, or arch LBRs (wow, > there's a lot of stuff getting thrown into XSTATE), so naturally those aren't > supported in XSS. > > KVM does virtualize Processor Trace (PT), but PT is a bit of a special snowflake. > E.g. the host kernel elects NOT to manage PT MSRs via XSTATE, but it would be > possible for KVM to the guest to manage PT MSRs via XSTATE. I must also note that PT doesn't always uses guest physical addresses to write its trace output, because there is a secondary execution control 'Intel PT uses guest physical addresses', however I see that KVM requires it, so yes, we could likely have supported PT xsaves component. > > I suspect the answer to PT is threefold: > > 1. Exposing a feature that isn't "supported" by the host kernel is scary. > 2. No one has pushed for the support, e.g. Linux guests obviously don't complain > about lack of XSS support for PT. > 3. Toggling PT MSR passthrough on XSAVES/XRSTORS accesses would be more complex > and less performant than KVM's current approach. > > Re: #3, KVM does passthrough PT MSRs, but only when the guest is actively using > PT. PT is basically a super fancy PMU feature, and so KVM "needs" to load guest > state as late as possible before VM-Entry, and load host state as early as possible > after VM-Exit. I.e. the context switch happens on *every* entry/exit pair. > Makes sense. > By passing through PT MSRs only when needed, KVM avoids a rather large pile of > RDMSRs and WRMSRs on every entry/exit, as the host values can be kept resident in > hardware so long as the main enable bit is cleared in the guest's control MSR > (which is context switch via a dedicated VMCS field). > > XSAVES isn't subject to MSR intercepts, but KVM could utilize VMX's XSS-exiting > bitmap to effectively intercept reads and writes to PT MSRs. Except that as you > note, KVM would either need to emulate XSAVES (oof) or save/load PT MSRs much more > frequently. > > So it's kind of an emulation thing, but I honestly doubt that emulating XSAVES > was ever seriously considered when KVM support for PT was added. > > CET is different than PT because the MSRs that need to be context switched at > every entry/exit have dedicated VMCS fields. The IA32_PLx_SSP MSRs don't have > VMCS fields, but they are consumed only in privelege level changes, i.e. can be > safely deferred until guest "FPU" state is put. > > > However it is possible to enable IA32_XSS bits in case the msrs XSAVES > > reads/writes can't do harm to the host, and then KVM can context switch these > > MSRs when the guest exits and that is what is done here with CET. > > This isn't really true. It's not a safety or correctness issue so much as it's > a performance issue. True as well, I haven't thought about it from this POV. > E.g. KVM could let the guest use XSS for any virtualized > feature, but it would effectively require context switching related state that > the host needs loaded "immediately" after VM-Exit. And for MSRs, that gets > very expensive without dedicated VMCS fields. Yes, unless allowing setting a MSR via xrstors causes harm to the host, (for example msr that has a physical address in it). Such MSRs cannot be allowed to be set by the guest even for the duration of the guest run, and that means that we cannot pass through the corresponding XSS state component. > > I mean, yeah, it's a correctness thing to not consume guest state in the host > and vice versa, but that's not unique to XSS in any way. > Best regards, Maxim Levitsky
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index e0b55c043dab..1258d1d6dd52 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1464,6 +1464,7 @@ static const u32 msrs_to_save_base[] = { MSR_IA32_UMWAIT_CONTROL, MSR_IA32_XFD, MSR_IA32_XFD_ERR, + MSR_IA32_XSS, }; static const u32 msrs_to_save_pmu[] = { @@ -7195,6 +7196,10 @@ static void kvm_probe_msr_to_save(u32 msr_index) if (!(kvm_get_arch_capabilities() & ARCH_CAP_TSX_CTRL_MSR)) return; break; + case MSR_IA32_XSS: + if (!kvm_caps.supported_xss) + return; + break; default: break; }