[PATCH v4 4/7] Drivers: hv: Use hv_setup_*() to set up hypercall arguments

mhkelley58@gmail.com posted 7 patches 2 months, 2 weeks ago
[PATCH v4 4/7] Drivers: hv: Use hv_setup_*() to set up hypercall arguments
Posted by mhkelley58@gmail.com 2 months, 2 weeks ago
From: Michael Kelley <mhklinux@outlook.com>

Update hypercall call sites to use the new hv_setup_*() functions
to set up hypercall arguments. Since these functions zero the
fixed portion of input memory, remove now redundant zero'ing of
input fields.

In hv_post_message(), the payload area is treated as an array to
avoid wasting cycles on zero'ing it and then overwriting with
memcpy().

Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---

Notes:
    Changes in v4:
    * Rename hv_hvcall_*() functions to hv_setup_*() [Easwar Hariharan]
    * Rename hv_hvcall_in_batch_size() to hv_get_input_batch_size()
      [Easwar Hariharan]
    
    Changes in v3:
    * Removed change to definition of struct hv_input_post_message so the
      'payload' remains a fixed size array. Adjust hv_post_message() so
      that the 'payload' array is not zero'ed. [Nuno Das Neves]
    * Added check of the batch size in hv_free_page_report(). [Nuno Das Neves]
    * In hv_call_deposit_pages(), use the new hv_hvcall_in_batch_size() to
      get the batch size at the start of the function, and check the
      'num_pages' input parameter against that batch size instead of against
      a separately defined constant. Also use the batch size to compute the
      size of the memory allocation. [Nuno Das Neves]

 drivers/hv/hv.c         |  4 +++-
 drivers/hv/hv_balloon.c |  8 ++++----
 drivers/hv/hv_common.c  |  9 +++------
 drivers/hv/hv_proc.c    | 23 ++++++++++-------------
 4 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index b14c5f9e0ef2..ad063f535f95 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -66,7 +66,9 @@ int hv_post_message(union hv_connection_id connection_id,
 	if (hv_isolation_type_tdx() && ms_hyperv.paravisor_present)
 		aligned_msg = this_cpu_ptr(hv_context.cpu_context)->post_msg_page;
 	else
-		aligned_msg = *this_cpu_ptr(hyperv_pcpu_input_arg);
+		hv_setup_in_array(&aligned_msg,
+				   offsetof(typeof(*aligned_msg), payload),
+				   sizeof(aligned_msg->payload[0]));
 
 	aligned_msg->connectionid = connection_id;
 	aligned_msg->reserved = 0;
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 2b4080e51f97..d9b569b204d2 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -1577,21 +1577,21 @@ static int hv_free_page_report(struct page_reporting_dev_info *pr_dev_info,
 {
 	unsigned long flags;
 	struct hv_memory_hint *hint;
-	int i, order;
+	int i, order, batch_size;
 	u64 status;
 	struct scatterlist *sg;
 
-	WARN_ON_ONCE(nents > HV_MEMORY_HINT_MAX_GPA_PAGE_RANGES);
 	WARN_ON_ONCE(sgl->length < (HV_HYP_PAGE_SIZE << page_reporting_order));
 	local_irq_save(flags);
-	hint = *this_cpu_ptr(hyperv_pcpu_input_arg);
+
+	batch_size = hv_setup_in_array(&hint, sizeof(*hint), sizeof(hint->ranges[0]));
 	if (!hint) {
 		local_irq_restore(flags);
 		return -ENOSPC;
 	}
+	WARN_ON_ONCE(nents > batch_size);
 
 	hint->heat_type = HV_EXTMEM_HEAT_HINT_COLD_DISCARD;
-	hint->reserved = 0;
 	for_each_sg(sgl, sg, nents, i) {
 		union hv_gpa_page_range *range;
 
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 49898d10faff..ae56397af1ed 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -267,7 +267,7 @@ void __init hv_get_partition_id(void)
 	u64 status, pt_id;
 
 	local_irq_save(flags);
-	output = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	hv_setup_inout(NULL, 0, &output, sizeof(*output));
 	status = hv_do_hypercall(HVCALL_GET_PARTITION_ID, NULL, output);
 	pt_id = output->partition_id;
 	local_irq_restore(flags);
@@ -288,13 +288,10 @@ u8 __init get_vtl(void)
 	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, names, 1));
+	hv_setup_inout_array(&input, sizeof(*input), sizeof(input->names[0]),
+			     &output, sizeof(*output), sizeof(output->values[0]));
 	input->partition_id = HV_PARTITION_ID_SELF;
 	input->vp_index = HV_VP_INDEX_SELF;
-	input->input_vtl.as_uint8 = 0;
 	input->names[0] = HV_REGISTER_VSM_VP_STATUS;
 
 	ret = hv_do_hypercall(control, input, output);
diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
index fbb4eb3901bb..bd63830b4141 100644
--- a/drivers/hv/hv_proc.c
+++ b/drivers/hv/hv_proc.c
@@ -9,12 +9,6 @@
 #include <linux/export.h>
 #include <asm/mshyperv.h>
 
-/*
- * See struct hv_deposit_memory. The first u64 is partition ID, the rest
- * are GPAs.
- */
-#define HV_DEPOSIT_MAX (HV_HYP_PAGE_SIZE / sizeof(u64) - 1)
-
 /* Deposits exact number of pages. Must be called with interrupts enabled.  */
 int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
 {
@@ -25,11 +19,13 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
 	int order;
 	u64 status;
 	int ret;
-	u64 base_pfn;
+	u64 base_pfn, batch_size;
 	struct hv_deposit_memory *input_page;
 	unsigned long flags;
 
-	if (num_pages > HV_DEPOSIT_MAX)
+	batch_size = hv_get_input_batch_size(sizeof(*input_page),
+			   sizeof(input_page->gpa_page_list[0]));
+	if (num_pages > batch_size)
 		return -E2BIG;
 	if (!num_pages)
 		return 0;
@@ -40,7 +36,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
 		return -ENOMEM;
 	pages = page_address(page);
 
-	counts = kcalloc(HV_DEPOSIT_MAX, sizeof(int), GFP_KERNEL);
+	counts = kcalloc(batch_size, sizeof(int), GFP_KERNEL);
 	if (!counts) {
 		free_page((unsigned long)pages);
 		return -ENOMEM;
@@ -74,7 +70,9 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
 
 	local_irq_save(flags);
 
-	input_page = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	/* Batch size is checked at the start of function; no need to repeat */
+	hv_setup_in_array(&input_page, sizeof(*input_page),
+			   sizeof(input_page->gpa_page_list[0]));
 
 	input_page->partition_id = partition_id;
 
@@ -126,9 +124,8 @@ int hv_call_add_logical_proc(int node, u32 lp_index, u32 apic_id)
 	do {
 		local_irq_save(flags);
 
-		input = *this_cpu_ptr(hyperv_pcpu_input_arg);
 		/* We don't do anything with the output right now */
-		output = *this_cpu_ptr(hyperv_pcpu_output_arg);
+		hv_setup_inout(&input, sizeof(*input), &output, sizeof(*output));
 
 		input->lp_index = lp_index;
 		input->apic_id = apic_id;
@@ -169,7 +166,7 @@ int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
 	do {
 		local_irq_save(irq_flags);
 
-		input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+		hv_setup_in(&input, sizeof(*input));
 
 		input->partition_id = partition_id;
 		input->vp_index = vp_index;
-- 
2.25.1
Re: [PATCH v4 4/7] Drivers: hv: Use hv_setup_*() to set up hypercall arguments
Posted by Nuno Das Neves 2 months, 1 week ago
On 7/17/2025 9:55 PM, mhkelley58@gmail.com wrote:
<snip>
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 2b4080e51f97..d9b569b204d2 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -1577,21 +1577,21 @@ static int hv_free_page_report(struct page_reporting_dev_info *pr_dev_info,
>  {
>  	unsigned long flags;
>  	struct hv_memory_hint *hint;
> -	int i, order;
> +	int i, order, batch_size;
>  	u64 status;
>  	struct scatterlist *sg;
>  
> -	WARN_ON_ONCE(nents > HV_MEMORY_HINT_MAX_GPA_PAGE_RANGES);
>  	WARN_ON_ONCE(sgl->length < (HV_HYP_PAGE_SIZE << page_reporting_order));
>  	local_irq_save(flags);
> -	hint = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +
> +	batch_size = hv_setup_in_array(&hint, sizeof(*hint), sizeof(hint->ranges[0]));
>  	if (!hint) {
>  		local_irq_restore(flags);
>  		return -ENOSPC;
>  	}
> +	WARN_ON_ONCE(nents > batch_size);
>  

I don't think WARN_ON_ONCE is sufficient here... this looks like a bug in the current code.
The loop below will go out of bounds of the input page if nents is too large.

Ideally this function would be refactored to batch the operation so that this isn't a
problem.

Nuno
>  	hint->heat_type = HV_EXTMEM_HEAT_HINT_COLD_DISCARD;
> -	hint->reserved = 0;
>  	for_each_sg(sgl, sg, nents, i) {
>  		union hv_gpa_page_range *range;
>  
<snip>