diff mbox series

Drivers: hv: vmbus: Remove the per-CPU post_msg_page

Message ID 20230408213441.15472-1-decui@microsoft.com
State New
Headers
Series Drivers: hv: vmbus: Remove the per-CPU post_msg_page |

Commit Message

Dexuan Cui April 8, 2023, 9:34 p.m. UTC 
  The post_msg_page was introduced in 2014 in
commit b29ef3546aec ("Drivers: hv: vmbus: Cleanup hv_post_message()")

Commit 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments") introduced
the hyperv_pcpu_input_arg in 2018, which can be used in hv_post_message().

Remove post_msg_page to simplify the code a little bit.

Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
 drivers/hv/hv.c           | 20 +++++---------------
 drivers/hv/hyperv_vmbus.h |  4 ----
 2 files changed, 5 insertions(+), 19 deletions(-)

Comments

Jinank Jain April 10, 2023, 4:28 a.m. UTC | #1 
Reviewed-by: Jinank Jain <jinankjain@linux.microsoft.com>

On 4/9/2023 3:04 AM, Dexuan Cui wrote:
> The post_msg_page was introduced in 2014 in
> commit b29ef3546aec ("Drivers: hv: vmbus: Cleanup hv_post_message()")
>
> Commit 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments") introduced
> the hyperv_pcpu_input_arg in 2018, which can be used in hv_post_message().
>
> Remove post_msg_page to simplify the code a little bit.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>   drivers/hv/hv.c           | 20 +++++---------------
>   drivers/hv/hyperv_vmbus.h |  4 ----
>   2 files changed, 5 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 8b0dd8e5244d7..30bf122a502ae 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -84,14 +84,15 @@ int hv_post_message(union hv_connection_id connection_id,
>   		  void *payload, size_t payload_size)
>   {
>   	struct hv_input_post_message *aligned_msg;
> -	struct hv_per_cpu_context *hv_cpu;
> +	unsigned long flags;
>   	u64 status;
>   
>   	if (payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT)
>   		return -EMSGSIZE;
>   
> -	hv_cpu = get_cpu_ptr(hv_context.cpu_context);
> -	aligned_msg = hv_cpu->post_msg_page;
> +	local_irq_save(flags);
> +
> +	aligned_msg = *this_cpu_ptr(hyperv_pcpu_input_arg);
>   	aligned_msg->connectionid = connection_id;
>   	aligned_msg->reserved = 0;
>   	aligned_msg->message_type = message_type;
> @@ -106,11 +107,7 @@ int hv_post_message(union hv_connection_id connection_id,
>   		status = hv_do_hypercall(HVCALL_POST_MESSAGE,
>   				aligned_msg, NULL);
>   
> -	/* Preemption must remain disabled until after the hypercall
> -	 * so some other thread can't get scheduled onto this cpu and
> -	 * corrupt the per-cpu post_msg_page
> -	 */
> -	put_cpu_ptr(hv_cpu);
> +	local_irq_restore(flags);
>   
>   	return hv_result(status);
>   }
> @@ -162,12 +159,6 @@ int hv_synic_alloc(void)
>   				goto err;
>   			}
>   		}
> -
> -		hv_cpu->post_msg_page = (void *)get_zeroed_page(GFP_ATOMIC);
> -		if (hv_cpu->post_msg_page == NULL) {
> -			pr_err("Unable to allocate post msg page\n");
> -			goto err;
> -		}
>   	}
>   
>   	return 0;
> @@ -190,7 +181,6 @@ void hv_synic_free(void)
>   
>   		free_page((unsigned long)hv_cpu->synic_event_page);
>   		free_page((unsigned long)hv_cpu->synic_message_page);
> -		free_page((unsigned long)hv_cpu->post_msg_page);
>   	}
>   
>   	kfree(hv_context.hv_numa_map);
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index dc673edf053c3..d8322049ecd08 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -122,10 +122,6 @@ enum {
>   struct hv_per_cpu_context {
>   	void *synic_message_page;
>   	void *synic_event_page;
> -	/*
> -	 * buffer to post messages to the host.
> -	 */
> -	void *post_msg_page;
>   
>   	/*
>   	 * Starting with win8, we can take channel interrupts on any CPU;
Michael Kelley (LINUX) April 11, 2023, 5:24 p.m. UTC | #2 
From: Dexuan Cui <decui@microsoft.com> Sent: Saturday, April 8, 2023 2:35 PM
> 
> The post_msg_page was introduced in 2014 in
> commit b29ef3546aec ("Drivers: hv: vmbus: Cleanup hv_post_message()")
> 
> Commit 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments") introduced
> the hyperv_pcpu_input_arg in 2018, which can be used in hv_post_message().
> 
> Remove post_msg_page to simplify the code a little bit.
> 
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>  drivers/hv/hv.c           | 20 +++++---------------
>  drivers/hv/hyperv_vmbus.h |  4 ----
>  2 files changed, 5 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 8b0dd8e5244d7..30bf122a502ae 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -84,14 +84,15 @@ int hv_post_message(union hv_connection_id connection_id,
>  		  void *payload, size_t payload_size)
>  {
>  	struct hv_input_post_message *aligned_msg;
> -	struct hv_per_cpu_context *hv_cpu;
> +	unsigned long flags;
>  	u64 status;
> 
>  	if (payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT)
>  		return -EMSGSIZE;
> 
> -	hv_cpu = get_cpu_ptr(hv_context.cpu_context);
> -	aligned_msg = hv_cpu->post_msg_page;
> +	local_irq_save(flags);
> +
> +	aligned_msg = *this_cpu_ptr(hyperv_pcpu_input_arg);
>  	aligned_msg->connectionid = connection_id;
>  	aligned_msg->reserved = 0;
>  	aligned_msg->message_type = message_type;
> @@ -106,11 +107,7 @@ int hv_post_message(union hv_connection_id connection_id,
>  		status = hv_do_hypercall(HVCALL_POST_MESSAGE,
>  				aligned_msg, NULL);
> 
> -	/* Preemption must remain disabled until after the hypercall
> -	 * so some other thread can't get scheduled onto this cpu and
> -	 * corrupt the per-cpu post_msg_page
> -	 */
> -	put_cpu_ptr(hv_cpu);
> +	local_irq_restore(flags);
> 
>  	return hv_result(status);
>  }
> @@ -162,12 +159,6 @@ int hv_synic_alloc(void)
>  				goto err;
>  			}
>  		}
> -
> -		hv_cpu->post_msg_page = (void *)get_zeroed_page(GFP_ATOMIC);
> -		if (hv_cpu->post_msg_page == NULL) {
> -			pr_err("Unable to allocate post msg page\n");
> -			goto err;
> -		}
>  	}
> 
>  	return 0;
> @@ -190,7 +181,6 @@ void hv_synic_free(void)
> 
>  		free_page((unsigned long)hv_cpu->synic_event_page);
>  		free_page((unsigned long)hv_cpu->synic_message_page);
> -		free_page((unsigned long)hv_cpu->post_msg_page);
>  	}
> 
>  	kfree(hv_context.hv_numa_map);
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index dc673edf053c3..d8322049ecd08 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -122,10 +122,6 @@ enum {
>  struct hv_per_cpu_context {
>  	void *synic_message_page;
>  	void *synic_event_page;
> -	/*
> -	 * buffer to post messages to the host.
> -	 */
> -	void *post_msg_page;
> 
>  	/*
>  	 * Starting with win8, we can take channel interrupts on any CPU;
> --
> 2.25.1

This looks good.  Glad to have this simplification done!

FWIW, this patch will cause conflicts with your TDX patch series.  If this
patch goes first, then you'll have merge errors with the TDX patches.
If the TDX series goes first, then this patch will have merge errors.  And
per my comments on Patch 5 of your TDX series, eliminating the
post_msg_page will simplify the error cleanup code in hv_synic_alloc().
It seems like making this patch part of the TDX series would be helpful
all around.

Reviewed-by: Michael Kelley
Wei Liu April 13, 2023, 1:19 a.m. UTC | #3 
On Sat, Apr 08, 2023 at 02:34:41PM -0700, Dexuan Cui wrote:
> The post_msg_page was introduced in 2014 in
> commit b29ef3546aec ("Drivers: hv: vmbus: Cleanup hv_post_message()")
> 
> Commit 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments") introduced
> the hyperv_pcpu_input_arg in 2018, which can be used in hv_post_message().
> 
> Remove post_msg_page to simplify the code a little bit.
> 
> Signed-off-by: Dexuan Cui <decui@microsoft.com>

Applied to hyperv-next.
Wei Liu April 13, 2023, 1:20 a.m. UTC | #4 
On Tue, Apr 11, 2023 at 05:24:31PM +0000, Michael Kelley (LINUX) wrote:
> > --
> > 2.25.1
> 
> This looks good.  Glad to have this simplification done!
> 
> FWIW, this patch will cause conflicts with your TDX patch series.  If this
> patch goes first, then you'll have merge errors with the TDX patches.
> If the TDX series goes first, then this patch will have merge errors.  And
> per my comments on Patch 5 of your TDX series, eliminating the
> post_msg_page will simplify the error cleanup code in hv_synic_alloc().
> It seems like making this patch part of the TDX series would be helpful
> all around.

Seeing that there are pending comments in the TDX series, I've applied
this patch. TDX series can be rebased.

Thanks,
Wei.
diff mbox series

Patch

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 8b0dd8e5244d7..30bf122a502ae 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -84,14 +84,15 @@  int hv_post_message(union hv_connection_id connection_id,
 		  void *payload, size_t payload_size)
 {
 	struct hv_input_post_message *aligned_msg;
-	struct hv_per_cpu_context *hv_cpu;
+	unsigned long flags;
 	u64 status;
 
 	if (payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT)
 		return -EMSGSIZE;
 
-	hv_cpu = get_cpu_ptr(hv_context.cpu_context);
-	aligned_msg = hv_cpu->post_msg_page;
+	local_irq_save(flags);
+
+	aligned_msg = *this_cpu_ptr(hyperv_pcpu_input_arg);
 	aligned_msg->connectionid = connection_id;
 	aligned_msg->reserved = 0;
 	aligned_msg->message_type = message_type;
@@ -106,11 +107,7 @@  int hv_post_message(union hv_connection_id connection_id,
 		status = hv_do_hypercall(HVCALL_POST_MESSAGE,
 				aligned_msg, NULL);
 
-	/* Preemption must remain disabled until after the hypercall
-	 * so some other thread can't get scheduled onto this cpu and
-	 * corrupt the per-cpu post_msg_page
-	 */
-	put_cpu_ptr(hv_cpu);
+	local_irq_restore(flags);
 
 	return hv_result(status);
 }
@@ -162,12 +159,6 @@  int hv_synic_alloc(void)
 				goto err;
 			}
 		}
-
-		hv_cpu->post_msg_page = (void *)get_zeroed_page(GFP_ATOMIC);
-		if (hv_cpu->post_msg_page == NULL) {
-			pr_err("Unable to allocate post msg page\n");
-			goto err;
-		}
 	}
 
 	return 0;
@@ -190,7 +181,6 @@  void hv_synic_free(void)
 
 		free_page((unsigned long)hv_cpu->synic_event_page);
 		free_page((unsigned long)hv_cpu->synic_message_page);
-		free_page((unsigned long)hv_cpu->post_msg_page);
 	}
 
 	kfree(hv_context.hv_numa_map);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index dc673edf053c3..d8322049ecd08 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -122,10 +122,6 @@  enum {
 struct hv_per_cpu_context {
 	void *synic_message_page;
 	void *synic_event_page;
-	/*
-	 * buffer to post messages to the host.
-	 */
-	void *post_msg_page;
 
 	/*
 	 * Starting with win8, we can take channel interrupts on any CPU;