On Wed, Jul 27, 2022, Paolo Bonzini wrote:
> On 7/26/22 19:32, Sean Christopherson wrote:
> > On Tue, Jul 26, 2022, Paolo Bonzini wrote:
> > > The "BIT" macro cannot be used in top-level assembly statements
> > > (it can be used in functions through the "i" constraint). To
> > > avoid having to hard-code EFLAGS.AC being bit 18, define the
> > > constants for CR0, CR4 and EFLAGS bits in terms of new macros
> > > for just the bit number.
> > >
> > > Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > > ---
> >
> > ...
> >
> > > diff --git a/x86/smap.c b/x86/smap.c
> > > index 0994c29..3f63ee1 100644
> > > --- a/x86/smap.c
> > > +++ b/x86/smap.c
> > > @@ -39,7 +39,7 @@ asm ("pf_tss:\n"
> > > #endif
> > > "add $"S", %"R "sp\n"
> > > #ifdef __x86_64__
> > > - "orl $" xstr(X86_EFLAGS_AC) ", 2*"S"(%"R "sp)\n" // set EFLAGS.AC and retry
> >
> > I don't understand, this compiles cleanly on both gcc and clang, and generates the
> > correct code. What am I missing?
>
> I saw a failure on older binutils where 1UL is not accepted by the
> assembler.
Mostly out of curiosity, how old? I thought I was running some ancient crud in some
of my VMs, but even they play nice with this.
> An alternative is to have some kind of __ASSEMBLY__ symbol as in Linux.
I've no objection to this approach, but can you reword the changelog to call out
that it's only older binutils that's problematic? I was truly confused by the
"cannot be used" blurb.
And a nit, add spaces around the shift (largely because they're needed around the
string), e.g.
"orl $(1 << " xstr(X86_EFLAGS_AC_BIT) "), 2*"S"(%"R "sp)\n" // set EFLAGS.AC and retry
I'm indifferent about the lack of spaces for the existing multiplication, I just
found the shift a little hard to read.
