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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

Jan


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