LinuxLists.cc - [PATCH] uio_pci_generic does not export memory resources

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, Jun 08, 2012 at 01:56:56PM +0200, Dominic Eschweiler wrote:
> 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 ...

Why is this needed?
What's wrong with mapping resources through
/sys/bus/pci/devices/XXXXXresourceX
?

>
> Signed-off-by: Dominic Eschweiler <[email protected]>
> ---
> 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));

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.

> + 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)
>
>
> --
> Gru?
> Dominic
>
> Frankfurt Institute for Advanced Studies (FIAS)
> Ruth-Moufang-Stra?e 1
> D-60438 Frankfurt am Main
> Germany
>
> Phone: +49 69 79844114

2012-06-08 13:17:09

by Jan Kiszka

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On 2012-06-08 15:03, Michael S. Tsirkin wrote:
>> + /* 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.

AKA: VFIO. :)

Dominic, maybe you want to have a look at Alex's work:
https://github.com/awilliam/linux-vfio

Jan

--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

2012-06-08 14:16:59

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, 2012-06-08 at 15:16 +0200, Jan Kiszka wrote:
> On 2012-06-08 15:03, Michael S. Tsirkin wrote:
> >> + /* 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.
>
> 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

2012-06-08 14:29:04

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

Am Freitag, den 08.06.2012, 16:03 +0300 schrieb Michael S. Tsirkin:
> 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.
>
> 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?

--
Gruß
Dominic

Frankfurt Institute for Advanced Studies (FIAS)
Ruth-Moufang-Straße 1
D-60438 Frankfurt am Main
Germany

Phone: +49 69 79844114

2012-06-08 14:47:52

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
> 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 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.

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

--
Gruß
Dominic

Frankfurt Institute for Advanced Studies (FIAS)
Ruth-Moufang-Straße 1
D-60438 Frankfurt am Main
Germany

Phone: +49 69 79844114

2012-06-08 15:07:04

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, 2012-06-08 at 16:47 +0200, Dominic Eschweiler wrote:
> Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
> > 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.

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

2012-06-08 15:18:56

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, Jun 08, 2012 at 04:28:58PM +0200, Dominic Eschweiler wrote:
> Am Freitag, den 08.06.2012, 16:03 +0300 schrieb Michael S. Tsirkin:
> > 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.

Then there's something fundamentally wrong in your driver. Check the return
value of uio_register_device().

> 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

2012-06-08 15:44:57

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

Am Freitag, den 08.06.2012, 17:18 +0200 schrieb Hans J. Koch:
> 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.

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

--
Gruß
Dominic

Frankfurt Institute for Advanced Studies (FIAS)
Ruth-Moufang-Straße 1
D-60438 Frankfurt am Main
Germany

Phone: +49 (0)

2012-06-08 15:57:41

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, Jun 08, 2012 at 05:45:12PM +0200, Dominic Eschweiler wrote:
> Am Freitag, den 08.06.2012, 17:18 +0200 schrieb Hans J. Koch:
> > 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.
>
> > > 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.

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

2012-06-08 16:08:09

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, Jun 08, 2012 at 01:56:56PM +0200, Dominic Eschweiler wrote:
> 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 ...
>
>
>
> Signed-off-by: Dominic Eschweiler <[email protected]>
> ---
> 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;
> + }
> + }
> +

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.

> 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
> + }

Do you really have x86 ports on your PCI card?

> +
> + 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

2012-06-08 16:19:02

by Andreas Hartmann

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

Hi Dominic,

Dominic Eschweiler wrote:
> Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
>> 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

2012-06-08 16:23:04

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

Am Freitag, den 08.06.2012, 17:57 +0200 schrieb Hans J. Koch:
> 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?

--
Gruß
Dominic

Frankfurt Institute for Advanced Studies (FIAS)
Ruth-Moufang-Straße 1
D-60438 Frankfurt am Main
Germany

Phone: +49 (0)

2012-06-08 16:37:41

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, Jun 08, 2012 at 06:23:19PM +0200, Dominic Eschweiler wrote:
> Am Freitag, den 08.06.2012, 17:57 +0200 schrieb Hans J. Koch:
> > 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.

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

>
> Anyway, what is wrong when the uio_pci_generic module works like
> described in the related documentation?

It does, the documentation is just not very explicit when it comes to
memory mapping.

Thanks,
Hans

2012-06-08 16:39:59

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, Jun 08, 2012 at 06:23:19PM +0200, Dominic Eschweiler wrote:
> Am Freitag, den 08.06.2012, 17:57 +0200 schrieb Hans J. Koch:
> > 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?

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.

> --
> Gru?
> Dominic
>
> Frankfurt Institute for Advanced Studies (FIAS)
> Ruth-Moufang-Stra?e 1
> D-60438 Frankfurt am Main
> Germany
>
> Phone: +49 (0)

2012-06-08 16:42:06

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, 2012-06-08 at 18:16 +0200, Andreas Hartmann wrote:
> Hi Dominic,
>
> Dominic Eschweiler wrote:
> > Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
> >> 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.

Oh, please post the patch for libvirt!

> 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:

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index ab0dba0..5c26804 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -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

2012-06-08 16:44:40

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, Jun 08, 2012 at 06:16:18PM +0200, Andreas Hartmann wrote:
> Hi Dominic,
>
> Dominic Eschweiler wrote:
> > Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
> >> 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

2012-06-08 16:59:25

by Jan Kiszka

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On 2012-06-08 18:44, Hans J. Koch wrote:
> On Fri, Jun 08, 2012 at 06:16:18PM +0200, Andreas Hartmann wrote:
>> Hi Dominic,
>>
>> Dominic Eschweiler wrote:
>>> Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
>>>> 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.

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

--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

2012-06-08 17:07:30

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

Am Freitag, den 08.06.2012, 18:37 +0200 schrieb Hans J. Koch:
> 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:

> 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?

--
Gruß
Dominic

Frankfurt Institute for Advanced Studies (FIAS)
Ruth-Moufang-Straße 1
D-60438 Frankfurt am Main
Germany

Phone: +49 (0)

2012-06-08 17:11:34

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, 2012-06-08 at 18:44 +0200, Hans J. Koch wrote:
> On Fri, Jun 08, 2012 at 06:16:18PM +0200, Andreas Hartmann wrote:
> > Hi Dominic,
> >
> > Dominic Eschweiler wrote:
> > > Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
> > >> 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.

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

2012-06-08 17:11:43

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, Jun 08, 2012 at 07:07:46PM +0200, Dominic Eschweiler wrote:
> Am Freitag, den 08.06.2012, 18:37 +0200 schrieb Hans J. Koch:
> > 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:
>
> > 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.

Ah OK, it's easy to misunderstand that.

>
> Therefore, would it be OK if I send a patch for the documentation next
> week?

Sure, that would be great!

Thanks,
Hans

2012-06-10 14:17:45

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

2012-06-10 16:09:42

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Sun, 2012-06-10 at 17:18 +0300, Michael S. Tsirkin wrote:
> On Fri, Jun 08, 2012 at 11:11:16AM -0600, Alex Williamson wrote:
> > On Fri, 2012-06-08 at 18:44 +0200, Hans J. Koch wrote:
> > > On Fri, Jun 08, 2012 at 06:16:18PM +0200, Andreas Hartmann wrote:
> > > > Hi Dominic,
> > > >
> > > > Dominic Eschweiler wrote:
> > > > > Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
> > > > >> 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.
> >
> > 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

2012-06-10 16:44:13

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Sun, Jun 10, 2012 at 10:09:26AM -0600, Alex Williamson wrote:
> On Sun, 2012-06-10 at 17:18 +0300, Michael S. Tsirkin wrote:
> > On Fri, Jun 08, 2012 at 11:11:16AM -0600, Alex Williamson wrote:
> > > On Fri, 2012-06-08 at 18:44 +0200, Hans J. Koch wrote:
> > > > On Fri, Jun 08, 2012 at 06:16:18PM +0200, Andreas Hartmann wrote:
> > > > > Hi Dominic,
> > > > >
> > > > > Dominic Eschweiler wrote:
> > > > > > Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
> > > > > >> 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.
> > >
> > > 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.

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.

> 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

IMHO all this means VFIO *works* not just for VFs.
Not that it's secure.

--
MST

2012-06-10 17:38:44

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Sun, 2012-06-10 at 19:44 +0300, Michael S. Tsirkin wrote:
> On Sun, Jun 10, 2012 at 10:09:26AM -0600, Alex Williamson wrote:
> > On Sun, 2012-06-10 at 17:18 +0300, Michael S. Tsirkin wrote:
> > > On Fri, Jun 08, 2012 at 11:11:16AM -0600, Alex Williamson wrote:
> > > > On Fri, 2012-06-08 at 18:44 +0200, Hans J. Koch wrote:
> > > > > On Fri, Jun 08, 2012 at 06:16:18PM +0200, Andreas Hartmann wrote:
> > > > > > Hi Dominic,
> > > > > >
> > > > > > Dominic Eschweiler wrote:
> > > > > > > Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
> > > > > > >> 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.
> > > >
> > > > 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.
>
> 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.

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.

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

2012-06-10 18:43:09

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Sun, Jun 10, 2012 at 11:38:25AM -0600, Alex Williamson wrote:
> On Sun, 2012-06-10 at 19:44 +0300, Michael S. Tsirkin wrote:
> > On Sun, Jun 10, 2012 at 10:09:26AM -0600, Alex Williamson wrote:
> > > On Sun, 2012-06-10 at 17:18 +0300, Michael S. Tsirkin wrote:
> > > > On Fri, Jun 08, 2012 at 11:11:16AM -0600, Alex Williamson wrote:
> > > > > On Fri, 2012-06-08 at 18:44 +0200, Hans J. Koch wrote:
> > > > > > On Fri, Jun 08, 2012 at 06:16:18PM +0200, Andreas Hartmann wrote:
> > > > > > > Hi Dominic,
> > > > > > >
> > > > > > > Dominic Eschweiler wrote:
> > > > > > > > Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
> > > > > > > >> 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.
> > > > >
> > > > > 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.
> >
> > 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.
>
> 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.

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

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

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

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

> 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?

--
MST

2012-06-10 19:00:52

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

2012-06-10 19:01:51

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Fri, Jun 08, 2012 at 11:11:16AM -0600, Alex Williamson wrote:
> On Fri, 2012-06-08 at 18:44 +0200, Hans J. Koch wrote:
> > On Fri, Jun 08, 2012 at 06:16:18PM +0200, Andreas Hartmann wrote:
> > > Hi Dominic,
> > >
> > > Dominic Eschweiler wrote:
> > > > Am Freitag, den 08.06.2012, 08:16 -0600 schrieb Alex Williamson:
> > > >> 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.
>
> 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

2012-06-10 19:11:46

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Sun, Jun 10, 2012 at 10:00:36PM +0300, Michael S. Tsirkin wrote:
>
> 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.

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

2012-06-10 19:16:42

[permalink] [raw]

Subject: Re: [PATCH] uio_pci_generic does not export memory resources

On Sun, Jun 10, 2012 at 09:11:30PM +0200, Hans J. Koch wrote:
> On Sun, Jun 10, 2012 at 10:00:36PM +0300, Michael S. Tsirkin wrote:
> >
> > 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.
>
> 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

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.

--
MST

2012-06-10 20:20:07