linux-kernel - Re: [Patch v3] uio_hv_generic: Set event for all channels on the device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250311041217.GA7165@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Date: Mon, 10 Mar 2025 21:12:17 -0700
From: Saurabh Singh Sengar <ssengar@...ux.microsoft.com>
To: longli@...uxonhyperv.com
Cc: "K. Y. Srinivasan" <kys@...rosoft.com>,
	Haiyang Zhang <haiyangz@...rosoft.com>,
	Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
	Long Li <longli@...rosoft.com>
Subject: Re: [Patch v3] uio_hv_generic: Set event for all channels on the
 device

On Mon, Mar 10, 2025 at 03:12:01PM -0700, longli@...uxonhyperv.com wrote:
> From: Long Li <longli@...rosoft.com>
> 
> Hyper-V may offer a non latency sensitive device with subchannels without
> monitor bit enabled. The decision is entirely on the Hyper-V host not
> configurable within guest.
> 
> When a device has subchannels, also signal events for the subchannel
> if its monitor bit is disabled.
> 
> This patch also removes the memory barrier when monitor bit is enabled
> as it is not necessary. The memory barrier is only needed between
> setting up interrupt mask and calling vmbus_set_event() when monitor
> bit is disabled.
> 
> Signed-off-by: Long Li <longli@...rosoft.com>
> ---
> Change log
> v2: Use vmbus_set_event() to avoid additional check on monitored bit
>     Lock vmbus_connection.channel_mutex when going through subchannels
> v3: Add details in commit messsage on the memory barrier.
> 
>  drivers/uio/uio_hv_generic.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index 3976360d0096..45be2f8baade 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -65,6 +65,16 @@ struct hv_uio_private_data {
>  	char	send_name[32];
>  };
>  
> +static void set_event(struct vmbus_channel *channel, s32 irq_state)
> +{
> +	channel->inbound.ring_buffer->interrupt_mask = !irq_state;
> +	if (!channel->offermsg.monitor_allocated && irq_state) {
> +		/* MB is needed for host to see the interrupt mask first */
> +		virt_mb();
> +		vmbus_set_event(channel);
> +	}
> +}
> +
>  /*
>   * This is the irqcontrol callback to be registered to uio_info.
>   * It can be used to disable/enable interrupt from user space processes.
> @@ -79,12 +89,15 @@ hv_uio_irqcontrol(struct uio_info *info, s32 irq_state)
>  {
>  	struct hv_uio_private_data *pdata = info->priv;
>  	struct hv_device *dev = pdata->device;
> +	struct vmbus_channel *primary, *sc;
>  
> -	dev->channel->inbound.ring_buffer->interrupt_mask = !irq_state;
> -	virt_mb();
> +	primary = dev->channel;
> +	set_event(primary, irq_state);
>  
> -	if (!dev->channel->offermsg.monitor_allocated && irq_state)
> -		vmbus_setevent(dev->channel);
> +	mutex_lock(&vmbus_connection.channel_mutex);
> +	list_for_each_entry(sc, &primary->sc_list, sc_list)
> +		set_event(sc, irq_state);
> +	mutex_unlock(&vmbus_connection.channel_mutex);
>  
>  	return 0;
>  }
> @@ -95,12 +108,19 @@ hv_uio_irqcontrol(struct uio_info *info, s32 irq_state)
>  static void hv_uio_channel_cb(void *context)
>  {
>  	struct vmbus_channel *chan = context;
> -	struct hv_device *hv_dev = chan->device_obj;
> -	struct hv_uio_private_data *pdata = hv_get_drvdata(hv_dev);
> +	struct hv_device *hv_dev;
> +	struct hv_uio_private_data *pdata;
>  
>  	chan->inbound.ring_buffer->interrupt_mask = 1;
>  	virt_mb();
>  
> +	/*
> +	 * The callback may come from a subchannel, in which case look
> +	 * for the hv device in the primary channel
> +	 */
> +	hv_dev = chan->primary_channel ?
> +		 chan->primary_channel->device_obj : chan->device_obj;
> +	pdata = hv_get_drvdata(hv_dev);
>  	uio_event_notify(&pdata->info);
>  }
>  
> -- 
> 2.34.1
> 

Reviewed-by: Saurabh Sengar <ssengar@...ux.microsoft.com>

- Saurabh