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@xxxxxxxxxxxxxxxxxxx>
> > > ---
> > > 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
