[PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()

[PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()

[Roman Kisel]

The get_vtl(void) function

* has got one bug when the code started using a wrong pointer type after
  refactoring, and also
* the both function in question don't adhere to the requirements of
  the Hypervisor Top-Level Funactional Specification[1, 2] as the code overlaps
  the input and output areas for a hypercall.

The first issue leads to a wrong 100% reproducible computation due to reading
a byte worth of data at a wrong offset. That in turn leads to using a nonsensical
value ("fortunately", could catch it easily!) for the current VTL when initiating
VMBus communications. As a repercussion from that, the system wouldn't boot. The
fix is straightforward: use the correct pointer type.

The second issue doesn't seem to lead to any reproducible breakage just yet. It is
fixed with using the output hypercall pages allocated per-CPU, and that isn't the
only or the most obvious choice so let me elaborate why that fix appears to be the
best one in my opinion out of the options I could conceive of.

The approach chosen for fixing the second issue makes two things shine through:

* these functions just get a vCPU register, no special treatment needs to be
  involved,
* VTLs and dom0 can and should share code as both exist to provide services to
  a guest(s), be that from within the partition or from outside of it.

The projected benefits include replacing the functions in question with a future
`hv_get_vp_registers` one shared between dom0 and VTLs to allow for a better test
coverage.

I have validated the fixes by booting the fixed kernel in VTL2 up using OpenVMM and
OpenHCL[3, 4].

[1] https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/hypercall-interface
[2] https://github.com/MicrosoftDocs/Virtualization-Documentation/tree/main/tlfs
[3] https://openvmm.dev/guide/user_guide/openhcl.html
[4] https://github.com/microsoft/OpenVMM

[v6]
  - Sending v6 a week after v5 to adhere to the requirements of the kernel
    documentation,
  - Added all tags the patches received prior to v6,
  - Fixed the bitfield layout for the ARM64 structures (patch 1),
  - Hoisted the common condition used in several if statements into
    a separate function to improve code maintainability (patch 3).

[v5]: https://lore.kernel.org/lkml/20241230180941.244418-1-romank@linux.microsoft.com/
  - In the first patch, removed some arch-specific #ifdef guards to fix the
    arm64 build and stick to the direction chosen for the Hyper-V header files.
    I could not remove all of them as some interrupt state structures
    are defined differently for x64 and arm64 and are found in the same
    enclosing `union hv_register_value`.

    No changes to other patches (approved in v4).

[v4]: https://lore.kernel.org/lkml/20241227183155.122827-1-romank@linux.microsoft.com/
  - Wrapped DECLARE_FLEX_ARRAY into a struct and added one more
    member as the documentation requires,
  - Removed superfluous type coercion,
  - Fixed tags,
  - Rebased onto the latest hyperv-next branch.

[v3]: https://lore.kernel.org/lkml/20241226213110.899497-1-romank@linux.microsoft.com/
  - Added a fix for hv_vtl_apicid_to_vp_id(),
  - Split out the patch for enabling the hypercall output page,
  - Updated the title of the patch series,

[v2]: https://lore.kernel.org/lkml/20241226203050.800524-1-romank@linux.microsoft.com/
  - Used the suggestions to define an additional structure to improve code readability,
  - Split out the patch with that definition.

[v1]: https://lore.kernel.org/lkml/20241218205421.319969-1-romank@linux.microsoft.com/

Roman Kisel (5):
  hyperv: Define struct hv_output_get_vp_registers
  hyperv: Fix pointer type in get_vtl(void)
  hyperv: Enable the hypercall output page for the VTL mode
  hyperv: Do not overlap the hvcall IO areas in get_vtl()
  hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id()

 arch/x86/hyperv/hv_init.c   |  6 +++---
 arch/x86/hyperv/hv_vtl.c    |  2 +-
 drivers/hv/hv_common.c      | 11 +++++++---
 include/hyperv/hvgdk_mini.h | 41 +++++++++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 7 deletions(-)


base-commit: 26e1b813fcd02984b1cac5f3decdf4b0bb56fe02
-- 
2.34.1

[PATCH v6 1/5] hyperv: Define struct hv_output_get_vp_registers

[Roman Kisel]

There is no definition of the output structure for the
GetVpRegisters hypercall. Hence, using the hypercall
is not possible when the output value has some structure
to it. Even getting a datum of a primitive type reads
as ad-hoc without that definition.

Define struct hv_output_get_vp_registers to enable using
the GetVpRegisters hypercall. Make provisions for all
supported architectures. No functional changes.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 include/hyperv/hvgdk_mini.h | 41 +++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
index db3d1aaf7330..4fffca9e16df 100644
--- a/include/hyperv/hvgdk_mini.h
+++ b/include/hyperv/hvgdk_mini.h
@@ -1068,6 +1068,35 @@ union hv_dispatch_suspend_register {
 	} __packed;
 };
 
+union hv_arm64_pending_interruption_register {
+	u64 as_uint64;
+	struct {
+		u64 interruption_pending : 1;
+		u64 interruption_type: 1;
+		u64 reserved : 30;
+		u64 error_code : 32;
+	} __packed;
+};
+
+union hv_arm64_interrupt_state_register {
+	u64 as_uint64;
+	struct {
+		u64 interrupt_shadow : 1;
+		u64 reserved : 63;
+	} __packed;
+};
+
+union hv_arm64_pending_synthetic_exception_event {
+	u64 as_uint64[2];
+	struct {
+		u8 event_pending : 1;
+		u8 event_type : 3;
+		u8 reserved : 4;
+		u8 rsvd[3];
+		u64 context;
+	} __packed;
+};
+
 union hv_x64_interrupt_state_register {
 	u64 as_uint64;
 	struct {
@@ -1103,8 +1132,20 @@ union hv_register_value {
 	union hv_explicit_suspend_register explicit_suspend;
 	union hv_intercept_suspend_register intercept_suspend;
 	union hv_dispatch_suspend_register dispatch_suspend;
+#ifdef CONFIG_ARM64
+	union hv_arm64_interrupt_state_register interrupt_state;
+	union hv_arm64_pending_interruption_register pending_interruption;
+#endif
+#ifdef CONFIG_X86
 	union hv_x64_interrupt_state_register interrupt_state;
 	union hv_x64_pending_interruption_register pending_interruption;
+#endif
+	union hv_arm64_pending_synthetic_exception_event pending_synthetic_exception_event;
+};
+
+/* NOTE: Linux helper struct - NOT from Hyper-V code. */
+struct hv_output_get_vp_registers {
+	DECLARE_FLEX_ARRAY(union hv_register_value, values);
 };
 
 #if defined(CONFIG_ARM64)
-- 
2.34.1

Re: [PATCH v6 1/5] hyperv: Define struct hv_output_get_vp_registers

[Nuno Das Neves]

On 1/8/2025 2:21 PM, Roman Kisel wrote:
> There is no definition of the output structure for the
> GetVpRegisters hypercall. Hence, using the hypercall
> is not possible when the output value has some structure
> to it. Even getting a datum of a primitive type reads
> as ad-hoc without that definition.
> 
> Define struct hv_output_get_vp_registers to enable using
> the GetVpRegisters hypercall. Make provisions for all
> supported architectures. No functional changes.
> 
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
>  include/hyperv/hvgdk_mini.h | 41 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> index db3d1aaf7330..4fffca9e16df 100644
> --- a/include/hyperv/hvgdk_mini.h
> +++ b/include/hyperv/hvgdk_mini.h
> @@ -1068,6 +1068,35 @@ union hv_dispatch_suspend_register {
>  	} __packed;
>  };
>  
> +union hv_arm64_pending_interruption_register {
> +	u64 as_uint64;
> +	struct {
> +		u64 interruption_pending : 1;
> +		u64 interruption_type: 1;
> +		u64 reserved : 30;
> +		u64 error_code : 32;
> +	} __packed;
> +};
> +
> +union hv_arm64_interrupt_state_register {
> +	u64 as_uint64;
> +	struct {
> +		u64 interrupt_shadow : 1;
> +		u64 reserved : 63;
> +	} __packed;
> +};
> +
> +union hv_arm64_pending_synthetic_exception_event {
> +	u64 as_uint64[2];
> +	struct {
> +		u8 event_pending : 1;
> +		u8 event_type : 3;
> +		u8 reserved : 4;
> +		u8 rsvd[3];
> +		u64 context;
> +	} __packed;
> +};
> +

You've omitted the exception_type field.
This is how it should be:

union hv_arm64_pending_synthetic_exception_event {
	u64 as_uint64[2];
	struct {
		u8 event_pending : 1;
		u8 event_type : 3;
		u8 reserved : 4;
		u8 rsvd[3];
		u32 exception_type;
		u64 context;
	} __packed;
};

>  union hv_x64_interrupt_state_register {
>  	u64 as_uint64;
>  	struct {
> @@ -1103,8 +1132,20 @@ union hv_register_value {
>  	union hv_explicit_suspend_register explicit_suspend;
>  	union hv_intercept_suspend_register intercept_suspend;
>  	union hv_dispatch_suspend_register dispatch_suspend;
> +#ifdef CONFIG_ARM64
> +	union hv_arm64_interrupt_state_register interrupt_state;
> +	union hv_arm64_pending_interruption_register pending_interruption;
> +#endif
> +#ifdef CONFIG_X86
>  	union hv_x64_interrupt_state_register interrupt_state;
>  	union hv_x64_pending_interruption_register pending_interruption;
> +#endif
> +	union hv_arm64_pending_synthetic_exception_event pending_synthetic_exception_event;
> +};
> +
> +/* NOTE: Linux helper struct - NOT from Hyper-V code. */
> +struct hv_output_get_vp_registers {
> +	DECLARE_FLEX_ARRAY(union hv_register_value, values);
>  };
>  
>  #if defined(CONFIG_ARM64)

Re: [PATCH v6 1/5] hyperv: Define struct hv_output_get_vp_registers

[Wei Liu]

On Wed, Jan 08, 2025 at 03:25:22PM -0800, Nuno Das Neves wrote:
> On 1/8/2025 2:21 PM, Roman Kisel wrote:
> > There is no definition of the output structure for the
> > GetVpRegisters hypercall. Hence, using the hypercall
> > is not possible when the output value has some structure
> > to it. Even getting a datum of a primitive type reads
> > as ad-hoc without that definition.
> > 
> > Define struct hv_output_get_vp_registers to enable using
> > the GetVpRegisters hypercall. Make provisions for all
> > supported architectures. No functional changes.
> > 
> > Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> > ---
> >  include/hyperv/hvgdk_mini.h | 41 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> > index db3d1aaf7330..4fffca9e16df 100644
> > --- a/include/hyperv/hvgdk_mini.h
> > +++ b/include/hyperv/hvgdk_mini.h
> > @@ -1068,6 +1068,35 @@ union hv_dispatch_suspend_register {
> >  	} __packed;
> >  };
> >  
> > +union hv_arm64_pending_interruption_register {
> > +	u64 as_uint64;
> > +	struct {
> > +		u64 interruption_pending : 1;
> > +		u64 interruption_type: 1;
> > +		u64 reserved : 30;
> > +		u64 error_code : 32;
> > +	} __packed;
> > +};
> > +
> > +union hv_arm64_interrupt_state_register {
> > +	u64 as_uint64;
> > +	struct {
> > +		u64 interrupt_shadow : 1;
> > +		u64 reserved : 63;
> > +	} __packed;
> > +};
> > +
> > +union hv_arm64_pending_synthetic_exception_event {
> > +	u64 as_uint64[2];
> > +	struct {
> > +		u8 event_pending : 1;
> > +		u8 event_type : 3;
> > +		u8 reserved : 4;
> > +		u8 rsvd[3];
> > +		u64 context;
> > +	} __packed;
> > +};
> > +
> 
> You've omitted the exception_type field.
> This is how it should be:
> 
> union hv_arm64_pending_synthetic_exception_event {
> 	u64 as_uint64[2];
> 	struct {
> 		u8 event_pending : 1;
> 		u8 event_type : 3;
> 		u8 reserved : 4;
> 		u8 rsvd[3];
> 		u32 exception_type;
> 		u64 context;
> 	} __packed;
> };
> 

I can fix this when I commit the change . This patch will be folded into
your old one anyway.

Thanks,
Wei.

Re: [PATCH v6 1/5] hyperv: Define struct hv_output_get_vp_registers

[Roman Kisel]

On 1/8/2025 9:50 PM, Wei Liu wrote:
> On Wed, Jan 08, 2025 at 03:25:22PM -0800, Nuno Das Neves wrote:
>> On 1/8/2025 2:21 PM, Roman Kisel wrote:

[...]

>>
> 
> I can fix this when I commit the change . This patch will be folded into
> your old one anyway.
> 
Nuno, thank you very much for spotting that! Wei, appreciate that!
Didn't mean to create more work for you, sorry about that.

> Thanks,
> Wei.

-- 
Thank you,
Roman

Re: [PATCH v6 1/5] hyperv: Define struct hv_output_get_vp_registers

[Wei Liu]

On Thu, Jan 09, 2025 at 09:25:58AM -0800, Roman Kisel wrote:
> 
> 
> On 1/8/2025 9:50 PM, Wei Liu wrote:
> > On Wed, Jan 08, 2025 at 03:25:22PM -0800, Nuno Das Neves wrote:
> > > On 1/8/2025 2:21 PM, Roman Kisel wrote:
> 
> [...]
> 
> > > 
> > 
> > I can fix this when I commit the change . This patch will be folded into
> > your old one anyway.
> > 
> Nuno, thank you very much for spotting that! Wei, appreciate that!
> Didn't mean to create more work for you, sorry about that.

No problem at all. I'm happy to help.

Wei.

> 
> > Thanks,
> > Wei.
> 
> -- 
> Thank you,
> Roman
> 
> 

[PATCH v6 2/5] hyperv: Fix pointer type in get_vtl(void)

[Roman Kisel]

Commit bc905fa8b633 ("hyperv: Switch from hyperv-tlfs.h to hyperv/hvhdk.h")
changed the type of the output pointer to `struct hv_register_assoc` from
`struct hv_get_vp_registers_output`. That leads to an incorrect computation,
and leaves the system broken.

Use the correct pointer type for the output of the GetVpRegisters hypercall.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Easwar Hariharan <eahariha@linux.microsoft.com>
Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
 arch/x86/hyperv/hv_init.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 9e5e8328df6b..f82d1aefaa8a 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -416,13 +416,13 @@ static u8 __init get_vtl(void)
 {
 	u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS;
 	struct hv_input_get_vp_registers *input;
-	struct hv_register_assoc *output;
+	struct hv_output_get_vp_registers *output;
 	unsigned long flags;
 	u64 ret;
 
 	local_irq_save(flags);
 	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
-	output = (struct hv_register_assoc *)input;
+	output = (struct hv_output_get_vp_registers *)input;
 
 	memset(input, 0, struct_size(input, names, 1));
 	input->partition_id = HV_PARTITION_ID_SELF;
@@ -432,7 +432,7 @@ static u8 __init get_vtl(void)
 
 	ret = hv_do_hypercall(control, input, output);
 	if (hv_result_success(ret)) {
-		ret = output->value.reg8 & HV_X64_VTL_MASK;
+		ret = output->values[0].reg8 & HV_X64_VTL_MASK;
 	} else {
 		pr_err("Failed to get VTL(error: %lld) exiting...\n", ret);
 		BUG();
-- 
2.34.1

[PATCH v6 3/5] hyperv: Enable the hypercall output page for the VTL mode

[Roman Kisel]

Due to the hypercall page not being allocated in the VTL mode,
the code resorts to using a part of the input page.

Allocate the hypercall output page in the VTL mode thus enabling
it to use it for output and share code with dom0.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 drivers/hv/hv_common.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index c6ed3ba4bf61..af5d1dc451f6 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -278,6 +278,11 @@ static void hv_kmsg_dump_register(void)
 	}
 }
 
+static inline bool hv_output_page_exists(void)
+{
+	return hv_root_partition || IS_ENABLED(CONFIG_HYPERV_VTL_MODE);
+}
+
 int __init hv_common_init(void)
 {
 	int i;
@@ -340,7 +345,7 @@ int __init hv_common_init(void)
 	BUG_ON(!hyperv_pcpu_input_arg);
 
 	/* Allocate the per-CPU state for output arg for root */
-	if (hv_root_partition) {
+	if (hv_output_page_exists()) {
 		hyperv_pcpu_output_arg = alloc_percpu(void *);
 		BUG_ON(!hyperv_pcpu_output_arg);
 	}
@@ -435,7 +440,7 @@ int hv_common_cpu_init(unsigned int cpu)
 	void **inputarg, **outputarg;
 	u64 msr_vp_index;
 	gfp_t flags;
-	int pgcount = hv_root_partition ? 2 : 1;
+	const int pgcount = hv_output_page_exists() ? 2 : 1;
 	void *mem;
 	int ret;
 
@@ -453,7 +458,7 @@ int hv_common_cpu_init(unsigned int cpu)
 		if (!mem)
 			return -ENOMEM;
 
-		if (hv_root_partition) {
+		if (hv_output_page_exists()) {
 			outputarg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
 			*outputarg = (char *)mem + HV_HYP_PAGE_SIZE;
 		}
-- 
2.34.1

Re: [PATCH v6 3/5] hyperv: Enable the hypercall output page for the VTL mode

[Nuno Das Neves]

On 1/8/2025 2:21 PM, Roman Kisel wrote:
> Due to the hypercall page not being allocated in the VTL mode,
> the code resorts to using a part of the input page.
> 
> Allocate the hypercall output page in the VTL mode thus enabling
> it to use it for output and share code with dom0.
> 
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
>  drivers/hv/hv_common.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index c6ed3ba4bf61..af5d1dc451f6 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -278,6 +278,11 @@ static void hv_kmsg_dump_register(void)
>  	}
>  }
>  
> +static inline bool hv_output_page_exists(void)
> +{
> +	return hv_root_partition || IS_ENABLED(CONFIG_HYPERV_VTL_MODE);
> +}
> +
>  int __init hv_common_init(void)
>  {
>  	int i;
> @@ -340,7 +345,7 @@ int __init hv_common_init(void)
>  	BUG_ON(!hyperv_pcpu_input_arg);
>  
>  	/* Allocate the per-CPU state for output arg for root */
> -	if (hv_root_partition) {
> +	if (hv_output_page_exists()) {
>  		hyperv_pcpu_output_arg = alloc_percpu(void *);
>  		BUG_ON(!hyperv_pcpu_output_arg);
>  	}
> @@ -435,7 +440,7 @@ int hv_common_cpu_init(unsigned int cpu)
>  	void **inputarg, **outputarg;
>  	u64 msr_vp_index;
>  	gfp_t flags;
> -	int pgcount = hv_root_partition ? 2 : 1;
> +	const int pgcount = hv_output_page_exists() ? 2 : 1;
>  	void *mem;
>  	int ret;
>  
> @@ -453,7 +458,7 @@ int hv_common_cpu_init(unsigned int cpu)
>  		if (!mem)
>  			return -ENOMEM;
>  
> -		if (hv_root_partition) {
> +		if (hv_output_page_exists()) {
>  			outputarg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
>  			*outputarg = (char *)mem + HV_HYP_PAGE_SIZE;
>  		}

Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>

[PATCH v6 4/5] hyperv: Do not overlap the hvcall IO areas in get_vtl()

[Roman Kisel]

The Top-Level Functional Specification for Hyper-V, Section 3.6 [1, 2],
disallows overlapping of the input and output hypercall areas, and
get_vtl(void) does overlap them.

Use the output hypercall page of the current vCPU for the hypercall.

[1] https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/hypercall-interface
[2] https://github.com/MicrosoftDocs/Virtualization-Documentation/tree/main/tlfs

Fixes: 8387ce06d70b ("x86/hyperv: Set Virtual Trust Level in VMBus init message")
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Tianyu Lan <tiala@microsoft.com>
Reviewed-by: Easwar Hariharan <eahariha@linux.microsoft.com>
---
 arch/x86/hyperv/hv_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index f82d1aefaa8a..173005e6a95d 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -422,7 +422,7 @@ static u8 __init get_vtl(void)
 
 	local_irq_save(flags);
 	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
-	output = (struct hv_output_get_vp_registers *)input;
+	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
 
 	memset(input, 0, struct_size(input, names, 1));
 	input->partition_id = HV_PARTITION_ID_SELF;
-- 
2.34.1

[PATCH v6 5/5] hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id()

[Roman Kisel]

The Top-Level Functional Specification for Hyper-V, Section 3.6 [1, 2],
disallows overlapping of the input and output hypercall areas, and
hv_vtl_apicid_to_vp_id() overlaps them.

Use the output hypercall page of the current vCPU for the hypercall.

[1] https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/hypercall-interface
[2] https://github.com/MicrosoftDocs/Virtualization-Documentation/tree/main/tlfs

Reported-by: Michael Kelley <mhklinux@outlook.com>
Closes: https://lore.kernel.org/lkml/SN6PR02MB4157B98CD34781CC87A9D921D40D2@SN6PR02MB4157.namprd02.prod.outlook.com/
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Easwar Hariharan <eahariha@linux.microsoft.com>
---
 arch/x86/hyperv/hv_vtl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 04775346369c..4e1b1e3b5658 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -189,7 +189,7 @@ static int hv_vtl_apicid_to_vp_id(u32 apic_id)
 	input->partition_id = HV_PARTITION_ID_SELF;
 	input->apic_ids[0] = apic_id;
 
-	output = (u32 *)input;
+	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
 
 	control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_ID_FROM_APIC_ID;
 	status = hv_do_hypercall(control, input, output);
-- 
2.34.1

Re: [PATCH v6 5/5] hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id()

[Nuno Das Neves]

On 1/8/2025 2:21 PM, Roman Kisel wrote:
> The Top-Level Functional Specification for Hyper-V, Section 3.6 [1, 2],
> disallows overlapping of the input and output hypercall areas, and
> hv_vtl_apicid_to_vp_id() overlaps them.
> 
> Use the output hypercall page of the current vCPU for the hypercall.
> 
> [1] https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/hypercall-interface
> [2] https://github.com/MicrosoftDocs/Virtualization-Documentation/tree/main/tlfs
> 
> Reported-by: Michael Kelley <mhklinux@outlook.com>
> Closes: https://lore.kernel.org/lkml/SN6PR02MB4157B98CD34781CC87A9D921D40D2@SN6PR02MB4157.namprd02.prod.outlook.com/
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Easwar Hariharan <eahariha@linux.microsoft.com>
> ---
>  arch/x86/hyperv/hv_vtl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> index 04775346369c..4e1b1e3b5658 100644
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -189,7 +189,7 @@ static int hv_vtl_apicid_to_vp_id(u32 apic_id)
>  	input->partition_id = HV_PARTITION_ID_SELF;
>  	input->apic_ids[0] = apic_id;
>  
> -	output = (u32 *)input;
> +	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
>  
>  	control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_ID_FROM_APIC_ID;
>  	status = hv_do_hypercall(control, input, output);

Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>

Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()

[Wei Liu]

On Wed, Jan 08, 2025 at 02:21:33PM -0800, Roman Kisel wrote:
[...]
> Roman Kisel (5):
>   hyperv: Define struct hv_output_get_vp_registers
>   hyperv: Fix pointer type in get_vtl(void)
>   hyperv: Enable the hypercall output page for the VTL mode
>   hyperv: Do not overlap the hvcall IO areas in get_vtl()
>   hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id()

The patches have been pushed to hyperv-next. Roman and Nuno, please
check the tree for correctness.

Thanks,
Wei.

Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()

[Nuno Das Neves]

On 1/9/2025 12:18 PM, Wei Liu wrote:
> On Wed, Jan 08, 2025 at 02:21:33PM -0800, Roman Kisel wrote:
> [...]
>> Roman Kisel (5):
>>   hyperv: Define struct hv_output_get_vp_registers
>>   hyperv: Fix pointer type in get_vtl(void)
>>   hyperv: Enable the hypercall output page for the VTL mode
>>   hyperv: Do not overlap the hvcall IO areas in get_vtl()
>>   hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id()
> 
> The patches have been pushed to hyperv-next. Roman and Nuno, please
> check the tree for correctness.
> 
> Thanks,
> Wei.

I checked, looks like the first two patches of the series are missing?

Nuno

Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()

[Roman Kisel]

On 1/9/2025 12:28 PM, Nuno Das Neves wrote:
> On 1/9/2025 12:18 PM, Wei Liu wrote:
>> On Wed, Jan 08, 2025 at 02:21:33PM -0800, Roman Kisel wrote:
>> [...]
>>> Roman Kisel (5):
>>>    hyperv: Define struct hv_output_get_vp_registers
>>>    hyperv: Fix pointer type in get_vtl(void)
>>>    hyperv: Enable the hypercall output page for the VTL mode
>>>    hyperv: Do not overlap the hvcall IO areas in get_vtl()
>>>    hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id()
>>
>> The patches have been pushed to hyperv-next. Roman and Nuno, please
>> check the tree for correctness.
>>
>> Thanks,
>> Wei.
> 
> I checked, looks like the first two patches of the series are missing?
IIUC, they were to be rolled up into your earlier patches

> 
> Nuno

-- 
Thank you,
Roman

Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()

[Roman Kisel]

On 1/9/2025 12:18 PM, Wei Liu wrote:
> On Wed, Jan 08, 2025 at 02:21:33PM -0800, Roman Kisel wrote:
> [...]
>> Roman Kisel (5):
>>    hyperv: Define struct hv_output_get_vp_registers
>>    hyperv: Fix pointer type in get_vtl(void)
>>    hyperv: Enable the hypercall output page for the VTL mode
>>    hyperv: Do not overlap the hvcall IO areas in get_vtl()
>>    hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id()
> 
> The patches have been pushed to hyperv-next. Roman and Nuno, please
> check the tree for correctness.

This

```c
union hv_arm64_pending_synthetic_exception_event {
	u64 as_uint64[2];
	struct {
		u8 event_pending : 1;
		u8 event_type : 3;
		u8 reserved : 4;
		u8 rsvd[3];
		u64 context;
	} __packed;
};
```

needs to have the `u32 exception_type;` field:

```c
union hv_arm64_pending_synthetic_exception_event {
	u64 as_uint64[2];
	struct {
		u8 event_pending : 1;
		u8 event_type : 3;
		u8 reserved : 4;
		u8 rsvd[3];
		u32 exception_type;
		u64 context;
	} __packed;
};
```
as otherwise the struct won't cover the array.
Testing the VMs currently with the latest hyperv-next.

> 
> Thanks,
> Wei.

-- 
Thank you,
Roman

Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()

[Wei Liu]

On Thu, Jan 09, 2025 at 01:40:34PM -0800, Roman Kisel wrote:
> 
> 
> On 1/9/2025 12:18 PM, Wei Liu wrote:
> > On Wed, Jan 08, 2025 at 02:21:33PM -0800, Roman Kisel wrote:
> > [...]
> > > Roman Kisel (5):
> > >    hyperv: Define struct hv_output_get_vp_registers
> > >    hyperv: Fix pointer type in get_vtl(void)
> > >    hyperv: Enable the hypercall output page for the VTL mode
> > >    hyperv: Do not overlap the hvcall IO areas in get_vtl()
> > >    hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id()
> > 
> > The patches have been pushed to hyperv-next. Roman and Nuno, please
> > check the tree for correctness.
> 
> This
> 
> ```c
> union hv_arm64_pending_synthetic_exception_event {
> 	u64 as_uint64[2];
> 	struct {
> 		u8 event_pending : 1;
> 		u8 event_type : 3;
> 		u8 reserved : 4;
> 		u8 rsvd[3];
> 		u64 context;
> 	} __packed;
> };
> ```
> 
> needs to have the `u32 exception_type;` field:
> 
> ```c
> union hv_arm64_pending_synthetic_exception_event {
> 	u64 as_uint64[2];
> 	struct {
> 		u8 event_pending : 1;
> 		u8 event_type : 3;
> 		u8 reserved : 4;
> 		u8 rsvd[3];
> 		u32 exception_type;
> 		u64 context;
> 	} __packed;
> };
> ```
> as otherwise the struct won't cover the array.
> Testing the VMs currently with the latest hyperv-next.

Fixed. I c&p'ed the code then deleted the right version of the struct.
Thanks for checking.

Wei.

Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()

[Roman Kisel]

On 1/9/2025 1:56 PM, Wei Liu wrote:
> On Thu, Jan 09, 2025 at 01:40:34PM -0800, Roman Kisel wrote:

[...]

>>
>> needs to have the `u32 exception_type;` field:
>>
>> ```c
>> union hv_arm64_pending_synthetic_exception_event {
>> 	u64 as_uint64[2];
>> 	struct {
>> 		u8 event_pending : 1;
>> 		u8 event_type : 3;
>> 		u8 reserved : 4;
>> 		u8 rsvd[3];
>> 		u32 exception_type;
>> 		u64 context;
>> 	} __packed;
>> };
>> ```
>> as otherwise the struct won't cover the array.
>> Testing the VMs currently with the latest hyperv-next.
> 
> Fixed. I c&p'ed the code then deleted the right version of the struct.
> Thanks for checking.

Happy to help :D

Validated with the VMs, and with the latest hyperv-next, the issue is
fixed!! Appreciate your help and guidance; thank you, Easwar, Michael,
Nuno, Stanislav, Tianyu and Wei for the suggestions that have let make 
this patchset so much better :)

Borislav, I apologize for sending the patchset versions too often. I'm
sorry for causing you trouble due to that. I have read up the kernel
documentation, and will be a better citizen of the LKML.

> 
> Wei.

-- 
Thank you,
Roman