Re: [PATCH v4 03/39] x86/cpufeatures: Add CPU feature flags for shadow stacks

[Edgecombe, Rick P]

On Wed, 2022-12-07 at 12:00 +0100, Borislav Petkov wrote:
> On Fri, Dec 02, 2022 at 04:35:30PM -0800, Rick Edgecombe wrote:
> > diff --git a/arch/x86/include/asm/cpufeatures.h
> > b/arch/x86/include/asm/cpufeatures.h
> > index 11a0e06362e4..aab7fa4104d7 100644
> > --- a/arch/x86/include/asm/cpufeatures.h
> > +++ b/arch/x86/include/asm/cpufeatures.h
> > @@ -307,6 +307,7 @@
> >   #define X86_FEATURE_SGX_EDECCSSA     (11*32+18) /* "" SGX
> > EDECCSSA user leaf function */
> >   #define X86_FEATURE_CALL_DEPTH               (11*32+19) /* ""
> > Call depth tracking for RSB stuffing */
> >   #define X86_FEATURE_MSR_TSX_CTRL     (11*32+20) /* "" MSR
> > IA32_TSX_CTRL (Intel) implemented */
> > +#define X86_FEATURE_USER_SHSTK               (11*32+21) /* Shadow
> > stack support for user mode applications */
> >   
> >   /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX),
> > word 12 */
> >   #define X86_FEATURE_AVX_VNNI         (12*32+ 4) /* AVX VNNI
> > instructions */
> > @@ -369,6 +370,7 @@
> >   #define X86_FEATURE_OSPKE            (16*32+ 4) /* OS Protection
> > Keys Enable */
> >   #define X86_FEATURE_WAITPKG          (16*32+ 5) /*
> > UMONITOR/UMWAIT/TPAUSE Instructions */
> >   #define X86_FEATURE_AVX512_VBMI2     (16*32+ 6) /* Additional
> > AVX512 Vector Bit Manipulation Instructions */
> > +#define X86_FEATURE_SHSTK            (16*32+ 7) /* Shadow Stack */
> >   #define X86_FEATURE_GFNI             (16*32+ 8) /* Galois Field
> > New Instructions */
> >   #define X86_FEATURE_VAES             (16*32+ 9) /* Vector AES */
> >   #define X86_FEATURE_VPCLMULQDQ               (16*32+10) /* Carry-
> > Less Multiplication Double Quadword */
> 
> What is the end goal here?
> 
> To have X86_FEATURE_KERNEL_SHSTK or so someday too?
> 
> If so, then the hardware bit X86_FEATURE_SHSTK should be hidden in
> /proc/cpuinfo, i.e.,
> 
> #define X86_FEATURE_SHSTK            (16*32+ 7) /* "" Shadow Stack
> hardware support */
> 
> note the "", otherwise you'll have people go:
> 
> "hey, I have "shstk" in /proc/cpuinfo on my machine. Why doesn't it
> do
> anything?"
> 
> Or am I misreading where this is headed?

Yes, the suggestion was to have one for kernel and one for user. But I
was also thinking about how KVM could hypothetically support shadow
stack in guests in the non !CONFIG_X86_USER_SHADOW_STACK case (it only
needs CET_U xsave support). So that configuration wouldn't expose
user_shstk and since KVM's guest feature support is retrieved
programmatically, it could be nice to have some hint for KVM users that
they could try. Maybe it's simpler to just tie KVM and host support
together though. I'll remove "shstk".