[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1 1/6] x86: Add support for STAC/CLAC instructions
>>> On 15.04.14 at 15:01, <feng.wu@xxxxxxxxx> wrote: > --- a/xen/include/asm-x86/x86_64/asm_defns.h > +++ b/xen/include/asm-x86/x86_64/asm_defns.h These changes should go into xen/include/asm-x86/asm_defns.h; the x86-64 one should get merged into the previously common one sooner or later, so let's not force that merging patch to be bigger than it needs to be. > @@ -228,4 +228,74 @@ __asm__( \ > # define _ASM_EX(p) #p "-." > #endif > > +/* "Raw" instruction opcodes */ > +#define __ASM_CLAC .byte 0x0f,0x01,0xca > +#define __ASM_STAC .byte 0x0f,0x01,0xcb > + > +/* Indirect stringification. Doing two levels allows the parameter to be a > + * macro itself. For example, compile with -DFOO=bar, __stringify(FOO) > + * converts to "bar". > + */ > +#define __stringify_1(x...) #x > +#define __stringify(x...) __stringify_1(x) Please use xen/stringify.h rather than repeating its defintions. > + > +#ifdef __ASSEMBLY__ > +#define X86_FEATURE_SMAP (7*32+20) No - how would be spot this if we were to re-arrange the feature array? Either use (or make usable) the definitions in cpufeature.h, or propagate the necessary values through asm-offsets.h. > +#define ASM_STAC \ > + pushq %rax; \ > + leaq boot_cpu_data(%rip),%rax; \ > + btl $X86_FEATURE_SMAP-7*32, CPUINFO86_leaf7_features(%rax); \ > + jnc 881f; \ > + movq %cr4,%rax; \ > + testl $X86_CR4_SMAP,%eax; \ > + jz 881f; \ > + __ASM_STAC; \ > +881: popq %rax > + > +#define ASM_CLAC \ > + pushq %rax; \ > + leaq boot_cpu_data(%rip),%rax; \ > + btl $X86_FEATURE_SMAP-7*32, CPUINFO86_leaf7_features(%rax); \ > + jnc 881f; \ > + movq %cr4,%rax; \ > + testl $X86_CR4_SMAP,%eax; \ > + jz 881f; \ > + __ASM_CLAC; \ > +881: popq %rax The only difference between the two macros appears to be the final instruction - please define one macro with an argument, and then make the two definitions here simple wrappers around that macro. That said, the macro contents itself is horrible too: A control register access and two conditional branches in code intended to be used in fast paths? Definitely not an option. Even the simplest possible solution - adding a global flag to be checked here - would already be questionable. Hence I think you should at least consider porting over proper instruction patching abstraction from Linux. > +#else > +#define ASM_STAC \ > + "\npushq %%rax\n\t" \ > + "leaq boot_cpu_data(%%rip),%%rax\n\t" \ > + "btl $" __stringify(X86_FEATURE_SMAP) "-7*32," \ > + __stringify(CPUINFO86_leaf7_features) "(%%rax)\n\t" \ > + "jnc 881f\n\t" \ > + "movq %%cr4,%%rax\n\t" \ > + "testl $" __stringify(X86_CR4_SMAP) ",%%eax\n\t" \ > + "jz 881f\n\t" \ > + __stringify(__ASM_STAC) "\n\t" \ > +"881: popq %%rax" > + > +#define ASM_CLAC \ > + "\npushq %%rax\n\t" \ > + "leaq boot_cpu_data(%%rip),%%rax\n\t" \ > + "btl $" __stringify(X86_FEATURE_SMAP) "-7*32," \ > + __stringify(CPUINFO86_leaf7_features) "(%%rax)\n\t" \ > + "jnc 881f\n\t" \ > + "movq %%cr4,%%rax\n\t" \ > + "testl $" __stringify(X86_CR4_SMAP) ",%%eax\n\t" \ > + "jz 881f\n\t" \ > + __stringify(__ASM_CLAC) "\n\t" \ > +"881: popq %%rax" All the same applies to these of course. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |