| Message ID | 20221119034633.1728632-7-ltykernel@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/hyperv/sev: Add AMD sev-snp enlightened guest support on hyperv | expand |
From: Tianyu Lan <ltykernel@gmail.com> Sent: Friday, November 18, 2022 7:46 PM > > In sev-snp enlightened guest, hvcall needs to use vmmcall to trigger What does "hvcall" refer to here? Is this a Hyper-V hypercall, or just a generic hypervisor call? > vmexit and notify hypervisor to handle hypercall request. > > Signed-off-by: Tianyu Lan <tiala@microsoft.com> > --- > arch/x86/include/asm/mshyperv.h | 66 ++++++++++++++++++++++----------- > 1 file changed, 45 insertions(+), 21 deletions(-) > > diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h > index 9b8c3f638845..28d5429e33c9 100644 > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -45,16 +45,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"); > + } > #else > u32 input_address_hi = upper_32_bits(input_address); > u32 input_address_lo = lower_32_bits(input_address); > @@ -82,12 +91,18 @@ static inline u64 hv_do_fast_hypercall8(u16 code, u64 input1) > u64 hv_status, control = (u64)code | HV_HYPERCALL_FAST_BIT; > > #ifdef CONFIG_X86_64 > - { > + if (hv_isolation_type_en_snp()) { > + __asm__ __volatile__( > + "vmmcall" > + : "=a" (hv_status), ASM_CALL_CONSTRAINT, > + "+c" (control), "+d" (input1) > + :: "cc", "r8", "r9", "r10", "r11"); > + } else { > __asm__ __volatile__(CALL_NOSPEC > - : "=a" (hv_status), ASM_CALL_CONSTRAINT, > - "+c" (control), "+d" (input1) > - : THUNK_TARGET(hv_hypercall_pg) > - : "cc", "r8", "r9", "r10", "r11"); > + : "=a" (hv_status), ASM_CALL_CONSTRAINT, > + "+c" (control), "+d" (input1) > + : THUNK_TARGET(hv_hypercall_pg) > + : "cc", "r8", "r9", "r10", "r11"); The above 4 lines appear to just have changes in indentation. Maybe there's value in having the same indentation as the new code you've added, so I won't object if you want to keep the changes. > } > #else > { > @@ -113,14 +128,21 @@ static inline u64 hv_do_fast_hypercall16(u16 code, u64 input1, u64 input2) > u64 hv_status, control = (u64)code | HV_HYPERCALL_FAST_BIT; > > #ifdef CONFIG_X86_64 > - { > + if (hv_isolation_type_en_snp()) { > __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"); > + "vmmcall" > + : "=a" (hv_status), ASM_CALL_CONSTRAINT, > + "+c" (control), "+d" (input1) > + : "r" (input2) > + : "cc", "r8", "r9", "r10", "r11"); > + } else { > + __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"); Same here. Above 5 lines appear to be changes only in indentation. > } > #else > { > @@ -177,6 +199,7 @@ int hv_map_ioapic_interrupt(int ioapic_id, bool level, int vcpu, > int vector, > struct hv_interrupt_entry *entry); > int hv_unmap_ioapic_interrupt(int ioapic_id, struct hv_interrupt_entry *entry); > int hv_set_mem_host_visibility(unsigned long addr, int numpages, bool visible); > +int hv_snp_boot_ap(int cpu, unsigned long start_ip); This declaration doesn't seem to belong in this patch. It should be in Patch 13 of the series. > > #ifdef CONFIG_AMD_MEM_ENCRYPT > void hv_ghcb_msr_write(u64 msr, u64 value); > @@ -191,6 +214,7 @@ static inline void hv_ghcb_terminate(unsigned int set, unsigned > int reason) {} > #endif > > extern bool hv_isolation_type_snp(void); > +extern bool hv_isolation_type_en_snp(void); This declaration seems to be a duplicate that doesn't belong in this patch. > > static inline bool hv_is_synic_reg(unsigned int reg) > { > -- > 2.25.1
On 12/14/2022 1:19 AM, Michael Kelley (LINUX) wrote: > From: Tianyu Lan <ltykernel@gmail.com> Sent: Friday, November 18, 2022 7:46 PM >> >> In sev-snp enlightened guest, hvcall needs to use vmmcall to trigger > > What does "hvcall" refer to here? Is this a Hyper-V hypercall, or just > a generic hypervisor call? It's should be Hyper-V hypercall. Will make it accurate in the next version. Thanks for reminder. > >> vmexit and notify hypervisor to handle hypercall request. >> >> Signed-off-by: Tianyu Lan <tiala@microsoft.com> >> --- >> arch/x86/include/asm/mshyperv.h | 66 ++++++++++++++++++++++----------- >> 1 file changed, 45 insertions(+), 21 deletions(-) >> >> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h >> index 9b8c3f638845..28d5429e33c9 100644 >> --- a/arch/x86/include/asm/mshyperv.h >> +++ b/arch/x86/include/asm/mshyperv.h >> @@ -45,16 +45,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"); >> + } >> #else >> u32 input_address_hi = upper_32_bits(input_address); >> u32 input_address_lo = lower_32_bits(input_address); >> @@ -82,12 +91,18 @@ static inline u64 hv_do_fast_hypercall8(u16 code, u64 input1) >> u64 hv_status, control = (u64)code | HV_HYPERCALL_FAST_BIT; >> >> #ifdef CONFIG_X86_64 >> - { >> + if (hv_isolation_type_en_snp()) { >> + __asm__ __volatile__( >> + "vmmcall" >> + : "=a" (hv_status), ASM_CALL_CONSTRAINT, >> + "+c" (control), "+d" (input1) >> + :: "cc", "r8", "r9", "r10", "r11"); >> + } else { >> __asm__ __volatile__(CALL_NOSPEC >> - : "=a" (hv_status), ASM_CALL_CONSTRAINT, >> - "+c" (control), "+d" (input1) >> - : THUNK_TARGET(hv_hypercall_pg) >> - : "cc", "r8", "r9", "r10", "r11"); >> + : "=a" (hv_status), ASM_CALL_CONSTRAINT, >> + "+c" (control), "+d" (input1) >> + : THUNK_TARGET(hv_hypercall_pg) >> + : "cc", "r8", "r9", "r10", "r11"); > > The above 4 lines appear to just have changes in indentation. Maybe > there's value in having the same indentation as the new code you've > added, so I won't object if you want to keep the changes. OK. Will update in the next version. > >> } >> #else >> { >> @@ -113,14 +128,21 @@ static inline u64 hv_do_fast_hypercall16(u16 code, u64 input1, u64 input2) >> u64 hv_status, control = (u64)code | HV_HYPERCALL_FAST_BIT; >> >> #ifdef CONFIG_X86_64 >> - { >> + if (hv_isolation_type_en_snp()) { >> __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"); >> + "vmmcall" >> + : "=a" (hv_status), ASM_CALL_CONSTRAINT, >> + "+c" (control), "+d" (input1) >> + : "r" (input2) >> + : "cc", "r8", "r9", "r10", "r11"); >> + } else { >> + __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"); >
> From: Tianyu Lan <ltykernel@gmail.com> > Sent: Friday, November 18, 2022 7:46 PM > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -45,16 +45,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"); Add a "return hv_status;" here to avoid chaning the indentation below? > + } 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"); > + } > #else
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h index 9b8c3f638845..28d5429e33c9 100644 --- a/arch/x86/include/asm/mshyperv.h +++ b/arch/x86/include/asm/mshyperv.h @@ -45,16 +45,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"); + } #else u32 input_address_hi = upper_32_bits(input_address); u32 input_address_lo = lower_32_bits(input_address); @@ -82,12 +91,18 @@ static inline u64 hv_do_fast_hypercall8(u16 code, u64 input1) u64 hv_status, control = (u64)code | HV_HYPERCALL_FAST_BIT; #ifdef CONFIG_X86_64 - { + if (hv_isolation_type_en_snp()) { + __asm__ __volatile__( + "vmmcall" + : "=a" (hv_status), ASM_CALL_CONSTRAINT, + "+c" (control), "+d" (input1) + :: "cc", "r8", "r9", "r10", "r11"); + } else { __asm__ __volatile__(CALL_NOSPEC - : "=a" (hv_status), ASM_CALL_CONSTRAINT, - "+c" (control), "+d" (input1) - : THUNK_TARGET(hv_hypercall_pg) - : "cc", "r8", "r9", "r10", "r11"); + : "=a" (hv_status), ASM_CALL_CONSTRAINT, + "+c" (control), "+d" (input1) + : THUNK_TARGET(hv_hypercall_pg) + : "cc", "r8", "r9", "r10", "r11"); } #else { @@ -113,14 +128,21 @@ static inline u64 hv_do_fast_hypercall16(u16 code, u64 input1, u64 input2) u64 hv_status, control = (u64)code | HV_HYPERCALL_FAST_BIT; #ifdef CONFIG_X86_64 - { + if (hv_isolation_type_en_snp()) { __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"); + "vmmcall" + : "=a" (hv_status), ASM_CALL_CONSTRAINT, + "+c" (control), "+d" (input1) + : "r" (input2) + : "cc", "r8", "r9", "r10", "r11"); + } else { + __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"); } #else { @@ -177,6 +199,7 @@ int hv_map_ioapic_interrupt(int ioapic_id, bool level, int vcpu, int vector, struct hv_interrupt_entry *entry); int hv_unmap_ioapic_interrupt(int ioapic_id, struct hv_interrupt_entry *entry); int hv_set_mem_host_visibility(unsigned long addr, int numpages, bool visible); +int hv_snp_boot_ap(int cpu, unsigned long start_ip); #ifdef CONFIG_AMD_MEM_ENCRYPT void hv_ghcb_msr_write(u64 msr, u64 value); @@ -191,6 +214,7 @@ static inline void hv_ghcb_terminate(unsigned int set, unsigned int reason) {} #endif extern bool hv_isolation_type_snp(void); +extern bool hv_isolation_type_en_snp(void); static inline bool hv_is_synic_reg(unsigned int reg) {