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, here are two ways to use R8 in
extended asm:
1) use explicit register variable as input
2) use supported constraint as input with a explicit MOV to R8 in
beginning of asm
The number of instructions of above two ways are same.
Asm code from 1)
38: 41 b8 00 00 00 80 mov $0x80000000,%r8d
3e: 48 89 c7 mov %rax,%rdi
41: 0f 01 c1 vmcall
Here, writes to the lower dword (%r8d) clear the upper dword of %r8 when
the CPU is in 64-bit mode.
Asm code from 2)
38: 48 89 c7 mov %rax,%rdi
3b: 49 b8 00 00 00 80 00 movabs $0x80000000,%r8
42: 00 00 00
45: 0f 01 c1 vmcall
Choose 1) for code simplicity and a little bit of code size
optimization.
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]>
---
arch/x86/include/asm/acrn.h | 57 +++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/arch/x86/include/asm/acrn.h b/arch/x86/include/asm/acrn.h
index a2d4aea3a80d..23a93b87edeb 100644
--- a/arch/x86/include/asm/acrn.h
+++ b/arch/x86/include/asm/acrn.h
@@ -14,4 +14,61 @@ 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.
+ */
+static inline long acrn_hypercall0(unsigned long hcall_id)
+{
+ register long r8 asm("r8");
+ long result;
+
+ /* Nothing can come between the r8 assignment and the asm: */
+ r8 = hcall_id;
+ asm volatile("vmcall\n\t"
+ : "=a" (result)
+ : "r" (r8)
+ : );
+
+ return result;
+}
+
+static inline long acrn_hypercall1(unsigned long hcall_id,
+ unsigned long param1)
+{
+ register long r8 asm("r8");
+ long result;
+
+ /* Nothing can come between the r8 assignment and the asm: */
+ r8 = hcall_id;
+ asm volatile("vmcall\n\t"
+ : "=a" (result)
+ : "r" (r8), "D" (param1)
+ : );
+
+ return result;
+}
+
+static inline long acrn_hypercall2(unsigned long hcall_id,
+ unsigned long param1,
+ unsigned long param2)
+{
+ register long r8 asm("r8");
+ long result;
+
+ /* Nothing can come between the r8 assignment and the asm: */
+ r8 = hcall_id;
+ asm volatile("vmcall\n\t"
+ : "=a" (result)
+ : "r" (r8), "D" (param1), "S" (param2)
+ : );
+
+ return result;
+}
+
#endif /* _ASM_X86_ACRN_H */
--
2.28.0
On Tue, Sep 22, 2020 at 07:42:58PM +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, here are two ways to use R8 in
> extended asm:
> 1) use explicit register variable as input
> 2) use supported constraint as input with a explicit MOV to R8 in
> beginning of asm
>
> The number of instructions of above two ways are same.
> Asm code from 1)
> 38: 41 b8 00 00 00 80 mov $0x80000000,%r8d
> 3e: 48 89 c7 mov %rax,%rdi
> 41: 0f 01 c1 vmcall
> Here, writes to the lower dword (%r8d) clear the upper dword of %r8 when
> the CPU is in 64-bit mode.
>
> Asm code from 2)
> 38: 48 89 c7 mov %rax,%rdi
> 3b: 49 b8 00 00 00 80 00 movabs $0x80000000,%r8
> 42: 00 00 00
> 45: 0f 01 c1 vmcall
>
> Choose 1) for code simplicity and a little bit of code size
> optimization.
>
> 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]>
> ---
> arch/x86/include/asm/acrn.h | 57 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/arch/x86/include/asm/acrn.h b/arch/x86/include/asm/acrn.h
> index a2d4aea3a80d..23a93b87edeb 100644
> --- a/arch/x86/include/asm/acrn.h
> +++ b/arch/x86/include/asm/acrn.h
> @@ -14,4 +14,61 @@ 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.
> + */
> +static inline long acrn_hypercall0(unsigned long hcall_id)
> +{
> + register long r8 asm("r8");
> + long result;
> +
> + /* Nothing can come between the r8 assignment and the asm: */
> + r8 = hcall_id;
> + asm volatile("vmcall\n\t"
> + : "=a" (result)
> + : "r" (r8)
> + : );
What keeps an interrupt from happening between the r8 assignment and the
asm: ?
Is this something that most hypercalls need to handle? I don't see
other ones needing this type of thing, is it just because of how these
are defined?
confused,
greg k-h
On Sun, Sep 27, 2020 at 12:51:52PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Sep 22, 2020 at 07:42:58PM +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, here are two ways to use R8 in
> > extended asm:
> > 1) use explicit register variable as input
> > 2) use supported constraint as input with a explicit MOV to R8 in
> > beginning of asm
> >
> > The number of instructions of above two ways are same.
> > Asm code from 1)
> > 38: 41 b8 00 00 00 80 mov $0x80000000,%r8d
> > 3e: 48 89 c7 mov %rax,%rdi
> > 41: 0f 01 c1 vmcall
> > Here, writes to the lower dword (%r8d) clear the upper dword of %r8 when
> > the CPU is in 64-bit mode.
> >
> > Asm code from 2)
> > 38: 48 89 c7 mov %rax,%rdi
> > 3b: 49 b8 00 00 00 80 00 movabs $0x80000000,%r8
> > 42: 00 00 00
> > 45: 0f 01 c1 vmcall
> >
> > Choose 1) for code simplicity and a little bit of code size
> > optimization.
> >
> > 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]>
> > ---
> > arch/x86/include/asm/acrn.h | 57 +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 57 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/acrn.h b/arch/x86/include/asm/acrn.h
> > index a2d4aea3a80d..23a93b87edeb 100644
> > --- a/arch/x86/include/asm/acrn.h
> > +++ b/arch/x86/include/asm/acrn.h
> > @@ -14,4 +14,61 @@ 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.
> > + */
> > +static inline long acrn_hypercall0(unsigned long hcall_id)
> > +{
> > + register long r8 asm("r8");
> > + long result;
> > +
> > + /* Nothing can come between the r8 assignment and the asm: */
> > + r8 = hcall_id;
> > + asm volatile("vmcall\n\t"
> > + : "=a" (result)
> > + : "r" (r8)
> > + : );
>
> What keeps an interrupt from happening between the r8 assignment and the
> asm: ?
>
> Is this something that most hypercalls need to handle? I don't see
> other ones needing this type of thing, is it just because of how these
> are defined?
Ah, the changelog above explains this. You should put that in the code
itself, as a comment, otherwise we will not know this at all in 5
years, when gcc is changed to allow r8 access :)
thanks,
greg k-h
On 9/27/20 3:51 AM, Greg Kroah-Hartman wrote:
>> +static inline long acrn_hypercall0(unsigned long hcall_id)
>> +{
>> + register long r8 asm("r8");
>> + long result;
>> +
>> + /* Nothing can come between the r8 assignment and the asm: */
>> + r8 = hcall_id;
>> + asm volatile("vmcall\n\t"
>> + : "=a" (result)
>> + : "r" (r8)
>> + : );
> What keeps an interrupt from happening between the r8 assignment and the
> asm: ?
It's probably better phrased something like: "No other C code can come
between this r8 assignment and the inline asm". An interrupt would
actually be fine in there because interrupts save and restore all
register state, including r8.
The problem (mentioned in the changelog) is that gcc does not let you
place data directly into r8. But, it does allow you to declare a
register variable that you can assign to use r8. There might be a
problem if a function calls was in between and clobber the register,
thus the "nothing can come between" comment.
The comment is really intended to scare away anyone from adding printk()'s.
More information about these register variables is here:
> https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html#Local-Register-Variables
Any better ideas for comments would be greatly appreciated. It has 4 or
5 copies so I wanted it to be succinct.
On Sun 27.Sep'20 at 12:53:14 +0200, Greg Kroah-Hartman wrote:
>On Sun, Sep 27, 2020 at 12:51:52PM +0200, Greg Kroah-Hartman wrote:
>> On Tue, Sep 22, 2020 at 07:42:58PM +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, here are two ways to use R8 in
>> > extended asm:
>> > 1) use explicit register variable as input
>> > 2) use supported constraint as input with a explicit MOV to R8 in
>> > beginning of asm
>> >
>> > The number of instructions of above two ways are same.
>> > Asm code from 1)
>> > 38: 41 b8 00 00 00 80 mov $0x80000000,%r8d
>> > 3e: 48 89 c7 mov %rax,%rdi
>> > 41: 0f 01 c1 vmcall
>> > Here, writes to the lower dword (%r8d) clear the upper dword of %r8 when
>> > the CPU is in 64-bit mode.
>> >
>> > Asm code from 2)
>> > 38: 48 89 c7 mov %rax,%rdi
>> > 3b: 49 b8 00 00 00 80 00 movabs $0x80000000,%r8
>> > 42: 00 00 00
>> > 45: 0f 01 c1 vmcall
>> >
>> > Choose 1) for code simplicity and a little bit of code size
>> > optimization.
>> >
>> > 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]>
>> > ---
>> > arch/x86/include/asm/acrn.h | 57 +++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 57 insertions(+)
>> >
>> > diff --git a/arch/x86/include/asm/acrn.h b/arch/x86/include/asm/acrn.h
>> > index a2d4aea3a80d..23a93b87edeb 100644
>> > --- a/arch/x86/include/asm/acrn.h
>> > +++ b/arch/x86/include/asm/acrn.h
>> > @@ -14,4 +14,61 @@ 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.
>> > + */
>> > +static inline long acrn_hypercall0(unsigned long hcall_id)
>> > +{
>> > + register long r8 asm("r8");
>> > + long result;
>> > +
>> > + /* Nothing can come between the r8 assignment and the asm: */
>> > + r8 = hcall_id;
>> > + asm volatile("vmcall\n\t"
>> > + : "=a" (result)
>> > + : "r" (r8)
>> > + : );
>>
>> What keeps an interrupt from happening between the r8 assignment and the
>> asm: ?
Dave gave a good explanation in another email. I will apply his better
comment that "No other C code can come between this r8 assignment and the
inline asm".
>>
>> Is this something that most hypercalls need to handle? I don't see
>> other ones needing this type of thing, is it just because of how these
>> are defined?
>
>Ah, the changelog above explains this. You should put that in the code
>itself, as a comment, otherwise we will not know this at all in 5
>years, when gcc is changed to allow r8 access :)
OK. I will copy that into code as well.
Thanks
shuo
On Tue, Sep 22, 2020 at 07:42:58PM +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.
I'm assuming this is already cast in stone in the HV and it cannot be
changed?
> Introduce the ACRN hypercall interfaces. Because GCC doesn't support R8
> register as direct register constraints, here are two ways to use R8 in
> extended asm:
> 1) use explicit register variable as input
> 2) use supported constraint as input with a explicit MOV to R8 in
> beginning of asm
>
> The number of instructions of above two ways are same.
> Asm code from 1)
> 38: 41 b8 00 00 00 80 mov $0x80000000,%r8d
> 3e: 48 89 c7 mov %rax,%rdi
> 41: 0f 01 c1 vmcall
> Here, writes to the lower dword (%r8d) clear the upper dword of %r8 when
> the CPU is in 64-bit mode.
>
> Asm code from 2)
> 38: 48 89 c7 mov %rax,%rdi
> 3b: 49 b8 00 00 00 80 00 movabs $0x80000000,%r8
> 42: 00 00 00
> 45: 0f 01 c1 vmcall
>
> Choose 1) for code simplicity and a little bit of code size
> optimization.
What?
How much "optimization" is this actually? A couple of bytes?
And all that for this
/* Nothing can come between the r8 assignment and the asm: */
restriction?
If it is only a couple of bytes, just do the explicit MOV to %r8 and
f'get about it.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Sun, Sep 27, 2020 at 08:38:03AM -0700, Dave Hansen wrote:
> On 9/27/20 3:51 AM, Greg Kroah-Hartman wrote:
> >> +static inline long acrn_hypercall0(unsigned long hcall_id)
> >> +{
> >> + register long r8 asm("r8");
> >> + long result;
> >> +
> >> + /* Nothing can come between the r8 assignment and the asm: */
> >> + r8 = hcall_id;
> >> + asm volatile("vmcall\n\t"
> >> + : "=a" (result)
> >> + : "r" (r8)
> >> + : );
> > What keeps an interrupt from happening between the r8 assignment and the
> > asm: ?
>
> It's probably better phrased something like: "No other C code can come
> between this r8 assignment and the inline asm". An interrupt would
> actually be fine in there because interrupts save and restore all
> register state, including r8.
>
> The problem (mentioned in the changelog) is that gcc does not let you
> place data directly into r8. But, it does allow you to declare a
> register variable that you can assign to use r8. There might be a
> problem if a function calls was in between and clobber the register,
> thus the "nothing can come between" comment.
>
> The comment is really intended to scare away anyone from adding printk()'s.
>
> More information about these register variables is here:
>
> > https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html#Local-Register-Variables
>
> Any better ideas for comments would be greatly appreciated. It has 4 or
> 5 copies so I wanted it to be succinct.
This is disguisting.. Segher, does this actually work? Nick, does clang
also support this?
Hi!
On Wed, Sep 30, 2020 at 01:16:12PM +0200, Peter Zijlstra wrote:
> On Sun, Sep 27, 2020 at 08:38:03AM -0700, Dave Hansen wrote:
> > On 9/27/20 3:51 AM, Greg Kroah-Hartman wrote:
> > >> +static inline long acrn_hypercall0(unsigned long hcall_id)
> > >> +{
> > >> + register long r8 asm("r8");
> > >> + long result;
> > >> +
> > >> + /* Nothing can come between the r8 assignment and the asm: */
> > >> + r8 = hcall_id;
> > >> + asm volatile("vmcall\n\t"
> > >> + : "=a" (result)
> > >> + : "r" (r8)
> > >> + : );
> > > What keeps an interrupt from happening between the r8 assignment and the
> > > asm: ?
> >
> > It's probably better phrased something like: "No other C code can come
> > between this r8 assignment and the inline asm". An interrupt would
> > actually be fine in there because interrupts save and restore all
> > register state, including r8.
> >
> > The problem (mentioned in the changelog) is that gcc does not let you
> > place data directly into r8. But, it does allow you to declare a
> > register variable that you can assign to use r8. There might be a
> > problem if a function calls was in between and clobber the register,
> > thus the "nothing can come between" comment.
> >
> > The comment is really intended to scare away anyone from adding printk()'s.
> >
> > More information about these register variables is here:
> >
> > > https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html#Local-Register-Variables
> >
> > Any better ideas for comments would be greatly appreciated. It has 4 or
> > 5 copies so I wanted it to be succinct.
>
> This is disguisting.. Segher, does this actually work? Nick, does clang
> also support this?
The C variable "r8" is just a variable like any other; it can live in
memory, or in any register, and different in all places, too. It can be
moved around too; where "the assignment to it" happens is a
philosophical question more than anything (the assignment there can be
optimised away completely, for example; it is just a C variable, there
is no magic).
Since this variable is a local register asm, on entry to the asm the
compiler guarantees that the value lives in the assigned register (the
"r8" hardware register in this case). This all works completely fine.
This is the only guaranteed behaviour for local register asm (well,
together with analogous behaviour for outputs).
If you want to *always* have it live in the hardware reg "r8", you have
to use a global register asm, and almost certainly do that for all
translation units, and use -ffixed-r8 as well. This of course is
extremely costly.
Segher
On Wed, Sep 30, 2020 at 11:10:36AM -0500, Segher Boessenkool wrote:
> Since this variable is a local register asm, on entry to the asm the
> compiler guarantees that the value lives in the assigned register (the
> "r8" hardware register in this case). This all works completely fine.
> This is the only guaranteed behaviour for local register asm (well,
> together with analogous behaviour for outputs).
Right, that's what they're trying to achieve. The hypervisor calling
convention needs that variable in %r8 (which is somewhat unfortunate).
AFAIK this is the first such use in the kernel, but at least the gcc-4.9
(our oldest supported version) claims to support this.
So now we need to know if clang will actually do this too..
On Wed, Sep 30, 2020 at 10:13 AM Peter Zijlstra <[email protected]> wrote:
>
> On Wed, Sep 30, 2020 at 11:10:36AM -0500, Segher Boessenkool wrote:
>
> > Since this variable is a local register asm, on entry to the asm the
> > compiler guarantees that the value lives in the assigned register (the
> > "r8" hardware register in this case). This all works completely fine.
> > This is the only guaranteed behaviour for local register asm (well,
> > together with analogous behaviour for outputs).
>
> Right, that's what they're trying to achieve. The hypervisor calling
> convention needs that variable in %r8 (which is somewhat unfortunate).
>
> AFAIK this is the first such use in the kernel, but at least the gcc-4.9
> (our oldest supported version) claims to support this.
>
> So now we need to know if clang will actually do this too..
Does clang support register local storage? Let's use godbolt.org to find out:
https://godbolt.org/z/YM45W5
Looks like yes. You can even check different GCC versions via the
dropdown in the top right.
The -ffixed-* flags are less well supported in Clang; they need to be
reimplemented on a per-backend basis. aarch64 is relatively well
supported, but other arches not so much IME.
Do we need register local storage here?
static inline long bar(unsigned long hcall_id)
{
long result;
asm volatile("movl %1, %%r8d\n\t"
"vmcall\n\t"
: "=a" (result)
: "ir" (hcall_id)
: );
return result;
}
--
Thanks,
~Nick Desaulniers
On Wed, Sep 30, 2020 at 12:14:03PM -0700, Nick Desaulniers wrote:
> On Wed, Sep 30, 2020 at 10:13 AM Peter Zijlstra <[email protected]> wrote:
> >
> > On Wed, Sep 30, 2020 at 11:10:36AM -0500, Segher Boessenkool wrote:
> >
> > > Since this variable is a local register asm, on entry to the asm the
> > > compiler guarantees that the value lives in the assigned register (the
> > > "r8" hardware register in this case). This all works completely fine.
> > > This is the only guaranteed behaviour for local register asm (well,
> > > together with analogous behaviour for outputs).
> >
> > Right, that's what they're trying to achieve. The hypervisor calling
> > convention needs that variable in %r8 (which is somewhat unfortunate).
> >
> > AFAIK this is the first such use in the kernel, but at least the gcc-4.9
> > (our oldest supported version) claims to support this.
> >
> > So now we need to know if clang will actually do this too..
>
> Does clang support register local storage? Let's use godbolt.org to find out:
> https://godbolt.org/z/YM45W5
> Looks like yes. You can even check different GCC versions via the
> dropdown in the top right.
That only tells me it compiles it, not if that (IMO) weird construct is
actually guaranteed to work as expected.
I'd almost dive into the GCC archives to read the back-story to this
'feature', it just seems to weird to me. A well, for another day that.
On Wed, Sep 30, 2020 at 12:14:03PM -0700, Nick Desaulniers wrote:
> On Wed, Sep 30, 2020 at 10:13 AM Peter Zijlstra <[email protected]> wrote:
> >
> > On Wed, Sep 30, 2020 at 11:10:36AM -0500, Segher Boessenkool wrote:
> >
> > > Since this variable is a local register asm, on entry to the asm the
> > > compiler guarantees that the value lives in the assigned register (the
> > > "r8" hardware register in this case). This all works completely fine.
> > > This is the only guaranteed behaviour for local register asm (well,
> > > together with analogous behaviour for outputs).
How strict is the guarantee? This is an inline function -- could the
compiler decide to reorder some other code in between the r8 assignment
and the asm statement when it gets inlined?
> >
> > Right, that's what they're trying to achieve. The hypervisor calling
> > convention needs that variable in %r8 (which is somewhat unfortunate).
> >
> > AFAIK this is the first such use in the kernel, but at least the gcc-4.9
> > (our oldest supported version) claims to support this.
> >
> > So now we need to know if clang will actually do this too..
>
> Does clang support register local storage? Let's use godbolt.org to find out:
> https://godbolt.org/z/YM45W5
> Looks like yes. You can even check different GCC versions via the
> dropdown in the top right.
>
> The -ffixed-* flags are less well supported in Clang; they need to be
> reimplemented on a per-backend basis. aarch64 is relatively well
> supported, but other arches not so much IME.
>
> Do we need register local storage here?
>
> static inline long bar(unsigned long hcall_id)
> {
> long result;
> asm volatile("movl %1, %%r8d\n\t"
> "vmcall\n\t"
> : "=a" (result)
> : "ir" (hcall_id)
> : );
> return result;
> }
This seems more robust, though you probably need an r8 clobber in there?
Is hcall_id actually just 32 bits or can it be >=2^32?
On Wed, Sep 30, 2020 at 03:59:15PM -0400, Arvind Sankar wrote:
> On Wed, Sep 30, 2020 at 12:14:03PM -0700, Nick Desaulniers wrote:
> > On Wed, Sep 30, 2020 at 10:13 AM Peter Zijlstra <[email protected]> wrote:
> > >
> > > On Wed, Sep 30, 2020 at 11:10:36AM -0500, Segher Boessenkool wrote:
> > >
> > > > Since this variable is a local register asm, on entry to the asm the
> > > > compiler guarantees that the value lives in the assigned register (the
> > > > "r8" hardware register in this case). This all works completely fine.
> > > > This is the only guaranteed behaviour for local register asm (well,
> > > > together with analogous behaviour for outputs).
>
> How strict is the guarantee? This is an inline function -- could the
> compiler decide to reorder some other code in between the r8 assignment
> and the asm statement when it gets inlined?
>
> > >
> > > Right, that's what they're trying to achieve. The hypervisor calling
> > > convention needs that variable in %r8 (which is somewhat unfortunate).
> > >
> > > AFAIK this is the first such use in the kernel, but at least the gcc-4.9
> > > (our oldest supported version) claims to support this.
> > >
> > > So now we need to know if clang will actually do this too..
> >
> > Does clang support register local storage? Let's use godbolt.org to find out:
> > https://godbolt.org/z/YM45W5
> > Looks like yes. You can even check different GCC versions via the
> > dropdown in the top right.
> >
> > The -ffixed-* flags are less well supported in Clang; they need to be
> > reimplemented on a per-backend basis. aarch64 is relatively well
> > supported, but other arches not so much IME.
> >
> > Do we need register local storage here?
> >
> > static inline long bar(unsigned long hcall_id)
> > {
> > long result;
> > asm volatile("movl %1, %%r8d\n\t"
> > "vmcall\n\t"
> > : "=a" (result)
> > : "ir" (hcall_id)
> > : );
> > return result;
> > }
>
> This seems more robust, though you probably need an r8 clobber in there?
> Is hcall_id actually just 32 bits or can it be >=2^32?
Also, I think you need memory clobbers for all of these in either case, no?
On Wed, Sep 30, 2020 at 12:14:03PM -0700, Nick Desaulniers wrote:
> Do we need register local storage here?
Depends what you want. It looks like you do:
> static inline long bar(unsigned long hcall_id)
> {
> long result;
> asm volatile("movl %1, %%r8d\n\t"
> "vmcall\n\t"
> : "=a" (result)
> : "ir" (hcall_id)
> : );
> return result;
> }
"result" as output from the asm is in %rax, and the compiler will
shuffle that to wherever it needs it as the function return value. That
part will work fine.
But how you are accessing %r8d is not correct, that needs to be a local
register asm (or r8 be made a fixed reg, probably not what you want ;-) )
Segher
On Wed, Sep 30, 2020 at 06:25:59PM -0500, Segher Boessenkool wrote:
> On Wed, Sep 30, 2020 at 12:14:03PM -0700, Nick Desaulniers wrote:
> > Do we need register local storage here?
>
> Depends what you want. It looks like you do:
>
> > static inline long bar(unsigned long hcall_id)
> > {
> > long result;
> > asm volatile("movl %1, %%r8d\n\t"
> > "vmcall\n\t"
> > : "=a" (result)
> > : "ir" (hcall_id)
> > : );
> > return result;
> > }
>
> "result" as output from the asm is in %rax, and the compiler will
> shuffle that to wherever it needs it as the function return value. That
> part will work fine.
>
> But how you are accessing %r8d is not correct, that needs to be a local
> register asm (or r8 be made a fixed reg, probably not what you want ;-) )
>
Doesn't it just need an "r8" clobber to allow using r8d?
>
> Segher
On Wed, Sep 30, 2020 at 09:42:40PM +0200, Peter Zijlstra wrote:
> > Looks like yes. You can even check different GCC versions via the
> > dropdown in the top right.
>
> That only tells me it compiles it, not if that (IMO) weird construct is
> actually guaranteed to work as expected.
>
> I'd almost dive into the GCC archives to read the back-story to this
> 'feature', it just seems to weird to me. A well, for another day that.
It was documented in 1996 (<https://gcc.gnu.org/g:c1f7febfcb10>), the
feature is older than that though.
In 2004 (in <https://gcc.gnu.org/g:805c33df1366>) the documentation for
this was made more explicit (it has been rewritten since, but it still
says the same thing).
Segher
On Wed, Sep 30, 2020 at 03:59:15PM -0400, Arvind Sankar wrote:
> On Wed, Sep 30, 2020 at 12:14:03PM -0700, Nick Desaulniers wrote:
> > On Wed, Sep 30, 2020 at 10:13 AM Peter Zijlstra <[email protected]> wrote:
> > >
> > > On Wed, Sep 30, 2020 at 11:10:36AM -0500, Segher Boessenkool wrote:
> > >
> > > > Since this variable is a local register asm, on entry to the asm the
> > > > compiler guarantees that the value lives in the assigned register (the
> > > > "r8" hardware register in this case). This all works completely fine.
> > > > This is the only guaranteed behaviour for local register asm (well,
> > > > together with analogous behaviour for outputs).
>
> How strict is the guarantee? This is an inline function -- could the
> compiler decide to reorder some other code in between the r8 assignment
> and the asm statement when it gets inlined?
Nope. It will be in r8 on entry to the asm. A guarantee is a
guarantee; it is not a "yeah maybe, we'll see".
> > Do we need register local storage here?
> >
> > static inline long bar(unsigned long hcall_id)
> > {
> > long result;
> > asm volatile("movl %1, %%r8d\n\t"
> > "vmcall\n\t"
> > : "=a" (result)
> > : "ir" (hcall_id)
> > : );
> > return result;
> > }
>
> This seems more robust, though you probably need an r8 clobber in there?
Oh, x86 has the operand order inverted, so this should work in fact.
Segher
On Wed, Sep 30, 2020 at 07:38:15PM -0400, Arvind Sankar wrote:
> On Wed, Sep 30, 2020 at 06:25:59PM -0500, Segher Boessenkool wrote:
> > On Wed, Sep 30, 2020 at 12:14:03PM -0700, Nick Desaulniers wrote:
> > > Do we need register local storage here?
> >
> > Depends what you want. It looks like you do:
> >
> > > static inline long bar(unsigned long hcall_id)
> > > {
> > > long result;
> > > asm volatile("movl %1, %%r8d\n\t"
> > > "vmcall\n\t"
> > > : "=a" (result)
> > > : "ir" (hcall_id)
> > > : );
> > > return result;
> > > }
> >
> > "result" as output from the asm is in %rax, and the compiler will
> > shuffle that to wherever it needs it as the function return value. That
> > part will work fine.
> >
> > But how you are accessing %r8d is not correct, that needs to be a local
> > register asm (or r8 be made a fixed reg, probably not what you want ;-) )
>
> Doesn't it just need an "r8" clobber to allow using r8d?
Yes, x86 asm is hard to read, what can I say :-) Sorry about that.
Segher
On Wed 30.Sep'20 at 12:14:03 -0700, Nick Desaulniers wrote:
>On Wed, Sep 30, 2020 at 10:13 AM Peter Zijlstra <[email protected]> wrote:
>>
>> On Wed, Sep 30, 2020 at 11:10:36AM -0500, Segher Boessenkool wrote:
>>
>> > Since this variable is a local register asm, on entry to the asm the
>> > compiler guarantees that the value lives in the assigned register (the
>> > "r8" hardware register in this case). This all works completely fine.
>> > This is the only guaranteed behaviour for local register asm (well,
>> > together with analogous behaviour for outputs).
>>
>> Right, that's what they're trying to achieve. The hypervisor calling
>> convention needs that variable in %r8 (which is somewhat unfortunate).
>>
>> AFAIK this is the first such use in the kernel, but at least the gcc-4.9
>> (our oldest supported version) claims to support this.
>>
>> So now we need to know if clang will actually do this too..
>
>Does clang support register local storage? Let's use godbolt.org to find out:
>https://godbolt.org/z/YM45W5
>Looks like yes. You can even check different GCC versions via the
>dropdown in the top right.
>
>The -ffixed-* flags are less well supported in Clang; they need to be
>reimplemented on a per-backend basis. aarch64 is relatively well
>supported, but other arches not so much IME.
>
>Do we need register local storage here?
>
>static inline long bar(unsigned long hcall_id)
>{
> long result;
> asm volatile("movl %1, %%r8d\n\t"
> "vmcall\n\t"
> : "=a" (result)
> : "ir" (hcall_id)
> : );
> return result;
>}
Yeah, this approach is also mentioned in the changelog. I will change to
this way to follow your preference. With an addtional "r8" clobber what
Arvind mentioned.
Thanks
shuo
On Mon, Oct 12, 2020 at 04:44:31PM +0800, Shuo A Liu wrote:
> On Wed 30.Sep'20 at 12:14:03 -0700, Nick Desaulniers wrote:
> >On Wed, Sep 30, 2020 at 10:13 AM Peter Zijlstra <[email protected]> wrote:
> >>
> >> On Wed, Sep 30, 2020 at 11:10:36AM -0500, Segher Boessenkool wrote:
> >>
> >> > Since this variable is a local register asm, on entry to the asm the
> >> > compiler guarantees that the value lives in the assigned register (the
> >> > "r8" hardware register in this case). This all works completely fine.
> >> > This is the only guaranteed behaviour for local register asm (well,
> >> > together with analogous behaviour for outputs).
> >>
> >> Right, that's what they're trying to achieve. The hypervisor calling
> >> convention needs that variable in %r8 (which is somewhat unfortunate).
> >>
> >> AFAIK this is the first such use in the kernel, but at least the gcc-4.9
> >> (our oldest supported version) claims to support this.
> >>
> >> So now we need to know if clang will actually do this too..
> >
> >Does clang support register local storage? Let's use godbolt.org to find out:
> >https://godbolt.org/z/YM45W5
> >Looks like yes. You can even check different GCC versions via the
> >dropdown in the top right.
> >
> >The -ffixed-* flags are less well supported in Clang; they need to be
> >reimplemented on a per-backend basis. aarch64 is relatively well
> >supported, but other arches not so much IME.
> >
> >Do we need register local storage here?
> >
> >static inline long bar(unsigned long hcall_id)
> >{
> > long result;
> > asm volatile("movl %1, %%r8d\n\t"
> > "vmcall\n\t"
> > : "=a" (result)
> > : "ir" (hcall_id)
> > : );
> > return result;
> >}
>
> Yeah, this approach is also mentioned in the changelog. I will change to
> this way to follow your preference. With an addtional "r8" clobber what
> Arvind mentioned.
>
> Thanks
> shuo
Btw, I noticed that arch/x86/xen/hypercall.h uses register-local
variables already for its hypercalls for quite some time, so this
wouldn't be unprecedented. [0]
Do these calls also need a memory clobber? The KVM/xen hypercall functions
all have one.
Thanks.
[0] e74359028d548 ("xen64: fix calls into hypercall page")
On Wed 30.Sep'20 at 12:54:08 +0200, Borislav Petkov wrote:
>On Tue, Sep 22, 2020 at 07:42:58PM +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.
>
>I'm assuming this is already cast in stone in the HV and it cannot be
>changed?
Yes, it is.
>
>> Introduce the ACRN hypercall interfaces. Because GCC doesn't support R8
>> register as direct register constraints, here are two ways to use R8 in
>> extended asm:
>> 1) use explicit register variable as input
>> 2) use supported constraint as input with a explicit MOV to R8 in
>> beginning of asm
>>
>> The number of instructions of above two ways are same.
>> Asm code from 1)
>> 38: 41 b8 00 00 00 80 mov $0x80000000,%r8d
>> 3e: 48 89 c7 mov %rax,%rdi
>> 41: 0f 01 c1 vmcall
>> Here, writes to the lower dword (%r8d) clear the upper dword of %r8 when
>> the CPU is in 64-bit mode.
>>
>> Asm code from 2)
>> 38: 48 89 c7 mov %rax,%rdi
>> 3b: 49 b8 00 00 00 80 00 movabs $0x80000000,%r8
>> 42: 00 00 00
>> 45: 0f 01 c1 vmcall
>>
>> Choose 1) for code simplicity and a little bit of code size
>> optimization.
>
>What?
>
>How much "optimization" is this actually? A couple of bytes?
>
>And all that for this
>
> /* Nothing can come between the r8 assignment and the asm: */
>
>restriction?
>
>If it is only a couple of bytes, just do the explicit MOV to %r8 and
>f'get about it.
Yes. Just a couple of bytes. Number of instructions is same.
sure, i can change to approach 2)
2) use supported constraint as input with a explicit MOV to R8
in beginning of asm
Thanks
shuo
On Mon 12.Oct'20 at 12:49:16 -0400, Arvind Sankar wrote:
>On Mon, Oct 12, 2020 at 04:44:31PM +0800, Shuo A Liu wrote:
>> On Wed 30.Sep'20 at 12:14:03 -0700, Nick Desaulniers wrote:
>> >On Wed, Sep 30, 2020 at 10:13 AM Peter Zijlstra <[email protected]> wrote:
>> >>
>> >> On Wed, Sep 30, 2020 at 11:10:36AM -0500, Segher Boessenkool wrote:
>> >>
>> >> > Since this variable is a local register asm, on entry to the asm the
>> >> > compiler guarantees that the value lives in the assigned register (the
>> >> > "r8" hardware register in this case). This all works completely fine.
>> >> > This is the only guaranteed behaviour for local register asm (well,
>> >> > together with analogous behaviour for outputs).
>> >>
>> >> Right, that's what they're trying to achieve. The hypervisor calling
>> >> convention needs that variable in %r8 (which is somewhat unfortunate).
>> >>
>> >> AFAIK this is the first such use in the kernel, but at least the gcc-4.9
>> >> (our oldest supported version) claims to support this.
>> >>
>> >> So now we need to know if clang will actually do this too..
>> >
>> >Does clang support register local storage? Let's use godbolt.org to find out:
>> >https://godbolt.org/z/YM45W5
>> >Looks like yes. You can even check different GCC versions via the
>> >dropdown in the top right.
>> >
>> >The -ffixed-* flags are less well supported in Clang; they need to be
>> >reimplemented on a per-backend basis. aarch64 is relatively well
>> >supported, but other arches not so much IME.
>> >
>> >Do we need register local storage here?
>> >
>> >static inline long bar(unsigned long hcall_id)
>> >{
>> > long result;
>> > asm volatile("movl %1, %%r8d\n\t"
>> > "vmcall\n\t"
>> > : "=a" (result)
>> > : "ir" (hcall_id)
>> > : );
>> > return result;
>> >}
>>
>> Yeah, this approach is also mentioned in the changelog. I will change to
>> this way to follow your preference. With an addtional "r8" clobber what
>> Arvind mentioned.
>>
>> Thanks
>> shuo
>
>Btw, I noticed that arch/x86/xen/hypercall.h uses register-local
>variables already for its hypercalls for quite some time, so this
>wouldn't be unprecedented. [0]
>
>Do these calls also need a memory clobber? The KVM/xen hypercall functions
>all have one.
Yes. it's needed. I will add it. Thanks
>
>Thanks.
>
>[0] e74359028d548 ("xen64: fix calls into hypercall page")