linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus devices
@ 2022-10-13 18:29 Saurabh Sengar
  2022-10-14  7:34 ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Saurabh Sengar @ 2022-10-13 18:29 UTC (permalink / raw)
  To: ssengar, kys, haiyangz, sthemmin, wei.liu, decui, longli, gregkh,
	linux-hyperv, linux-kernel, mikelley

Hyper-V is adding some "specialty" synthetic devices. Instead of writing
new kernel-level VMBus drivers for these devices, the devices will be
presented to user space via this existing Hyper-V generic UIO driver, so
that a user space driver can handle the device. Since these new synthetic
devices are low speed devices, they don't support monitor bits and we must
use vmbus_setevent() to enable interrupts from the host.

Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
 drivers/uio/uio_hv_generic.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index c08a6cfd119f..8e5aa4a1247f 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -84,6 +84,9 @@ hv_uio_irqcontrol(struct uio_info *info, s32 irq_state)
 	dev->channel->inbound.ring_buffer->interrupt_mask = !irq_state;
 	virt_mb();
 
+	if (!dev->channel->offermsg.monitor_allocated && irq_state)
+		vmbus_setevent(dev->channel);
+
 	return 0;
 }
 
@@ -239,12 +242,6 @@ hv_uio_probe(struct hv_device *dev,
 	void *ring_buffer;
 	int ret;
 
-	/* Communicating with host has to be via shared memory not hypercall */
-	if (!channel->offermsg.monitor_allocated) {
-		dev_err(&dev->device, "vmbus channel requires hypercall\n");
-		return -ENOTSUPP;
-	}
-
 	pdata = devm_kzalloc(&dev->device, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return -ENOMEM;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus devices
  2022-10-13 18:29 [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus devices Saurabh Sengar
@ 2022-10-14  7:34 ` Greg KH
  2022-10-18  6:31   ` Long Li
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2022-10-14  7:34 UTC (permalink / raw)
  To: Saurabh Sengar
  Cc: ssengar, kys, haiyangz, sthemmin, wei.liu, decui, longli,
	linux-hyperv, linux-kernel, mikelley

On Thu, Oct 13, 2022 at 11:29:14AM -0700, Saurabh Sengar wrote:
> Hyper-V is adding some "specialty" synthetic devices.

What devices are those specifically?

> Instead of writing new kernel-level VMBus drivers for these devices,
> the devices will be presented to user space via this existing Hyper-V
> generic UIO driver, so that a user space driver can handle the device.
> Since these new synthetic devices are low speed devices, they don't
> support monitor bits and we must use vmbus_setevent() to enable
> interrupts from the host.

That is not what the UIO interface is for.  Please write real drivers so
that they tie into the specific user/kernel apis for those device types.

Without a specific list of what these devices are, I can not recommend
that anyone use the UIO api for them as that's probably not a good idea.

Also, if you do do this, you need to list where the source for that
userspace code is so that users can get it and have their distros
package it up for them.  I do not see that here at all.


> 
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
>  drivers/uio/uio_hv_generic.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index c08a6cfd119f..8e5aa4a1247f 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -84,6 +84,9 @@ hv_uio_irqcontrol(struct uio_info *info, s32 irq_state)
>  	dev->channel->inbound.ring_buffer->interrupt_mask = !irq_state;
>  	virt_mb();
>  
> +	if (!dev->channel->offermsg.monitor_allocated && irq_state)
> +		vmbus_setevent(dev->channel);
> +
>  	return 0;
>  }
>  
> @@ -239,12 +242,6 @@ hv_uio_probe(struct hv_device *dev,
>  	void *ring_buffer;
>  	int ret;
>  
> -	/* Communicating with host has to be via shared memory not hypercall */
> -	if (!channel->offermsg.monitor_allocated) {
> -		dev_err(&dev->device, "vmbus channel requires hypercall\n");

I do not understand, why is this check not made anymore here?  Why
constantly make the call above in the irq handler instead?  Isn't that
going to be massively slow?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus devices
  2022-10-14  7:34 ` Greg KH
@ 2022-10-18  6:31   ` Long Li
  2022-10-18  6:40     ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Long Li @ 2022-10-18  6:31 UTC (permalink / raw)
  To: Greg KH, Saurabh Sengar
  Cc: Saurabh Singh Sengar, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, wei.liu, Dexuan Cui, linux-hyperv,
	linux-kernel, Michael Kelley (LINUX)

> Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus
> devices
> 
> On Thu, Oct 13, 2022 at 11:29:14AM -0700, Saurabh Sengar wrote:
> > Hyper-V is adding some "specialty" synthetic devices.
> 
> What devices are those specifically?
> 
> > Instead of writing new kernel-level VMBus drivers for these devices,
> > the devices will be presented to user space via this existing Hyper-V
> > generic UIO driver, so that a user space driver can handle the device.
> > Since these new synthetic devices are low speed devices, they don't
> > support monitor bits and we must use vmbus_setevent() to enable
> > interrupts from the host.
> 
> That is not what the UIO interface is for.  Please write real drivers so that
> they tie into the specific user/kernel apis for those device types.
> 
> Without a specific list of what these devices are, I can not recommend that
> anyone use the UIO api for them as that's probably not a good idea.

There are some VMBUS drivers currently not implemented in Linux. Out of all
VMBUS drivers, two use "monitored bits": they are network and storage drivers.
All the rest VMBUS drivers use hypercall for host notification and signal for next
interrupt. One example of such driver is to collect process level crash information
for diagnostic purposes.

Also, we want to move some existing kernel mode VMBUS drivers to user-space,
such as hv_kvp and hv_filecopy. They don't really fit into an existing kernel API, and
they create their own devices under /dev and communicates with a user-mode
daemon to do most of the work. It's a better model that we can move those drivers
entirely into user-mode.

> 
> Also, if you do do this, you need to list where the source for that userspace
> code is so that users can get it and have their distros package it up for them.  I
> do not see that here at all.
> 
> 
> >
> > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> > ---
> >  drivers/uio/uio_hv_generic.c | 9 +++------
> >  1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/uio/uio_hv_generic.c
> > b/drivers/uio/uio_hv_generic.c index c08a6cfd119f..8e5aa4a1247f 100644
> > --- a/drivers/uio/uio_hv_generic.c
> > +++ b/drivers/uio/uio_hv_generic.c
> > @@ -84,6 +84,9 @@ hv_uio_irqcontrol(struct uio_info *info, s32 irq_state)
> >  	dev->channel->inbound.ring_buffer->interrupt_mask = !irq_state;
> >  	virt_mb();
> >
> > +	if (!dev->channel->offermsg.monitor_allocated && irq_state)
> > +		vmbus_setevent(dev->channel);
> > +
> >  	return 0;
> >  }
> >
> > @@ -239,12 +242,6 @@ hv_uio_probe(struct hv_device *dev,
> >  	void *ring_buffer;
> >  	int ret;
> >
> > -	/* Communicating with host has to be via shared memory not
> hypercall */
> > -	if (!channel->offermsg.monitor_allocated) {
> > -		dev_err(&dev->device, "vmbus channel requires
> hypercall\n");
> 
> I do not understand, why is this check not made anymore here?  Why
> constantly make the call above in the irq handler instead?  Isn't that going to
> be massively slow?

Some VMBUS devices exposed by the Hyper-V are not modeled as high speed, 
they use hypercall, not monitored bits. Because they don't fit into other kernel
API (as explained above), can we use UIO for those devices?

Thanks,
Long

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus devices
  2022-10-18  6:31   ` Long Li
@ 2022-10-18  6:40     ` Greg KH
  2022-10-18  6:57       ` Long Li
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2022-10-18  6:40 UTC (permalink / raw)
  To: Long Li
  Cc: Saurabh Sengar, Saurabh Singh Sengar, KY Srinivasan,
	Haiyang Zhang, Stephen Hemminger, wei.liu, Dexuan Cui,
	linux-hyperv, linux-kernel, Michael Kelley (LINUX)

On Tue, Oct 18, 2022 at 06:31:16AM +0000, Long Li wrote:
> > Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus
> > devices
> > 
> > On Thu, Oct 13, 2022 at 11:29:14AM -0700, Saurabh Sengar wrote:
> > > Hyper-V is adding some "specialty" synthetic devices.
> > 
> > What devices are those specifically?
> > 
> > > Instead of writing new kernel-level VMBus drivers for these devices,
> > > the devices will be presented to user space via this existing Hyper-V
> > > generic UIO driver, so that a user space driver can handle the device.
> > > Since these new synthetic devices are low speed devices, they don't
> > > support monitor bits and we must use vmbus_setevent() to enable
> > > interrupts from the host.
> > 
> > That is not what the UIO interface is for.  Please write real drivers so that
> > they tie into the specific user/kernel apis for those device types.
> > 
> > Without a specific list of what these devices are, I can not recommend that
> > anyone use the UIO api for them as that's probably not a good idea.
> 
> There are some VMBUS drivers currently not implemented in Linux. Out of all
> VMBUS drivers, two use "monitored bits": they are network and storage drivers.
> All the rest VMBUS drivers use hypercall for host notification and signal for next
> interrupt. One example of such driver is to collect process level crash information
> for diagnostic purposes.
> 
> Also, we want to move some existing kernel mode VMBUS drivers to user-space,
> such as hv_kvp and hv_filecopy. They don't really fit into an existing kernel API, and
> they create their own devices under /dev and communicates with a user-mode
> daemon to do most of the work. It's a better model that we can move those drivers
> entirely into user-mode.

How are you going to be able to remove drivers that export an existing
user/kernel api and not break current systems?

> > Also, if you do do this, you need to list where the source for that userspace
> > code is so that users can get it and have their distros package it up for them.  I
> > do not see that here at all.
> > 
> > 
> > >
> > > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> > > ---
> > >  drivers/uio/uio_hv_generic.c | 9 +++------
> > >  1 file changed, 3 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/uio/uio_hv_generic.c
> > > b/drivers/uio/uio_hv_generic.c index c08a6cfd119f..8e5aa4a1247f 100644
> > > --- a/drivers/uio/uio_hv_generic.c
> > > +++ b/drivers/uio/uio_hv_generic.c
> > > @@ -84,6 +84,9 @@ hv_uio_irqcontrol(struct uio_info *info, s32 irq_state)
> > >  	dev->channel->inbound.ring_buffer->interrupt_mask = !irq_state;
> > >  	virt_mb();
> > >
> > > +	if (!dev->channel->offermsg.monitor_allocated && irq_state)
> > > +		vmbus_setevent(dev->channel);
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -239,12 +242,6 @@ hv_uio_probe(struct hv_device *dev,
> > >  	void *ring_buffer;
> > >  	int ret;
> > >
> > > -	/* Communicating with host has to be via shared memory not
> > hypercall */
> > > -	if (!channel->offermsg.monitor_allocated) {
> > > -		dev_err(&dev->device, "vmbus channel requires
> > hypercall\n");
> > 
> > I do not understand, why is this check not made anymore here?  Why
> > constantly make the call above in the irq handler instead?  Isn't that going to
> > be massively slow?
> 
> Some VMBUS devices exposed by the Hyper-V are not modeled as high speed, 
> they use hypercall, not monitored bits. Because they don't fit into other kernel
> API (as explained above), can we use UIO for those devices?

UIO is for mmaped memory regions, like PCI devices, how is this a valid
Hyper-V api at all?

confused,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus devices
  2022-10-18  6:40     ` Greg KH
@ 2022-10-18  6:57       ` Long Li
  2022-10-18  7:04         ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Long Li @ 2022-10-18  6:57 UTC (permalink / raw)
  To: Greg KH
  Cc: Saurabh Sengar, Saurabh Singh Sengar, KY Srinivasan,
	Haiyang Zhang, Stephen Hemminger, wei.liu, Dexuan Cui,
	linux-hyperv, linux-kernel, Michael Kelley (LINUX)

> Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus
> devices
> 
> On Tue, Oct 18, 2022 at 06:31:16AM +0000, Long Li wrote:
> > > Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low speed
> > > VMBus devices
> > >
> > > On Thu, Oct 13, 2022 at 11:29:14AM -0700, Saurabh Sengar wrote:
> > > > Hyper-V is adding some "specialty" synthetic devices.
> > >
> > > What devices are those specifically?
> > >
> > > > Instead of writing new kernel-level VMBus drivers for these
> > > > devices, the devices will be presented to user space via this
> > > > existing Hyper-V generic UIO driver, so that a user space driver can
> handle the device.
> > > > Since these new synthetic devices are low speed devices, they
> > > > don't support monitor bits and we must use vmbus_setevent() to
> > > > enable interrupts from the host.
> > >
> > > That is not what the UIO interface is for.  Please write real
> > > drivers so that they tie into the specific user/kernel apis for those device
> types.
> > >
> > > Without a specific list of what these devices are, I can not
> > > recommend that anyone use the UIO api for them as that's probably not
> a good idea.
> >
> > There are some VMBUS drivers currently not implemented in Linux. Out
> > of all VMBUS drivers, two use "monitored bits": they are network and
> storage drivers.
> > All the rest VMBUS drivers use hypercall for host notification and
> > signal for next interrupt. One example of such driver is to collect
> > process level crash information for diagnostic purposes.
> >
> > Also, we want to move some existing kernel mode VMBUS drivers to
> > user-space, such as hv_kvp and hv_filecopy. They don't really fit into
> > an existing kernel API, and they create their own devices under /dev
> > and communicates with a user-mode daemon to do most of the work. It's
> > a better model that we can move those drivers entirely into user-mode.
> 
> How are you going to be able to remove drivers that export an existing
> user/kernel api and not break current systems?

It will be some configuration challenges, but it's doable. Newer Linux
VMs can be configured in a way to use the UIO interfaces. This doesn't break
any existing VMs because the old model will continue to work when UIO is not
used. Also, this doesn't require any Hyper-V changes.

> 
> > > Also, if you do do this, you need to list where the source for that
> > > userspace code is so that users can get it and have their distros
> > > package it up for them.  I do not see that here at all.
> > >
> > >
> > > >
> > > > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> > > > ---
> > > >  drivers/uio/uio_hv_generic.c | 9 +++------
> > > >  1 file changed, 3 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/uio/uio_hv_generic.c
> > > > b/drivers/uio/uio_hv_generic.c index c08a6cfd119f..8e5aa4a1247f
> > > > 100644
> > > > --- a/drivers/uio/uio_hv_generic.c
> > > > +++ b/drivers/uio/uio_hv_generic.c
> > > > @@ -84,6 +84,9 @@ hv_uio_irqcontrol(struct uio_info *info, s32
> irq_state)
> > > >  	dev->channel->inbound.ring_buffer->interrupt_mask = !irq_state;
> > > >  	virt_mb();
> > > >
> > > > +	if (!dev->channel->offermsg.monitor_allocated && irq_state)
> > > > +		vmbus_setevent(dev->channel);
> > > > +
> > > >  	return 0;
> > > >  }
> > > >
> > > > @@ -239,12 +242,6 @@ hv_uio_probe(struct hv_device *dev,
> > > >  	void *ring_buffer;
> > > >  	int ret;
> > > >
> > > > -	/* Communicating with host has to be via shared memory not
> > > hypercall */
> > > > -	if (!channel->offermsg.monitor_allocated) {
> > > > -		dev_err(&dev->device, "vmbus channel requires
> > > hypercall\n");
> > >
> > > I do not understand, why is this check not made anymore here?  Why
> > > constantly make the call above in the irq handler instead?  Isn't
> > > that going to be massively slow?
> >
> > Some VMBUS devices exposed by the Hyper-V are not modeled as high
> > speed, they use hypercall, not monitored bits. Because they don't fit
> > into other kernel API (as explained above), can we use UIO for those
> devices?
> 
> UIO is for mmaped memory regions, like PCI devices, how is this a valid
> Hyper-V api at all?

Currently uio_hv_generic is used to mmap VMBUS ring buffers to user-mode.
The primary use case is for DPDK. However, the same mmap model can be
used for all other VMBUS devices, as they all use the same ring buffers model.

Long

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus devices
  2022-10-18  6:57       ` Long Li
@ 2022-10-18  7:04         ` Greg KH
  2022-10-18  7:36           ` Long Li
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2022-10-18  7:04 UTC (permalink / raw)
  To: Long Li
  Cc: Saurabh Sengar, Saurabh Singh Sengar, KY Srinivasan,
	Haiyang Zhang, Stephen Hemminger, wei.liu, Dexuan Cui,
	linux-hyperv, linux-kernel, Michael Kelley (LINUX)

On Tue, Oct 18, 2022 at 06:57:33AM +0000, Long Li wrote:
> > Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus
> > devices
> > 
> > On Tue, Oct 18, 2022 at 06:31:16AM +0000, Long Li wrote:
> > > > Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low speed
> > > > VMBus devices
> > > >
> > > > On Thu, Oct 13, 2022 at 11:29:14AM -0700, Saurabh Sengar wrote:
> > > > > Hyper-V is adding some "specialty" synthetic devices.
> > > >
> > > > What devices are those specifically?
> > > >
> > > > > Instead of writing new kernel-level VMBus drivers for these
> > > > > devices, the devices will be presented to user space via this
> > > > > existing Hyper-V generic UIO driver, so that a user space driver can
> > handle the device.
> > > > > Since these new synthetic devices are low speed devices, they
> > > > > don't support monitor bits and we must use vmbus_setevent() to
> > > > > enable interrupts from the host.
> > > >
> > > > That is not what the UIO interface is for.  Please write real
> > > > drivers so that they tie into the specific user/kernel apis for those device
> > types.
> > > >
> > > > Without a specific list of what these devices are, I can not
> > > > recommend that anyone use the UIO api for them as that's probably not
> > a good idea.
> > >
> > > There are some VMBUS drivers currently not implemented in Linux. Out
> > > of all VMBUS drivers, two use "monitored bits": they are network and
> > storage drivers.
> > > All the rest VMBUS drivers use hypercall for host notification and
> > > signal for next interrupt. One example of such driver is to collect
> > > process level crash information for diagnostic purposes.
> > >
> > > Also, we want to move some existing kernel mode VMBUS drivers to
> > > user-space, such as hv_kvp and hv_filecopy. They don't really fit into
> > > an existing kernel API, and they create their own devices under /dev
> > > and communicates with a user-mode daemon to do most of the work. It's
> > > a better model that we can move those drivers entirely into user-mode.
> > 
> > How are you going to be able to remove drivers that export an existing
> > user/kernel api and not break current systems?
> 
> It will be some configuration challenges, but it's doable.

How exactly?

We can not break userspace when you upgrade a kernel version.

> Newer Linux
> VMs can be configured in a way to use the UIO interfaces. This doesn't break
> any existing VMs because the old model will continue to work when UIO is not
> used. Also, this doesn't require any Hyper-V changes.

Hyper-v changes are not the issue here :)

Again, you have to support the situation where a system upgrades to a
new kernel and all the same functionality must be there.  How will you
do that if you remove drivers from a newer kernel?

> > UIO is for mmaped memory regions, like PCI devices, how is this a valid
> > Hyper-V api at all?
> 
> Currently uio_hv_generic is used to mmap VMBUS ring buffers to user-mode.
> The primary use case is for DPDK. However, the same mmap model can be
> used for all other VMBUS devices, as they all use the same ring buffers model.

Ok, that feels like overkill...

You need to also post the source for these new userspace drivers
somewhere for us to review.  I don't see a link to them in the changelog
text :(

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus devices
  2022-10-18  7:04         ` Greg KH
@ 2022-10-18  7:36           ` Long Li
  2022-10-18  7:53             ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Long Li @ 2022-10-18  7:36 UTC (permalink / raw)
  To: Greg KH
  Cc: Saurabh Sengar, Saurabh Singh Sengar, KY Srinivasan,
	Haiyang Zhang, Stephen Hemminger, wei.liu, Dexuan Cui,
	linux-hyperv, linux-kernel, Michael Kelley (LINUX)

> Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus
> devices
> 
> On Tue, Oct 18, 2022 at 06:57:33AM +0000, Long Li wrote:
> > > Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low speed
> > > VMBus devices
> > >
> > > On Tue, Oct 18, 2022 at 06:31:16AM +0000, Long Li wrote:
> > > > > Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low
> > > > > speed VMBus devices
> > > > >
> > > > > On Thu, Oct 13, 2022 at 11:29:14AM -0700, Saurabh Sengar wrote:
> > > > > > Hyper-V is adding some "specialty" synthetic devices.
> > > > >
> > > > > What devices are those specifically?
> > > > >
> > > > > > Instead of writing new kernel-level VMBus drivers for these
> > > > > > devices, the devices will be presented to user space via this
> > > > > > existing Hyper-V generic UIO driver, so that a user space
> > > > > > driver can
> > > handle the device.
> > > > > > Since these new synthetic devices are low speed devices, they
> > > > > > don't support monitor bits and we must use vmbus_setevent() to
> > > > > > enable interrupts from the host.
> > > > >
> > > > > That is not what the UIO interface is for.  Please write real
> > > > > drivers so that they tie into the specific user/kernel apis for
> > > > > those device
> > > types.
> > > > >
> > > > > Without a specific list of what these devices are, I can not
> > > > > recommend that anyone use the UIO api for them as that's
> > > > > probably not
> > > a good idea.
> > > >
> > > > There are some VMBUS drivers currently not implemented in Linux.
> > > > Out of all VMBUS drivers, two use "monitored bits": they are
> > > > network and
> > > storage drivers.
> > > > All the rest VMBUS drivers use hypercall for host notification and
> > > > signal for next interrupt. One example of such driver is to
> > > > collect process level crash information for diagnostic purposes.
> > > >
> > > > Also, we want to move some existing kernel mode VMBUS drivers to
> > > > user-space, such as hv_kvp and hv_filecopy. They don't really fit
> > > > into an existing kernel API, and they create their own devices
> > > > under /dev and communicates with a user-mode daemon to do most of
> > > > the work. It's a better model that we can move those drivers entirely
> into user-mode.
> > >
> > > How are you going to be able to remove drivers that export an
> > > existing user/kernel api and not break current systems?
> >
> > It will be some configuration challenges, but it's doable.
> 
> How exactly?
> 
> We can not break userspace when you upgrade a kernel version.
> 
> > Newer Linux
> > VMs can be configured in a way to use the UIO interfaces. This doesn't
> > break any existing VMs because the old model will continue to work
> > when UIO is not used. Also, this doesn't require any Hyper-V changes.
> 
> Hyper-v changes are not the issue here :)
> 
> Again, you have to support the situation where a system upgrades to a new
> kernel and all the same functionality must be there.  How will you do that if
> you remove drivers from a newer kernel?

Currently there is a hv_kvp_daemon that interacts with the /dev/device 
created by the hv_kvp kernel driver, this is the only program interacts with
this device. This program is acting like a user-space driver, in a sense.

With the new kernel, if the KVP kernel mode is not present, this kvp daemon
will not start. All the KVP functionality is handled by the new UIO driver.

> 
> > > UIO is for mmaped memory regions, like PCI devices, how is this a
> > > valid Hyper-V api at all?
> >
> > Currently uio_hv_generic is used to mmap VMBUS ring buffers to user-
> mode.
> > The primary use case is for DPDK. However, the same mmap model can be
> > used for all other VMBUS devices, as they all use the same ring buffers
> model.
> 
> Ok, that feels like overkill...
> 
> You need to also post the source for these new userspace drivers
> somewhere for us to review.  I don't see a link to them in the changelog
> text :(

Yes, we will share the user space VMBUS drivers. What's the concern of creating
VMBus driver in user-mode? There are many examples of kernel drivers moving to
user-mode. For example, DPDK wants a network driver in user-mode,
SPDK wants a storage driver in user-mode, although they already have corresponding kernel
drivers.

Long

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus devices
  2022-10-18  7:36           ` Long Li
@ 2022-10-18  7:53             ` Greg KH
  0 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2022-10-18  7:53 UTC (permalink / raw)
  To: Long Li
  Cc: Saurabh Sengar, Saurabh Singh Sengar, KY Srinivasan,
	Haiyang Zhang, Stephen Hemminger, wei.liu, Dexuan Cui,
	linux-hyperv, linux-kernel, Michael Kelley (LINUX)

On Tue, Oct 18, 2022 at 07:36:54AM +0000, Long Li wrote:
> > Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus
> > devices
> > 
> > On Tue, Oct 18, 2022 at 06:57:33AM +0000, Long Li wrote:
> > > > Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low speed
> > > > VMBus devices
> > > >
> > > > On Tue, Oct 18, 2022 at 06:31:16AM +0000, Long Li wrote:
> > > > > > Subject: Re: [PATCH] uio_hv_generic: Enable interrupt for low
> > > > > > speed VMBus devices
> > > > > >
> > > > > > On Thu, Oct 13, 2022 at 11:29:14AM -0700, Saurabh Sengar wrote:
> > > > > > > Hyper-V is adding some "specialty" synthetic devices.
> > > > > >
> > > > > > What devices are those specifically?
> > > > > >
> > > > > > > Instead of writing new kernel-level VMBus drivers for these
> > > > > > > devices, the devices will be presented to user space via this
> > > > > > > existing Hyper-V generic UIO driver, so that a user space
> > > > > > > driver can
> > > > handle the device.
> > > > > > > Since these new synthetic devices are low speed devices, they
> > > > > > > don't support monitor bits and we must use vmbus_setevent() to
> > > > > > > enable interrupts from the host.
> > > > > >
> > > > > > That is not what the UIO interface is for.  Please write real
> > > > > > drivers so that they tie into the specific user/kernel apis for
> > > > > > those device
> > > > types.
> > > > > >
> > > > > > Without a specific list of what these devices are, I can not
> > > > > > recommend that anyone use the UIO api for them as that's
> > > > > > probably not
> > > > a good idea.
> > > > >
> > > > > There are some VMBUS drivers currently not implemented in Linux.
> > > > > Out of all VMBUS drivers, two use "monitored bits": they are
> > > > > network and
> > > > storage drivers.
> > > > > All the rest VMBUS drivers use hypercall for host notification and
> > > > > signal for next interrupt. One example of such driver is to
> > > > > collect process level crash information for diagnostic purposes.
> > > > >
> > > > > Also, we want to move some existing kernel mode VMBUS drivers to
> > > > > user-space, such as hv_kvp and hv_filecopy. They don't really fit
> > > > > into an existing kernel API, and they create their own devices
> > > > > under /dev and communicates with a user-mode daemon to do most of
> > > > > the work. It's a better model that we can move those drivers entirely
> > into user-mode.
> > > >
> > > > How are you going to be able to remove drivers that export an
> > > > existing user/kernel api and not break current systems?
> > >
> > > It will be some configuration challenges, but it's doable.
> > 
> > How exactly?
> > 
> > We can not break userspace when you upgrade a kernel version.
> > 
> > > Newer Linux
> > > VMs can be configured in a way to use the UIO interfaces. This doesn't
> > > break any existing VMs because the old model will continue to work
> > > when UIO is not used. Also, this doesn't require any Hyper-V changes.
> > 
> > Hyper-v changes are not the issue here :)
> > 
> > Again, you have to support the situation where a system upgrades to a new
> > kernel and all the same functionality must be there.  How will you do that if
> > you remove drivers from a newer kernel?
> 
> Currently there is a hv_kvp_daemon that interacts with the /dev/device 
> created by the hv_kvp kernel driver, this is the only program interacts with
> this device. This program is acting like a user-space driver, in a sense.
> 
> With the new kernel, if the KVP kernel mode is not present, this kvp daemon
> will not start. All the KVP functionality is handled by the new UIO driver.

And those changes have already been implemented and pushed out
somewhere?

> > > > UIO is for mmaped memory regions, like PCI devices, how is this a
> > > > valid Hyper-V api at all?
> > >
> > > Currently uio_hv_generic is used to mmap VMBUS ring buffers to user-
> > mode.
> > > The primary use case is for DPDK. However, the same mmap model can be
> > > used for all other VMBUS devices, as they all use the same ring buffers
> > model.
> > 
> > Ok, that feels like overkill...
> > 
> > You need to also post the source for these new userspace drivers
> > somewhere for us to review.  I don't see a link to them in the changelog
> > text :(
> 
> Yes, we will share the user space VMBUS drivers. What's the concern of creating
> VMBus driver in user-mode? There are many examples of kernel drivers moving to
> user-mode. For example, DPDK wants a network driver in user-mode,
> SPDK wants a storage driver in user-mode, although they already have corresponding kernel
> drivers.

When moving out of the kernel to userspace, you loose the common and
neutral user/kernel api and are now forced to rely on a userspace
program for access to that device.  no longer can you do something like
normal socket calls, but instead, you have to rely on a library for the
same functionality.

Same for block devices.  Do you really want to give up that common
interface to your block device and rely on a library instead of the
kernel?

If so, great, but don't take away the functionality from all users, as
you will break their systems.

thanks

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-10-18  7:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-13 18:29 [PATCH] uio_hv_generic: Enable interrupt for low speed VMBus devices Saurabh Sengar
2022-10-14  7:34 ` Greg KH
2022-10-18  6:31   ` Long Li
2022-10-18  6:40     ` Greg KH
2022-10-18  6:57       ` Long Li
2022-10-18  7:04         ` Greg KH
2022-10-18  7:36           ` Long Li
2022-10-18  7:53             ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).