| Message ID | 20230122024607.788454-4-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: Saturday, January 21, 2023 6:46 PM > > sev-snp guest provides vtl(Virtual Trust Level) and > get it from hyperv hvcall via HVCALL_GET_VP_REGISTERS. > Set target vtl in the vmbus init message. I'm still wondering why this is necessary in an SNP VM, vs. just assuming VTL 0. Also, I had several comments on v2 of this patch that don't appear to have been taken into account. I strongly think the code should use the standard helper functions for checking hypercall results. Some of my other code comments are more nit-picky and could perhaps be ignored. :-) > > Signed-off-by: Tianyu Lan <tiala@microsoft.com> > --- > Change since RFC v2: > * Rename get_current_vtl() to get_vtl() > * Fix some coding style issues > --- > arch/x86/hyperv/hv_init.c | 37 ++++++++++++++++++++++++++++++ > arch/x86/include/asm/hyperv-tlfs.h | 4 ++++ > drivers/hv/connection.c | 1 + > include/asm-generic/mshyperv.h | 2 ++ > include/linux/hyperv.h | 4 ++-- > 5 files changed, 46 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index 24154c1ee12b..9e9757049915 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -384,6 +384,40 @@ static void __init hv_get_partition_id(void) > local_irq_restore(flags); > } > > +static u8 __init get_vtl(void) > +{ > + u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; > + struct hv_get_vp_registers_input *input = NULL; > + struct hv_get_vp_registers_output *output = NULL; > + u64 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) { > + local_irq_restore(flags); > + goto done; > + } > + > + memset(input, 0, sizeof(*input) + sizeof(input->element[0])); > + input->header.partitionid = HV_PARTITION_ID_SELF; > + input->header.vpindex = HV_VP_INDEX_SELF; > + input->header.inputvtl = 0; > + input->element[0].name0 = HV_X64_REGISTER_VSM_VP_STATUS; > + > + ret = hv_do_hypercall(control, input, output); > + if (ret == 0) > + vtl = output->as64.low & HV_X64_VTL_MASK; > + else > + pr_err("Hyper-V: failed to get 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. > @@ -512,6 +546,9 @@ void __init hyperv_init(void) > /* Query the VMs extended capability once, so that it can be cached. */ > hv_query_ext_cap(0); > > + /* Find the VTL */ > + ms_hyperv.vtl = get_vtl(); > + > return; > > clean_guest_os_id: > diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h > index db2202d985bd..6dcbb21aac2b 100644 > --- a/arch/x86/include/asm/hyperv-tlfs.h > +++ b/arch/x86/include/asm/hyperv-tlfs.h > @@ -36,6 +36,10 @@ > #define HYPERV_CPUID_MIN 0x40000005 > #define HYPERV_CPUID_MAX 0x4000ffff > > +/* Support for HVCALL_GET_VP_REGISTERS hvcall */ The above comment isn't really right, in that these definitions aren't for the hypercall. They are for the specific synthetic register. > +#define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003 > +#define HV_X64_VTL_MASK GENMASK(3, 0) Hyper-V synthetic registers have two different numbering schemes. For registers that have synthetic MSR equivalents, there's a full list starting with HV_X64_MSR_GUEST_OS_ID, which defines the MSR address. But these registers also have register numbers that are not the same as the MSR address. These register numbers aren't defined anywhere in x86 Linux code because we don't access them using the register number. (The register numbers *are* defined in ARM64 code since ARM64 doesn't have MSRs.) But this register is an exception on x86. There's no MSR equivalent so we must use a hypercall to fetch the value. I'd suggest starting a separate list after the definition of HV_X64_MSR_REFERENCE_TSC and make clear in a comment about the list that this is a list of register numbers, not MSR addresses. > + > /* > * Group D Features. The bit assignments are custom to each architecture. > * On x86/x64 these are HYPERV_CPUID_FEATURES.EDX bits. > diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c > index f670cfd2e056..e4c39f4016ad 100644 > --- a/drivers/hv/connection.c > +++ b/drivers/hv/connection.c > @@ -98,6 +98,7 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo > *msginfo, u32 version) > */ > if (version >= VERSION_WIN10_V5) { > msg->msg_sint = VMBUS_MESSAGE_SINT; > + msg->msg_vtl = ms_hyperv.vtl; > vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID_4; > } else { > msg->interrupt_page = virt_to_phys(vmbus_connection.int_page); > diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h > index f2c0856f1797..44e56777fea7 100644 > --- a/include/asm-generic/mshyperv.h > +++ b/include/asm-generic/mshyperv.h > @@ -48,6 +48,7 @@ struct ms_hyperv_info { > }; > }; > u64 shared_gpa_boundary; > + u8 vtl; > }; > extern struct ms_hyperv_info ms_hyperv; > > @@ -57,6 +58,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) > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h > index 85f7c5a63aa6..65121b21b0af 100644 > --- a/include/linux/hyperv.h > +++ b/include/linux/hyperv.h > @@ -665,8 +665,8 @@ struct vmbus_channel_initiate_contact { > u64 interrupt_page; > struct { > u8 msg_sint; > - u8 padding1[3]; > - u32 padding2; > + u8 msg_vtl; > + u8 reserved[6]; > }; > }; > u64 monitor_page1; > -- > 2.25.1
On 2/1/2023 1:55 AM, Michael Kelley (LINUX) wrote: >> index db2202d985bd..6dcbb21aac2b 100644 >> --- a/arch/x86/include/asm/hyperv-tlfs.h >> +++ b/arch/x86/include/asm/hyperv-tlfs.h >> @@ -36,6 +36,10 @@ >> #define HYPERV_CPUID_MIN 0x40000005 >> #define HYPERV_CPUID_MAX 0x4000ffff >> >> +/* Support for HVCALL_GET_VP_REGISTERS hvcall */ > The above comment isn't really right, in that these definitions > aren't for the hypercall. They are for the specific synthetic register. > >> +#define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003 >> +#define HV_X64_VTL_MASK GENMASK(3, 0) > Hyper-V synthetic registers have two different numbering schemes. > For registers that have synthetic MSR equivalents, there's a full list > starting with HV_X64_MSR_GUEST_OS_ID, which defines the MSR > address. But these registers also have register numbers that are > not the same as the MSR address. These register numbers > aren't defined anywhere in x86 Linux code because we don't access > them using the register number. (The register numbers*are* > defined in ARM64 code since ARM64 doesn't have MSRs.) But this > register is an exception on x86. There's no MSR equivalent so we > must use a hypercall to fetch the value. > > I'd suggest starting a separate list after the definition of > HV_X64_MSR_REFERENCE_TSC and make clear in a comment > about the list that this is a list of register numbers, not MSR addresses. > Agree. Will update in the next version.
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index 24154c1ee12b..9e9757049915 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -384,6 +384,40 @@ static void __init hv_get_partition_id(void) local_irq_restore(flags); } +static u8 __init get_vtl(void) +{ + u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; + struct hv_get_vp_registers_input *input = NULL; + struct hv_get_vp_registers_output *output = NULL; + u64 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) { + local_irq_restore(flags); + goto done; + } + + memset(input, 0, sizeof(*input) + sizeof(input->element[0])); + input->header.partitionid = HV_PARTITION_ID_SELF; + input->header.vpindex = HV_VP_INDEX_SELF; + input->header.inputvtl = 0; + input->element[0].name0 = HV_X64_REGISTER_VSM_VP_STATUS; + + ret = hv_do_hypercall(control, input, output); + if (ret == 0) + vtl = output->as64.low & HV_X64_VTL_MASK; + else + pr_err("Hyper-V: failed to get 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. @@ -512,6 +546,9 @@ void __init hyperv_init(void) /* Query the VMs extended capability once, so that it can be cached. */ hv_query_ext_cap(0); + /* Find the VTL */ + ms_hyperv.vtl = get_vtl(); + return; clean_guest_os_id: diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h index db2202d985bd..6dcbb21aac2b 100644 --- a/arch/x86/include/asm/hyperv-tlfs.h +++ b/arch/x86/include/asm/hyperv-tlfs.h @@ -36,6 +36,10 @@ #define HYPERV_CPUID_MIN 0x40000005 #define HYPERV_CPUID_MAX 0x4000ffff +/* Support for HVCALL_GET_VP_REGISTERS hvcall */ +#define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003 +#define HV_X64_VTL_MASK GENMASK(3, 0) + /* * Group D Features. The bit assignments are custom to each architecture. * On x86/x64 these are HYPERV_CPUID_FEATURES.EDX bits. diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index f670cfd2e056..e4c39f4016ad 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -98,6 +98,7 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version) */ if (version >= VERSION_WIN10_V5) { msg->msg_sint = VMBUS_MESSAGE_SINT; + msg->msg_vtl = ms_hyperv.vtl; vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID_4; } else { msg->interrupt_page = virt_to_phys(vmbus_connection.int_page); diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h index f2c0856f1797..44e56777fea7 100644 --- a/include/asm-generic/mshyperv.h +++ b/include/asm-generic/mshyperv.h @@ -48,6 +48,7 @@ struct ms_hyperv_info { }; }; u64 shared_gpa_boundary; + u8 vtl; }; extern struct ms_hyperv_info ms_hyperv; @@ -57,6 +58,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) diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 85f7c5a63aa6..65121b21b0af 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -665,8 +665,8 @@ struct vmbus_channel_initiate_contact { u64 interrupt_page; struct { u8 msg_sint; - u8 padding1[3]; - u32 padding2; + u8 msg_vtl; + u8 reserved[6]; }; }; u64 monitor_page1;