Re: [PATCH v2] KVM: VMX: Improve handle_external_interrupt_irqoff inline assembly

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Mon, Apr 27, 2020 at 10:08:01PM +0200, Uros Bizjak wrote:
> On Mon, Apr 27, 2020 at 9:25 PM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> >
> > On Sun, Apr 26, 2020 at 01:52:55PM +0200, Uros Bizjak wrote:
> > > Improve handle_external_interrupt_irqoff inline assembly in several ways:
> > > - use "n" operand constraint instead of "i" and remove
> >
> > What's the motivation for using 'n'?  The 'i' variant is much more common,
> > i.e. less likely to trip up readers.
> >
> >   $ git grep -E "\"i\"\s*\(" | wc -l
> >   768
> >   $ git grep -E "\"n\"\s*\(" | wc -l
> >   11

...

> PUSH is able to use "i" here, since the operand is word wide. But, do
> we really want to allow symbol references and labels here?

No, but on the other hand, I doubt this particular code is going to change
much.  I don't have a strong preference.

> > >   unneeded %c operand modifiers and "$" prefixes
> > > - use %rsp instead of _ASM_SP, since we are in CONFIG_X86_64 part
> > > - use $-16 immediate to align %rsp
> >
> > Heh, this one depends on the reader, I find 0xfffffffffffffff0 to be much
> > more intuitive, though admittedly also far easier to screw up.
> 
> I was beaten by this in the past ... but don't want to bikeshed here.

I'm good with either approach.  Same as above, the argument for keeping the
existing code is that it's there, it works, and from some people it's more
readable.