RE: [PATCH v3 3/7] Drivers: hv: Provide arch-neutral implementation of get_vtl()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Roman Kisel <romank@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx>
> ---
>  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
> 






[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux