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] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y0kRIcXG+wNbcGx0@kroah.com>
Date:   Fri, 14 Oct 2022 09:34:57 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Saurabh Sengar <ssengar@...ux.microsoft.com>
Cc:     ssengar@...rosoft.com, kys@...rosoft.com, haiyangz@...rosoft.com,
        sthemmin@...rosoft.com, wei.liu@...nel.org, decui@...rosoft.com,
        longli@...rosoft.com, linux-hyperv@...r.kernel.org,
        linux-kernel@...r.kernel.org, mikelley@...rosoft.com
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.

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@...ux.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ