diff mbox series

[V5,5/8] x86/hyperv: Use vmmcall to implement Hyper-V hypercall in sev-snp enlightened guest

Message ID 20230810160412.820246-6-ltykernel@gmail.com
State New
Headers
Series x86/hyperv: Add AMD sev-snp enlightened guest support on hyperv |

Commit Message

Tianyu Lan Aug. 10, 2023, 4:04 p.m. UTC 
  From: Tianyu Lan <tiala@microsoft.com>

In sev-snp enlightened guest, Hyper-V hypercall needs
to use vmmcall to trigger vmexit and notify hypervisor
to handle hypercall request.

Signed-off-by: Tianyu Lan <tiala@microsoft.com>
---
 arch/x86/include/asm/mshyperv.h | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

Comments

Dexuan Cui Aug. 10, 2023, 11:43 p.m. UTC | #1 
> From: Tianyu Lan <ltykernel@gmail.com>
> Sent: Thursday, August 10, 2023 9:04 AM
>  [...]
> @@ -103,7 +103,8 @@ static inline u64 _hv_do_fast_hypercall8(u64 control,
> u64 input1)
> 
>  #ifdef CONFIG_X86_64
>  	{
> -		__asm__ __volatile__(CALL_NOSPEC
> +		__asm__ __volatile__("mov %[thunk_target], %%r8\n"

The "mov %[thunk_target], %%r8\n" is dubious.

I removed it and the kernel still worked fine for my regular VM (on an AMD host)
and for my SNP VM (with HCL). 

I suspect a fully enlightened SNP VM also doesn't need it as this hypercall
doesn't really need an output param. 

I noticed your 
[PATCH V5 8/8] x86/hyperv: Add hyperv-specific handling for VMMCALL under SEV-ES
exposes r8 to the hypervisor:

+static void hv_sev_es_hcall_prepare(struct ghcb *ghcb, struct pt_regs *regs)
+{
+       /* RAX and CPL are already in the GHCB */
+       ghcb_set_rcx(ghcb, regs->cx);
+       ghcb_set_rdx(ghcb, regs->dx);
+       ghcb_set_r8(ghcb, regs->r8);
+}

I guess the intent here is that we want to pass a deterministic value in R8 (rather
a random value) to the hypervisor for security's purpose. If so, can we just set
R8 to 0 rather than %[thunk_target]?

Please add a comment.

Sorry, I was not in the earlier discussion, so I may be missing something.

> +				     ALTERNATIVE(CALL_NOSPEC, "vmmcall",
> X86_FEATURE_SEV_ES)
>  				     : "=a" (hv_status),
> ASM_CALL_CONSTRAINT,
>  				       "+c" (control), "+d" (input1)
>  				     : THUNK_TARGET(hv_hypercall_pg)
diff mbox series

Patch

diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 9f11f0495950..07cad6c2af56 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -62,12 +62,12 @@  static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
 	if (!hv_hypercall_pg)
 		return U64_MAX;
 
-	__asm__ __volatile__("mov %4, %%r8\n"
-			     CALL_NOSPEC
+	__asm__ __volatile__("mov %[output], %%r8\n"
+			     ALTERNATIVE(CALL_NOSPEC, "vmmcall", X86_FEATURE_SEV_ES)
 			     : "=a" (hv_status), ASM_CALL_CONSTRAINT,
-			       "+c" (control), "+d" (input_address)
-			     :  "r" (output_address),
-				THUNK_TARGET(hv_hypercall_pg)
+			     "+c" (control), "+d" (input_address)
+			     : [output] "r" (output_address),
+			       THUNK_TARGET(hv_hypercall_pg)
 			     : "cc", "memory", "r8", "r9", "r10", "r11");
 #else
 	u32 input_address_hi = upper_32_bits(input_address);
@@ -103,7 +103,8 @@  static inline u64 _hv_do_fast_hypercall8(u64 control, u64 input1)
 
 #ifdef CONFIG_X86_64
 	{
-		__asm__ __volatile__(CALL_NOSPEC
+		__asm__ __volatile__("mov %[thunk_target], %%r8\n"
+				     ALTERNATIVE(CALL_NOSPEC, "vmmcall", X86_FEATURE_SEV_ES)
 				     : "=a" (hv_status), ASM_CALL_CONSTRAINT,
 				       "+c" (control), "+d" (input1)
 				     : THUNK_TARGET(hv_hypercall_pg)
@@ -148,13 +149,13 @@  static inline u64 _hv_do_fast_hypercall16(u64 control, u64 input1, u64 input2)
 
 #ifdef CONFIG_X86_64
 	{
-		__asm__ __volatile__("mov %4, %%r8\n"
-				     CALL_NOSPEC
-				     : "=a" (hv_status), ASM_CALL_CONSTRAINT,
-				       "+c" (control), "+d" (input1)
-				     : "r" (input2),
-				       THUNK_TARGET(hv_hypercall_pg)
-				     : "cc", "r8", "r9", "r10", "r11");
+		__asm__ __volatile__("mov %[output], %%r8\n"
+		     ALTERNATIVE(CALL_NOSPEC, "vmmcall", X86_FEATURE_SEV_ES)
+		     : "=a" (hv_status), ASM_CALL_CONSTRAINT,
+		       "+c" (control), "+d" (input1)
+		     : [output] "r" (input2),
+		       THUNK_TARGET(hv_hypercall_pg)
+		     : "cc", "r8", "r9", "r10", "r11");
 	}
 #else
 	{