From: Shuo Liu <[email protected]>
The Service VM communicates with the hypervisor via conventional
hypercalls. VMCALL instruction is used to make the hypercalls.
ACRN hypercall ABI:
* Hypercall number is in R8 register.
* Up to 2 parameters are in RDI and RSI registers.
* Return value is in RAX register.
Introduce the ACRN hypercall interfaces. Because GCC doesn't support R8
register as direct register constraints, use supported constraint as
input with a explicit MOV to R8 in beginning of asm.
Originally-by: Yakui Zhao <[email protected]>
Signed-off-by: Shuo Liu <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Sean Christopherson <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Fengwei Yin <[email protected]>
Cc: Zhi Wang <[email protected]>
Cc: Zhenyu Wang <[email protected]>
Cc: Yu Wang <[email protected]>
Cc: Reinette Chatre <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Arvind Sankar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Segher Boessenkool <[email protected]>
---
arch/x86/include/asm/acrn.h | 54 +++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/arch/x86/include/asm/acrn.h b/arch/x86/include/asm/acrn.h
index a2d4aea3a80d..03e420245505 100644
--- a/arch/x86/include/asm/acrn.h
+++ b/arch/x86/include/asm/acrn.h
@@ -14,4 +14,58 @@ void acrn_setup_intr_handler(void (*handler)(void));
void acrn_remove_intr_handler(void);
bool acrn_is_privileged_vm(void);
+/*
+ * Hypercalls for ACRN
+ *
+ * - VMCALL instruction is used to implement ACRN hypercalls.
+ * - ACRN hypercall ABI:
+ * - Hypercall number is passed in R8 register.
+ * - Up to 2 arguments are passed in RDI, RSI.
+ * - Return value will be placed in RAX.
+ *
+ * Because GCC doesn't support R8 register as direct register constraints, use
+ * supported constraint as input with a explicit MOV to R8 in beginning of asm.
+ */
+static inline long acrn_hypercall0(unsigned long hcall_id)
+{
+ long result;
+
+ asm volatile("movl %1, %%r8d\n\t"
+ "vmcall\n\t"
+ : "=a" (result)
+ : "ir" (hcall_id)
+ : "r8", "memory");
+
+ return result;
+}
+
+static inline long acrn_hypercall1(unsigned long hcall_id,
+ unsigned long param1)
+{
+ long result;
+
+ asm volatile("movl %1, %%r8d\n\t"
+ "vmcall\n\t"
+ : "=a" (result)
+ : "ir" (hcall_id), "D" (param1)
+ : "r8", "memory");
+
+ return result;
+}
+
+static inline long acrn_hypercall2(unsigned long hcall_id,
+ unsigned long param1,
+ unsigned long param2)
+{
+ long result;
+
+ asm volatile("movl %1, %%r8d\n\t"
+ "vmcall\n\t"
+ : "=a" (result)
+ : "ir" (hcall_id), "D" (param1), "S" (param2)
+ : "r8", "memory");
+
+ return result;
+}
+
#endif /* _ASM_X86_ACRN_H */
--
2.28.0
On Sun, Oct 18, 2020 at 11:18 PM <[email protected]> wrote:
>
> From: Shuo Liu <[email protected]>
>
> The Service VM communicates with the hypervisor via conventional
> hypercalls. VMCALL instruction is used to make the hypercalls.
>
> ACRN hypercall ABI:
> * Hypercall number is in R8 register.
> * Up to 2 parameters are in RDI and RSI registers.
> * Return value is in RAX register.
>
> Introduce the ACRN hypercall interfaces. Because GCC doesn't support R8
> register as direct register constraints, use supported constraint as
> input with a explicit MOV to R8 in beginning of asm.
Reviewed-by: Nick Desaulniers <[email protected]>
>
> Originally-by: Yakui Zhao <[email protected]>
> Signed-off-by: Shuo Liu <[email protected]>
> Reviewed-by: Reinette Chatre <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Sean Christopherson <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Fengwei Yin <[email protected]>
> Cc: Zhi Wang <[email protected]>
> Cc: Zhenyu Wang <[email protected]>
> Cc: Yu Wang <[email protected]>
> Cc: Reinette Chatre <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Arvind Sankar <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Segher Boessenkool <[email protected]>
> ---
> arch/x86/include/asm/acrn.h | 54 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/arch/x86/include/asm/acrn.h b/arch/x86/include/asm/acrn.h
> index a2d4aea3a80d..03e420245505 100644
> --- a/arch/x86/include/asm/acrn.h
> +++ b/arch/x86/include/asm/acrn.h
> @@ -14,4 +14,58 @@ void acrn_setup_intr_handler(void (*handler)(void));
> void acrn_remove_intr_handler(void);
> bool acrn_is_privileged_vm(void);
>
> +/*
> + * Hypercalls for ACRN
> + *
> + * - VMCALL instruction is used to implement ACRN hypercalls.
> + * - ACRN hypercall ABI:
> + * - Hypercall number is passed in R8 register.
> + * - Up to 2 arguments are passed in RDI, RSI.
> + * - Return value will be placed in RAX.
> + *
> + * Because GCC doesn't support R8 register as direct register constraints, use
> + * supported constraint as input with a explicit MOV to R8 in beginning of asm.
> + */
> +static inline long acrn_hypercall0(unsigned long hcall_id)
> +{
> + long result;
> +
> + asm volatile("movl %1, %%r8d\n\t"
> + "vmcall\n\t"
> + : "=a" (result)
> + : "ir" (hcall_id)
> + : "r8", "memory");
> +
> + return result;
> +}
> +
> +static inline long acrn_hypercall1(unsigned long hcall_id,
> + unsigned long param1)
> +{
> + long result;
> +
> + asm volatile("movl %1, %%r8d\n\t"
> + "vmcall\n\t"
> + : "=a" (result)
> + : "ir" (hcall_id), "D" (param1)
> + : "r8", "memory");
> +
> + return result;
> +}
> +
> +static inline long acrn_hypercall2(unsigned long hcall_id,
> + unsigned long param1,
> + unsigned long param2)
> +{
> + long result;
> +
> + asm volatile("movl %1, %%r8d\n\t"
> + "vmcall\n\t"
> + : "=a" (result)
> + : "ir" (hcall_id), "D" (param1), "S" (param2)
> + : "r8", "memory");
> +
> + return result;
> +}
> +
> #endif /* _ASM_X86_ACRN_H */
> --
> 2.28.0
>
--
Thanks,
~Nick Desaulniers
On Tue, Oct 20, 2020 at 10:30:17AM +0800, Shuo A Liu wrote:
> On Mon 19.Oct'20 at 22:08:51 -0400, Arvind Sankar wrote:
> >On Tue, Oct 20, 2020 at 09:38:09AM +0800, Shuo A Liu wrote:
> >> On Mon 19.Oct'20 at 18:15:15 -0400, Arvind Sankar wrote:
> >> >On Mon, Oct 19, 2020 at 02:17:50PM +0800, [email protected] wrote:
> >> >> From: Shuo Liu <[email protected]>
> >> >>
> >> >> The Service VM communicates with the hypervisor via conventional
> >> >> hypercalls. VMCALL instruction is used to make the hypercalls.
> >> >>
> >> >> ACRN hypercall ABI:
> >> >> * Hypercall number is in R8 register.
> >> >> * Up to 2 parameters are in RDI and RSI registers.
> >> >> * Return value is in RAX register.
> >> >>
> >> >> Introduce the ACRN hypercall interfaces. Because GCC doesn't support R8
> >> >> register as direct register constraints, use supported constraint as
> >> >> input with a explicit MOV to R8 in beginning of asm.
> >> >>
> >> >> +static inline long acrn_hypercall0(unsigned long hcall_id)
> >> >> +{
> >> >> + long result;
> >> >> +
> >> >> + asm volatile("movl %1, %%r8d\n\t"
> >> >> + "vmcall\n\t"
> >> >> + : "=a" (result)
> >> >> + : "ir" (hcall_id)
> >> >
> >> >Is the hypercall id an unsigned long (64 bits) or an unsigned int (32
> >> >bits)? This will generate broken assembly if the "r" option is chosen,
> >> >eg something like
> >> > movl %rdi, %r8d
> >>
> >> Yes, it can be an unsigned long. So do MOV explicitly.
> >>
> >> asm volatile("movq %1, %%r8\n\t"
> >> "vmcall\n\t"
> >> : "=a" (result)
> >> : "ir" (hcall_id)
> >>
> >> Thanks
> >
> >All the hypercall ID's defined seem to be only 32 bits though?
>
> Yes, they are.
> The paramter is unsigned long, use movq to align it.
I don't understand what you mean by alignment here, but I was asking why
hcall_id is unsigned long and not unsigned int (or u32) if you only need
32 bits?
On Mon, Oct 19, 2020 at 02:17:50PM +0800, [email protected] wrote:
> From: Shuo Liu <[email protected]>
>
> The Service VM communicates with the hypervisor via conventional
> hypercalls. VMCALL instruction is used to make the hypercalls.
>
> ACRN hypercall ABI:
> * Hypercall number is in R8 register.
> * Up to 2 parameters are in RDI and RSI registers.
> * Return value is in RAX register.
>
> Introduce the ACRN hypercall interfaces. Because GCC doesn't support R8
> register as direct register constraints, use supported constraint as
> input with a explicit MOV to R8 in beginning of asm.
>
> +static inline long acrn_hypercall0(unsigned long hcall_id)
> +{
> + long result;
> +
> + asm volatile("movl %1, %%r8d\n\t"
> + "vmcall\n\t"
> + : "=a" (result)
> + : "ir" (hcall_id)
Is the hypercall id an unsigned long (64 bits) or an unsigned int (32
bits)? This will generate broken assembly if the "r" option is chosen,
eg something like
movl %rdi, %r8d
> + : "r8", "memory");
> +
> + return result;
> +}
> +
> +static inline long acrn_hypercall1(unsigned long hcall_id,
> + unsigned long param1)
> +{
> + long result;
> +
> + asm volatile("movl %1, %%r8d\n\t"
> + "vmcall\n\t"
> + : "=a" (result)
> + : "ir" (hcall_id), "D" (param1)
> + : "r8", "memory");
> +
> + return result;
> +}
> +
> +static inline long acrn_hypercall2(unsigned long hcall_id,
> + unsigned long param1,
> + unsigned long param2)
> +{
> + long result;
> +
> + asm volatile("movl %1, %%r8d\n\t"
> + "vmcall\n\t"
> + : "=a" (result)
> + : "ir" (hcall_id), "D" (param1), "S" (param2)
> + : "r8", "memory");
> +
> + return result;
> +}
> +
> #endif /* _ASM_X86_ACRN_H */
> --
> 2.28.0
>
On Tue, Oct 20, 2020 at 09:38:09AM +0800, Shuo A Liu wrote:
> On Mon 19.Oct'20 at 18:15:15 -0400, Arvind Sankar wrote:
> >On Mon, Oct 19, 2020 at 02:17:50PM +0800, [email protected] wrote:
> >> From: Shuo Liu <[email protected]>
> >>
> >> The Service VM communicates with the hypervisor via conventional
> >> hypercalls. VMCALL instruction is used to make the hypercalls.
> >>
> >> ACRN hypercall ABI:
> >> * Hypercall number is in R8 register.
> >> * Up to 2 parameters are in RDI and RSI registers.
> >> * Return value is in RAX register.
> >>
> >> Introduce the ACRN hypercall interfaces. Because GCC doesn't support R8
> >> register as direct register constraints, use supported constraint as
> >> input with a explicit MOV to R8 in beginning of asm.
> >>
> >> +static inline long acrn_hypercall0(unsigned long hcall_id)
> >> +{
> >> + long result;
> >> +
> >> + asm volatile("movl %1, %%r8d\n\t"
> >> + "vmcall\n\t"
> >> + : "=a" (result)
> >> + : "ir" (hcall_id)
> >
> >Is the hypercall id an unsigned long (64 bits) or an unsigned int (32
> >bits)? This will generate broken assembly if the "r" option is chosen,
> >eg something like
> > movl %rdi, %r8d
>
> Yes, it can be an unsigned long. So do MOV explicitly.
>
> asm volatile("movq %1, %%r8\n\t"
> "vmcall\n\t"
> : "=a" (result)
> : "ir" (hcall_id)
>
> Thanks
All the hypercall ID's defined seem to be only 32 bits though?
On Mon 19.Oct'20 at 18:15:15 -0400, Arvind Sankar wrote:
>On Mon, Oct 19, 2020 at 02:17:50PM +0800, [email protected] wrote:
>> From: Shuo Liu <[email protected]>
>>
>> The Service VM communicates with the hypervisor via conventional
>> hypercalls. VMCALL instruction is used to make the hypercalls.
>>
>> ACRN hypercall ABI:
>> * Hypercall number is in R8 register.
>> * Up to 2 parameters are in RDI and RSI registers.
>> * Return value is in RAX register.
>>
>> Introduce the ACRN hypercall interfaces. Because GCC doesn't support R8
>> register as direct register constraints, use supported constraint as
>> input with a explicit MOV to R8 in beginning of asm.
>>
>> +static inline long acrn_hypercall0(unsigned long hcall_id)
>> +{
>> + long result;
>> +
>> + asm volatile("movl %1, %%r8d\n\t"
>> + "vmcall\n\t"
>> + : "=a" (result)
>> + : "ir" (hcall_id)
>
>Is the hypercall id an unsigned long (64 bits) or an unsigned int (32
>bits)? This will generate broken assembly if the "r" option is chosen,
>eg something like
> movl %rdi, %r8d
Yes, it can be an unsigned long. So do MOV explicitly.
asm volatile("movq %1, %%r8\n\t"
"vmcall\n\t"
: "=a" (result)
: "ir" (hcall_id)
Thanks
On Mon 19.Oct'20 at 22:08:51 -0400, Arvind Sankar wrote:
>On Tue, Oct 20, 2020 at 09:38:09AM +0800, Shuo A Liu wrote:
>> On Mon 19.Oct'20 at 18:15:15 -0400, Arvind Sankar wrote:
>> >On Mon, Oct 19, 2020 at 02:17:50PM +0800, [email protected] wrote:
>> >> From: Shuo Liu <[email protected]>
>> >>
>> >> The Service VM communicates with the hypervisor via conventional
>> >> hypercalls. VMCALL instruction is used to make the hypercalls.
>> >>
>> >> ACRN hypercall ABI:
>> >> * Hypercall number is in R8 register.
>> >> * Up to 2 parameters are in RDI and RSI registers.
>> >> * Return value is in RAX register.
>> >>
>> >> Introduce the ACRN hypercall interfaces. Because GCC doesn't support R8
>> >> register as direct register constraints, use supported constraint as
>> >> input with a explicit MOV to R8 in beginning of asm.
>> >>
>> >> +static inline long acrn_hypercall0(unsigned long hcall_id)
>> >> +{
>> >> + long result;
>> >> +
>> >> + asm volatile("movl %1, %%r8d\n\t"
>> >> + "vmcall\n\t"
>> >> + : "=a" (result)
>> >> + : "ir" (hcall_id)
>> >
>> >Is the hypercall id an unsigned long (64 bits) or an unsigned int (32
>> >bits)? This will generate broken assembly if the "r" option is chosen,
>> >eg something like
>> > movl %rdi, %r8d
>>
>> Yes, it can be an unsigned long. So do MOV explicitly.
>>
>> asm volatile("movq %1, %%r8\n\t"
>> "vmcall\n\t"
>> : "=a" (result)
>> : "ir" (hcall_id)
>>
>> Thanks
>
>All the hypercall ID's defined seem to be only 32 bits though?
Yes, they are.
The paramter is unsigned long, use movq to align it.
On Tue 20.Oct'20 at 10:16:02 -0400, Arvind Sankar wrote:
>On Tue, Oct 20, 2020 at 10:30:17AM +0800, Shuo A Liu wrote:
>> On Mon 19.Oct'20 at 22:08:51 -0400, Arvind Sankar wrote:
>> >On Tue, Oct 20, 2020 at 09:38:09AM +0800, Shuo A Liu wrote:
>> >> On Mon 19.Oct'20 at 18:15:15 -0400, Arvind Sankar wrote:
>> >> >On Mon, Oct 19, 2020 at 02:17:50PM +0800, [email protected] wrote:
>> >> >> From: Shuo Liu <[email protected]>
>> >> >>
>> >> >> The Service VM communicates with the hypervisor via conventional
>> >> >> hypercalls. VMCALL instruction is used to make the hypercalls.
>> >> >>
>> >> >> ACRN hypercall ABI:
>> >> >> * Hypercall number is in R8 register.
>> >> >> * Up to 2 parameters are in RDI and RSI registers.
>> >> >> * Return value is in RAX register.
>> >> >>
>> >> >> Introduce the ACRN hypercall interfaces. Because GCC doesn't support R8
>> >> >> register as direct register constraints, use supported constraint as
>> >> >> input with a explicit MOV to R8 in beginning of asm.
>> >> >>
>> >> >> +static inline long acrn_hypercall0(unsigned long hcall_id)
>> >> >> +{
>> >> >> + long result;
>> >> >> +
>> >> >> + asm volatile("movl %1, %%r8d\n\t"
>> >> >> + "vmcall\n\t"
>> >> >> + : "=a" (result)
>> >> >> + : "ir" (hcall_id)
>> >> >
>> >> >Is the hypercall id an unsigned long (64 bits) or an unsigned int (32
>> >> >bits)? This will generate broken assembly if the "r" option is chosen,
>> >> >eg something like
>> >> > movl %rdi, %r8d
>> >>
>> >> Yes, it can be an unsigned long. So do MOV explicitly.
>> >>
>> >> asm volatile("movq %1, %%r8\n\t"
>> >> "vmcall\n\t"
>> >> : "=a" (result)
>> >> : "ir" (hcall_id)
>> >>
>> >> Thanks
>> >
>> >All the hypercall ID's defined seem to be only 32 bits though?
>>
>> Yes, they are.
>> The paramter is unsigned long, use movq to align it.
>
>I don't understand what you mean by alignment here, but I was asking why
>hcall_id is unsigned long and not unsigned int (or u32) if you only need
>32 bits?
The hypervisor is using R8 as the input of hcall_id. So i just want to
keep it consistent.
On Mon, Oct 19, 2020 at 02:17:50PM +0800, [email protected] wrote:
> +static inline long acrn_hypercall0(unsigned long hcall_id)
> +{
> + long result;
> +
> + asm volatile("movl %1, %%r8d\n\t"
> + "vmcall\n\t"
> + : "=a" (result)
> + : "ir" (hcall_id)
^^
Not "irm"? Or simply "g" in that case?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Mon, Nov 02, 2020 at 03:56:57PM +0100, Borislav Petkov wrote:
> On Mon, Oct 19, 2020 at 02:17:50PM +0800, [email protected] wrote:
> > +static inline long acrn_hypercall0(unsigned long hcall_id)
> > +{
> > + long result;
> > +
> > + asm volatile("movl %1, %%r8d\n\t"
> > + "vmcall\n\t"
> > + : "=a" (result)
> > + : "ir" (hcall_id)
> ^^
>
> Not "irm"? Or simply "g" in that case?
I think that will work for x86_64. But it won't matter much, most of
the time you give an immediate number. It is a tiny bit neater of
course (if anyone still remembers what "g" is, you cannot use it much
these days).
Segher
On Mon, Nov 02, 2020 at 10:09:01AM -0600, Segher Boessenkool wrote:
> I think that will work for x86_64. But it won't matter much, most of
> the time you give an immediate number.
Yeah, my question is more along the lines of "is this constraint somehow
special, i.e., 'ir' (and not 'm') or can it be whatever, i.e., 'g'"?
> It is a tiny bit neater of course (if anyone still remembers what "g"
> is, you cannot use it much these days).
Oh I always remember what it is because it is right there in the docs:
"6.47.3.1 Simple Constraints
...
‘g’ Any register, memory or immediate integer operand is allowed,
except for registers that are not general registers."
;-)
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Hi!
On Mon, Nov 02, 2020 at 06:19:50PM +0100, Borislav Petkov wrote:
> On Mon, Nov 02, 2020 at 10:09:01AM -0600, Segher Boessenkool wrote:
> > I think that will work for x86_64. But it won't matter much, most of
> > the time you give an immediate number.
>
> Yeah, my question is more along the lines of "is this constraint somehow
> special, i.e., 'ir' (and not 'm') or can it be whatever, i.e., 'g'"?
movl works for moving anything g->r. This is not true for most insns,
and not for most architectures at all (usually loading from memory has a
separate mnemonic; moving an immediate number often as well (and it does
not usually allow *every* immediate anyway, not even all numbers!)
> > It is a tiny bit neater of course (if anyone still remembers what "g"
> > is, you cannot use it much these days).
>
> Oh I always remember what it is because it is right there in the docs:
>
> "6.47.3.1 Simple Constraints
>
> ...
>
> ‘g’ Any register, memory or immediate integer operand is allowed,
> except for registers that are not general registers."
Most people do not read the documentation, they just copy from (bad)
examples (and arguably, any example you do not really understand is a
bad example).
(It does not allow *all* memory and *all* constants, btw... And the
condition for registers is not really "general register", whatever that
means... I hope no one ever told you our documentation does not have
white lies!)
Segher
On Mon, Nov 02, 2020 at 12:10:00PM -0600, Segher Boessenkool wrote:
> movl works for moving anything g->r.
Right.
> This is not true for most insns, and not for most architectures at
> all (usually loading from memory has a separate mnemonic; moving an
> immediate number often as well (and it does not usually allow *every*
> immediate anyway, not even all numbers!)
So I've heard from other architectures. Every arch has its own problems.
:-)
> Most people do not read the documentation, they just copy from (bad)
> examples (and arguably, any example you do not really understand is a
> bad example).
Unfortunately.
> (It does not allow *all* memory and *all* constants, btw... And the
> condition for registers is not really "general register", whatever that
> means...
I think that means general purpose registers. I.e., it won't use, say
FPU, XMM or whatever special regs.
What does the asm() parsing code in gcc do for "g"? There should be
some logic which constraints what register is valid...
> I hope no one ever told you our documentation does not have white
> lies!)
I have convinced myself of this a couple of times already so I either go
ask our gcc friends or go look straight at gcc source. It is useful to
know folks which hack on it so that I can ask them stupid questions and
not go off into the weeds, trying to figure out what the documentation
says.
But hey, if that gets the documentation improved, that's a win-win
situation right there. Might even make people copying from bad examples
to go look at the docs first...
:-)
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Mon, Nov 02, 2020 at 07:34:30PM +0100, Borislav Petkov wrote:
> On Mon, Nov 02, 2020 at 12:10:00PM -0600, Segher Boessenkool wrote:
> > (It does not allow *all* memory and *all* constants, btw... And the
> > condition for registers is not really "general register", whatever that
> > means...
>
> I think that means general purpose registers. I.e., it won't use, say
> FPU, XMM or whatever special regs.
>
> What does the asm() parsing code in gcc do for "g"? There should be
> some logic which constraints what register is valid...
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. And pseudos usually
are allocated a simple integer register during register allocation, in
an asm that is.
> > I hope no one ever told you our documentation does not have white
> > lies!)
>
> I have convinced myself of this a couple of times already so I either go
> ask our gcc friends or go look straight at gcc source. It is useful to
> know folks which hack on it so that I can ask them stupid questions and
> not go off into the weeds, trying to figure out what the documentation
> says.
>
> But hey, if that gets the documentation improved, that's a win-win
> situation right there.
Yes :-)
> Might even make people copying from bad examples
> to go look at the docs first...
Optimism is cool :-)
Segher
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
---
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
---
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.
> 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.
:-)))
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
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
On Mon, Nov 02, 2020 at 05:18:09PM -0600, Segher Boessenkool wrote:
> 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.
Screams how?
It builds fine without a single peep with -Wall here.
Btw, that's a MOVD - not a MOVL. MOVD can do xmm -> gpr moves. And
singlestepping it with gdb does, well, something, which is clearly
wrong but nothing complains:
=> 0x555555555131 <main+12>: movd %xmm0,%eax
and %xmm0 has:
(gdb) p $xmm0
$2 = {v4_float = {0.99000001, 0, 0, 0}, v2_double = {5.2627153433055495e-315, 0}, v16_int8 = {-92, 112, 125, 63,
^^^^^^^^^^
so that is correct.
0 <repeats 12 times>}, v8_int16 = {28836, 16253, 0, 0, 0, 0, 0, 0}, v4_int32 = {1065185444, 0, 0, 0}, v2_int64 = {
1065185444, 0}, uint128 = 1065185444}
Then:
movd %xmm0,%eax
rax 0x3f7d70a4
and that's the hex representation of the 0.99 float.
and that same value goes into %r8d:
mov %eax,%r8d
r8 0x3f7d70a4
:-)
> Yes. But GCC doing what you should have said instead of doing what you
> said, is not good.
Oh well, should I open a low prio bug, would that help?
I probably should test with the latest gcc first, though...
> 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.
The problem every big project has.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, Nov 03, 2020 at 05:44:35PM +0100, Borislav Petkov wrote:
> On Mon, Nov 02, 2020 at 05:18:09PM -0600, Segher Boessenkool wrote:
> > 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.
>
> Screams how?
$ cat xmm1.s
movl %xmm0,%eax
$ x86_64-linux-as xmm1.s -o xmm1.o
xmm1.s: Assembler messages:
xmm1.s:1: Error: unsupported instruction `mov'
(This isn't an existing insn IIUC.)
> It builds fine without a single peep with -Wall here.
>
> Btw, that's a MOVD - not a MOVL. MOVD can do xmm -> gpr moves. And
> singlestepping it with gdb does, well, something, which is clearly
> wrong but nothing complains:
>
> => 0x555555555131 <main+12>: movd %xmm0,%eax
>
> and %xmm0 has:
>
> (gdb) p $xmm0
> $2 = {v4_float = {0.99000001, 0, 0, 0}, v2_double = {5.2627153433055495e-315, 0}, v16_int8 = {-92, 112, 125, 63,
> ^^^^^^^^^^
>
> so that is correct.
The original code had movl. And movl is needed for GPRs.
> and that same value goes into %r8d:
>
> mov %eax,%r8d
Which violates what is required by register asm :-(
> > Yes. But GCC doing what you should have said instead of doing what you
> > said, is not good.
>
> Oh well, should I open a low prio bug, would that help?
Sure, thanks!
> I probably should test with the latest gcc first, though...
Yeah... FWIW, I tested with
x86_64-linux-gcc (GCC) 11.0.0 20201015 (experimental)
so I doubt current ToT will have it fixed, but who knows.
Thanks,
Segher
On Tue, Nov 03, 2020 at 12:47:39PM -0600, Segher Boessenkool wrote:
> > Oh well, should I open a low prio bug, would that help?
>
> Sure, thanks!
>
> > I probably should test with the latest gcc first, though...
>
> Yeah... FWIW, I tested with
> x86_64-linux-gcc (GCC) 11.0.0 20201015 (experimental)
> so I doubt current ToT will have it fixed, but who knows.
Ok, I'll open it and put you on CC and if they ask me to test latest, I
will build gcc from git.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette