| Message ID | 20221119034633.1728632-6-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 > > sev-snp guest provides vtl(Virtual Trust Level) and get it from > hyperv hvcall via HVCALL_GET_VP_REGISTERS. Two general comments: 1) Could this patch be combined with Patch 9 of the series? It seems like they go together since Patch 9 is the consumer of the VTL. 2) What is the bigger picture motivation for this patch and Patch 9 being part of the patch series for support fully enlightened SEV-SNP guests? Won't the VTL always be 0 in such a guest? The code currently assumes VTL 0, so it seems like this patch doesn't change anything. Or maybe there's a scenario that I'm not aware of where the VTL is other than 0. > > Signed-off-by: Tianyu Lan <tiala@microsoft.com> > --- > arch/x86/hyperv/hv_init.c | 35 ++++++++++++++++++++++++++++++++++ > include/asm-generic/mshyperv.h | 2 ++ > 2 files changed, 37 insertions(+) > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index 4600c5941957..5b919d4d24c0 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -390,6 +390,39 @@ static void __init hv_get_partition_id(void) > local_irq_restore(flags); > } > > +static u8 __init get_current_vtl(void) The name get_current_vtl() seems to imply that there might be a "previous" VTL, or that the VTL might change over time. I'm not aware that either is the case. Couldn't this just be get_vtl()? > +{ > + u64 control = ((u64)1 << HV_HYPERCALL_REP_COMP_OFFSET) | HVCALL_GET_VP_REGISTERS; Simplify by using HV_HYPERCALL_REP_COMP_1. > + struct hv_get_vp_registers_input *input = NULL; > + struct hv_get_vp_registers_output *output = NULL; It doesn't seem like the above two initializations to NULL are needed. > + u8 vtl = 0; > + int ret; The result of hv_do_hypercall() should always be a u64. > + unsigned long flags; > + > + local_irq_save(flags); > + input = *(struct hv_get_vp_registers_input **)this_cpu_ptr(hyperv_pcpu_input_arg); > + output = (struct hv_get_vp_registers_output *)input; > + if (!input || !output) { Don't need to check both values since one is assigned from the other. :-) > + pr_err("Hyper-V: cannot allocate a shared page!"); Error message text isn't correct. > + goto done; Need to do local_irq_restore() before goto done. > + } > + > + memset(input, 0, sizeof(*input) + sizeof(input->element[0])); > + input->header.partitionid = HV_PARTITION_ID_SELF; > + input->header.inputvtl = 0; > + input->element[0].name0 = 0x000D0003; This constant should go in one of the hyperv-tlfs.h header files. If I recall correctly, we're currently treating VTLs as x86-specific, so should go in arch/x86/include/asm/hyperv-tlfs.h. > + > + ret = hv_do_hypercall(control, input, output); > + if (ret == 0) Use hv_result_success(ret). > + vtl = output->as64.low & 0xf; The 0xF mask should be defined in the hyperv-tlfs.h per above. > + else > + pr_err("Hyper-V: failed to get the current VTL!"); Again, drop the word "current". And the exclamation mark isn't needed. :-) > + local_irq_restore(flags); > + > +done: > + return vtl; > +} > + > /* > * This function is to be invoked early in the boot sequence after the > * hypervisor has been detected. > @@ -527,6 +560,8 @@ void __init hyperv_init(void) > if (hv_is_isolation_supported()) > swiotlb_update_mem_attributes(); > #endif > + /* Find the current VTL */ > + ms_hyperv.vtl = get_current_vtl(); Drop "current" in the comment and function name. > > return; > > diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h > index bfb9eb9d7215..68133de044ec 100644 > --- a/include/asm-generic/mshyperv.h > +++ b/include/asm-generic/mshyperv.h > @@ -46,6 +46,7 @@ struct ms_hyperv_info { > }; > }; > u64 shared_gpa_boundary; > + u8 vtl; > }; > extern struct ms_hyperv_info ms_hyperv; > > @@ -55,6 +56,7 @@ extern void * __percpu *hyperv_pcpu_output_arg; > extern u64 hv_do_hypercall(u64 control, void *inputaddr, void *outputaddr); > extern u64 hv_do_fast_hypercall8(u16 control, u64 input8); > extern bool hv_isolation_type_snp(void); > +extern bool hv_isolation_type_en_snp(void); This declaration of hv_isolation_type_en_snp() shouldn't be needed here as it has already been added to arch/x86/include/asm/mshyperv.h. The declaration of hv_isolation_type_snp() occurs both places, but I think that's some sloppiness from the past that could be fixed. In fact, hv_isolation_type_snp() occurs *twice* in include/asm-generic/mshyperv.h. > > /* Helper functions that provide a consistent pattern for checking Hyper-V hypercall > status. */ > static inline int hv_result(u64 status) > -- > 2.25.1
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index 4600c5941957..5b919d4d24c0 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -390,6 +390,39 @@ static void __init hv_get_partition_id(void) local_irq_restore(flags); } +static u8 __init get_current_vtl(void) +{ + u64 control = ((u64)1 << HV_HYPERCALL_REP_COMP_OFFSET) | HVCALL_GET_VP_REGISTERS; + struct hv_get_vp_registers_input *input = NULL; + struct hv_get_vp_registers_output *output = NULL; + u8 vtl = 0; + int ret; + unsigned long flags; + + local_irq_save(flags); + input = *(struct hv_get_vp_registers_input **)this_cpu_ptr(hyperv_pcpu_input_arg); + output = (struct hv_get_vp_registers_output *)input; + if (!input || !output) { + pr_err("Hyper-V: cannot allocate a shared page!"); + goto done; + } + + memset(input, 0, sizeof(*input) + sizeof(input->element[0])); + input->header.partitionid = HV_PARTITION_ID_SELF; + input->header.inputvtl = 0; + input->element[0].name0 = 0x000D0003; + + ret = hv_do_hypercall(control, input, output); + if (ret == 0) + vtl = output->as64.low & 0xf; + else + pr_err("Hyper-V: failed to get the current VTL!"); + local_irq_restore(flags); + +done: + return vtl; +} + /* * This function is to be invoked early in the boot sequence after the * hypervisor has been detected. @@ -527,6 +560,8 @@ void __init hyperv_init(void) if (hv_is_isolation_supported()) swiotlb_update_mem_attributes(); #endif + /* Find the current VTL */ + ms_hyperv.vtl = get_current_vtl(); return; diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h index bfb9eb9d7215..68133de044ec 100644 --- a/include/asm-generic/mshyperv.h +++ b/include/asm-generic/mshyperv.h @@ -46,6 +46,7 @@ struct ms_hyperv_info { }; }; u64 shared_gpa_boundary; + u8 vtl; }; extern struct ms_hyperv_info ms_hyperv; @@ -55,6 +56,7 @@ extern void * __percpu *hyperv_pcpu_output_arg; extern u64 hv_do_hypercall(u64 control, void *inputaddr, void *outputaddr); extern u64 hv_do_fast_hypercall8(u16 control, u64 input8); extern bool hv_isolation_type_snp(void); +extern bool hv_isolation_type_en_snp(void); /* Helper functions that provide a consistent pattern for checking Hyper-V hypercall status. */ static inline int hv_result(u64 status)