[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 1/7] x86: Add support for STAC/CLAC instructions
On 25/04/14 09:51, Wu, Feng wrote: > >> -----Original Message----- >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx] >> Sent: Wednesday, April 23, 2014 8:46 PM >> To: Wu, Feng >> Cc: andrew.cooper3@xxxxxxxxxx; ian.campbell@xxxxxxxxxx; Dong, Eddie; >> Nakajima, Jun; Tian, Kevin; xen-devel@xxxxxxxxxxxxx >> Subject: RE: [PATCH v2 1/7] x86: Add support for STAC/CLAC instructions >> >>>>> On 23.04.14 at 14:32, <feng.wu@xxxxxxxxx> wrote: >>>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx] >>>>>>> On 23.04.14 at 16:34, <feng.wu@xxxxxxxxx> wrote: >>>>> +/* "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 >>>> So why are you pushing/popping %rax here? There's no need for the >>>> lea. >>>> >>>> And the hard coded 7 here should be replaced too; I don't see a need >>>> for CPUINFO86_leaf7_features either - just calculate everything you >>>> need from X86_FEATURE_SMAP (these are all constants, so other than >>>> the expression getting a little long there's nothing keeping this from >>>> being a single btl). >>> In my understanding, CPUINFO86_leaf7_features is the offset for >>> x86_capability[i] in struct cpuinfo_x86{}, seems we cannot get the >>> right offset only from X86_FEATURE_SMAP? >> Of course you need to add in the offset of x86_capability[]. >> See my other reply just sent. >> >>>>> +#define ASM_STAC(prefix) ASM_AC(__ASM_STAC, prefix) >>>>> +#define ASM_CLAC(prefix) ASM_AC(__ASM_CLAC, prefix) >>>> What is "prefix" good for here, i.e. why can't you put the % right >>>> in the macro? >>> Because this macro will be used in the basic inline assembly (use "%" as the >>> register prefix) >>> and extended assembly (use "%%" as the register prefix). >> Perhaps worth avoiding the basic uses then, by converting them to >> extended? Passing these % or %% to the macro looks rather ugly, >> so if the suggestion isn't viable, some other trick can certainly be >> found to avoid this. > Need to add CLAC in the beginning of interrupt in the following macro, which > uses > the basic inline assembly, seems it is hard to convert this one to extended > format. I > have been thinking about this for some time and tried several method, but I > am kind of > run out of ideas about it. Jan, do you have any suggestion about this? What do you mean about "basic" and "extended" format ? > > Thanks very much in advance! > > #define BUILD_COMMON_IRQ() \ > __asm__( \ > "\n" __ALIGN_STR"\n" \ > "common_interrupt:\n\t" \ > STR(SAVE_ALL) "\n\t" \ > "movq %rsp,%rdi\n\t" \ > "callq " STR(do_IRQ) "\n\t" \ > "jmp ret_from_intr\n"); Independently of the SMAP question, this code chunk would probably be better living in in entry.S than as a macro in a header file. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |