| Message ID | 20240726225910.1912537-4-romank@linux.microsoft.com |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: hyperv: Support Virtual Trust Level Boot | expand |
On Fri, Jul 26, 2024 at 03:59:06PM -0700, Roman Kisel wrote: > To run in the VTL mode, Hyper-V drivers have to know what > VTL the system boots in, and the arm64/hyperv code does not > have the means to compute that. > > Refactor the code to hoist the function that detects VTL, > make it arch-neutral to be able to employ it to get the VTL > on arm64. Fix the hypercall output address in `get_vtl(void)` > not to overlap with the hypercall input area to adhere to > the Hyper-V TLFS. Can you split the fix out? That potentially can be backported. > > Signed-off-by: Roman Kisel <romank@linux.microsoft.com> > --- > + > +#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) > +u8 __init get_vtl(void) > +{ > + u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; > + struct hv_get_vp_registers_input *input; > + struct hv_get_vp_registers_output *output; > + unsigned long flags; > + u64 ret; > + > + local_irq_save(flags); > + input = *this_cpu_ptr(hyperv_pcpu_input_arg); > + output = *this_cpu_ptr(hyperv_pcpu_output_arg); Hmm... I don't remember why the old code didn't use hyperv_pcpu_output_arg but instead reused input+OFFSET as output. Saurabh, can you comment on this? Thanks, Wei.
From: Roman Kisel <romank@linux.microsoft.com> Sent: Friday, July 26, 2024 3:59 PM > > To run in the VTL mode, Hyper-V drivers have to know what > VTL the system boots in, and the arm64/hyperv code does not > have the means to compute that. > > Refactor the code to hoist the function that detects VTL, > make it arch-neutral to be able to employ it to get the VTL > on arm64. Fix the hypercall output address in `get_vtl(void)` > not to overlap with the hypercall input area to adhere to > the Hyper-V TLFS. > > Signed-off-by: Roman Kisel <romank@linux.microsoft.com> > --- > arch/x86/hyperv/hv_init.c | 34 --------------------- > arch/x86/include/asm/hyperv-tlfs.h | 7 ----- > drivers/hv/hv_common.c | 47 ++++++++++++++++++++++++++++-- > include/asm-generic/hyperv-tlfs.h | 7 +++++ > include/asm-generic/mshyperv.h | 6 ++++ > 5 files changed, 58 insertions(+), 43 deletions(-) > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index 17a71e92a343..c350fa05ee59 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -413,40 +413,6 @@ static void __init hv_get_partition_id(void) > local_irq_restore(flags); > } > > -#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) > -static u8 __init get_vtl(void) > -{ > - u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; > - struct hv_get_vp_registers_input *input; > - struct hv_get_vp_registers_output *output; > - unsigned long flags; > - u64 ret; > - > - local_irq_save(flags); > - input = *this_cpu_ptr(hyperv_pcpu_input_arg); > - output = (struct hv_get_vp_registers_output *)input; > - > - memset(input, 0, struct_size(input, element, 1)); > - 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 (hv_result_success(ret)) { > - ret = output->as64.low & HV_X64_VTL_MASK; > - } else { > - pr_err("Failed to get VTL(error: %lld) exiting...\n", ret); > - BUG(); > - } > - > - local_irq_restore(flags); > - return ret; > -} > -#else > -static inline u8 get_vtl(void) { return 0; } > -#endif > - > /* > * This function is to be invoked early in the boot sequence after the > * hypervisor has been detected. > diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h > index 3787d26810c1..9ee68eb8e6ff 100644 > --- a/arch/x86/include/asm/hyperv-tlfs.h > +++ b/arch/x86/include/asm/hyperv-tlfs.h > @@ -309,13 +309,6 @@ enum hv_isolation_type { > #define HV_MSR_STIMER0_CONFIG (HV_X64_MSR_STIMER0_CONFIG) > #define HV_MSR_STIMER0_COUNT (HV_X64_MSR_STIMER0_COUNT) > > -/* > - * Registers are only accessible via HVCALL_GET_VP_REGISTERS hvcall and > - * there is not associated MSR address. > - */ > -#define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003 > -#define HV_X64_VTL_MASK GENMASK(3, 0) > - > /* Hyper-V memory host visibility */ > enum hv_mem_host_visibility { > VMBUS_PAGE_NOT_VISIBLE = 0, > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c > index 9c452bfbd571..7d6c1523b0b5 100644 > --- a/drivers/hv/hv_common.c > +++ b/drivers/hv/hv_common.c > @@ -339,8 +339,8 @@ int __init hv_common_init(void) > hyperv_pcpu_input_arg = alloc_percpu(void *); > BUG_ON(!hyperv_pcpu_input_arg); > > - /* Allocate the per-CPU state for output arg for root */ > - if (hv_root_partition) { > + /* Allocate the per-CPU state for output arg for root or a VTL */ > + if (hv_root_partition || IS_ENABLED(CONFIG_HYPERV_VTL_MODE)) { > hyperv_pcpu_output_arg = alloc_percpu(void *); > BUG_ON(!hyperv_pcpu_output_arg); > } > @@ -656,3 +656,46 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 > param2) > return HV_STATUS_INVALID_PARAMETER; > } > EXPORT_SYMBOL_GPL(hv_tdx_hypercall); > + > +#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) > +u8 __init get_vtl(void) > +{ > + u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; > + struct hv_get_vp_registers_input *input; > + struct hv_get_vp_registers_output *output; > + unsigned long flags; > + u64 ret; > + > + local_irq_save(flags); > + input = *this_cpu_ptr(hyperv_pcpu_input_arg); > + output = *this_cpu_ptr(hyperv_pcpu_output_arg); Rather than use the hyperv_pcpu_output_arg here, it's OK to use a different area of the hyperv_pcpu_input_arg page. For example, output = (void *)input + HV_HYP_PAGE_SIZE/2; The TLFS does not require that the input and output be in separate pages. While using the hyperv_pcpu_output_arg is conceptually a bit cleaner, doing so requires allocating a 4K page per CPU that is not otherwise used. The VTL 2 code wants to be frugal with memory, and this seems like a good step in that direction. :-) The hyperv_pcpu_output_arg was added for the cases where up to a full page is needed for input and output on the same hypercall. So far, the only case of that is when running in the root partition. > + > + memset(input, 0, struct_size(input, element, 1)); > + input->header.partitionid = HV_PARTITION_ID_SELF; > + input->header.vpindex = HV_VP_INDEX_SELF; > + input->header.inputvtl = 0; > + input->element[0].name0 = HV_REGISTER_VSM_VP_STATUS; > + > + ret = hv_do_hypercall(control, input, output); > + if (hv_result_success(ret)) { > + ret = output->as64.low & HV_VTL_MASK; > + } else { > + pr_err("Failed to get VTL(error: %lld) exiting...\n", ret); > + > + /* > + * This is a dead end, something fundamental is broken. > + * > + * There is no sensible way of continuing as the Hyper-V drivers > + * transitively depend via the vmbus driver on knowing which VTL > + * they run in to establish communication with the host. The kernel > + * is going to be worse off if continued booting than a panicked one, > + * just hung and stuck, producing second-order failures, with neither > + * a way to recover nor to provide expected services. > + */ > + BUG(); > + } > + > + local_irq_restore(flags); > + return ret; > +} > +#endif > diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h > index 814207e7c37f..271c365973d6 100644 > --- a/include/asm-generic/hyperv-tlfs.h > +++ b/include/asm-generic/hyperv-tlfs.h > @@ -75,6 +75,13 @@ > /* AccessTscInvariantControls privilege */ > #define HV_ACCESS_TSC_INVARIANT BIT(15) > > +/* > + * This synthetic register is only accessible via the HVCALL_GET_VP_REGISTERS > + * hvcall, and there is no an associated MSR on x86. s/there is no an associated/there is no associated/ > + */ > +#define HV_REGISTER_VSM_VP_STATUS 0x000D0003 > +#define HV_VTL_MASK GENMASK(3, 0) Further down in hyperv-tlfs.h is a section devoted to register definitions. It seems like this definition should go there in numeric order, which is after HV_REGISTER_STIMER0_COUNT. Michael > + > /* > * Group B features. > */ > diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h > index 8fe7aaab2599..85a5b8cb1702 100644 > --- a/include/asm-generic/mshyperv.h > +++ b/include/asm-generic/mshyperv.h > @@ -315,4 +315,10 @@ static inline enum hv_isolation_type > hv_get_isolation_type(void) > } > #endif /* CONFIG_HYPERV */ > > +#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) > +u8 __init get_vtl(void); > +#else > +static inline u8 get_vtl(void) { return 0; } > +#endif > + > #endif > -- > 2.34.1 >
On Sat, Aug 03, 2024 at 01:21:52AM +0000, Wei Liu wrote: > On Fri, Jul 26, 2024 at 03:59:06PM -0700, Roman Kisel wrote: > > To run in the VTL mode, Hyper-V drivers have to know what > > VTL the system boots in, and the arm64/hyperv code does not > > have the means to compute that. > > > > Refactor the code to hoist the function that detects VTL, > > make it arch-neutral to be able to employ it to get the VTL > > on arm64. Fix the hypercall output address in `get_vtl(void)` > > not to overlap with the hypercall input area to adhere to > > the Hyper-V TLFS. > > Can you split the fix out? That potentially can be backported. > > > > > Signed-off-by: Roman Kisel <romank@linux.microsoft.com> > > --- > > + > > +#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) > > +u8 __init get_vtl(void) > > +{ > > + u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; > > + struct hv_get_vp_registers_input *input; > > + struct hv_get_vp_registers_output *output; > > + unsigned long flags; > > + u64 ret; > > + > > + local_irq_save(flags); > > + input = *this_cpu_ptr(hyperv_pcpu_input_arg); > > + output = *this_cpu_ptr(hyperv_pcpu_output_arg); > > Hmm... I don't remember why the old code didn't use > hyperv_pcpu_output_arg but instead reused input+OFFSET as output. > > Saurabh, can you comment on this? This was done to optimize memory usage. Michael has provided more details on this in his review of the patch today.
On 8/4/2024 8:02 PM, Michael Kelley wrote: > From: Roman Kisel <romank@linux.microsoft.com> Sent: Friday, July 26, 2024 3:59 PM >> >> To run in the VTL mode, Hyper-V drivers have to know what >> VTL the system boots in, and the arm64/hyperv code does not >> have the means to compute that. >> >> Refactor the code to hoist the function that detects VTL, >> make it arch-neutral to be able to employ it to get the VTL >> on arm64. Fix the hypercall output address in `get_vtl(void)` >> not to overlap with the hypercall input area to adhere to >> the Hyper-V TLFS. >> >> Signed-off-by: Roman Kisel <romank@linux.microsoft.com> >> --- >> arch/x86/hyperv/hv_init.c | 34 --------------------- >> arch/x86/include/asm/hyperv-tlfs.h | 7 ----- >> drivers/hv/hv_common.c | 47 ++++++++++++++++++++++++++++-- >> include/asm-generic/hyperv-tlfs.h | 7 +++++ >> include/asm-generic/mshyperv.h | 6 ++++ >> 5 files changed, 58 insertions(+), 43 deletions(-) >> >> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c >> index 17a71e92a343..c350fa05ee59 100644 >> --- a/arch/x86/hyperv/hv_init.c >> +++ b/arch/x86/hyperv/hv_init.c >> @@ -413,40 +413,6 @@ static void __init hv_get_partition_id(void) >> local_irq_restore(flags); >> } >> >> -#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) >> -static u8 __init get_vtl(void) >> -{ >> - u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; >> - struct hv_get_vp_registers_input *input; >> - struct hv_get_vp_registers_output *output; >> - unsigned long flags; >> - u64 ret; >> - >> - local_irq_save(flags); >> - input = *this_cpu_ptr(hyperv_pcpu_input_arg); >> - output = (struct hv_get_vp_registers_output *)input; >> - >> - memset(input, 0, struct_size(input, element, 1)); >> - 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 (hv_result_success(ret)) { >> - ret = output->as64.low & HV_X64_VTL_MASK; >> - } else { >> - pr_err("Failed to get VTL(error: %lld) exiting...\n", ret); >> - BUG(); >> - } >> - >> - local_irq_restore(flags); >> - return ret; >> -} >> -#else >> -static inline u8 get_vtl(void) { return 0; } >> -#endif >> - >> /* >> * This function is to be invoked early in the boot sequence after the >> * hypervisor has been detected. >> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h >> index 3787d26810c1..9ee68eb8e6ff 100644 >> --- a/arch/x86/include/asm/hyperv-tlfs.h >> +++ b/arch/x86/include/asm/hyperv-tlfs.h >> @@ -309,13 +309,6 @@ enum hv_isolation_type { >> #define HV_MSR_STIMER0_CONFIG (HV_X64_MSR_STIMER0_CONFIG) >> #define HV_MSR_STIMER0_COUNT (HV_X64_MSR_STIMER0_COUNT) >> >> -/* >> - * Registers are only accessible via HVCALL_GET_VP_REGISTERS hvcall and >> - * there is not associated MSR address. >> - */ >> -#define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003 >> -#define HV_X64_VTL_MASK GENMASK(3, 0) >> - >> /* Hyper-V memory host visibility */ >> enum hv_mem_host_visibility { >> VMBUS_PAGE_NOT_VISIBLE = 0, >> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c >> index 9c452bfbd571..7d6c1523b0b5 100644 >> --- a/drivers/hv/hv_common.c >> +++ b/drivers/hv/hv_common.c >> @@ -339,8 +339,8 @@ int __init hv_common_init(void) >> hyperv_pcpu_input_arg = alloc_percpu(void *); >> BUG_ON(!hyperv_pcpu_input_arg); >> >> - /* Allocate the per-CPU state for output arg for root */ >> - if (hv_root_partition) { >> + /* Allocate the per-CPU state for output arg for root or a VTL */ >> + if (hv_root_partition || IS_ENABLED(CONFIG_HYPERV_VTL_MODE)) { >> hyperv_pcpu_output_arg = alloc_percpu(void *); >> BUG_ON(!hyperv_pcpu_output_arg); >> } >> @@ -656,3 +656,46 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 >> param2) >> return HV_STATUS_INVALID_PARAMETER; >> } >> EXPORT_SYMBOL_GPL(hv_tdx_hypercall); >> + >> +#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) >> +u8 __init get_vtl(void) >> +{ >> + u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; >> + struct hv_get_vp_registers_input *input; >> + struct hv_get_vp_registers_output *output; >> + unsigned long flags; >> + u64 ret; >> + >> + local_irq_save(flags); >> + input = *this_cpu_ptr(hyperv_pcpu_input_arg); >> + output = *this_cpu_ptr(hyperv_pcpu_output_arg); > > Rather than use the hyperv_pcpu_output_arg here, it's OK to > use a different area of the hyperv_pcpu_input_arg page. For > example, > > output = (void *)input + HV_HYP_PAGE_SIZE/2; > > The TLFS does not require that the input and output be in > separate pages. > > While using the hyperv_pcpu_output_arg is conceptually a > bit cleaner, doing so requires allocating a 4K page per CPU that > is not otherwise used. The VTL 2 code wants to be frugal with > memory, and this seems like a good step in that direction. :-) > I agree on the both counts: the code looks conceptually cleaner now and VTL2 wants to be frugal with memory, esp that the output hypercall page is per-CPU so we have O(n) as the CPU count increases. Still, the output page will be needed for VTL2 (say to get/set registers just as done here). That said, with this patch we can achieve both the conceptual cleanliness and being ready to grow more on the primitives being built out in the VTL support patches. > The hyperv_pcpu_output_arg was added for the cases where up > to a full page is needed for input and output on the same hypercall. > So far, the only case of that is when running in the root partition. > >> + >> + memset(input, 0, struct_size(input, element, 1)); >> + input->header.partitionid = HV_PARTITION_ID_SELF; >> + input->header.vpindex = HV_VP_INDEX_SELF; >> + input->header.inputvtl = 0; >> + input->element[0].name0 = HV_REGISTER_VSM_VP_STATUS; >> + >> + ret = hv_do_hypercall(control, input, output); >> + if (hv_result_success(ret)) { >> + ret = output->as64.low & HV_VTL_MASK; >> + } else { >> + pr_err("Failed to get VTL(error: %lld) exiting...\n", ret); >> + >> + /* >> + * This is a dead end, something fundamental is broken. >> + * >> + * There is no sensible way of continuing as the Hyper-V drivers >> + * transitively depend via the vmbus driver on knowing which VTL >> + * they run in to establish communication with the host. The kernel >> + * is going to be worse off if continued booting than a panicked one, >> + * just hung and stuck, producing second-order failures, with neither >> + * a way to recover nor to provide expected services. >> + */ >> + BUG(); >> + } >> + >> + local_irq_restore(flags); >> + return ret; >> +} >> +#endif >> diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h >> index 814207e7c37f..271c365973d6 100644 >> --- a/include/asm-generic/hyperv-tlfs.h >> +++ b/include/asm-generic/hyperv-tlfs.h >> @@ -75,6 +75,13 @@ >> /* AccessTscInvariantControls privilege */ >> #define HV_ACCESS_TSC_INVARIANT BIT(15) >> >> +/* >> + * This synthetic register is only accessible via the HVCALL_GET_VP_REGISTERS >> + * hvcall, and there is no an associated MSR on x86. > > s/there is no an associated/there is no associated/ > Much appreciated, will fix this! >> + */ >> +#define HV_REGISTER_VSM_VP_STATUS 0x000D0003 >> +#define HV_VTL_MASK GENMASK(3, 0) > > Further down in hyperv-tlfs.h is a section devoted to register > definitions. It seems like this definition should go there in > numeric order, which is after HV_REGISTER_STIMER0_COUNT. > Agreed, will fix, thanks! > Michael > >> + >> /* >> * Group B features. >> */ >> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h >> index 8fe7aaab2599..85a5b8cb1702 100644 >> --- a/include/asm-generic/mshyperv.h >> +++ b/include/asm-generic/mshyperv.h >> @@ -315,4 +315,10 @@ static inline enum hv_isolation_type >> hv_get_isolation_type(void) >> } >> #endif /* CONFIG_HYPERV */ >> >> +#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) >> +u8 __init get_vtl(void); >> +#else >> +static inline u8 get_vtl(void) { return 0; } >> +#endif >> + >> #endif >> -- >> 2.34.1 >>
On 8/5/2024 1:13 PM, Michael Kelley wrote: > From: Roman Kisel <romank@linux.microsoft.com> Sent: Monday, August 5, 2024 9:20 AM >> >> On 8/4/2024 8:02 PM, Michael Kelley wrote: >>> From: Roman Kisel <romank@linux.microsoft.com> Sent: Friday, July 26, 2024 3:59 >> PM >>>> >>>> To run in the VTL mode, Hyper-V drivers have to know what >>>> VTL the system boots in, and the arm64/hyperv code does not >>>> have the means to compute that. >>>> >>>> Refactor the code to hoist the function that detects VTL, >>>> make it arch-neutral to be able to employ it to get the VTL >>>> on arm64. Fix the hypercall output address in `get_vtl(void)` >>>> not to overlap with the hypercall input area to adhere to >>>> the Hyper-V TLFS. >>>> >>>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com> >>>> --- >>>> arch/x86/hyperv/hv_init.c | 34 --------------------- >>>> arch/x86/include/asm/hyperv-tlfs.h | 7 ----- >>>> drivers/hv/hv_common.c | 47 ++++++++++++++++++++++++++++-- >>>> include/asm-generic/hyperv-tlfs.h | 7 +++++ >>>> include/asm-generic/mshyperv.h | 6 ++++ >>>> 5 files changed, 58 insertions(+), 43 deletions(-) >>>> >>>> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c >>>> index 17a71e92a343..c350fa05ee59 100644 >>>> --- a/arch/x86/hyperv/hv_init.c >>>> +++ b/arch/x86/hyperv/hv_init.c >>>> @@ -413,40 +413,6 @@ static void __init hv_get_partition_id(void) >>>> local_irq_restore(flags); >>>> } >>>> >>>> -#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) >>>> -static u8 __init get_vtl(void) >>>> -{ >>>> - u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; >>>> - struct hv_get_vp_registers_input *input; >>>> - struct hv_get_vp_registers_output *output; >>>> - unsigned long flags; >>>> - u64 ret; >>>> - >>>> - local_irq_save(flags); >>>> - input = *this_cpu_ptr(hyperv_pcpu_input_arg); >>>> - output = (struct hv_get_vp_registers_output *)input; >>>> - >>>> - memset(input, 0, struct_size(input, element, 1)); >>>> - 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 (hv_result_success(ret)) { >>>> - ret = output->as64.low & HV_X64_VTL_MASK; >>>> - } else { >>>> - pr_err("Failed to get VTL(error: %lld) exiting...\n", ret); >>>> - BUG(); >>>> - } >>>> - >>>> - local_irq_restore(flags); >>>> - return ret; >>>> -} >>>> -#else >>>> -static inline u8 get_vtl(void) { return 0; } >>>> -#endif >>>> - >>>> /* >>>> * This function is to be invoked early in the boot sequence after the >>>> * hypervisor has been detected. >>>> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h >>>> index 3787d26810c1..9ee68eb8e6ff 100644 >>>> --- a/arch/x86/include/asm/hyperv-tlfs.h >>>> +++ b/arch/x86/include/asm/hyperv-tlfs.h >>>> @@ -309,13 +309,6 @@ enum hv_isolation_type { >>>> #define HV_MSR_STIMER0_CONFIG (HV_X64_MSR_STIMER0_CONFIG) >>>> #define HV_MSR_STIMER0_COUNT (HV_X64_MSR_STIMER0_COUNT) >>>> >>>> -/* >>>> - * Registers are only accessible via HVCALL_GET_VP_REGISTERS hvcall and >>>> - * there is not associated MSR address. >>>> - */ >>>> -#define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003 >>>> -#define HV_X64_VTL_MASK GENMASK(3, 0) >>>> - >>>> /* Hyper-V memory host visibility */ >>>> enum hv_mem_host_visibility { >>>> VMBUS_PAGE_NOT_VISIBLE = 0, >>>> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c >>>> index 9c452bfbd571..7d6c1523b0b5 100644 >>>> --- a/drivers/hv/hv_common.c >>>> +++ b/drivers/hv/hv_common.c >>>> @@ -339,8 +339,8 @@ int __init hv_common_init(void) >>>> hyperv_pcpu_input_arg = alloc_percpu(void *); >>>> BUG_ON(!hyperv_pcpu_input_arg); >>>> >>>> - /* Allocate the per-CPU state for output arg for root */ >>>> - if (hv_root_partition) { >>>> + /* Allocate the per-CPU state for output arg for root or a VTL */ >>>> + if (hv_root_partition || IS_ENABLED(CONFIG_HYPERV_VTL_MODE)) { >>>> hyperv_pcpu_output_arg = alloc_percpu(void *); >>>> BUG_ON(!hyperv_pcpu_output_arg); >>>> } >>>> @@ -656,3 +656,46 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2) >>>> return HV_STATUS_INVALID_PARAMETER; >>>> } >>>> EXPORT_SYMBOL_GPL(hv_tdx_hypercall); >>>> + >>>> +#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) >>>> +u8 __init get_vtl(void) >>>> +{ >>>> + u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; >>>> + struct hv_get_vp_registers_input *input; >>>> + struct hv_get_vp_registers_output *output; >>>> + unsigned long flags; >>>> + u64 ret; >>>> + >>>> + local_irq_save(flags); >>>> + input = *this_cpu_ptr(hyperv_pcpu_input_arg); >>>> + output = *this_cpu_ptr(hyperv_pcpu_output_arg); >>> >>> Rather than use the hyperv_pcpu_output_arg here, it's OK to >>> use a different area of the hyperv_pcpu_input_arg page. For >>> example, >>> >>> output = (void *)input + HV_HYP_PAGE_SIZE/2; >>> >>> The TLFS does not require that the input and output be in >>> separate pages. >>> >>> While using the hyperv_pcpu_output_arg is conceptually a >>> bit cleaner, doing so requires allocating a 4K page per CPU that >>> is not otherwise used. The VTL 2 code wants to be frugal with >>> memory, and this seems like a good step in that direction. :-) >>> >> I agree on the both counts: the code looks conceptually cleaner now and >> VTL2 wants to be frugal with memory, esp that the output hypercall page >> is per-CPU so we have O(n) as the CPU count increases. Still, the output >> page will be needed for VTL2 (say to get/set registers just as done >> here). That said, with this patch we can achieve both the conceptual >> cleanliness and being ready to grow more on the primitives being built >> out in the VTL support patches. >> > > Could you elaborate further on why the output page is needed for > VTL2? The get/set register hypercalls can operate with just the input > page (again, splitting it into two halves for input and output args) as > long as the number of registers acted on by a single hypercall isn't > more than a few dozen. > > If you really *do* need the output page in VTL2 for other reasons > that I'm not aware of, then my suggestion isn't relevant and there's > no memory to be saved. VTL2 might potentially use any hypercalls being in some sense an exclave of the hypervisor living inside the guest quite similarly to the VBS/VTL1/SecureKernel. The tradeoff here would be to save a page per processor at the cost of specializing the hypercall issuing code that would use a part of the input page to save memory (quite likely limiting which hypercalls can be used), or use the common implementation at the cost of spending one more page per processor. Less code means less maintenance usually so seems beneficial to choose the latter option although at the cost of using more memory. > > Michael
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index 17a71e92a343..c350fa05ee59 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -413,40 +413,6 @@ static void __init hv_get_partition_id(void) local_irq_restore(flags); } -#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) -static u8 __init get_vtl(void) -{ - u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; - struct hv_get_vp_registers_input *input; - struct hv_get_vp_registers_output *output; - unsigned long flags; - u64 ret; - - local_irq_save(flags); - input = *this_cpu_ptr(hyperv_pcpu_input_arg); - output = (struct hv_get_vp_registers_output *)input; - - memset(input, 0, struct_size(input, element, 1)); - 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 (hv_result_success(ret)) { - ret = output->as64.low & HV_X64_VTL_MASK; - } else { - pr_err("Failed to get VTL(error: %lld) exiting...\n", ret); - BUG(); - } - - local_irq_restore(flags); - return ret; -} -#else -static inline u8 get_vtl(void) { return 0; } -#endif - /* * This function is to be invoked early in the boot sequence after the * hypervisor has been detected. diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h index 3787d26810c1..9ee68eb8e6ff 100644 --- a/arch/x86/include/asm/hyperv-tlfs.h +++ b/arch/x86/include/asm/hyperv-tlfs.h @@ -309,13 +309,6 @@ enum hv_isolation_type { #define HV_MSR_STIMER0_CONFIG (HV_X64_MSR_STIMER0_CONFIG) #define HV_MSR_STIMER0_COUNT (HV_X64_MSR_STIMER0_COUNT) -/* - * Registers are only accessible via HVCALL_GET_VP_REGISTERS hvcall and - * there is not associated MSR address. - */ -#define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003 -#define HV_X64_VTL_MASK GENMASK(3, 0) - /* Hyper-V memory host visibility */ enum hv_mem_host_visibility { VMBUS_PAGE_NOT_VISIBLE = 0, diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c index 9c452bfbd571..7d6c1523b0b5 100644 --- a/drivers/hv/hv_common.c +++ b/drivers/hv/hv_common.c @@ -339,8 +339,8 @@ int __init hv_common_init(void) hyperv_pcpu_input_arg = alloc_percpu(void *); BUG_ON(!hyperv_pcpu_input_arg); - /* Allocate the per-CPU state for output arg for root */ - if (hv_root_partition) { + /* Allocate the per-CPU state for output arg for root or a VTL */ + if (hv_root_partition || IS_ENABLED(CONFIG_HYPERV_VTL_MODE)) { hyperv_pcpu_output_arg = alloc_percpu(void *); BUG_ON(!hyperv_pcpu_output_arg); } @@ -656,3 +656,46 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2) return HV_STATUS_INVALID_PARAMETER; } EXPORT_SYMBOL_GPL(hv_tdx_hypercall); + +#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) +u8 __init get_vtl(void) +{ + u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS; + struct hv_get_vp_registers_input *input; + struct hv_get_vp_registers_output *output; + unsigned long flags; + u64 ret; + + local_irq_save(flags); + input = *this_cpu_ptr(hyperv_pcpu_input_arg); + output = *this_cpu_ptr(hyperv_pcpu_output_arg); + + memset(input, 0, struct_size(input, element, 1)); + input->header.partitionid = HV_PARTITION_ID_SELF; + input->header.vpindex = HV_VP_INDEX_SELF; + input->header.inputvtl = 0; + input->element[0].name0 = HV_REGISTER_VSM_VP_STATUS; + + ret = hv_do_hypercall(control, input, output); + if (hv_result_success(ret)) { + ret = output->as64.low & HV_VTL_MASK; + } else { + pr_err("Failed to get VTL(error: %lld) exiting...\n", ret); + + /* + * This is a dead end, something fundamental is broken. + * + * There is no sensible way of continuing as the Hyper-V drivers + * transitively depend via the vmbus driver on knowing which VTL + * they run in to establish communication with the host. The kernel + * is going to be worse off if continued booting than a panicked one, + * just hung and stuck, producing second-order failures, with neither + * a way to recover nor to provide expected services. + */ + BUG(); + } + + local_irq_restore(flags); + return ret; +} +#endif diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h index 814207e7c37f..271c365973d6 100644 --- a/include/asm-generic/hyperv-tlfs.h +++ b/include/asm-generic/hyperv-tlfs.h @@ -75,6 +75,13 @@ /* AccessTscInvariantControls privilege */ #define HV_ACCESS_TSC_INVARIANT BIT(15) +/* + * This synthetic register is only accessible via the HVCALL_GET_VP_REGISTERS + * hvcall, and there is no an associated MSR on x86. + */ +#define HV_REGISTER_VSM_VP_STATUS 0x000D0003 +#define HV_VTL_MASK GENMASK(3, 0) + /* * Group B features. */ diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h index 8fe7aaab2599..85a5b8cb1702 100644 --- a/include/asm-generic/mshyperv.h +++ b/include/asm-generic/mshyperv.h @@ -315,4 +315,10 @@ static inline enum hv_isolation_type hv_get_isolation_type(void) } #endif /* CONFIG_HYPERV */ +#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE) +u8 __init get_vtl(void); +#else +static inline u8 get_vtl(void) { return 0; } +#endif + #endif
To run in the VTL mode, Hyper-V drivers have to know what VTL the system boots in, and the arm64/hyperv code does not have the means to compute that. Refactor the code to hoist the function that detects VTL, make it arch-neutral to be able to employ it to get the VTL on arm64. Fix the hypercall output address in `get_vtl(void)` not to overlap with the hypercall input area to adhere to the Hyper-V TLFS. Signed-off-by: Roman Kisel <romank@linux.microsoft.com> --- arch/x86/hyperv/hv_init.c | 34 --------------------- arch/x86/include/asm/hyperv-tlfs.h | 7 ----- drivers/hv/hv_common.c | 47 ++++++++++++++++++++++++++++-- include/asm-generic/hyperv-tlfs.h | 7 +++++ include/asm-generic/mshyperv.h | 6 ++++ 5 files changed, 58 insertions(+), 43 deletions(-)