All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] uio/uio_pci_generic: don't fail probe if pdev->irq == NULL
@ 2017-04-30 19:29 Jim Harris
  2017-05-01  1:17 ` Michael S. Tsirkin
  2017-05-02  9:20 ` kbuild test robot
  0 siblings, 2 replies; 4+ messages in thread
From: Jim Harris @ 2017-04-30 19:29 UTC (permalink / raw)
  To: mst, kvm; +Cc: gregkh, linux, keith.busch, jonathan.derrick, Jim Harris

Some userspace drivers and frameworks only poll and do not
require interrupts to be available and enabled on the
PCI device.  So remove the requirement that an IRQ is
assigned.  If an IRQ is not assigned and a userspace
driver tries to read()/write(), the generic uio
framework will just return -EIO.

This allows binding uio_pci_generic to devices which
cannot get an IRQ assigned, such as an NVMe controller
behind Intel Volume Management Device (VMD), since VMD
does not support INTx interrupts.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
---
 drivers/uio/uio_pci_generic.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/uio/uio_pci_generic.c b/drivers/uio/uio_pci_generic.c
index d0b508b68f3c..81c59b4f8552 100644
--- a/drivers/uio/uio_pci_generic.c
+++ b/drivers/uio/uio_pci_generic.c
@@ -66,14 +66,7 @@ static int probe(struct pci_dev *pdev,
 		return err;
 	}
 
-	if (!pdev->irq) {
-		dev_warn(&pdev->dev, "No IRQ assigned to device: "
-			 "no support for interrupts?\n");
-		pci_disable_device(pdev);
-		return -ENODEV;
-	}
-
-	if (!pci_intx_mask_supported(pdev)) {
+	if (pci->irq && !pci_intx_mask_supported(pdev)) {
 		err = -ENODEV;
 		goto err_verify;
 	}
@@ -86,10 +79,15 @@ static int probe(struct pci_dev *pdev,
 
 	gdev->info.name = "uio_pci_generic";
 	gdev->info.version = DRIVER_VERSION;
-	gdev->info.irq = pdev->irq;
-	gdev->info.irq_flags = IRQF_SHARED;
-	gdev->info.handler = irqhandler;
 	gdev->pdev = pdev;
+	if (pdev->irq) {
+		gdev->info.irq = pdev->irq;
+		gdev->info.irq_flags = IRQF_SHARED;
+		gdev->info.handler = irqhandler;
+	} else {
+		dev_warn(&pdev->dev, "No IRQ assigned to device: "
+			 "no support for interrupts?\n");
+	}
 
 	err = uio_register_device(&pdev->dev, &gdev->info);
 	if (err)
-- 
2.12.2

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

* Re: [PATCH] uio/uio_pci_generic: don't fail probe if pdev->irq == NULL
  2017-04-30 19:29 [PATCH] uio/uio_pci_generic: don't fail probe if pdev->irq == NULL Jim Harris
@ 2017-05-01  1:17 ` Michael S. Tsirkin
  2017-05-01 21:21   ` Harris, James R
  2017-05-02  9:20 ` kbuild test robot
  1 sibling, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2017-05-01  1:17 UTC (permalink / raw)
  To: Jim Harris; +Cc: kvm, gregkh, linux, keith.busch, jonathan.derrick

On Sun, Apr 30, 2017 at 12:29:55PM -0700, Jim Harris wrote:
> Some userspace drivers and frameworks only poll and do not
> require interrupts to be available and enabled on the
> PCI device.  So remove the requirement that an IRQ is
> assigned.  If an IRQ is not assigned and a userspace
> driver tries to read()/write(), the generic uio
> framework will just return -EIO.
> 
> This allows binding uio_pci_generic to devices which
> cannot get an IRQ assigned, such as an NVMe controller
> behind Intel Volume Management Device (VMD), since VMD
> does not support INTx interrupts.
> 
> Signed-off-by: Jim Harris <james.r.harris@intel.com>

Without interrupts, why do you want uio? All it does is
forward interrupts to userspace.

> ---
>  drivers/uio/uio_pci_generic.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/uio/uio_pci_generic.c b/drivers/uio/uio_pci_generic.c
> index d0b508b68f3c..81c59b4f8552 100644
> --- a/drivers/uio/uio_pci_generic.c
> +++ b/drivers/uio/uio_pci_generic.c
> @@ -66,14 +66,7 @@ static int probe(struct pci_dev *pdev,
>  		return err;
>  	}
>  
> -	if (!pdev->irq) {
> -		dev_warn(&pdev->dev, "No IRQ assigned to device: "
> -			 "no support for interrupts?\n");
> -		pci_disable_device(pdev);
> -		return -ENODEV;
> -	}
> -
> -	if (!pci_intx_mask_supported(pdev)) {
> +	if (pci->irq && !pci_intx_mask_supported(pdev)) {
>  		err = -ENODEV;
>  		goto err_verify;
>  	}
> @@ -86,10 +79,15 @@ static int probe(struct pci_dev *pdev,
>  
>  	gdev->info.name = "uio_pci_generic";
>  	gdev->info.version = DRIVER_VERSION;
> -	gdev->info.irq = pdev->irq;
> -	gdev->info.irq_flags = IRQF_SHARED;
> -	gdev->info.handler = irqhandler;
>  	gdev->pdev = pdev;
> +	if (pdev->irq) {
> +		gdev->info.irq = pdev->irq;
> +		gdev->info.irq_flags = IRQF_SHARED;
> +		gdev->info.handler = irqhandler;
> +	} else {
> +		dev_warn(&pdev->dev, "No IRQ assigned to device: "
> +			 "no support for interrupts?\n");
> +	}
>  
>  	err = uio_register_device(&pdev->dev, &gdev->info);
>  	if (err)
> -- 
> 2.12.2

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

* Re: [PATCH] uio/uio_pci_generic: don't fail probe if pdev->irq == NULL
  2017-05-01  1:17 ` Michael S. Tsirkin
@ 2017-05-01 21:21   ` Harris, James R
  0 siblings, 0 replies; 4+ messages in thread
From: Harris, James R @ 2017-05-01 21:21 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: kvm, gregkh, Busch, Keith, Derrick, Jonathan


> On Apr 30, 2017, at 6:17 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> On Sun, Apr 30, 2017 at 12:29:55PM -0700, Jim Harris wrote:
>> Some userspace drivers and frameworks only poll and do not
>> require interrupts to be available and enabled on the
>> PCI device.  So remove the requirement that an IRQ is
>> assigned.  If an IRQ is not assigned and a userspace
>> driver tries to read()/write(), the generic uio
>> framework will just return -EIO.
>> 
>> This allows binding uio_pci_generic to devices which
>> cannot get an IRQ assigned, such as an NVMe controller
>> behind Intel Volume Management Device (VMD), since VMD
>> does not support INTx interrupts.
>> 
>> Signed-off-by: Jim Harris <james.r.harris@intel.com>
> 
> Without interrupts, why do you want uio? All it does is
> forward interrupts to userspace.

Primarily so that a driver is bound to the device while using it from userspace.  It can also be helpful to have the uioX sysfs symlinks to the underlying PCI config/resources.

Some userspace drivers/frameworks such as DPDK and SPDK primarily run in polled-only mode - not using interrupts at all.  But DPDK does provide a facility to drop into interrupt mode which uio facilitates.  So looking ahead to NVMe devices behind a VMD endpoint, we want to be able to at least bind uio to these NVMe devices - DPDK/SPDK just will not be able to drop into interrupt mode on these devices.

This is different obviously for vfio where we can allocate MSI/MSIX vectors.  But for dev systems with IOMMU disabled, or passing a VMD endpoint to a guest VM, uio support is still desired.

> 
>> ---
>> drivers/uio/uio_pci_generic.c | 20 +++++++++-----------
>> 1 file changed, 9 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/uio/uio_pci_generic.c b/drivers/uio/uio_pci_generic.c
>> index d0b508b68f3c..81c59b4f8552 100644
>> --- a/drivers/uio/uio_pci_generic.c
>> +++ b/drivers/uio/uio_pci_generic.c
>> @@ -66,14 +66,7 @@ static int probe(struct pci_dev *pdev,
>> 		return err;
>> 	}
>> 
>> -	if (!pdev->irq) {
>> -		dev_warn(&pdev->dev, "No IRQ assigned to device: "
>> -			 "no support for interrupts?\n");
>> -		pci_disable_device(pdev);
>> -		return -ENODEV;
>> -	}
>> -
>> -	if (!pci_intx_mask_supported(pdev)) {
>> +	if (pci->irq && !pci_intx_mask_supported(pdev)) {
>> 		err = -ENODEV;
>> 		goto err_verify;
>> 	}
>> @@ -86,10 +79,15 @@ static int probe(struct pci_dev *pdev,
>> 
>> 	gdev->info.name = "uio_pci_generic";
>> 	gdev->info.version = DRIVER_VERSION;
>> -	gdev->info.irq = pdev->irq;
>> -	gdev->info.irq_flags = IRQF_SHARED;
>> -	gdev->info.handler = irqhandler;
>> 	gdev->pdev = pdev;
>> +	if (pdev->irq) {
>> +		gdev->info.irq = pdev->irq;
>> +		gdev->info.irq_flags = IRQF_SHARED;
>> +		gdev->info.handler = irqhandler;
>> +	} else {
>> +		dev_warn(&pdev->dev, "No IRQ assigned to device: "
>> +			 "no support for interrupts?\n");
>> +	}
>> 
>> 	err = uio_register_device(&pdev->dev, &gdev->info);
>> 	if (err)
>> -- 
>> 2.12.2

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

* Re: [PATCH] uio/uio_pci_generic: don't fail probe if pdev->irq == NULL
  2017-04-30 19:29 [PATCH] uio/uio_pci_generic: don't fail probe if pdev->irq == NULL Jim Harris
  2017-05-01  1:17 ` Michael S. Tsirkin
@ 2017-05-02  9:20 ` kbuild test robot
  1 sibling, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2017-05-02  9:20 UTC (permalink / raw)
  To: Jim Harris
  Cc: kbuild-all, mst, kvm, gregkh, linux, keith.busch,
	jonathan.derrick, Jim Harris

[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]

Hi Jim,

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v4.11 next-20170501]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jim-Harris/uio-uio_pci_generic-don-t-fail-probe-if-pdev-irq-NULL/20170501-040733
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/uio/uio_pci_generic.c: In function 'probe':
>> drivers/uio/uio_pci_generic.c:69:6: error: 'pci' undeclared (first use in this function)
     if (pci->irq && !pci_intx_mask_supported(pdev)) {
         ^~~
   drivers/uio/uio_pci_generic.c:69:6: note: each undeclared identifier is reported only once for each function it appears in

vim +/pci +69 drivers/uio/uio_pci_generic.c

    63		if (err) {
    64			dev_err(&pdev->dev, "%s: pci_enable_device failed: %d\n",
    65				__func__, err);
    66			return err;
    67		}
    68	
  > 69		if (pci->irq && !pci_intx_mask_supported(pdev)) {
    70			err = -ENODEV;
    71			goto err_verify;
    72		}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38807 bytes --]

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

end of thread, other threads:[~2017-05-02  9:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-30 19:29 [PATCH] uio/uio_pci_generic: don't fail probe if pdev->irq == NULL Jim Harris
2017-05-01  1:17 ` Michael S. Tsirkin
2017-05-01 21:21   ` Harris, James R
2017-05-02  9:20 ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.