On Mon, Jan 28, 2019 at 07:31:10AM -0800, Sean Christopherson wrote:
> On Sun, Jan 27, 2019 at 11:48:47PM +0800, Yang Weijiang wrote:
> > On Fri, Jan 25, 2019 at 02:40:15PM -0800, Sean Christopherson wrote:
> > > On Wed, Jan 23, 2019 at 04:59:05AM +0800, Yang Weijiang wrote:
> > > > CET xsaves component size is queried through CPUID.(EAX=0xD, ECX=11)
> > > > and CPUID.(EAX=0xD, ECX=12).
> > > >
> > > > Signed-off-by: Zhang Yi Z <yi.z.zhang@xxxxxxxxxxxxxxx>
> > > > Signed-off-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
> > > > ---
> > > > arch/x86/kvm/cpuid.c | 60 +++++++++++++++++++++++++++++++++-----------
> > > > arch/x86/kvm/x86.c | 4 +++
> > > > arch/x86/kvm/x86.h | 4 +++
> > > > 3 files changed, 54 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > > > index cb1aece25b17..dbeb4e7904eb 100644
> > > > --- a/arch/x86/kvm/cpuid.c
> > > > +++ b/arch/x86/kvm/cpuid.c
> > > > @@ -27,6 +27,8 @@
> > > > #include "trace.h"
> > > > #include "pmu.h"
> > > >
> > > > +extern u64 host_xss;
> > >
> > > Probably better to put this in x86.h next to host_xcr0.
> > >
> > > > +
> > > > static u32 xstate_required_size(u64 xstate_bv, bool compacted)
> > > > {
> > > > int feature_bit = 0;
> > > > @@ -65,6 +67,19 @@ u64 kvm_supported_xcr0(void)
> > > > return xcr0;
> > > > }
> > > >
> > > > +u64 kvm_supported_xss(void)
> > > > +{
> > > > + u64 xss = host_xss & KVM_SUPPORTED_XSS;
> > > > +
> > > > + /*
> > > > + * Either SHSTK or IBT feature depends on the xsaves component.
> > > > + */
> > > > + if (!boot_cpu_has(X86_FEATURE_SHSTK) && !boot_cpu_has(X86_FEATURE_IBT))
> > > > + xss &= ~(XFEATURE_MASK_SHSTK_USER | XFEATURE_MASK_SHSTK_KERNEL);
> > >
> > > This looks wrong, e.g. the SHSTK bits are allowed for SHSTK=false && IBT=true?
> > >
> > > And isn't this redundant, i.e. if the features aren't supported by the
> > > boot cpu then shouldn't the associated bits be cleared in host_xss?
> > >
> > In current host CET realization, the kernel will clear the SHSTK/IBT bit
> > once it detects they're disabled via cmdline, but before that, the
> > kernel has set XSS based on early cpu capability identification(in
> > boot_cpu_data). so actually, the host XSS state deviates from boot_cpu_data
>
> Right, so aren't those bits already cleared in host_xss?
from what I observed in test, those bits are not cleared in this case.
Do you want the host to clear XSS if CET is not enabled on host?
