2023-06-27 03:37:08

by Tianyu Lan

[permalink] [raw]
Subject: [PATCH V2 2/9] x86/hyperv: Set Virtual Trust Level in VMBus init message

From: Tianyu Lan <[email protected]>

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 <[email protected]>
---
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



2023-07-04 15:06:16

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH V2 2/9] x86/hyperv: Set Virtual Trust Level in VMBus init message

From: Tianyu Lan <[email protected]> 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 <[email protected]>
> ---
> 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


2023-07-07 10:17:19

by Saurabh Singh Sengar

[permalink] [raw]
Subject: RE: [EXTERNAL] [PATCH V2 2/9] x86/hyperv: Set Virtual Trust Level in VMBus init message



> -----Original Message-----
> From: Tianyu Lan <[email protected]>
> Sent: Tuesday, June 27, 2023 8:53 AM
> To: KY Srinivasan <[email protected]>; Haiyang Zhang
> <[email protected]>; [email protected]; Dexuan Cui
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Michael Kelley
> (LINUX) <[email protected]>
> Cc: Tianyu Lan <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: [EXTERNAL] [PATCH V2 2/9] x86/hyperv: Set Virtual Trust Level in
> VMBus init message
>
> From: Tianyu Lan <[email protected]>
>
> 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 <[email protected]>
> ---
> 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


2023-08-07 06:23:25

by Wei Liu

[permalink] [raw]
Subject: Re: [EXTERNAL] [PATCH V2 2/9] x86/hyperv: Set Virtual Trust Level in VMBus init message

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?

2023-08-10 18:20:38

by Tianyu Lan

[permalink] [raw]
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.

2023-08-11 04:52:26

by Saurabh Singh Sengar

[permalink] [raw]
Subject: RE: [EXTERNAL] [PATCH V2 2/9] x86/hyperv: Set Virtual Trust Level in VMBus init message



> -----Original Message-----
> From: Tianyu Lan <[email protected]>
> Sent: Thursday, August 10, 2023 9:53 PM
> To: Wei Liu <[email protected]>; Saurabh Singh Sengar
> <[email protected]>
> Cc: KY Srinivasan <[email protected]>; Haiyang Zhang
> <[email protected]>; Dexuan Cui <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Michael Kelley (LINUX)
> <[email protected]>; Tianyu Lan <[email protected]>; linux-
> [email protected]; [email protected]; linux-
> [email protected]; vkuznets <[email protected]>
> 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.