Re: [PATCH v5 04/17] x86/acrn: Introduce hypercall interfaces

From: Segher Boessenkool
Date: Mon Nov 02 2020 - 18:24:45 EST


On Mon, Nov 02, 2020 at 11:54:39PM +0100, Borislav Petkov wrote:
> On Mon, Nov 02, 2020 at 02:01:13PM -0600, Segher Boessenkool wrote:
> > It just asks the general_operand function, which (for registers) accepts
> > the hard registers that are accessible. This does include the float and
> > vector etc. registers, normally.
> >
> > But you usually have a pseudo-register there (which is always allowed
> > here), unless you used some register asm variable.
>
> You mean like this:
>
> ---
> int main(void)
> {
> register float foo asm ("xmm0") = 0.99f;
>
> asm volatile("movl %0, %%r8d\n\t"
> "vmcall\n\t"
> :: "g" (foo));
>
> return 0;
> }
> ---
>
> That works ok AFAICT:
>
> ---
>
> 0000000000000000 <main>:
> 0: 55 push %rbp
> 1: 48 89 e5 mov %rsp,%rbp
> 4: f3 0f 10 05 00 00 00 movss 0x0(%rip),%xmm0 # c <main+0xc>
> b: 00
> c: 66 0f 7e c0 movd %xmm0,%eax
> 10: 41 89 c0 mov %eax,%r8d
> 13: 0f 01 c1 vmcall
> 16: b8 00 00 00 00 mov $0x0,%eax
> 1b: 5d pop %rbp
> 1c: c3 retq
>
> ---

That is invalid actually: local register asm as input to an inline asm
should use *that* register!

This is all correct until LRA ("reload"). Not that "movl %xmm0,$eax"
works, but at least it screams its head off, as it should. And then LRA
puts it in %eax, a different register than asked for.

> gcc smartypants shuffles it through a GPR before sticking it into %r8.
>
> It works too If I use a float immediate:
>
> ---
> int main(void)
> {
> asm volatile("movl %0, %%r8d\n\t"
> "vmcall\n\t"
> :: "g" (0.99f));
>
> return 0;
> }
> ---
>
> ---
> 0000000000000000 <main>:
> 0: 55 push %rbp
> 1: 48 89 e5 mov %rsp,%rbp
> 4: 41 b8 a4 70 7d 3f mov $0x3f7d70a4,%r8d
> a: 0f 01 c1 vmcall
> d: b8 00 00 00 00 mov $0x0,%eax
> 12: 5d pop %rbp
> 13: c3 retq
> ---

(this one is correct code)

> or maybe I'm missing some freaky way to specify the input operand so
> that I can make it take a float register. But even if I could, it would
> error out when generating the asm, I presume, due to invalid insn or
> whatnot.

Yes. But GCC doing what you should have said instead of doing what you
said, is not good.

> > And pseudos usually are allocated a simple integer register during
> > register allocation, in an asm that is.
>
> Interesting.
>
> > > Might even make people copying from bad examples
> > > to go look at the docs first...
> >
> > Optimism is cool :-)
>
> In my experience so far, documenting stuff better might not always bring
> the expected results but sometimes it does move people in the most
> unexpected way and miracles happen.
>
> :-)))

Now if only we had time to document what we wrote! We *do* write docs
to go with new code (maybe not enough always), but no one spends a lot
of time on documenting the existing compiler, or with a larger view than
just a single aspect of the compiler. Alas.


Segher