uio_pci_generic does not export memory resources
Subsystems (4)
PATCH 1/1 uio_pci_generic does not export memory resources
Hello,
the current version of the uio_pci_generic module does not export memory resources, such as BARs. As far as I can see, the related module does only support interrupts, which is not really useful. My suggestion in this case would be to either fix this issue, or to completely remove it. The latter would not be an option for us, since we need this functionality at some stuff at CERN.
Therefore, here is a patch that fixes the issue. Please give me further advice, since I'm doing this the first time ...
---
diff -uNr linux-3.4_old/drivers/uio/uio_pci_generic.c linux-3.4_new/drivers/uio/uio_pci_generic.c
--- linux-3.4_old/drivers/uio/uio_pci_generic.c 2012-05-21
00:29:13.000000000 +0200
+++ linux-3.4_new/drivers/uio/uio_pci_generic.c 2012-06-08
13:01:12.000000000 +0200
@@ -25,10 +25,12 @@
#include <linux/slab.h>
#include <linux/uio_driver.h>
-#define DRIVER_VERSION "0.01.0"
+#define DRIVER_VERSION "0.02.0"
#define DRIVER_AUTHOR "Michael S. Tsirkin <[email protected]>"
#define DRIVER_DESC "Generic UIO driver for PCI 2.3 devices"
+#define DRV_NAME "uio_pci_generic"
+
struct uio_pci_generic_dev {
struct uio_info info;
struct pci_dev *pdev;
@@ -58,6 +60,7 @@
{
struct uio_pci_generic_dev *gdev;
int err;
+ int i;
err = pci_enable_device(pdev);
if (err) {
@@ -66,9 +69,33 @@
return err;
}
+ /* set master */
+ pci_set_master(pdev);
+
+ /* set DMA mask */
+ err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
+ if (err) {
+ dev_warn(&pdev->dev, "Warning: couldn't set 64-bit PCI DMA mask.\n");
+ err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+ if (err) {
+ dev_err(&pdev->dev, "Can't set PCI DMA mask, aborting\n");
+ return -ENODEV;
+ }
+ }
+
+ /* set consistent DMA mask */
+ err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
+ if (err) {
+ dev_warn(&pdev->dev, "Warning: couldn't set 64-bit consistent PCI DMA
mask.\n");
+ err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
+ if (err) {
+ dev_err(&pdev->dev, "Can't set consistent PCI DMA mask, aborting
\n");
+ return -ENODEV;
+ }
+ }
+
if (!pdev->irq) {
- dev_warn(&pdev->dev, "No IRQ assigned to device: "
- "no support for interrupts?\n");
+ dev_warn(&pdev->dev, "No IRQ assigned to device: no support for
interrupts?\n");
pci_disable_device(pdev);
return -ENODEV;
}
@@ -91,10 +118,40 @@
gdev->info.handler = irqhandler;
gdev->pdev = pdev;
+ /* request regions */
+ err = pci_request_regions(pdev, DRV_NAME);
+ if (err) {
+ dev_err(&pdev->dev, "Couldn't get PCI resources, aborting\n");
+ return err;
+ }
+
+ /* create attributes for BAR mappings */
+ for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+ if (pdev->resource[i].flags &&
+ (pdev->resource[i].flags & IORESOURCE_IO)) {
+ gdev->info.port[i].size = 0;
+ gdev->info.port[i].porttype = UIO_PORT_OTHER;
+ #ifdef CONFIG_X86
+ gdev->info.port[i].porttype = UIO_PORT_X86;
+ #endif
+ }
+
+ if (pdev->resource[i].flags &&
+ (pdev->resource[i].flags & IORESOURCE_MEM)) {
+ gdev->info.mem[i].addr = pci_resource_start(pdev, i);
+ gdev->info.mem[i].size = pci_resource_len(pdev, i);
+ gdev->info.mem[i].internal_addr = NULL;
+ gdev->info.mem[i].memtype = UIO_MEM_PHYS;
+ }
+ }
+
if (uio_register_device(&pdev->dev, &gdev->info))
goto err_register;
pci_set_drvdata(pdev, gdev);
+ pr_info("UIO_PCI_GENERIC : initialized new device (%x %x)\n",
+ pdev->vendor, pdev->device);
+
return 0;
err_register:
kfree(gdev);
@@ -107,17 +164,21 @@
static void remove(struct pci_dev *pdev)
{
struct uio_pci_generic_dev *gdev = pci_get_drvdata(pdev);
-
uio_unregister_device(&gdev->info);
+
+ pci_release_regions(pdev);
pci_disable_device(pdev);
kfree(gdev);
+
+ pr_info("UIO_PCI_GENERIC : removed device (%x %x)\n",
+ pdev->vendor, pdev->device);
}
static struct pci_driver driver = {
- .name = "uio_pci_generic",
+ .name = DRV_NAME,
.id_table = NULL, /* only dynamic id's */
- .probe = probe,
- .remove = remove,
+ .probe = probe,
+ .remove = remove,
};
static int __init init(void)
Signature · 9 lines
Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 69 79844114
---
drivers/uio/uio_pci_generic.c +68 -7
@@ -25,10 +25,12 @@#include <linux/slab.h> #include <linux/uio_driver.h> -#define DRIVER_VERSION "0.01.0" +#define DRIVER_VERSION "0.02.0"#define DRIVER_AUTHOR "Michael S. Tsirkin <[email protected]>"#define DRIVER_DESC "Generic UIO driver for PCI 2.3 devices" +#define DRV_NAME "uio_pci_generic" +struct uio_pci_generic_dev {struct uio_info info;struct pci_dev *pdev;
@@ -58,6 +60,7 @@ {struct uio_pci_generic_dev *gdev;int err; +int i; err =pci_enable_device (pdev);if (err) {
@@ -66,9 +69,33 @@return err; } +/* set master */ +pci_set_master (pdev); + +/* set DMA mask */ + err =pci_set_dma_mask (pdev,DMA_BIT_MASK (64 )); +if (err) { +dev_warn (&pdev->dev,"Warning: couldn't set 64-bit PCI DMA mask.\n" ); + err =pci_set_dma_mask (pdev,DMA_BIT_MASK (32 )); +if (err) { +dev_err (&pdev->dev,"Can't set PCI DMA mask, aborting\n" ); +return -ENODEV ; + } + } + +/* set consistent DMA mask */ + err =pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (64 )); +if (err) { +dev_warn (&pdev->dev,"Warning: couldn't set 64-bit consistent PCI DMA mask.\n" ); + err =pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (32 )); +if (err) { +dev_err (&pdev->dev,"Can't set consistent PCI DMA mask, aborting\n" ); +return -ENODEV ; + } + } +if (!pdev->irq) { -dev_warn (&pdev->dev,"No IRQ assigned to device: " -"no support for interrupts?\n" ); +dev_warn (&pdev->dev,"No IRQ assigned to device: no support for interrupts?\n" );pci_disable_device (pdev);return -ENODEV ; }
@@ -91,10 +118,40 @@ gdev->info.handler = irqhandler; gdev->pdev = pdev; +/* request regions */ + err =pci_request_regions (pdev,DRV_NAME ); +if (err) { +dev_err (&pdev->dev,"Couldn't get PCI resources, aborting\n" ); +return err; + } + +/* create attributes for BAR mappings */ +for (i =0 ; i <PCI_NUM_RESOURCES ; i++) { +if (pdev->resource[i].flags && + (pdev->resource[i].flags &IORESOURCE_IO )) { + gdev->info.port[i].size =0 ; + gdev->info.port[i].porttype =UIO_PORT_OTHER ; +#ifdef CONFIG_X86 + gdev->info.port[i].porttype =UIO_PORT_X86 ; +#endif + } + +if (pdev->resource[i].flags && + (pdev->resource[i].flags &IORESOURCE_MEM )) { + gdev->info.mem[i].addr =pci_resource_start (pdev, i); + gdev->info.mem[i].size =pci_resource_len (pdev, i); + gdev->info.mem[i].internal_addr =NULL ; + gdev->info.mem[i].memtype =UIO_MEM_PHYS ; + } + } +if (uio_register_device (&pdev->dev, &gdev->info)) goto err_register;pci_set_drvdata (pdev, gdev); +pr_info ("UIO_PCI_GENERIC : initialized new device (%x %x)\n" , + pdev->vendor, pdev->device); +return 0 ; err_register:kfree (gdev);
@@ -107,17 +164,21 @@static void remove (struct pci_dev *pdev) {struct uio_pci_generic_dev *gdev =pci_get_drvdata (pdev); -uio_unregister_device (&gdev->info); + +pci_release_regions (pdev);pci_disable_device (pdev);kfree (gdev); + +pr_info ("UIO_PCI_GENERIC : removed device (%x %x)\n" , + pdev->vendor, pdev->device); }static struct pci_driver driver = { - .name ="uio_pci_generic" , + .name =DRV_NAME , .id_table =NULL ,/* only dynamic id's */ - .probe = probe, - .remove = remove, + .probe = probe, + .remove = remove, };static int __init init (void )
Michael S. Tsirkin
maintainerQuoted 11 lines · Dominic Eschweiler
Hello,
the current version of the
uio_pci_genericmodule does not export memory resources, such as BARs. As far as I can see, the related module does only support interrupts, which is not really useful. My suggestion in this case would be to either fix this issue, or to completely remove it. The latter would not be an option for us, since we need this functionality at some stuff at CERN.Therefore, here is a patch that fixes the issue. Please give me further advice, since I'm doing this the first time ...
Why is this needed?
What's wrong with mapping resources through
/sys/bus/pci/devices/XXXXXresourceX
?
Quoted 32 lines
Signed-off-by Dominic Eschweiler <[email protected]>---
diff -uNrlinux-3.4_old/drivers/uio/uio_pci_generic.clinux-3.4_new/drivers/uio/uio_pci_generic.c
---linux-3.4_old/drivers/uio/uio_pci_generic.c2012-05-21
00:29:13.000000000 +0200
+++linux-3.4_new/drivers/uio/uio_pci_generic.c2012-06-08
13:01:12.000000000 +0200drivers/uio/uio_pci_generic.c +4
-1@@ -25,10 +25,12 @@#include <linux/slab.h> #include <linux/uio_driver.h> -#define DRIVER_VERSION "0.01.0" +#define DRIVER_VERSION "0.02.0"#define DRIVER_AUTHOR "Michael S. Tsirkin <[email protected]>"#define DRIVER_DESC "Generic UIO driver for PCI 2.3 devices" +#define DRV_NAME "uio_pci_generic" +struct uio_pci_generic_dev {struct uio_info info;struct pci_dev *pdev;@@ -58,6 +60,7 @@ {struct uio_pci_generic_dev *gdev;int err; +int i; err =pci_enable_device (pdev);if (err) {
drivers/uio/uio_pci_generic.c line 76
Quoted 8 lines
drivers/uio/uio_pci_generic.c +5
-0@@ -66,9 +69,33 @@return err; } +/* set master */ +pci_set_master (pdev); + +/* set DMA mask */ + err =pci_set_dma_mask (pdev,DMA_BIT_MASK (64 ));
uio currently only supports devices which do not do DMA.
DMA from uio controlled devices is a no no unless it's behind an IOMMU which can protect us from random memory corruptions this could cause.
In the later case it's OK but we need some code to check this and program the IOMMU appropriately.
Quoted 109 lines
drivers/uio/uio_pci_generic.c +21
-2+if (err) { +dev_warn (&pdev->dev,"Warning: couldn't set 64-bit PCI DMA mask.\n" ); + err =pci_set_dma_mask (pdev,DMA_BIT_MASK (32 )); +if (err) { +dev_err (&pdev->dev,"Can't set PCI DMA mask, aborting\n" ); +return -ENODEV ; + } + } + +/* set consistent DMA mask */ + err =pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (64 )); +if (err) { + dev_warn(&pdev->dev, "Warning: couldn't set 64-bit consistent PCI DMA mask.\n"); + err =pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (32 )); +if (err) { +dev_err (&pdev->dev,"Can't set consistent PCI DMA mask, aborting \n" ); +return -ENODEV ; + } + } + if (!pdev->irq) { - dev_warn(&pdev->dev, "No IRQ assigned to device: " - "no support for interrupts?\n"); + dev_warn(&pdev->dev, "No IRQ assigned to device: no support for interrupts?\n");drivers/uio/uio_pci_generic.c +38
-4pci_disable_device (pdev);return -ENODEV ; }@@ -91,10 +118,40 @@ gdev->info.handler = irqhandler; gdev->pdev = pdev; +/* request regions */ + err =pci_request_regions (pdev,DRV_NAME ); +if (err) { +dev_err (&pdev->dev,"Couldn't get PCI resources, aborting\n" ); +return err; + } + +/* create attributes for BAR mappings */ +for (i =0 ; i <PCI_NUM_RESOURCES ; i++) { +if (pdev->resource[i].flags && + (pdev->resource[i].flags &IORESOURCE_IO )) { + gdev->info.port[i].size =0 ; + gdev->info.port[i].porttype =UIO_PORT_OTHER ; +#ifdef CONFIG_X86 + gdev->info.port[i].porttype =UIO_PORT_X86 ; +#endif + } + +if (pdev->resource[i].flags && + (pdev->resource[i].flags &IORESOURCE_MEM )) { + gdev->info.mem[i].addr =pci_resource_start (pdev, i); + gdev->info.mem[i].size =pci_resource_len (pdev, i); + gdev->info.mem[i].internal_addr =NULL ; + gdev->info.mem[i].memtype =UIO_MEM_PHYS ; + } + } +if (uio_register_device (&pdev->dev, &gdev->info)) goto err_register;pci_set_drvdata (pdev, gdev); +pr_info ("UIO_PCI_GENERIC : initialized new device (%x %x)\n" , + pdev->vendor, pdev->device); +return 0 ; err_register:kfree (gdev);@@ -107,17 +164,21 @@static void remove (struct pci_dev *pdev) {struct uio_pci_generic_dev *gdev =pci_get_drvdata (pdev); -uio_unregister_device (&gdev->info); + +pci_release_regions (pdev);pci_disable_device (pdev);kfree (gdev); + +pr_info ("UIO_PCI_GENERIC : removed device (%x %x)\n" , + pdev->vendor, pdev->device); }static struct pci_driver driver = { - .name ="uio_pci_generic" , + .name =DRV_NAME , .id_table =NULL ,/* only dynamic id's */ - .probe = probe, - .remove = remove, + .probe = probe, + .remove = remove, };static int __init init (void )Signature · 10 lines
-- Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 69 79844114
Quoted 9 lines · Michael S. Tsirkin
drivers/uio/uio_pci_generic.c line 76
Quoted 5 lines
drivers/uio/uio_pci_generic.c +5
-0+/* set master */ +pci_set_master (pdev); + +/* set DMA mask */ + err =pci_set_dma_mask (pdev,DMA_BIT_MASK (64 ));uio currently only supports devices which do not do DMA.
Quoted 6 lines
DMA from uio controlled devices is a no no unless it's behind an IOMMU which can protect us from random memory corruptions this could cause.
In the later case it's OK but we need some code to check this and program the IOMMU appropriately.
AKA: VFIO. :)
Dominic, maybe you want to have a look at Alex's work:
https://github.com/awilliam/linux-vfio
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Quoted 14 lines · Jan Kiszka
Quoted 13 lines · Michael S. Tsirkin
drivers/uio/uio_pci_generic.c line 76
Quoted 5 lines
drivers/uio/uio_pci_generic.c +5
-0+/* set master */ +pci_set_master (pdev); + +/* set DMA mask */ + err =pci_set_dma_mask (pdev,DMA_BIT_MASK (64 ));uio currently only supports devices which do not do DMA.
DMA from uio controlled devices is a no no unless it's behind an IOMMU which can protect us from random memory corruptions this could cause.
Quoted 7 lines
Quoted 2 lines
In the later case it's OK but we need some code to check this and program the IOMMU appropriately.
AKA: VFIO. :)
Dominic, maybe you want to have a look at Alex's work:
https://github.com/awilliam/linux-vfio
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace. I'm currently working to get this upstream, probably targeting 3.6 at this point, and would love to have more users to help make that happen. Please take a look at the vfio-3.4 branch in the tree above. See this tree for Qemu's usage of VFIO for device assignment:
https://github.com/awilliam/qemu-vfio (vfio-ng)
Look forward to your feedback. Thanks,
Alex
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
I know about VFIO, but we need some support for that stuff relatively soon. That's the reason why I'm currently working on it to make UIO DMA capable. My extensions probably do not play well with IOMMUs and they therefore won't make it to mainline anyhow (i learned that today ;-).
Quoted 3 lines
I'm currently working to get this upstream, probably targeting 3.6 at this point, and would love to have more users to help make that happen.
Yes, you are faster than I'm able to ask. 3.6 is a good target and really would like to support that.
Quoted 2 lines
Please take a look at the vfio-3.4 branch in the tree above. See this tree for Qemu's usage of VFIO for device assignment ...
OK, I take look at it and report back (off the list). Please write me an email if you need support.
Signature · 9 lines
Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 69 79844114
Quoted 9 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
I know about VFIO, but we need some support for that stuff relatively soon. That's the reason why I'm currently working on it to make UIO DMA capable. My extensions probably do not play well with IOMMUs and they therefore won't make it to mainline anyhow (i learned that today ;-).
Yeah, VFIO was originally started by Tom Lyon as a uio extension to support DMA mapping and more extensive interrupt handling, but it became an entire re-write of uio. Now we've had to extend in to iommu grouping to get the DMA protection that we're really after.
Quoted 4 lines
Quoted 3 lines
I'm currently working to get this upstream, probably targeting 3.6 at this point, and would love to have more users to help make that happen.
Quoted 8 lines
Yes, you are faster than I'm able to ask. 3.6 is a good target and really would like to support that.
Quoted 2 lines
Please take a look at the vfio-3.4 branch in the tree above. See this tree for Qemu's usage of VFIO for device assignment ...
OK, I take look at it and report back (off the list). Please write me an email if you need support.
Thanks,
Alex
Hi Dominic,
Quoted 9 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
I know about VFIO, but we need some support for that stuff relatively soon. That's the reason why I'm currently working on it to make UIO DMA capable. My extensions probably do not play well with IOMMUs and they therefore won't make it to mainline anyhow (i learned that today ;-).
I'm not sure if vfio covers your needs completely, but I tested it here very successfully. I was able to create a patch, which can be applied to opensuse 3.4.1 kernel and which seams to run well.
I even managed to integrate it into libvirt :-). So it is usable as every other traditional VM, too.
Besides the problem with AMD IOMMU, which requires to unbind a whole group of devices in some cases (PCI passthrough - not PCIe), it's really cool! And it's usable now!
Regards,
Andreas
Quoted 13 lines · Andreas Hartmann
Hi Dominic,
Quoted 9 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
I know about VFIO, but we need some support for that stuff relatively soon. That's the reason why I'm currently working on it to make UIO DMA capable. My extensions probably do not play well with IOMMUs and they therefore won't make it to mainline anyhow (i learned that today ;-).
Quoted 6 lines
I'm not sure if vfio covers your needs completely, but I tested it here very successfully. I was able to create a patch, which can be applied to opensuse 3.4.1 kernel and which seams to run well.
I even managed to integrate it into libvirt :-). So it is usable as every other traditional VM, too.
Oh, please post the patch for libvirt!
Quoted 3 lines
Besides the problem with AMD IOMMU, which requires to unbind a whole group of devices in some cases (PCI passthrough - not PCIe), it's really cool! And it's usable now!
If you're feeling adventurous (and know that this may not make it upstream), you can do something like this:
drivers/pci/quirks.c +11 -0
@@ -3161,11 +3161,22 @@ struct pci_dev *pci_get_dma_source(struct pci_dev *dev)return pci_dev_get (dev); } +static int pci_func_supports_acs (struct pci_dev *dev,u16 acs_flags) +{ +return 1 ; +} +static const struct pci_dev_acs_enabled {u16 vendor;u16 device;int (*acs_enabled)(struct pci_dev *dev,u16 acs_flags); } pci_dev_acs_enabled[] = { + {0x1002 ,0x4385 , pci_func_supports_acs }, + {0x1002 ,0x439c , pci_func_supports_acs }, + {0x1002 ,0x4383 , pci_func_supports_acs }, + {0x1002 ,0x439d , pci_func_supports_acs }, + {0x1002 ,0x4384 , pci_func_supports_acs }, + {0x1002 ,0x4399 , pci_func_supports_acs }, {0 } };
Hmm, I wonder if we should make a kernel boot parameter that allows whitelisting some devices. I think it would have to taint the kernel but there's probably sufficient interest for usability vs supportability. Thanks,
Alex
Quoted 1 line · Alex Williamson
On Fri, 2012-06-08 at 18:16 +0200, Andreas Hartmann wrote:
[...]
Quoted 6 lines
Quoted 3 lines
Besides the problem with AMD IOMMU, which requires to unbind a whole group of devices in some cases (PCI passthrough - not PCIe), it's really cool! And it's usable now!
If you're feeling adventurous (and know that this may not make it upstream), you can do something like this:
Hmm, even without this patch, it's not necessary to bind all devices to pci-stub. Just to remember:
-[0000:00]-+-14.0 Advanced Micro Devices [AMD] nee ATI SBx00 SMBus Controller
+-14.1 Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 IDE Controller
+-14.2 Advanced Micro Devices [AMD] nee ATI SBx00 Azalia (Intel HDA)
+-14.3 Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 LPC host controller
+-14.4-[06]--
\-14.5 Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB OHCI2 ControllerDevice 06:07.0 should be passed through.
The devices 14.0, 14.3 and 14.4 need not to be bound to pci-stub - the VM starts (and seems to work fine) anyway. Maybe because there isn't any driver anyway for these devices?
The USB device seems to be an internal device only - all of my external USB jacks are working fine.
The only thing I'm missing is the sound device.
Quoted 16 lines
drivers/pci/quirks.c +5
-0@@ -3161,11 +3161,22 @@ struct pci_dev *pci_get_dma_source(struct pci_dev *dev)return pci_dev_get (dev); } +static int pci_func_supports_acs (struct pci_dev *dev,u16 acs_flags) +{ +return 1 ; +} + static const struct pci_dev_acs_enabled { u16 vendor; u16 device;
drivers/pci/quirks.c struct pci_dev *pci_get_dma_source(struct pci_dev *dev) · line 3181
Quoted 10 lines
drivers/pci/quirks.c +6
-0int (*acs_enabled)(struct pci_dev *dev,u16 acs_flags); } pci_dev_acs_enabled[] = { + { 0x1002, 0x4385, pci_func_supports_acs }, + { 0x1002, 0x439c, pci_func_supports_acs }, + { 0x1002, 0x4383, pci_func_supports_acs }, + { 0x1002, 0x439d, pci_func_supports_acs }, + { 0x1002, 0x4384, pci_func_supports_acs }, + { 0x1002, 0x4399, pci_func_supports_acs }, { 0 } };
What's the risk of this patch? Machine crash? Data loss for an active file in an application? Complete filesystem damage? The latter would be worse.
Quoted 4 lines
Hmm, I wonder if we should make a kernel boot parameter that allows whitelisting some devices. I think it would have to taint the kernel but there's probably sufficient interest for usability vs supportability.
Good idea. I would print an additional big fat warning of dataloss / filesystem damage / crash if this could be the case.
Thanks,
regards,
Andreas
Quoted 20 lines · Andreas Hartmann
Quoted 1 line · Alex Williamson
On Fri, 2012-06-08 at 18:16 +0200, Andreas Hartmann wrote:
[...]
Quoted 6 lines
Quoted 3 lines
Besides the problem with AMD IOMMU, which requires to unbind a whole group of devices in some cases (PCI passthrough - not PCIe), it's really cool! And it's usable now!
If you're feeling adventurous (and know that this may not make it upstream), you can do something like this:
Hmm, even without this patch, it's not necessary to bind all devices to pci-stub. Just to remember:
-[0000:00]-+-14.0 Advanced Micro Devices [AMD] nee ATI SBx00 SMBus Controller +-14.1 Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 IDE Controller +-14.2 Advanced Micro Devices [AMD] nee ATI SBx00 Azalia (Intel HDA) +-14.3 Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 LPC host controller +-14.4-[06]-- \-14.5 Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB OHCI2 Controller
Quoted 5 lines
Device 06:07.0 should be passed through.
The devices 14.0, 14.3 and 14.4 need not to be bound to pci-stub - the VM starts (and seems to work fine) anyway. Maybe because there isn't any driver anyway for these devices?
Right, the actual rule is that any device you want to access needs to be bound to vfio-pci. The remaining devices within the group can either be bound to vfio-pci or pci-stub or no driver. The recommendation for avoiding that no-driver case is because if later a driver is loaded into the kernel that would attempt to bind to any of those no-driver devices, you end up back in the situation of the security of the group being violated and hit the BUGON you saw previously.
Quoted 27 lines
The USB device seems to be an internal device only - all of my external USB jacks are working fine.
The only thing I'm missing is the sound device.
Quoted 22 lines
drivers/pci/quirks.c +9
-0@@ -3161,11 +3161,22 @@ struct pci_dev *pci_get_dma_source(struct pci_dev *dev)return pci_dev_get (dev); } +static int pci_func_supports_acs (struct pci_dev *dev,u16 acs_flags) +{ +return 1 ; +} +static const struct pci_dev_acs_enabled {u16 vendor;u16 device;int (*acs_enabled)(struct pci_dev *dev,u16 acs_flags); } pci_dev_acs_enabled[] = { + {0x1002 ,0x4385 , pci_func_supports_acs }, + {0x1002 ,0x439c , pci_func_supports_acs }, + {0x1002 ,0x4383 , pci_func_supports_acs }, + {0x1002 ,0x439d , pci_func_supports_acs },
Quoted 8 lines
drivers/pci/quirks.c struct pci_dev *pci_get_dma_source(struct pci_dev *dev) · line 3181
Quoted 4 lines
drivers/pci/quirks.c +2
-0+ { 0x1002, 0x4384, pci_func_supports_acs }, + { 0x1002, 0x4399, pci_func_supports_acs }, { 0 } };What's the risk of this patch? Machine crash? Data loss for an active file in an application? Complete filesystem damage? The latter would be worse.
What we're trying to prevent by testing whether devices support ACS is peer-to-peer transactions that would not be translated via the IOMMU. For instance, imagine if your WLAN controller behind 14.4 does a DMA write to an address that happens to be within the MMIO resources of device 14.2, the audio device. Instead of the transaction going up to the IOMMU and resulting in a memory write, internal routing on device 14.x results in that transaction being redirected to the 14.2. So you're looking at potential data loss from the guest as well as corrupting device state in the host.
Quoted 7 lines
Quoted 4 lines
Hmm, I wonder if we should make a kernel boot parameter that allows whitelisting some devices. I think it would have to taint the kernel but there's probably sufficient interest for usability vs supportability.
Good idea. I would print an additional big fat warning of dataloss / filesystem damage / crash if this could be the case.
Well, outlining the risk above makes me a little more nervous about making such a config option, even if it taints the kernel, available so easily... :^\ Thanks,
Alex
Quoted 1 line · Alex Williamson
On Sat, 2012-06-09 at 11:28 +0200, Andreas Hartmann wrote:
[...]
Quoted 9 lines
Quoted 3 lines
What's the risk of this patch? Machine crash? Data loss for an active file in an application? Complete filesystem damage? The latter would be worse.
What we're trying to prevent by testing whether devices support ACS is peer-to-peer transactions that would not be translated via the IOMMU. For instance, imagine if your WLAN controller behind 14.4 does a DMA write to an address that happens to be within the MMIO resources of device 14.2, the audio device.
Why should it do that intentionally - I can't see any reason. Remains unintentionally. But that's already enough :-) .
Quoted 5 lines
Instead of the transaction going up to
the IOMMU and resulting in a memory write, internal routing on device 14.x results in that transaction being redirected to the 14.2. So you're looking at potential data loss from the guest as well as corrupting device state in the host.
My guest does have no data. Besides that, the VM can be easily backuped before.
The corrupting device state probably is limited to the devices behind the bridge. Correct?
Quoted 11 lines
Quoted 7 lines
Quoted 4 lines
Hmm, I wonder if we should make a kernel boot parameter that allows whitelisting some devices. I think it would have to taint the kernel but there's probably sufficient interest for usability vs supportability.
Good idea. I would print an additional big fat warning of dataloss / filesystem damage / crash if this could be the case.
Well, outlining the risk above makes me a little more nervous about making such a config option, even if it taints the kernel, available so easily... :^\
That's true, too. Anyway, out of curiosity, if the corruption of the device states is limited to the devices behind the bridge, I'm going to test it. I hope that a hardware damage isn't possible by that action.
Thanks,
regards,
Andreas
Quoted 9 lines · Andreas Hartmann
Quoted 1 line · Alex Williamson
On Sat, 2012-06-09 at 11:28 +0200, Andreas Hartmann wrote:
[...]
Quoted 4 lines
Quoted 3 lines
What's the risk of this patch? Machine crash? Data loss for an active file in an application? Complete filesystem damage? The latter would be worse.
Quoted 8 lines
Quoted 5 lines
What we're trying to prevent by testing whether devices support ACS is peer-to-peer transactions that would not be translated via the IOMMU. For instance, imagine if your WLAN controller behind 14.4 does a DMA write to an address that happens to be within the MMIO resources of device 14.2, the audio device.
Why should it do that intentionally - I can't see any reason. Remains unintentionally. But that's already enough :-) .
We don't know the internal design of multifunction devices.
Quoted 10 lines
Quoted 5 lines
Instead of the transaction going up to
the IOMMU and resulting in a memory write, internal routing on device 14.x results in that transaction being redirected to the 14.2. So you're looking at potential data loss from the guest as well as corrupting device state in the host.My guest does have no data. Besides that, the VM can be easily backuped before.
The corrupting device state probably is limited to the devices behind the bridge. Correct?
No, we're talking about the multifunction device here. A legacy PCI bus is always susceptible to this as it's a shared bus. Another device on the bus can claim the transaction. The IOMMU also only has visibility to the bridge devices, all devices behind it are masked. So the difference we're looking at is whether 14.4 is grouped with all the other devices on 14.x or not. Bus 6 will always be grouped with device 14.4. So then you have to consider what can happen if your guest that owns 14.4 and bus 6 can corrupt the state of device 14.2 and how that corrupted state could be propagated out to the host.
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
Hmm, I wonder if we should make a kernel boot parameter that allows whitelisting some devices. I think it would have to taint the kernel but there's probably sufficient interest for usability vs supportability.
Good idea. I would print an additional big fat warning of dataloss / filesystem damage / crash if this could be the case.
Quoted 7 lines
Quoted 3 lines
Well, outlining the risk above makes me a little more nervous about making such a config option, even if it taints the kernel, available so easily... :^\
That's true, too. Anyway, out of curiosity, if the corruption of the device states is limited to the devices behind the bridge, I'm going to test it. I hope that a hardware damage isn't possible by that action.
Hardware damage should not be possible, but the interactions would be between host and guest as noted above. Thanks,
Alex
Quoted 16 lines · Alex Williamson
Quoted 15 lines · Andreas Hartmann
Quoted 1 line · Alex Williamson
On Sat, 2012-06-09 at 11:28 +0200, Andreas Hartmann wrote:
[...]
Quoted 9 lines
Quoted 3 lines
What's the risk of this patch? Machine crash? Data loss for an active file in an application? Complete filesystem damage? The latter would be worse.
What we're trying to prevent by testing whether devices support ACS is peer-to-peer transactions that would not be translated via the IOMMU. For instance, imagine if your WLAN controller behind 14.4 does a DMA write to an address that happens to be within the MMIO resources of device 14.2, the audio device.
Quoted 4 lines
Quoted 2 lines
Why should it do that intentionally - I can't see any reason. Remains unintentionally. But that's already enough :-) .
We don't know the internal design of multifunction devices.
_peer-to-peer transaction_
My current idea: this transaction is done without the knowledge of the system software at all.
Couldn't this happen too, even if there is no VM at all but bare metal? What would be the result in this case?
Quoted 11 lines
Quoted 10 lines
Quoted 5 lines
Instead of the transaction going up to
the IOMMU and resulting in a memory write, internal routing on device 14.x results in that transaction being redirected to the 14.2. So you're looking at potential data loss from the guest as well as corrupting device state in the host.My guest does have no data. Besides that, the VM can be easily backuped before.
The corrupting device state probably is limited to the devices behind the bridge. Correct?
Quoted 9 lines
No, we're talking about the multifunction device here. A legacy PCI bus is always susceptible to this as it's a shared bus. Another device on the bus can claim the transaction. The IOMMU also only has visibility to the bridge devices, all devices behind it are masked. So the difference we're looking at is whether 14.4 is grouped with all the other devices on 14.x or not. Bus 6 will always be grouped with device 14.4. So then you have to consider what can happen if your guest that owns 14.4 and bus 6 can corrupt the state of device 14.2 and how that corrupted state could be propagated out to the host.
To make it short: I applied the patch as proposed. I disabled binding / unbinding of 14.2 (Sound), started the VM, started hostapd in the VM (rt2860pci is set to do no hw crypt) and started a netperf session sending data from a client via AP to host and vice versa. At the same time, I started some sound device interaction (only output) onto the host.
If there are any problems, I would have expected:
- Encryption / decryption of WLAN data would have failed. I couldn't see any errors / warnings at this side. The 4 and 2 way key exchanges worked without any problem. EAP-TLS authentication has been working fine, too, apart from the already known problems.
- Distortions on sound output: I couldn't hear any.
- Some other malfunction, e.g. on graphics output (fglrx). I couldn't see any problem.
These are tests from the point of the view of an user. Do you have any idea to intentionally activate a failure situation?
Kind regards,
Andreas
Quoted 65 lines · Andreas Hartmann
Quoted 16 lines · Alex Williamson
Quoted 15 lines · Andreas Hartmann
Quoted 1 line · Alex Williamson
On Sat, 2012-06-09 at 11:28 +0200, Andreas Hartmann wrote:
[...]
Quoted 9 lines
Quoted 3 lines
What's the risk of this patch? Machine crash? Data loss for an active file in an application? Complete filesystem damage? The latter would be worse.
What we're trying to prevent by testing whether devices support ACS is peer-to-peer transactions that would not be translated via the IOMMU. For instance, imagine if your WLAN controller behind 14.4 does a DMA write to an address that happens to be within the MMIO resources of device 14.2, the audio device.
Quoted 4 lines
Quoted 2 lines
Why should it do that intentionally - I can't see any reason. Remains unintentionally. But that's already enough :-) .
We don't know the internal design of multifunction devices.
_peer-to-peer
transaction_My current idea: this transaction is done without the knowledge of the system software at all.
Couldn't this happen too, even if there is no VM at all but bare metal? What would be the result in this case?Quoted 11 lines
Quoted 10 lines
Quoted 5 lines
Instead of the transaction going up to
the IOMMU and resulting in a memory write, internal routing on device 14.x results in that transaction being redirected to the 14.2. So you're looking at potential data loss from the guest as well as corrupting device state in the host.My guest does have no data. Besides that, the VM can be easily backuped before.
The corrupting device state probably is limited to the devices behind the bridge. Correct?Quoted 9 lines
No, we're talking about the multifunction device here. A legacy PCI bus is always susceptible to this as it's a shared bus. Another device on the bus can claim the transaction. The IOMMU also only has visibility to the bridge devices, all devices behind it are masked. So the difference we're looking at is whether 14.4 is grouped with all the other devices on 14.x or not. Bus 6 will always be grouped with device 14.4. So then you have to consider what can happen if your guest that owns 14.4 and bus 6 can corrupt the state of device 14.2 and how that corrupted state could be propagated out to the host.
To make it short: I applied the patch as proposed. I disabled binding / unbinding of 14.2 (Sound), started the VM, started hostapd in the VM (rt2860pci is set to do no hw crypt) and started a netperf session sending data from a client via AP to host and vice versa. At the same time, I started some sound device interaction (only output) onto the host.
If there are any problems, I would have expected:
- Encryption / decryption of WLAN data would have failed. I couldn't see any errors / warnings at this side. The 4 and 2 way key exchanges worked without any problem. EAP-TLS authentication has been working fine, too, apart from the already known problems.
- Distortions on sound output: I couldn't hear any.
- Some other malfunction, e.g. on graphics output (fglrx). I couldn't see any problem.
Quoted 2 lines
These are tests from the point of the view of an user. Do you have any idea to intentionally activate a failure situation?
I did one more test:
- Set up a sound server (pulseaudio) on the host using the sound device 14.2 for output.
- Configure the WLAN-client to use this sound device as output device.
- The VM is the AP again, configured with rt2800pci hw encryption on or off
Result:
Again, I didn't face any problem (until now). The machine just worked as expected. Even after a s2ram/resume cycle (all VM's have been shut down before).
As I don't need the USB or IDE device, the ISA-bridge or the SMBus-Controller at all here, the patch should be reducible to
+ { 0x1002, 0x4383, pci_func_supports_acs }, // Sound
+ { 0x1002, 0x4384, pci_func_supports_acs }, // PCI-to-PCI bridge
Would this work, too?
BTW:
Isn't there a deterministic possibility to check if the MF device does peer-to-peer transfers? It could be a external program, which tells the user, if the hardware could be used the desired way.
Thanks,
kind regards,
Andreas
Michael S. Tsirkin
maintainerQuoted 5 lines · Alex Williamson
Quoted 5 lines
Quoted 4 lines
Hmm, I wonder if we should make a kernel boot parameter that allows whitelisting some devices. I think it would have to taint the kernel but there's probably sufficient interest for usability vs supportability.
Quoted 8 lines
Quoted 2 lines
Good idea. I would print an additional big fat warning of dataloss / filesystem damage / crash if this could be the case.
Well, outlining the risk above makes me a little more nervous about making such a config option, even if it taints the kernel, available so easily... :^\ Thanks,
Alex
Yes, please do not do it. Users will use it and we'll have an unfixable security bug on our hands.
Signature · 1 line
MST
Quoted 7 lines · Andreas Hartmann
Hi Dominic,
Quoted 4 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
VFIO is not a "better UIO". It *requires* an IOMMU. Dominic didn't say on what CPU he's working, so it's not clear if he can use VFIO at all.
UIO is intended for general use with devices that have mappable registers and don't fit into any other subsystem. No more, no less.
Thanks,
Hans
Quoted 9 lines · Hans J. Koch
Quoted 7 lines · Andreas Hartmann
Hi Dominic,
Quoted 4 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
Quoted 5 lines
VFIO is not a "better UIO". It *requires* an IOMMU. Dominic didn't say on what CPU he's working, so it's not clear if he can use VFIO at all.
UIO is intended for general use with devices that have mappable registers and don't fit into any other subsystem. No more, no less.
Maybe it is worth mentioning in the docs that UIO cannot support DMA, thus bus master mode. Apparently people do not realize this constraint and expect more of it than it is supposed to deliver.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Quoted 9 lines · Hans J. Koch
Quoted 7 lines · Andreas Hartmann
Hi Dominic,
Quoted 4 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
Quoted 5 lines
VFIO is not a "better UIO". It *requires* an IOMMU. Dominic didn't say on what CPU he's working, so it's not clear if he can use VFIO at all.
UIO is intended for general use with devices that have mappable registers and don't fit into any other subsystem. No more, no less.
VFIO is a secure UIO. An IOMMU is required for any model that allows a user to program DMA of a device, whether VFIO or something else. As Michael noted, if we allow UIO PCI to enable bus master, we open ourselves up to a whole world of problems with a user suddenly having access to and DMA'able memory in the system. So really, add that the device must only use PIO to your list of requirements for UIO, which rules out any kind of high speed device. Thanks,
Alex
Michael S. Tsirkin
maintainerQuoted 10 lines · Alex Williamson
Quoted 9 lines · Hans J. Koch
Quoted 7 lines · Andreas Hartmann
Hi Dominic,
Quoted 4 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
Quoted 7 lines
Quoted 5 lines
VFIO is not a "better UIO". It *requires* an IOMMU. Dominic didn't say on what CPU he's working, so it's not clear if he can use VFIO at all.
UIO is intended for general use with devices that have mappable registers and don't fit into any other subsystem. No more, no less.
VFIO is a secure UIO.
A secure UIO *for VFs*. I think that's why it's called VFIO :). Other stuff sometimes also works but no real guarantees, though VFIO tries to make sure you don't burn yourself too badly if it breaks.
Signature · 1 line
MST
Quoted 17 lines · Michael S. Tsirkin
Quoted 16 lines · Alex Williamson
Quoted 14 lines · Hans J. Koch
Quoted 7 lines · Andreas Hartmann
Hi Dominic,
Quoted 4 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
VFIO is not a "better UIO". It *requires* an IOMMU. Dominic didn't say on what CPU he's working, so it's not clear if he can use VFIO at all.
UIO is intended for general use with devices that have mappable registers and don't fit into any other subsystem. No more, no less.
Quoted 6 lines
Quoted 1 line
VFIO is a secure UIO.
A secure UIO *for VFs*. I think that's why it's called VFIO :). Other stuff sometimes also works but no real guarantees, though VFIO tries to make sure you don't burn yourself too badly if it breaks.
We do a little better than that. Multifunction devices that don't explicitly report ACS support are grouped together, so we have security for multifunction devices as well. Either single of multifunction PFs can have an option ROM, but since there's no defined mechanism to program the ROM, we can't protect it. Secure boot actually helps us here since the ROM loaded by the host BIOS or drivers would need to verify the ROM before using it. Note that secure boot will likely close off the pci-sysfs path uio_pci and KVM device assignment use to get resources since it allows unprotected access to the system. VFIO provides an interface where we control secure access, so should be compatible with secure boot. Thanks,
Alex
Michael S. Tsirkin
maintainerQuoted 20 lines · Alex Williamson
Quoted 19 lines · Michael S. Tsirkin
Quoted 17 lines · Alex Williamson
Quoted 9 lines · Hans J. Koch
Quoted 7 lines · Andreas Hartmann
Hi Dominic,
Quoted 4 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
Quoted 5 lines
VFIO is not a "better UIO". It *requires* an IOMMU. Dominic didn't say on what CPU he's working, so it's not clear if he can use VFIO at all.
UIO is intended for general use with devices that have mappable registers and don't fit into any other subsystem. No more, no less.
VFIO is a secure UIO.
Quoted 8 lines
Quoted 4 lines
A secure UIO *for VFs*. I think that's why it's called VFIO :). Other stuff sometimes also works but no real guarantees, though VFIO tries to make sure you don't burn yourself too badly if it breaks.
We do a little better than that. Multifunction devices that don't explicitly report ACS support are grouped together, so we have security for multifunction devices as well.
How can you get security with insecure hardware?
So you prevent the device from writing to host memory? Cool. Now guest puts a virus on an on-card flash, the moment device is assigned to another VM it will own that, or host if it's enabled in host.
I can make up more silliness. Buggy userspace can brick the device, e.g. by damaging the internal eeprom memory, and these things were known to happen even by accident.
Simply put if you want secure userspace drivers you must be able to trust your hardware for security and the only hardware that promises you security is a VF in an SRIOV device.
Quoted 11 lines
Either single of multifunction PFs
can have an option ROM, but since there's no defined mechanism to program the ROM, we can't protect it. Secure boot actually helps us here since the ROM loaded by the host BIOS or drivers would need to verify the ROM before using it. Note that secure boot will likely close off the pci-sysfs pathuio_pciand KVM device assignment use to get resources since it allows unprotected access to the system. VFIO provides an interface where we control secure access, so should be compatible with secure boot. Thanks,Alex
IMHO all this means VFIO *works* not just for VFs. Not that it's secure.
Signature · 1 line
MST
Quoted 37 lines · Michael S. Tsirkin
Quoted 20 lines · Alex Williamson
Quoted 19 lines · Michael S. Tsirkin
Quoted 17 lines · Alex Williamson
Quoted 9 lines · Hans J. Koch
Quoted 7 lines · Andreas Hartmann
Hi Dominic,
Quoted 4 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
Quoted 5 lines
VFIO is not a "better UIO". It *requires* an IOMMU. Dominic didn't say on what CPU he's working, so it's not clear if he can use VFIO at all.
UIO is intended for general use with devices that have mappable registers and don't fit into any other subsystem. No more, no less.
VFIO is a secure UIO.
Quoted 8 lines
Quoted 4 lines
A secure UIO *for VFs*. I think that's why it's called VFIO :). Other stuff sometimes also works but no real guarantees, though VFIO tries to make sure you don't burn yourself too badly if it breaks.
We do a little better than that. Multifunction devices that don't explicitly report ACS support are grouped together, so we have security for multifunction devices as well.
How can you get security with insecure hardware?
So you prevent the device from writing to host memory? Cool. Now guest puts a virus on an on-card flash, the moment device is assigned to another VM it will own that, or host if it's enabled in host.
Quoted 7 lines
I can make up more silliness. Buggy userspace can brick the device, e.g. by damaging the internal eeprom memory, and these things were known to happen even by accident.
Simply put if you want secure userspace drivers you must be able to trust your hardware for security and the only hardware that promises you security is a VF in an SRIOV device.
Next I suppose you're going to say assigning a NIC to a guest is insecure because it could host a malicious OS that infects other systems on the network. So to clarify, by secure, I mean that users of VFIO devices don't have access to the host. The host still needs to be suspicious of any data the user might have tainted after a device is returned.
Quoted 10 lines
Quoted 10 lines
Either single of multifunction PFs
can have an option ROM, but since there's no defined mechanism to program the ROM, we can't protect it. Secure boot actually helps us here since the ROM loaded by the host BIOS or drivers would need to verify the ROM before using it. Note that secure boot will likely close off the pci-sysfs pathuio_pciand KVM device assignment use to get resources since it allows unprotected access to the system. VFIO provides an interface where we control secure access, so should be compatible with secure boot. Thanks,
Quoted 4 lines
Quoted 1 line
Alex
IMHO all this means VFIO *works* not just for VFs. Not that it's secure.
By your argument above, not even VFs are "secure". A user could just as easily taint a disk attached to an HBA VF...
Michael S. Tsirkin
maintainerQuoted 42 lines · Alex Williamson
Quoted 41 lines · Michael S. Tsirkin
Quoted 20 lines · Alex Williamson
Quoted 19 lines · Michael S. Tsirkin
Quoted 17 lines · Alex Williamson
Quoted 9 lines · Hans J. Koch
Quoted 7 lines · Andreas Hartmann
Hi Dominic,
Quoted 4 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
Quoted 5 lines
VFIO is not a "better UIO". It *requires* an IOMMU. Dominic didn't say on what CPU he's working, so it's not clear if he can use VFIO at all.
UIO is intended for general use with devices that have mappable registers and don't fit into any other subsystem. No more, no less.
VFIO is a secure UIO.
Quoted 8 lines
Quoted 4 lines
A secure UIO *for VFs*. I think that's why it's called VFIO :). Other stuff sometimes also works but no real guarantees, though VFIO tries to make sure you don't burn yourself too badly if it breaks.
We do a little better than that. Multifunction devices that don't explicitly report ACS support are grouped together, so we have security for multifunction devices as well.
How can you get security with insecure hardware?
So you prevent the device from writing to host memory? Cool. Now guest puts a virus on an on-card flash, the moment device is assigned to another VM it will own that, or host if it's enabled in host.
I can make up more silliness. Buggy userspace can brick the device, e.g. by damaging the internal eeprom memory, and these things were known to happen even by accident.
Quoted 7 lines
Quoted 3 lines
Simply put if you want secure userspace drivers you must be able to trust your hardware for security and the only hardware that promises you security is a VF in an SRIOV device.
Next I suppose you're going to say assigning a NIC to a guest is insecure because it could host a malicious OS that infects other systems on the network.
*Of course* it is less secure than a firewalled guest with a virtual NIC. You argue this is not true?
But at least there are ways to contain a NIC on a network. So it depends on the setup.
Not so for an assigned PF. It depends on the internals of the PF which you have no idea about.
Quoted 2 lines
So to clarify, by secure, I mean that users of VFIO devices don't have access to the host.
Yes. And since you can't guarantee it for PFs, it's insecure.
Quoted 2 lines
The host still needs to be
suspicious of any data the user might have tainted
That's not the only point. Host data might also leak to guest when device is assigned.
Quoted 1 line
after a device is returned.
For a VF you have a way to validate what the VF does. For a PF there is no way to be suspicious of the device state.
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Either single of multifunction PFs
can have an option ROM, but since there's no defined mechanism to program the ROM, we can't protect it. Secure boot actually helps us here since the ROM loaded by the host BIOS or drivers would need to verify the ROM before using it. Note that secure boot will likely close off the pci-sysfs pathuio_pciand KVM device assignment use to get resources since it allows unprotected access to the system. VFIO provides an interface where we control secure access, so should be compatible with secure boot. Thanks,
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Alex
IMHO all this means VFIO *works* not just for VFs. Not that it's secure.
By your argument above, not even VFs are "secure".
VFs can be secure if PF hardware and driver are secure.
There's no sure way to secure a PF.
Quoted 2 lines
A user could just as
easily taint a disk attached to an HBA VF...
But if I don't run stuff from this disk I am safe.
What is the way to guarantee security with an assigned PF?
Signature · 1 line
MST
Michael S. Tsirkin
maintainerQuoted 38 lines · Alex Williamson
Quoted 37 lines · Michael S. Tsirkin
Quoted 20 lines · Alex Williamson
Quoted 19 lines · Michael S. Tsirkin
Quoted 17 lines · Alex Williamson
Quoted 9 lines · Hans J. Koch
Quoted 7 lines · Andreas Hartmann
Hi Dominic,
Quoted 4 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
Quoted 5 lines
VFIO is not a "better UIO". It *requires* an IOMMU. Dominic didn't say on what CPU he's working, so it's not clear if he can use VFIO at all.
UIO is intended for general use with devices that have mappable registers and don't fit into any other subsystem. No more, no less.
VFIO is a secure UIO.
Quoted 8 lines
Quoted 4 lines
A secure UIO *for VFs*. I think that's why it's called VFIO :). Other stuff sometimes also works but no real guarantees, though VFIO tries to make sure you don't burn yourself too badly if it breaks.
We do a little better than that. Multifunction devices that don't explicitly report ACS support are grouped together, so we have security for multifunction devices as well.
How can you get security with insecure hardware?
So you prevent the device from writing to host memory? Cool. Now guest puts a virus on an on-card flash, the moment device is assigned to another VM it will own that, or host if it's enabled in host.
Quoted 7 lines
Quoted 7 lines
I can make up more silliness. Buggy userspace can brick the device, e.g. by damaging the internal eeprom memory, and these things were known to happen even by accident.
Simply put if you want secure userspace drivers you must be able to trust your hardware for security and the only hardware that promises you security is a VF in an SRIOV device.
One thing I stand corrected on: assigning a PF that does DMA with VFIO *might* be secure, and sometimes, maybe often, is.
There's just no way to make sure.
This is unlike uio_pci_generic where it would always be insecure.
Signature · 1 line
MST
Quoted 5 lines · Michael S. Tsirkin
One thing I stand corrected on: assigning a PF that does DMA with VFIO *might* be secure, and sometimes, maybe often, is.
There's just no way to make sure.
This is unlikeuio_pci_genericwhere it would always be insecure.
You need to be root to access a UIO device, and if you're root, you can compromise a system in many ways. Before UIO, people used /dev/mem for similar purposes, and UIO is certainly a seccurity improvement over that.
But of course, UIO presents security risks. Like many other things below /dev, you need to know what you're doing, and who gets access to /dev/uioX.
Thanks,
Hans
Michael S. Tsirkin
maintainerQuoted 11 lines · Hans J. Koch
Quoted 5 lines · Michael S. Tsirkin
One thing I stand corrected on: assigning a PF that does DMA with VFIO *might* be secure, and sometimes, maybe often, is.
There's just no way to make sure.
This is unlikeuio_pci_genericwhere it would always be insecure.You need to be root to access a UIO device, and if you're root, you can compromise a system in many ways. Before UIO, people used
/dev/memfor similar purposes, and UIO is certainly a seccurity improvement over that.
Quoted 5 lines
But of course, UIO presents security risks. Like many other things below /dev, you need to know what you're doing, and who gets access to
/dev/uioX.Thanks,
Hans
Sorry I might not have explained myself clearly. uio_pci_generic would be insecure if used with a device doing DMA. I am not speaking about UIO in general at all.
Signature · 1 line
MST
Quoted 15 lines · Michael S. Tsirkin
Quoted 14 lines · Hans J. Koch
Quoted 5 lines · Michael S. Tsirkin
One thing I stand corrected on: assigning a PF that does DMA with VFIO *might* be secure, and sometimes, maybe often, is.
There's just no way to make sure.
This is unlikeuio_pci_genericwhere it would always be insecure.You need to be root to access a UIO device, and if you're root, you can compromise a system in many ways. Before UIO, people used
/dev/memfor similar purposes, and UIO is certainly a seccurity improvement over that.But of course, UIO presents security risks. Like many other things below /dev, you need to know what you're doing, and who gets access to
/dev/uioX.
Quoted 6 lines
Quoted 2 lines
Thanks,
HansSorry I might not have explained myself clearly.
uio_pci_genericwould be insecure if used with a device doing DMA. I am not speaking about UIO in general at all.
Oh, I do. There are many more risks than just DMA. I come from the embedded systems world, and there it is not uncommon that some strange device can simply turn the power off of some of your chips or even the whole system if programmed properly. And there are a lot of things that might be fine from the kernel's point of view, but render the system unusable from a user's point of view.
UIO is a very thin layer on top of strange hardware. It just fills a gap for a certain class of devices that don't fit in anywhere else. Although I'm glad if somebody posts his UIO driver, I'm even more glad if another subsystem (IIO, VFIO) can be found for the damn chip ;-)
Thanks,
Hans
Quoted 16 lines · Alex Williamson
Quoted 9 lines · Hans J. Koch
Quoted 7 lines · Andreas Hartmann
Hi Dominic,
Quoted 4 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
Quoted 3 lines
Yes, thanks Jan. This is exactly what VFIO does. VFIO provides secure config space access, resource access, DMA mapping services, and full interrupt support to userspace.
Quoted 5 lines
VFIO is not a "better UIO". It *requires* an IOMMU. Dominic didn't say on what CPU he's working, so it's not clear if he can use VFIO at all.
UIO is intended for general use with devices that have mappable registers and don't fit into any other subsystem. No more, no less.
Quoted 7 lines
VFIO is a secure UIO. An IOMMU is required for any model that allows a user to program DMA of a device, whether VFIO or something else. As Michael noted, if we allow UIO PCI to enable bus master, we open ourselves up to a whole world of problems with a user suddenly having access to and DMA'able memory in the system. So really, add that the device must only use PIO to your list of requirements for UIO, which rules out any kind of high speed device. Thanks,
You're living in a completely different world. If you want to discuss UIO, it would be best if you forget PCI, DMA, and virtualization first.
UIO was introduced to handle crazy hardware that doesn't fit into an existing subsystem, e.g. a customer specific FPGA. Before UIO, people wrote kernel char device drivers with 5000+ lines of code for that. They controlled their device with 50 new ioctls, the driver was buggy and could never make it to mainline.
With UIO, they write a very simple kernel driver, typically around 150 LOC. The rest can be done in userspace. That's the purpose of UIO. And since strange devices like the FPGA mentioned above are mainly found in Embedded Devices, it's absolutely neccesary that UIO works on ANY CPU, including ARM, MIPS, older x86 like LX800, or PowerPC. Not just some shiny new Core i7 with IOMMU. And it's not only for PCI, most of the drivers use platform devices and the chips are directly attached to some SoC bus.
And one last word about DMA: The reason the UIO core doesn't support DMA is simply that so far nobody wanted it. In five years, I never had a device on my desk where a UIO driver needed DMA support by the core. On the other hand, I met several people who said "Bad thing that UIO doesn't support DMA". When I asked what they're about to do, everyone answered "Oh well, I don't need DMA, but UIO should support it, don't you think so?". So much for that.
One fine day some time ago (2010?), somebody came up with a patch set that attempted to change half the UIO core. In the resulting discussion, we quickly found out that he had completly different requirements for a completely different purpose. The result of that insight was the birth of VFIO. A new subsystem for new requirements.
If you say "VFIO is a secure UIO", then please post a replacement for uio_pdrv_genirq.c that runs on an ARM9 and offers the advantages you're talking about. If that is impossible, please stop comparing VFIO with UIO.
Thanks,
Hans
Am Freitag, den 08.06.2012, 16:03 +0300 schrieb Michael S. Tsirkin:
Quoted 5 lines
Why is this needed?
What's wrong with mapping resources through
/sys/bus/pci/devices/XXXXXresourceX
?
Mmmh ok, the problem here is, that the UIO documentation states:
"/dev/uioX is used to access the address space of the card. Just use mmap() to access registers or RAM locations of your card."
and
"From userspace, the different mappings are distinguished by adjusting the offset parameter of the mmap() call."
This does not work and the "/sys/class/uio/uioX/maps/mapX/" directories do also not appear. I was very confused when I tried UIO the first time and it did not behave like it is described in the documentation.
Anyway, the patch adds the described behaviour and I would more than happy, if it would be accepted for the mainline kernel.
Quoted 10 lines
uio currently only supports devices which do not do DMA.
DMA from uio controlled devices is a no no unless it's behind an IOMMU which can protect us from random memory corruptions this could cause.
In the later case it's OK but we need some code to check this and program the IOMMU appropriately.
OK, I see. Might it be OK if I just remove the DMA stuff from this patch and resubmit it?
Signature · 9 lines
Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 69 79844114
Quoted 11 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 16:03 +0300 schrieb Michael S. Tsirkin:
Quoted 5 lines
Why is this needed?
What's wrong with mapping resources through
/sys/bus/pci/devices/XXXXXresourceX
?Mmmh ok, the problem here is, that the UIO documentation states:
"
/dev/uioXis used to access the address space of the card. Just usemmap()to access registers or RAM locations of your card."
Quoted 8 lines
and
"From userspace, the different mappings are distinguished by adjusting the offset parameter of the
mmap()call."This does not work and the "
/sys/class/uio/uioX/maps/mapX/" directories do also not appear.
Then there's something fundamentally wrong in your driver. Check the return value of uio_register_device().
Quoted 2 lines
I was very confused when I tried UIO the first time and it did not behave like it is described in the documentation.
UIO is the mainline since 2007, and I can assure you it works like described. Lots of people use it.
Try and fix your driver, then post it. If you absolutely don't get it working, post a non-working version for review.
Thanks,
Hans
Am Freitag, den 08.06.2012, 17:18 +0200 schrieb Hans J. Koch:
Quoted 3 lines
Then there's something fundamentally wrong in your driver. Check the return value of
uio_register_device().
I'm talking about the uio_pci_generic module, not about UIO in general.
Quoted 6 lines
Quoted 2 lines
I was very confused when I tried UIO the first time and it did not behave like it is described in the documentation.
UIO is the mainline since 2007, and I can assure you it works like described. Lots of people use it.
I know and I really don't have a problem with UIO itself. The uio_pci_generic module does not export correctly, maybe you take a look on its code? My guess is, that the uio_pci_generic module isn't commonly used and therefore the problem wasn't reported before.
Signature · 9 lines
Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 (0)
Quoted 9 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 17:18 +0200 schrieb Hans J. Koch:
Quoted 3 lines
Then there's something fundamentally wrong in your driver. Check the return value of
uio_register_device().I'm talking about the
uio_pci_genericmodule, not about UIO in general.Quoted 3 lines
Quoted 2 lines
I was very confused when I tried UIO the first time and it did not behave like it is described in the documentation.
Quoted 7 lines
Quoted 3 lines
UIO is the mainline since 2007, and I can assure you it works like described. Lots of people use it.
I know and I really don't have a problem with UIO itself. The
uio_pci_genericmodule does not export correctly, maybe you take a look on its code? My guess is, that theuio_pci_genericmodule isn't commonly used and therefore the problem wasn't reported before.
As Michael already said, the intention was to map BAR ressources using /sys/bus/pci/devices/...
OK, I agree, that's the standard PCI way of doing this, not the standard UIO way.
What problem do you have with this approach?
Thanks,
Hans
Am Freitag, den 08.06.2012, 17:57 +0200 schrieb Hans J. Koch:
Quoted 1 line
What problem do you have with this approach?
Nothing itself. There is obviously a documentation issue, which really confuses people who are new to that stuff. I thought that fixing it, by just adding a view lines of code (really a view, when I remove the DMA masque part) might enhance this particular module.
Anyway, what is wrong when the uio_pci_generic module works like described in the related documentation?
Signature · 9 lines
Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 (0)
Quoted 5 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 17:57 +0200 schrieb Hans J. Koch:
Quoted 1 line
What problem do you have with this approach?
Nothing itself. There is obviously a documentation issue, which really confuses people who are new to that stuff.
Well, the documentation says "Userspace driver can use pci sysfs interface, or the libpci libray that wraps it, to talk to the device..."
I agree that this is not very clear, especially since the example code there doesn't use this.
Quoted 3 lines
I thought that fixing it, by
just adding a view lines of code (really a view, when I remove the DMA masque part) might enhance this particular module.
I'm not sure if talking to the device the UIO way can confuse the PCI subsystem. Are you? Any hints from somebody else?
Unfortunately, I can't remember why we agreed to do it the way it is...
Quoted 3 lines
Anyway, what is wrong when the
uio_pci_genericmodule works like described in the related documentation?
It does, the documentation is just not very explicit when it comes to memory mapping.
Thanks,
Hans
Am Freitag, den 08.06.2012, 18:37 +0200 schrieb Hans J. Koch:
Quoted 2 lines
It does, the documentation is just not very explicit when it comes to memory mapping.
OK, quoting the other mail:
Am Freitag, den 08.06.2012, 19:39 +0300 schrieb Michael S. Tsirkin:
Quoted 5 lines
Documentation says this:
Userspace driver can use pci sysfs interface, or the libpci libray that wraps it, to talk to the device and to re-enable interrupts by writing to the command register.
I think should ditch the discussion about the patch and I'm going to revise the documentation? I read this part a couple of times and allways thought the "sysfs interface" refers to the UIO sysfs entry.
Therefore, would it be OK if I send a patch for the documentation next week?
Signature · 9 lines
Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 (0)
Quoted 10 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 18:37 +0200 schrieb Hans J. Koch:
Quoted 2 lines
It does, the documentation is just not very explicit when it comes to memory mapping.
OK, quoting the other mail:
Am Freitag, den 08.06.2012, 19:39 +0300 schrieb Michael S. Tsirkin:
Quoted 2 lines
Documentation says this:
Quoted 7 lines
Quoted 3 lines
Userspace driver can use pci sysfs interface, or the libpci libray that wraps it, to talk to the device and to re-enable interrupts by writing to the command register.
I think should ditch the discussion about the patch and I'm going to revise the documentation? I read this part a couple of times and allways thought the "sysfs interface" refers to the UIO sysfs entry.
Ah OK, it's easy to misunderstand that.
Quoted 3 lines
Therefore, would it be OK if I send a patch for the documentation next week?
Sure, that would be great!
Thanks,
Hans
Michael S. Tsirkin
maintainerQuoted 10 lines · Dominic Eschweiler
Am Freitag, den 08.06.2012, 17:57 +0200 schrieb Hans J. Koch:
Quoted 1 line
What problem do you have with this approach?
Nothing itself. There is obviously a documentation issue, which really confuses people who are new to that stuff. I thought that fixing it, by just adding a view lines of code (really a view, when I remove the DMA masque part) might enhance this particular module.
Anyway, what is wrong when the
uio_pci_genericmodule works like described in the related documentation?
Documentation says this:
Userspace driver can use pci sysfs interface, or the libpci libray that wraps it, to talk to the device and to re-enable interrupts by writing to the command register.
maybe we should make the point that *all* interaction with the device should be done via sysfs, explicit.
Quoted 10 lines
Signature · 9 lines
Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 (0)
Quoted 65 lines · Dominic Eschweiler
Hello,
the current version of the
uio_pci_genericmodule does not export memory resources, such as BARs. As far as I can see, the related module does only support interrupts, which is not really useful. My suggestion in this case would be to either fix this issue, or to completely remove it. The latter would not be an option for us, since we need this functionality at some stuff at CERN.Therefore, here is a patch that fixes the issue. Please give me further advice, since I'm doing this the first time ...
Signed-off-by Dominic Eschweiler <[email protected]>---
diff -uNrlinux-3.4_old/drivers/uio/uio_pci_generic.clinux-3.4_new/drivers/uio/uio_pci_generic.c
---linux-3.4_old/drivers/uio/uio_pci_generic.c2012-05-21
00:29:13.000000000 +0200
+++linux-3.4_new/drivers/uio/uio_pci_generic.c2012-06-08
13:01:12.000000000 +0200drivers/uio/uio_pci_generic.c +21
-1@@ -25,10 +25,12 @@#include <linux/slab.h> #include <linux/uio_driver.h> -#define DRIVER_VERSION "0.01.0" +#define DRIVER_VERSION "0.02.0"#define DRIVER_AUTHOR "Michael S. Tsirkin <[email protected]>"#define DRIVER_DESC "Generic UIO driver for PCI 2.3 devices" +#define DRV_NAME "uio_pci_generic" +struct uio_pci_generic_dev {struct uio_info info;struct pci_dev *pdev;@@ -58,6 +60,7 @@ {struct uio_pci_generic_dev *gdev;int err; +int i; err =pci_enable_device (pdev);if (err) {@@ -66,9 +69,33 @@return err; } +/* set master */ +pci_set_master (pdev); + +/* set DMA mask */ + err =pci_set_dma_mask (pdev,DMA_BIT_MASK (64 )); +if (err) { +dev_warn (&pdev->dev,"Warning: couldn't set 64-bit PCI DMA mask.\n" ); + err =pci_set_dma_mask (pdev,DMA_BIT_MASK (32 )); +if (err) { +dev_err (&pdev->dev,"Can't set PCI DMA mask, aborting\n" ); +return -ENODEV ; + } + } + +/* set consistent DMA mask */ + err =pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (64 )); +if (err) {
drivers/uio/uio_pci_generic.c line 98
Quoted 10 lines
drivers/uio/uio_pci_generic.c +8
-0+ dev_warn(&pdev->dev, "Warning: couldn't set 64-bit consistent PCI DMA mask.\n"); + err =pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (32 )); +if (err) { +dev_err (&pdev->dev,"Can't set consistent PCI DMA mask, aborting \n" ); +return -ENODEV ; + } + } +
All this DMA stuff doesn't fit into a "uio_pci_generic" driver. There might be users who need other kinds of DMA handling.
If you need this, please make it a new driver and name it after your device.
Quoted 19 lines
drivers/uio/uio_pci_generic.c +1
-2if (!pdev->irq) { - dev_warn(&pdev->dev, "No IRQ assigned to device: " - "no support for interrupts?\n"); + dev_warn(&pdev->dev, "No IRQ assigned to device: no support for interrupts?\n");drivers/uio/uio_pci_generic.c +7
-0pci_disable_device (pdev);return -ENODEV ; }@@ -91,10 +118,40 @@ gdev->info.handler = irqhandler; gdev->pdev = pdev; +/* request regions */ + err =pci_request_regions (pdev,DRV_NAME ); +if (err) { +dev_err (&pdev->dev,"Couldn't get PCI resources, aborting\n" ); +return err; + } +
drivers/uio/uio_pci_generic.c line 137
Quoted 10 lines
drivers/uio/uio_pci_generic.c +10
-0+/* create attributes for BAR mappings */ +for (i =0 ; i <PCI_NUM_RESOURCES ; i++) { +if (pdev->resource[i].flags && + (pdev->resource[i].flags &IORESOURCE_IO )) { + gdev->info.port[i].size =0 ; + gdev->info.port[i].porttype =UIO_PORT_OTHER ; +#ifdef CONFIG_X86 + gdev->info.port[i].porttype =UIO_PORT_X86 ; +#endif + }
Do you really have x86 ports on your PCI card?
drivers/uio/uio_pci_generic.c line 146
Quoted 9 lines
drivers/uio/uio_pci_generic.c +9
-0+ +if (pdev->resource[i].flags && + (pdev->resource[i].flags &IORESOURCE_MEM )) { + gdev->info.mem[i].addr =pci_resource_start (pdev, i); + gdev->info.mem[i].size =pci_resource_len (pdev, i); + gdev->info.mem[i].internal_addr =NULL ; + gdev->info.mem[i].memtype =UIO_MEM_PHYS ; + } + }
As Michael said, why don't you just use the existing PCI sysfs files?
I don't really object to your approach, but I'd like to hear a reason why you can't live with the existing possibilities.
Thanks,
Hans