Re: [Xen-devel] [PATCH v2 1/7] x86: Add support for STAC/CLAC instructions

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: Wednesday, April 23, 2014 6:06 PM
> To: Wu, Feng
> Cc: xen-devel@xxxxxxxxxxxxx; JBeulich@xxxxxxxx; Tian, Kevin; Dong, Eddie;
> Nakajima, Jun; ian.campbell@xxxxxxxxxx
> Subject: Re: [PATCH v2 1/7] x86: Add support for STAC/CLAC instructions
> 
> On 23/04/14 15:34, Feng Wu wrote:
> > The STAC/CLAC instructions are only available when SMAP is enabled,
> > but on the other hand they aren't needed if SMAP is not available,
> > or before we start to run userspace, in that case, the functions and
> > macros do nothing.
> >
> > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
> > ---
> >  xen/arch/x86/x86_64/asm-offsets.c |  1 +
> >  xen/include/asm-x86/asm_defns.h   | 43
> +++++++++++++++++++++++++++++++++++++++
> >  xen/include/asm-x86/cpufeature.h  |  4 ++++
> >  3 files changed, 48 insertions(+)
> >
> > diff --git a/xen/arch/x86/x86_64/asm-offsets.c
> b/xen/arch/x86/x86_64/asm-offsets.c
> > index b0098b3..fa4cbb6 100644
> > --- a/xen/arch/x86/x86_64/asm-offsets.c
> > +++ b/xen/arch/x86/x86_64/asm-offsets.c
> > @@ -160,6 +160,7 @@ void __dummy__(void)
> >      BLANK();
> >
> >      OFFSET(CPUINFO86_ext_features, struct cpuinfo_x86,
> x86_capability[1]);
> > +    OFFSET(CPUINFO86_leaf7_features, struct cpuinfo_x86,
> x86_capability[7]);
> >      BLANK();
> >
> >      OFFSET(MB_flags, multiboot_info_t, flags);
> > diff --git a/xen/include/asm-x86/asm_defns.h
> b/xen/include/asm-x86/asm_defns.h
> > index a4601ba..0f18728 100644
> > --- a/xen/include/asm-x86/asm_defns.h
> > +++ b/xen/include/asm-x86/asm_defns.h
> > @@ -7,6 +7,8 @@
> >  #include <asm/asm-offsets.h>
> >  #endif
> >  #include <asm/processor.h>
> > +#include <xen/stringify.h>
> > +#include <asm/cpufeature.h>
> >
> >  #ifndef __ASSEMBLY__
> >  void ret_from_intr(void);
> > @@ -103,4 +105,45 @@ void ret_from_intr(void);
> >
> >  #endif
> >
> > +/* "Raw" instruction opcodes */
> > +#define __ASM_CLAC      .byte 0x0f,0x01,0xca
> > +#define __ASM_STAC      .byte 0x0f,0x01,0xcb
> > +
> > +#ifdef __ASSEMBLY__
> > +#define ASM_AC(op)                                       \
> > +        pushq %rax;                                      \
> > +        leaq boot_cpu_data(%rip), %rax;                  \
> > +        btl $X86_FEATURE_SMAP-7*32, CPUINFO86_leaf7_features(%rax);
> \
> > +        jnc 881f;                                        \
> > +        op;                                              \
> > +881:    popq %rax
> 
> While it is unlikely to change going forwards, $(X86_FEATURE_SMAP & 31)
> looks more obviously correct.

Okay.
> 
> Also, given that boot_cpu_data(%rip) is an assembler know constant
> value, and CPUINFO86_leaf7_features is a constant offset from that, is
> there any way of persuading the assembler to conjoin the two and forgo
> the push, lea and pop.  (I have had a go, but lack sufficient caffeine
> this early in the day)
> 
> ~Andrew

I thought about this idea before posting this version, but I didn't get a good
solution. I will continue to find a better way to handle this.

> 
> > +
> > +#define ASM_STAC ASM_AC(__ASM_STAC)
> > +#define ASM_CLAC ASM_AC(__ASM_CLAC)
> > +#else
> > +#define ASM_AC(op, prefix)
> \
> > +        "\npushq " __stringify(prefix) "rax\n\t"                   \
> > +        "leaq boot_cpu_data(" __stringify(prefix) "rip),"          \
> > +        __stringify(prefix) "rax\n\t"                              \
> > +        "btl $" __stringify(X86_FEATURE_SMAP) "-7*32,"             \
> > +        __stringify(CPUINFO86_leaf7_features) "("                  \
> > +        __stringify(prefix) "rax)\n\t"                             \
> > +        "jnc 881f\n\t"
> \
> > +         __stringify(op) "\n\t"
> \
> > +"881:    popq " __stringify(prefix) "rax"
> > +
> > +#define ASM_STAC(prefix) ASM_AC(__ASM_STAC, prefix)
> > +#define ASM_CLAC(prefix) ASM_AC(__ASM_CLAC, prefix)
> > +
> > +static inline void clac(void)
> > +{
> > +    asm volatile (ASM_CLAC(%));
> > +}
> > +
> > +static inline void stac(void)
> > +{
> > +    asm volatile (ASM_STAC(%));
> > +}
> > +#endif
> > +
> >  #endif /* __X86_ASM_DEFNS_H__ */
> > diff --git a/xen/include/asm-x86/cpufeature.h
> b/xen/include/asm-x86/cpufeature.h
> > index 0c4d6c1..3dfb875 100644
> > --- a/xen/include/asm-x86/cpufeature.h
> > +++ b/xen/include/asm-x86/cpufeature.h
> > @@ -7,7 +7,9 @@
> >  #ifndef __ASM_I386_CPUFEATURE_H
> >  #define __ASM_I386_CPUFEATURE_H
> >
> > +#ifndef __ASSEMBLY__
> >  #include <xen/bitops.h>
> > +#endif
> >
> >  #define NCAPINTS   8       /* N 32-bit words worth of info */
> >
> > @@ -151,6 +153,7 @@
> >  #define X86_FEATURE_ADX            (7*32+19) /* ADCX, ADOX instructions
> */
> >  #define X86_FEATURE_SMAP   (7*32+20) /* Supervisor Mode Access
> Prevention */
> >
> > +#ifndef __ASSEMBLY__
> >  #define cpu_has(c, bit)            test_bit(bit, (c)->x86_capability)
> >  #define boot_cpu_has(bit)  test_bit(bit, boot_cpu_data.x86_capability)
> >  #define cpufeat_mask(idx)       (1u << ((idx) & 31))
> > @@ -209,6 +212,7 @@
> >  #define cpu_has_vmx                boot_cpu_has(X86_FEATURE_VMXE)
> >
> >  #define cpu_has_cpuid_faulting
>       boot_cpu_has(X86_FEATURE_CPUID_FAULTING)
> > +#endif
> >
> >  #endif /* __ASM_I386_CPUFEATURE_H */
> >

Thanks,
Feng

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel