Re: [PATCH kvm-unit-tests] x86: add and use *_BIT constants for CR0, CR4, EFLAGS

[Sean Christopherson <seanjc@google.com>]

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@redhat.com>
> > > ---
> > 
> > ...
> > 
> > > 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.