[V2,2/9] x86/hyperv: Set Virtual Trust Level in VMBus init message

  From: Tianyu Lan <tiala@microsoft.com>

SEV-SNP guest provides vtl(Virtual Trust Level) and
get it from Hyper-V hvcall via register hvcall HVCALL_
GET_VP_REGISTERS.

During initialization of VMBus, vtl needs to be set in the
VMBus init message.

Signed-off-by: Tianyu Lan <tiala@microsoft.com>
---
 arch/x86/hyperv/hv_init.c          | 36 ++++++++++++++++++++++++++++++
 arch/x86/include/asm/hyperv-tlfs.h |  7 ++++++
 drivers/hv/connection.c            |  1 +
 include/asm-generic/mshyperv.h     |  1 +
 include/linux/hyperv.h             |  4 ++--
 5 files changed, 47 insertions(+), 2 deletions(-)
  

From: Tianyu Lan <ltykernel@gmail.com> Sent: Monday, June 26, 2023 8:23 PM
> 
> SEV-SNP guest provides vtl(Virtual Trust Level) and
> get it from Hyper-V hvcall via register hvcall HVCALL_
> GET_VP_REGISTERS.
> 
> During initialization of VMBus, vtl needs to be set in the
> VMBus init message.

I had suggested a revised version of the commit message, which you
agreed to use.  But this is still the old commit message.

Michael

> 
> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> ---
>  arch/x86/hyperv/hv_init.c          | 36 ++++++++++++++++++++++++++++++
>  arch/x86/include/asm/hyperv-tlfs.h |  7 ++++++
>  drivers/hv/connection.c            |  1 +
>  include/asm-generic/mshyperv.h     |  1 +
>  include/linux/hyperv.h             |  4 ++--
>  5 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 6c04b52f139b..1ba367a9686e 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -378,6 +378,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;
> +	struct hv_get_vp_registers_output *output;
> +	u64 vtl = 0;
> +	u64 ret;
> +	unsigned long flags;
> +
> +	local_irq_save(flags);
> +	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +	output = (struct hv_get_vp_registers_output *)input;
> +	if (!input) {
> +		local_irq_restore(flags);
> +		goto done;
> +	}
> +
> +	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))
> +		vtl = output->as64.low & HV_X64_VTL_MASK;
> +	else
> +		pr_err("Hyper-V: failed to get VTL! %lld", ret);
> +	local_irq_restore(flags);
> +
> +done:
> +	return vtl;
> +}
> +
>  /*
>   * This function is to be invoked early in the boot sequence after the
>   * hypervisor has been detected.
> @@ -506,6 +540,8 @@ 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 cea95dcd27c2..4bf0b315b0ce 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -301,6 +301,13 @@ enum hv_isolation_type {
>  #define HV_X64_MSR_TIME_REF_COUNT	HV_REGISTER_TIME_REF_COUNT
>  #define HV_X64_MSR_REFERENCE_TSC	HV_REGISTER_REFERENCE_TSC
> 
> +/*
> + * 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/connection.c b/drivers/hv/connection.c
> index 5978e9dbc286..02b54f85dc60 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 6b5c41f90398..f73a044ecaa7 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -54,6 +54,7 @@ struct ms_hyperv_info {
>  		};
>  	};
>  	u64 shared_gpa_boundary;
> +	u8 vtl;
>  };
>  extern struct ms_hyperv_info ms_hyperv;
>  extern bool hv_nested;
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index bfbc37ce223b..1f2bfec4abde 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
  

> -----Original Message-----
> From: Tianyu Lan <ltykernel@gmail.com>
> Sent: Tuesday, June 27, 2023 8:53 AM
> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; wei.liu@kernel.org; Dexuan Cui
> <decui@microsoft.com>; tglx@linutronix.de; mingo@redhat.com;
> bp@alien8.de; dave.hansen@linux.intel.com; x86@kernel.org;
> hpa@zytor.com; daniel.lezcano@linaro.org; arnd@arndb.de; Michael Kelley
> (LINUX) <mikelley@microsoft.com>
> Cc: Tianyu Lan <Tianyu.Lan@microsoft.com>; linux-arch@vger.kernel.org;
> linux-hyperv@vger.kernel.org; linux-kernel@vger.kernel.org;
> vkuznets@redhat.com
> Subject: [EXTERNAL] [PATCH V2 2/9] x86/hyperv: Set Virtual Trust Level in
> VMBus init message
> 
> From: Tianyu Lan <tiala@microsoft.com>
> 
> SEV-SNP guest provides vtl(Virtual Trust Level) and get it from Hyper-V hvcall
> via register hvcall HVCALL_ GET_VP_REGISTERS.
> 
> During initialization of VMBus, vtl needs to be set in the VMBus init message.
> 
> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> ---
>  arch/x86/hyperv/hv_init.c          | 36 ++++++++++++++++++++++++++++++
>  arch/x86/include/asm/hyperv-tlfs.h |  7 ++++++
>  drivers/hv/connection.c            |  1 +
>  include/asm-generic/mshyperv.h     |  1 +
>  include/linux/hyperv.h             |  4 ++--
>  5 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index
> 6c04b52f139b..1ba367a9686e 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -378,6 +378,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;
> +	struct hv_get_vp_registers_output *output;
> +	u64 vtl = 0;
> +	u64 ret;
> +	unsigned long flags;
> +
> +	local_irq_save(flags);
> +	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +	output = (struct hv_get_vp_registers_output *)input;
> +	if (!input) {
> +		local_irq_restore(flags);
> +		goto done;
> +	}
> +
> +	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))
> +		vtl = output->as64.low & HV_X64_VTL_MASK;
> +	else
> +		pr_err("Hyper-V: failed to get VTL! %lld", ret);

In case of error this function will return vtl=0, which can be the valid value of vtl.
I suggest we initialize vtl with -1 so and then check for its return.

This could be a good utility function which can be used for any Hyper-V VTL system, so think
of making it global ?

- Saurabh

> +	local_irq_restore(flags);
> +
> +done:
> +	return vtl;
> +}
> +
>  /*
>   * This function is to be invoked early in the boot sequence after the
>   * hypervisor has been detected.
> @@ -506,6 +540,8 @@ 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 cea95dcd27c2..4bf0b315b0ce 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -301,6 +301,13 @@ enum hv_isolation_type {
>  #define HV_X64_MSR_TIME_REF_COUNT
> 	HV_REGISTER_TIME_REF_COUNT
>  #define HV_X64_MSR_REFERENCE_TSC
> 	HV_REGISTER_REFERENCE_TSC
> 
> +/*
> + * 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/connection.c b/drivers/hv/connection.c index
> 5978e9dbc286..02b54f85dc60 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 6b5c41f90398..f73a044ecaa7 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -54,6 +54,7 @@ struct ms_hyperv_info {
>  		};
>  	};
>  	u64 shared_gpa_boundary;
> +	u8 vtl;
>  };
>  extern struct ms_hyperv_info ms_hyperv;  extern bool hv_nested; diff --git
> a/include/linux/hyperv.h b/include/linux/hyperv.h index
> bfbc37ce223b..1f2bfec4abde 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 Fri, Jul 07, 2023 at 09:07:54AM +0000, Saurabh Singh Sengar wrote:
> 
> 
[...]
> > +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;
> > +	u64 vtl = 0;
> > +	u64 ret;
> > +	unsigned long flags;
> > +
> > +	local_irq_save(flags);
> > +	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> > +	output = (struct hv_get_vp_registers_output *)input;
> > +	if (!input) {
> > +		local_irq_restore(flags);
> > +		goto done;
> > +	}
> > +
> > +	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))
> > +		vtl = output->as64.low & HV_X64_VTL_MASK;
> > +	else
> > +		pr_err("Hyper-V: failed to get VTL! %lld", ret);
> 
> In case of error this function will return vtl=0, which can be the valid value of vtl.
> I suggest we initialize vtl with -1 so and then check for its return.
> 
> This could be a good utility function which can be used for any Hyper-V VTL system, so think
> of making it global ?
> 

Tianyu -- your thought on this?
  

On 8/7/2023 12:48 PM, Wei Liu wrote:
> On Fri, Jul 07, 2023 at 09:07:54AM +0000, Saurabh Singh Sengar wrote:
>>
>>
>>> +
>>> +	ret = hv_do_hypercall(control, input, output);
>>> +	if (hv_result_success(ret))
>>> +		vtl = output->as64.low & HV_X64_VTL_MASK;
>>> +	else
>>> +		pr_err("Hyper-V: failed to get VTL! %lld", ret);
>>
>> In case of error this function will return vtl=0, which can be the valid value of vtl.
>> I suggest we initialize vtl with -1 so and then check for its return.
>>
>> This could be a good utility function which can be used for any Hyper-V VTL system, so think
>> of making it global ?
>>
> 
> Tianyu -- your thought on this?

In current user cases, the guest only runs in VTL0 and Hyper-V may
return VTL error in some cases but kernel still may run with 0 as VTL.

I just sent out v5 and set VTL to 0 by default if fail to get VTL from
Hyper-V and give out a warning log. The get_vtl() is only called on 
enlightened SEV-SNP guest. If there is new case that needs handle the 
error from Hyper-V when call VTL hvcall, we may add the logic later.
  

> -----Original Message-----
> From: Tianyu Lan <ltykernel@gmail.com>
> Sent: Thursday, August 10, 2023 9:53 PM
> To: Wei Liu <wei.liu@kernel.org>; Saurabh Singh Sengar
> <ssengar@microsoft.com>
> Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Dexuan Cui <decui@microsoft.com>;
> tglx@linutronix.de; mingo@redhat.com; bp@alien8.de;
> dave.hansen@linux.intel.com; x86@kernel.org; hpa@zytor.com;
> daniel.lezcano@linaro.org; arnd@arndb.de; Michael Kelley (LINUX)
> <mikelley@microsoft.com>; Tianyu Lan <Tianyu.Lan@microsoft.com>; linux-
> arch@vger.kernel.org; linux-hyperv@vger.kernel.org; linux-
> kernel@vger.kernel.org; vkuznets <vkuznets@redhat.com>
> Subject: Re: [EXTERNAL] [PATCH V2 2/9] x86/hyperv: Set Virtual Trust Level in
> VMBus init message
> 
> On 8/7/2023 12:48 PM, Wei Liu wrote:
> > On Fri, Jul 07, 2023 at 09:07:54AM +0000, Saurabh Singh Sengar wrote:
> >>
> >>
> >>> +
> >>> +	ret = hv_do_hypercall(control, input, output);
> >>> +	if (hv_result_success(ret))
> >>> +		vtl = output->as64.low & HV_X64_VTL_MASK;
> >>> +	else
> >>> +		pr_err("Hyper-V: failed to get VTL! %lld", ret);
> >>
> >> In case of error this function will return vtl=0, which can be the valid value
> of vtl.
> >> I suggest we initialize vtl with -1 so and then check for its return.
> >>
> >> This could be a good utility function which can be used for any
> >> Hyper-V VTL system, so think of making it global ?
> >>
> >
> > Tianyu -- your thought on this?
> 
> In current user cases, the guest only runs in VTL0 and Hyper-V may return VTL
> error in some cases but kernel still may run with 0 as VTL.
> 
> I just sent out v5 and set VTL to 0 by default if fail to get VTL from Hyper-V and
> give out a warning log. The get_vtl() is only called on enlightened SEV-SNP
> guest. If there is new case that needs handle the error from Hyper-V when
> call VTL hvcall, we may add the logic later.

I understand your requirement.
IMO it's cleaner if function returns errors, rather than relying solely on error prints.
While caller of this function can choose to ignore errors, the function must not overlook them.
something like this should work for your requirement:

ret = get_vtl()
ms_hyperv.vtl = ret < 0 ? 0 : ret;

This way desired functionality will be intact, and function also will be self-contained.