[v4,0/3] UIO driver for low speed Hyper-V devices

Message ID 1691132996-11706-1-git-send-email-ssengar@linux.microsoft.com
Headers
Series UIO driver for low speed Hyper-V devices |

Message

Saurabh Singh Sengar Aug. 4, 2023, 7:09 a.m. UTC
  Hyper-V is adding multiple low speed "speciality" synthetic devices.
Instead of writing a new kernel-level VMBus driver for each device,
make the devices accessible to user space through a UIO-based
hv_vmbus_client driver. Each device can then be supported by a user
space driver. This approach optimizes the development process and
provides flexibility to user space applications to control the key
interactions with the VMBus ring buffer.

The new synthetic devices are low speed devices that don't support
VMBus monitor bits, and so they must use vmbus_setevent() to notify
the host of ring buffer updates. The new driver provides this
functionality along with a configurable ring buffer size.

Moreover, this series of patches incorporates an update to the fcopy
application, enabling it to seamlessly utilize the new interface. The
older fcopy driver and application will be phased out gradually.
Development of other similar userspace drivers is still underway.

Moreover, this patch series adds a new implementation of the fcopy
application that uses the new UIO driver. The older fcopy driver and
application will be phased out gradually. Development of other similar
userspace drivers is still underway.

[V4]
- Modify comment to remove RingDataStartOffset mention
- Change downward ALIGN macro to upward ALIGN
- Add error check for setting ring_size value in sysfs entry
- Add error handling in fcopy_get_instance_id for instance id not found case

[V3]
- Removed ringbuffer sysfs entry and used uio framework for mmap
- Remove ".id_table = NULL"
- kasprintf -> devm_kasprintf
- Change global variable ring_size to per device
- More checks on value which can be set for ring_size
- Remove driverctl, and used echo command instead for driver documentation
- Remove unnecessary one time use macros
- Change kernel version and date for sysfs documentation
- Update documentation.
- Made ring buffer data offset depend on page size
- remove rte_smp_rwmb macro and reused rte_compiler_barrier instead
- Added legal counsel sign-off
- simplify mmap
- Removed "Link:" tag 
- Improve cover letter and commit messages
- Improve debug prints
- Instead of hardcoded instance id, query from class id sysfs
- Set the ring_size value from application
- new application compilation dependent on x86
- Not removing the old driver and application for backward compatibility

[V2]
- Update driver info in Documentation/driver-api/uio-howto.rst
- Update ring_size sysfs info in Documentation/ABI/stable/sysfs-bus-vmbus
- Remove DRIVER_VERSION
- Remove refcnt
- scnprintf -> sysfs_emit
- sysfs_create_file -> ATTRIBUTE_GROUPS + ".driver.groups";
- sysfs_create_bin_file -> device_create_bin_file
- dev_notice -> dev_err
- remove MODULE_VERSION
- simpler sysfs path, less parsing

Saurabh Sengar (3):
  uio: Add hv_vmbus_client driver
  tools: hv: Add vmbus_bufring
  tools: hv: Add new fcopy application based on uio driver

 Documentation/ABI/stable/sysfs-bus-vmbus |  10 +
 Documentation/driver-api/uio-howto.rst   |  54 +++
 drivers/uio/Kconfig                      |  12 +
 drivers/uio/Makefile                     |   1 +
 drivers/uio/uio_hv_vmbus_client.c        | 218 +++++++++
 tools/hv/Build                           |   2 +
 tools/hv/Makefile                        |  21 +-
 tools/hv/hv_fcopy_uio_daemon.c           | 587 +++++++++++++++++++++++
 tools/hv/vmbus_bufring.c                 | 297 ++++++++++++
 tools/hv/vmbus_bufring.h                 | 154 ++++++
 10 files changed, 1355 insertions(+), 1 deletion(-)
 create mode 100644 drivers/uio/uio_hv_vmbus_client.c
 create mode 100644 tools/hv/hv_fcopy_uio_daemon.c
 create mode 100644 tools/hv/vmbus_bufring.c
 create mode 100644 tools/hv/vmbus_bufring.h
  

Comments

Greg KH Aug. 12, 2023, 11:14 a.m. UTC | #1
On Fri, Aug 04, 2023 at 12:09:53AM -0700, Saurabh Sengar wrote:
> Hyper-V is adding multiple low speed "speciality" synthetic devices.
> Instead of writing a new kernel-level VMBus driver for each device,
> make the devices accessible to user space through a UIO-based
> hv_vmbus_client driver. Each device can then be supported by a user
> space driver. This approach optimizes the development process and
> provides flexibility to user space applications to control the key
> interactions with the VMBus ring buffer.

Why is it faster to write userspace drivers here?  Where are those new
drivers, and why can't they be proper kernel drivers?  Are all hyper-v
drivers going to move to userspace now?

> The new synthetic devices are low speed devices that don't support
> VMBus monitor bits, and so they must use vmbus_setevent() to notify
> the host of ring buffer updates. The new driver provides this
> functionality along with a configurable ring buffer size.
> 
> Moreover, this series of patches incorporates an update to the fcopy
> application, enabling it to seamlessly utilize the new interface. The
> older fcopy driver and application will be phased out gradually.
> Development of other similar userspace drivers is still underway.
> 
> Moreover, this patch series adds a new implementation of the fcopy
> application that uses the new UIO driver. The older fcopy driver and
> application will be phased out gradually. Development of other similar
> userspace drivers is still underway.

You are adding a new user api with the "ring buffer" size api, which is
odd for normal UIO drivers as that's not something that UIO was designed
for.

Why not just make you own generic type uiofs type kernel api if you
really want to do all of this type of thing in userspace instead of in
the kernel?

And finally, if this is going to replace the fcopy driver, then it
should be part of this series as well, removing it to show that you
really mean it when you say it will be deleted, otherwise we all know
that will never happen.

thanks,

greg k-h
  
Saurabh Singh Sengar Sept. 26, 2023, 12:41 p.m. UTC | #2
On Wed, Sep 06, 2023 at 05:23:07AM -0700, Saurabh Singh Sengar wrote:
> On Tue, Aug 22, 2023 at 01:48:03PM +0200, Greg KH wrote:
> > On Mon, Aug 21, 2023 at 07:36:18AM +0000, Saurabh Singh Sengar wrote:
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: Greg KH <gregkh@linuxfoundation.org>
> > > > Sent: Saturday, August 12, 2023 4:45 PM
> > > > To: Saurabh Sengar <ssengar@linux.microsoft.com>
> > > > Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> > > > <haiyangz@microsoft.com>; wei.liu@kernel.org; Dexuan Cui
> > > > <decui@microsoft.com>; Michael Kelley (LINUX) <mikelley@microsoft.com>;
> > > > corbet@lwn.net; linux-kernel@vger.kernel.org; linux-hyperv@vger.kernel.org;
> > > > linux-doc@vger.kernel.org
> > > > Subject: [EXTERNAL] Re: [PATCH v4 0/3] UIO driver for low speed Hyper-V
> > > > devices
> > > > 
> > > > On Fri, Aug 04, 2023 at 12:09:53AM -0700, Saurabh Sengar wrote:
> > > > > Hyper-V is adding multiple low speed "speciality" synthetic devices.
> > > > > Instead of writing a new kernel-level VMBus driver for each device,
> > > > > make the devices accessible to user space through a UIO-based
> > > > > hv_vmbus_client driver. Each device can then be supported by a user
> > > > > space driver. This approach optimizes the development process and
> > > > > provides flexibility to user space applications to control the key
> > > > > interactions with the VMBus ring buffer.
> > > > 
> > > > Why is it faster to write userspace drivers here?  Where are those new drivers,
> > > > and why can't they be proper kernel drivers?  Are all hyper-v drivers going to
> > > > move to userspace now?
> > > 
> > > Hi Greg,
> > > 
> > > You are correct; it isn't faster. However, the developers working on these userspace
> > > drivers can concentrate entirely on the business logic of these devices. The more
> > > intricate aspects of the kernel, such as interrupt management and host communication,
> > > can be encapsulated within the uio driver.
> > 
> > Yes, kernel drivers are hard, we all know that.
> > 
> > But if you do it right, it doesn't have to be, saying "it's too hard for
> > our programmers to write good code for our platform" isn't exactly a
> > good endorcement of either your programmers, or your platform :)
> > 
> > > The quantity of Hyper-V devices is substantial, and their numbers are consistently
> > > increasing. Presently, all of these drivers are in a development/planning phase and
> > > rely significantly on the acceptance of this UIO driver as a prerequisite.
> > 
> > Don't make my acceptance of something that you haven't submitted before
> > a business decision that I need to make, that's disenginous.
> > 
> > > Not all hyper-v drivers will move to userspace, but many a new slow Hyperv-V
> > > devices will use this framework and will avoid introducing a new kernel driver. We
> > > will also plan to remove some of the existing drivers like kvp/vss.
> > 
> > Define "slow" please.
> 
> In the Hyper-V environment, most devices, with the exception of network and storage,
> typically do not require extensive data read/write exchanges with the host. Such
> devices are considered to be 'slow' devices.
> 
> > 
> > > > > The new synthetic devices are low speed devices that don't support
> > > > > VMBus monitor bits, and so they must use vmbus_setevent() to notify
> > > > > the host of ring buffer updates. The new driver provides this
> > > > > functionality along with a configurable ring buffer size.
> > > > >
> > > > > Moreover, this series of patches incorporates an update to the fcopy
> > > > > application, enabling it to seamlessly utilize the new interface. The
> > > > > older fcopy driver and application will be phased out gradually.
> > > > > Development of other similar userspace drivers is still underway.
> > > > >
> > > > > Moreover, this patch series adds a new implementation of the fcopy
> > > > > application that uses the new UIO driver. The older fcopy driver and
> > > > > application will be phased out gradually. Development of other similar
> > > > > userspace drivers is still underway.
> > > > 
> > > > You are adding a new user api with the "ring buffer" size api, which is odd for
> > > > normal UIO drivers as that's not something that UIO was designed for.
> > > > 
> > > > Why not just make you own generic type uiofs type kernel api if you really
> > > > want to do all of this type of thing in userspace instead of in the kernel?
> > > 
> > > Could you please elaborate more on this suggestion. I couldn't understand it
> > > completely.
> > 
> > Why is uio the requirement here?  Why not make your own framework to
> > write hv drivers in userspace that fits in better with the overall goal?
> > Call it "hvfs" or something like that, much like we have usbfs for
> > writing usb drivers in userspace.
> > 
> > Bolting on HV drivers to UIO seems very odd as that is not what this
> > framework is supposed to be providing at all.  UIO was to enable "pass
> > through" memory-mapped drivers that only wanted an interrupt and access
> > to raw memory locations in the hardware.
> > 
> > Now you are adding ring buffer managment and all other sorts of things
> > just for your platform.  So make it a real subsystem tuned exactly for
> > what you need and NOT try to force it into the UIO interface (which
> > should know nothing about ring buffers...)
> 
> Thank you for elaborating the details. I will drop the plan to introduce a
> new UIO driver for this effort. However, I would like to know your thoughts
> on enhancing existing 'uio_hv_generic' driver to achieve the same.  We
> already have 'uio_hv_generic' driver in linux kernel, which is used for
> developing userspace drivers for 'fast Hyper-V devices'.
> 
> Since these newly introduced synthetic devices operate at a lower speed,
> they do not have the capability to support monitor bits. Instead, we must
> utilize the 'vmbus_setevent()' method to enable interrupts from the host.
> Earlier we made an attempt to support slow devices by uio_hv_generic :
> https://lore.kernel.org/lkml/1665685754-13971-1-git-send-email-ssengar@linux.microsoft.com/.
> At that time, the absence of userspace code (fcopy) hindered progress
> in this direction.
> 
> Acknowledging your valid concerns about introducing a new UIO driver for
> Hyper-V, I propose exploring the potential to enhance the existing
> 'uio_hv_generic' driver to accommodate slower devices effectively. My
> commitment to this endeavour includes ensuring the seamless operation of
> the existing 'fcopy' functionality with the modified 'uio_hv_generic'
> driver. Additionally, I will undertake the task of removing the current
> 'fcopy' kernel driver and userspace daemon as part of this effort.
> 
> Please let me know your thoughts. I look forward to your feedback and
> the opportunity to discuss this proposal further. 

Greg,

May I know if enhancing uio_hv_generic.c to support 'slow devices' is
an accptable approach ? I'm willing to undertake this task and propose
the necessary modifications.

- Saurabh

> 
> - Saurabh
  
Saurabh Singh Sengar Oct. 6, 2023, 4:28 a.m. UTC | #3
On Tue, Sep 26, 2023 at 05:41:26AM -0700, Saurabh Singh Sengar wrote:
> On Wed, Sep 06, 2023 at 05:23:07AM -0700, Saurabh Singh Sengar wrote:
> > On Tue, Aug 22, 2023 at 01:48:03PM +0200, Greg KH wrote:
> > > On Mon, Aug 21, 2023 at 07:36:18AM +0000, Saurabh Singh Sengar wrote:
> > > > 
> > > > 
> > > > > -----Original Message-----
> > > > > From: Greg KH <gregkh@linuxfoundation.org>
> > > > > Sent: Saturday, August 12, 2023 4:45 PM
> > > > > To: Saurabh Sengar <ssengar@linux.microsoft.com>
> > > > > Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> > > > > <haiyangz@microsoft.com>; wei.liu@kernel.org; Dexuan Cui
> > > > > <decui@microsoft.com>; Michael Kelley (LINUX) <mikelley@microsoft.com>;
> > > > > corbet@lwn.net; linux-kernel@vger.kernel.org; linux-hyperv@vger.kernel.org;
> > > > > linux-doc@vger.kernel.org
> > > > > Subject: [EXTERNAL] Re: [PATCH v4 0/3] UIO driver for low speed Hyper-V
> > > > > devices
> > > > > 
> > > > > On Fri, Aug 04, 2023 at 12:09:53AM -0700, Saurabh Sengar wrote:
> > > > > > Hyper-V is adding multiple low speed "speciality" synthetic devices.
> > > > > > Instead of writing a new kernel-level VMBus driver for each device,
> > > > > > make the devices accessible to user space through a UIO-based
> > > > > > hv_vmbus_client driver. Each device can then be supported by a user
> > > > > > space driver. This approach optimizes the development process and
> > > > > > provides flexibility to user space applications to control the key
> > > > > > interactions with the VMBus ring buffer.
> > > > > 
> > > > > Why is it faster to write userspace drivers here?  Where are those new drivers,
> > > > > and why can't they be proper kernel drivers?  Are all hyper-v drivers going to
> > > > > move to userspace now?
> > > > 
> > > > Hi Greg,
> > > > 
> > > > You are correct; it isn't faster. However, the developers working on these userspace
> > > > drivers can concentrate entirely on the business logic of these devices. The more
> > > > intricate aspects of the kernel, such as interrupt management and host communication,
> > > > can be encapsulated within the uio driver.
> > > 
> > > Yes, kernel drivers are hard, we all know that.
> > > 
> > > But if you do it right, it doesn't have to be, saying "it's too hard for
> > > our programmers to write good code for our platform" isn't exactly a
> > > good endorcement of either your programmers, or your platform :)
> > > 
> > > > The quantity of Hyper-V devices is substantial, and their numbers are consistently
> > > > increasing. Presently, all of these drivers are in a development/planning phase and
> > > > rely significantly on the acceptance of this UIO driver as a prerequisite.
> > > 
> > > Don't make my acceptance of something that you haven't submitted before
> > > a business decision that I need to make, that's disenginous.
> > > 
> > > > Not all hyper-v drivers will move to userspace, but many a new slow Hyperv-V
> > > > devices will use this framework and will avoid introducing a new kernel driver. We
> > > > will also plan to remove some of the existing drivers like kvp/vss.
> > > 
> > > Define "slow" please.
> > 
> > In the Hyper-V environment, most devices, with the exception of network and storage,
> > typically do not require extensive data read/write exchanges with the host. Such
> > devices are considered to be 'slow' devices.
> > 
> > > 
> > > > > > The new synthetic devices are low speed devices that don't support
> > > > > > VMBus monitor bits, and so they must use vmbus_setevent() to notify
> > > > > > the host of ring buffer updates. The new driver provides this
> > > > > > functionality along with a configurable ring buffer size.
> > > > > >
> > > > > > Moreover, this series of patches incorporates an update to the fcopy
> > > > > > application, enabling it to seamlessly utilize the new interface. The
> > > > > > older fcopy driver and application will be phased out gradually.
> > > > > > Development of other similar userspace drivers is still underway.
> > > > > >
> > > > > > Moreover, this patch series adds a new implementation of the fcopy
> > > > > > application that uses the new UIO driver. The older fcopy driver and
> > > > > > application will be phased out gradually. Development of other similar
> > > > > > userspace drivers is still underway.
> > > > > 
> > > > > You are adding a new user api with the "ring buffer" size api, which is odd for
> > > > > normal UIO drivers as that's not something that UIO was designed for.
> > > > > 
> > > > > Why not just make you own generic type uiofs type kernel api if you really
> > > > > want to do all of this type of thing in userspace instead of in the kernel?
> > > > 
> > > > Could you please elaborate more on this suggestion. I couldn't understand it
> > > > completely.
> > > 
> > > Why is uio the requirement here?  Why not make your own framework to
> > > write hv drivers in userspace that fits in better with the overall goal?
> > > Call it "hvfs" or something like that, much like we have usbfs for
> > > writing usb drivers in userspace.
> > > 
> > > Bolting on HV drivers to UIO seems very odd as that is not what this
> > > framework is supposed to be providing at all.  UIO was to enable "pass
> > > through" memory-mapped drivers that only wanted an interrupt and access
> > > to raw memory locations in the hardware.
> > > 
> > > Now you are adding ring buffer managment and all other sorts of things
> > > just for your platform.  So make it a real subsystem tuned exactly for
> > > what you need and NOT try to force it into the UIO interface (which
> > > should know nothing about ring buffers...)
> > 
> > Thank you for elaborating the details. I will drop the plan to introduce a
> > new UIO driver for this effort. However, I would like to know your thoughts
> > on enhancing existing 'uio_hv_generic' driver to achieve the same.  We
> > already have 'uio_hv_generic' driver in linux kernel, which is used for
> > developing userspace drivers for 'fast Hyper-V devices'.
> > 
> > Since these newly introduced synthetic devices operate at a lower speed,
> > they do not have the capability to support monitor bits. Instead, we must
> > utilize the 'vmbus_setevent()' method to enable interrupts from the host.
> > Earlier we made an attempt to support slow devices by uio_hv_generic :
> > https://lore.kernel.org/lkml/1665685754-13971-1-git-send-email-ssengar@linux.microsoft.com/.
> > At that time, the absence of userspace code (fcopy) hindered progress
> > in this direction.
> > 
> > Acknowledging your valid concerns about introducing a new UIO driver for
> > Hyper-V, I propose exploring the potential to enhance the existing
> > 'uio_hv_generic' driver to accommodate slower devices effectively. My
> > commitment to this endeavour includes ensuring the seamless operation of
> > the existing 'fcopy' functionality with the modified 'uio_hv_generic'
> > driver. Additionally, I will undertake the task of removing the current
> > 'fcopy' kernel driver and userspace daemon as part of this effort.
> > 
> > Please let me know your thoughts. I look forward to your feedback and
> > the opportunity to discuss this proposal further. 
> 
> Greg,
> 
> May I know if enhancing uio_hv_generic.c to support 'slow devices' is
> an accptable approach ? I'm willing to undertake this task and propose
> the necessary modifications.
> 
> - Saurabh

ping

> 
> > 
> > - Saurabh