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

From: Shuo A Liu
Date: Sun Sep 27 2020 - 23:38:57 EST


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, shuo.a.liu@xxxxxxxxx wrote:
> From: Shuo Liu <shuo.a.liu@xxxxxxxxx>
>
> 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 <yakui.zhao@xxxxxxxxx>
> Signed-off-by: Shuo Liu <shuo.a.liu@xxxxxxxxx>
> Reviewed-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Cc: Dave Hansen <dave.hansen@xxxxxxxxx>
> Cc: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
> Cc: Fengwei Yin <fengwei.yin@xxxxxxxxx>
> Cc: Zhi Wang <zhi.a.wang@xxxxxxxxx>
> Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>
> Cc: Yu Wang <yu1.wang@xxxxxxxxx>
> Cc: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
> 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