Messages in this thread | | | From | Dexuan Cui <> | Subject | RE: [PATCH v6 5/8] x86/hyperv: Use vmmcall to implement Hyper-V hypercall in sev-snp enlightened guest | Date | Wed, 16 Aug 2023 21:24:09 +0000 |
| |
> From: Tianyu Lan <ltykernel@gmail.com> > Sent: Wednesday, August 16, 2023 8:59 AM > To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang > <haiyangz@microsoft.com>; wei.liu@kernel.org; Dexuan Cui > [...] > 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> > --- > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -59,16 +59,25 @@ static inline u64 hv_do_hypercall(u64 control, void > *input, void *output) > u64 hv_status; > > #ifdef CONFIG_X86_64 > - if (!hv_hypercall_pg) > - return U64_MAX; > + if (hv_isolation_type_en_snp()) { > + __asm__ __volatile__("mov %4, %%r8\n" > + "vmmcall" > + : "=a" (hv_status), > ASM_CALL_CONSTRAINT, > + "+c" (control), "+d" (input_address) > + : "r" (output_address) > + : "cc", "memory", "r8", "r9", "r10", "r11"); > + } else { > + if (!hv_hypercall_pg) > + return U64_MAX; > > - __asm__ __volatile__("mov %4, %%r8\n" > - CALL_NOSPEC > - : "=a" (hv_status), ASM_CALL_CONSTRAINT, > - "+c" (control), "+d" (input_address) > - : "r" (output_address), > - THUNK_TARGET(hv_hypercall_pg) > - : "cc", "memory", "r8", "r9", "r10", "r11"); > + __asm__ __volatile__("mov %4, %%r8\n" > + CALL_NOSPEC > + : "=a" (hv_status), > ASM_CALL_CONSTRAINT, > + "+c" (control), "+d" (input_address) > + : "r" (output_address), > + THUNK_TARGET(hv_hypercall_pg) > + : "cc", "memory", "r8", "r9", "r10", "r11"); > + }
IMO it's better if we add a "return hv_status;" for the SNP case, and don't move the assembly code for the regular VM. I made a patch: https://github.com/dcui/tdx/commit/f81013578605aa02939a3186afa9fc76791b3acd
You may want to explain briefly why the earlier approach ALTERNATIVE(CALL_NOSPEC, "vmmcall", X86_FEATURE_SEV_ES) doesn't work:
start_kernel() calls hyperv_init() before alternative_instructions(), and hyperv_init() already uses hypercalls, e.g. the newly-added get_vtl() in your patch 2.
start_kernel: late_time_init x86_late_time_init x86_init.irqs.intr_mode_init apic_intr_mode_init x86_platform.apic_post_init hyperv_init ==> it already uses hypercalls, e.g. the newly-added get_vtl() in your patch 2.
arch_cpu_finalize_init() alternative_instructions()
We can move the get_vtl hypercall to a later place, but there are also other hypercalls before alternative_instructions(). IMO it may be unsafe to run ALTERNATIVE code before alternative_instructions() is called.
| |