Skip to contentSkip to conversation

uio_hv_generic: Set event for all channels on the device

PATCH In review [email protected] 5 participants 6 messages Mar 10, 2025 – Mar 25, 2025

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 uio_hv_generic: Set event for all channels on the device

From: Long Li <[email protected]>

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.

---
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(-)
drivers/uio/uio_hv_generic.c +26 -6
index 3976360d0096..45be2f8baade 100644
@@ -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);
 }
 
Signature · 1 line
2.34.1
Reviewed-by on v3 1/1
#1 lore raw

From: [email protected] <[email protected]> Sent: Monday, March 10, 2025 3:12 PM

Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 75 lines

---
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(-)
drivers/uio/uio_hv_generic.c +26 -7
index 3976360d0096..45be2f8baade 100644
@@ -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 on v3 1/1
#2 lore raw
In reply to [email protected]
Quoted 90 lines · On Mon, Mar 10, 2025 at 03:12:01PM -0700, [email protected] wrote:

From: Long Li <[email protected]>

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.

---
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(-)
drivers/uio/uio_hv_generic.c +26 -6
index 3976360d0096..45be2f8baade 100644
@@ -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);
 }
 
Signature · 1 line
2.34.1

- Saurabh

maintainer
#3 lore raw
In reply to [email protected]
Quoted 9 lines · On Mon, Mar 10, 2025 at 03:12:01PM -0700, [email protected] wrote:

From: Long Li <[email protected]>

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.

Quoted 6 lines

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.

Greg, are you going to take this patch?

I can take it if you want.

Thanks,
Wei.

Quoted 75 lines

---
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(-)
drivers/uio/uio_hv_generic.c +26 -6
index 3976360d0096..45be2f8baade 100644
@@ -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);
 }
 
Signature · 1 line
2.34.1
#4 lore raw
Quoted 15 lines · Wei Liu
Quoted 14 lines · On Mon, Mar 10, 2025 at 03:12:01PM -0700, [email protected] wrote:

From: Long Li <[email protected]>

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.

Quoted 5 lines
Quoted 1 line

Greg, are you going to take this patch?

I can take it if you want.

It's the merge window right now, neither of us should be taking it. Let me look into it after -rc1 is out.

thanks,

greg k-h

maintainer
#5 lore raw
Quoted 16 lines · Greg Kroah-Hartman
Quoted 15 lines · Wei Liu
Quoted 14 lines · On Mon, Mar 10, 2025 at 03:12:01PM -0700, [email protected] wrote:

From: Long Li <[email protected]>

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.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

Greg, are you going to take this patch?

I can take it if you want.

It's the merge window right now, neither of us should be taking it. Let me look into it after -rc1 is out.

Understood. Thank you for your response.

Quoted 4 lines

thanks,

greg k-h