Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Aug 2023 00:10:03 +0000
From:   Wei Liu <wei.liu@kernel.org>
To:     Nuno Das Neves <nunodasneves@linux.microsoft.com>
Cc:     linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
        x86@kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-arch@vger.kernel.org, mikelley@microsoft.com,
        kys@microsoft.com, wei.liu@kernel.org, haiyangz@microsoft.com,
        decui@microsoft.com, ssengar@linux.microsoft.com,
        mukeshrathor@microsoft.com, stanislav.kinsburskiy@gmail.com,
        jinankjain@linux.microsoft.com, apais@linux.microsoft.com,
        Tianyu.Lan@microsoft.com, vkuznets@redhat.com, tglx@linutronix.de,
        mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
        hpa@zytor.com, will@kernel.org, catalin.marinas@arm.com
Subject: Re: [PATCH 08/15] Drivers: hv: Introduce per-cpu event ring tail
Message-ID: <ZMrwW1m65V1flOpi@liuwe-devbox-debian-v2>
References: <1690487690-2428-1-git-send-email-nunodasneves@linux.microsoft.com>
 <1690487690-2428-9-git-send-email-nunodasneves@linux.microsoft.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1690487690-2428-9-git-send-email-nunodasneves@linux.microsoft.com>
Precedence: bulk

On Thu, Jul 27, 2023 at 12:54:43PM -0700, Nuno Das Neves wrote:
> Add a pointer hv_synic_eventring_tail to track the tail pointer for the
> SynIC event ring buffer for each SINT.
> This will be used by the mshv driver, but must be tracked independently
> since the driver module could be removed and re-inserted.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>

I understand the general idea, but see below.

> ---
>  drivers/hv/hv_common.c         | 25 +++++++++++++++++++++++++
>  include/asm-generic/mshyperv.h |  2 ++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 9f9c3dc89bb2..99d9b262b8a7 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -61,6 +61,16 @@ static void hv_kmsg_dump_unregister(void);
>  
>  static struct ctl_table_header *hv_ctl_table_hdr;
>  
> +/*
> + * Per-cpu array holding the tail pointer for the SynIC event ring buffer
> + * for each SINT.
> + *
> + * We cannot maintain this in mshv driver because the tail pointer should
> + * persist even if the mshv driver is unloaded.
> + */
> +u8 __percpu **hv_synic_eventring_tail;
> +EXPORT_SYMBOL_GPL(hv_synic_eventring_tail);
> +
>  /*
>   * Hyper-V specific initialization and shutdown code that is
>   * common across all architectures.  Called from architecture
> @@ -332,6 +342,8 @@ int __init hv_common_init(void)
>  	if (hv_root_partition) {
>  		hyperv_pcpu_output_arg = alloc_percpu(void *);
>  		BUG_ON(!hyperv_pcpu_output_arg);
> +		hv_synic_eventring_tail = alloc_percpu(u8 *);
> +		BUG_ON(hv_synic_eventring_tail == NULL);
>  	}
>  
>  	hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
> @@ -356,6 +368,7 @@ int __init hv_common_init(void)
>  int hv_common_cpu_init(unsigned int cpu)
>  {
>  	void **inputarg, **outputarg;
> +	u8 **synic_eventring_tail;
>  	u64 msr_vp_index;
>  	gfp_t flags;
>  	int pgcount = hv_root_partition ? 2 : 1;
> @@ -371,6 +384,14 @@ int hv_common_cpu_init(unsigned int cpu)
>  	if (hv_root_partition) {
>  		outputarg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
>  		*outputarg = (char *)(*inputarg) + HV_HYP_PAGE_SIZE;
> +		synic_eventring_tail = (u8 **)this_cpu_ptr(hv_synic_eventring_tail);
> +		*synic_eventring_tail = kcalloc(HV_SYNIC_SINT_COUNT, sizeof(u8),
> +						flags);
> +
> +		if (unlikely(!*synic_eventring_tail)) {
> +			kfree(*inputarg);
> +			return -ENOMEM;
> +		}
>  	}
>  
>  	msr_vp_index = hv_get_register(HV_MSR_VP_INDEX);
> @@ -387,6 +408,7 @@ int hv_common_cpu_die(unsigned int cpu)
>  {
>  	unsigned long flags;
>  	void **inputarg, **outputarg;
> +	u8 **synic_eventring_tail;
>  	void *mem;
>  
>  	local_irq_save(flags);
> @@ -398,6 +420,9 @@ int hv_common_cpu_die(unsigned int cpu)
>  	if (hv_root_partition) {
>  		outputarg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
>  		*outputarg = NULL;
> +		synic_eventring_tail = (u8 **)this_cpu_ptr(hv_synic_eventring_tail);
> +		kfree(*synic_eventring_tail);
> +		*synic_eventring_tail = NULL;

The upstream code looks different now. See 9636be85cc.

>  	}
>  
>  	local_irq_restore(flags);
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 0c94d20b4d44..9118d678b27a 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -73,6 +73,8 @@ extern bool hv_nested;
>  extern void * __percpu *hyperv_pcpu_input_arg;
>  extern void * __percpu *hyperv_pcpu_output_arg;
>  
> +extern u8 __percpu **hv_synic_eventring_tail;
> +
>  extern u64 hv_do_hypercall(u64 control, void *inputaddr, void *outputaddr);
>  extern u64 hv_do_fast_hypercall8(u16 control, u64 input8);
>  extern bool hv_isolation_type_snp(void);
> -- 
> 2.25.1
>