Re: UIO: missing resource mapping
Hello Hans Am Donnerstag, den 19.07.2012, 01:47 +0200 schrieb Hans J. Koch: > You'll hear from me soon, thanks for your work! Comments and reviews > from others are welcome... Is there any progress on this topic? -- Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 69 79844114 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: UIO: missing resource mapping
Hello Hans Am Donnerstag, den 19.07.2012, 01:47 +0200 schrieb Hans J. Koch: You'll hear from me soon, thanks for your work! Comments and reviews from others are welcome... Is there any progress on this topic? -- Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 69 79844114 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: UIO: missing resource mapping
Am Montag, den 16.07.2012, 23:58 +0200 schrieb Hans J. Koch:
> Try to hack up a patch to add generic BAR mapping to uio_pci_generic.c
> and post it for review.
>
Here we go ...
>
Signed-off-by: Dominic Eschweiler
diff --git a/drivers/uio/uio_pci_generic.c
b/drivers/uio/uio_pci_generic.c
index 0bd08ef..e25991e 100644
--- a/drivers/uio/uio_pci_generic.c
+++ b/drivers/uio/uio_pci_generic.c
@@ -25,10 +25,12 @@
#include
#include
-#define DRIVER_VERSION "0.01.0"
+#define DRIVER_VERSION "0.02.0"
#define DRIVER_AUTHOR "Michael S. Tsirkin "
#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 @@ static int __devinit probe(struct pci_dev *pdev,
{
struct uio_pci_generic_dev *gdev;
int err;
+ int i;
err = pci_enable_device(pdev);
if (err) {
@@ -67,8 +70,7 @@ static int __devinit probe(struct pci_dev *pdev,
}
if (!pdev->irq) {
- dev_warn(>dev, "No IRQ assigned to device: "
-"no support for interrupts?\n");
+ dev_warn(>dev, "No IRQ assigned to device: no support for
interrupts?\n");
pci_disable_device(pdev);
return -ENODEV;
}
@@ -91,10 +93,31 @@ static int __devinit probe(struct pci_dev *pdev,
gdev->info.handler = irqhandler;
gdev->pdev = pdev;
+ /* request regions */
+ err = pci_request_regions(pdev, DRV_NAME);
+ if (err) {
+ dev_err(>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_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(>dev, >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 +130,21 @@ err_verify:
static void remove(struct pci_dev *pdev)
{
struct uio_pci_generic_dev *gdev = pci_get_drvdata(pdev);
-
uio_unregister_device(>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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: UIO: missing resource mapping
Am Montag, den 16.07.2012, 23:58 +0200 schrieb Hans J. Koch:
Try to hack up a patch to add generic BAR mapping to uio_pci_generic.c
and post it for review.
Here we go ...
Signed-off-by: Dominic Eschweiler eschwei...@fias.uni-frankfurt.de
diff --git a/drivers/uio/uio_pci_generic.c
b/drivers/uio/uio_pci_generic.c
index 0bd08ef..e25991e 100644
--- a/drivers/uio/uio_pci_generic.c
+++ b/drivers/uio/uio_pci_generic.c
@@ -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 m...@redhat.com
#define DRIVER_DESCGeneric 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 @@ static int __devinit probe(struct pci_dev *pdev,
{
struct uio_pci_generic_dev *gdev;
int err;
+ int i;
err = pci_enable_device(pdev);
if (err) {
@@ -67,8 +70,7 @@ static int __devinit probe(struct pci_dev *pdev,
}
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 +93,31 @@ static int __devinit probe(struct pci_dev *pdev,
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_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 +130,21 @@ err_verify:
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: UIO: missing resource mapping
Am Freitag, den 13.07.2012, 21:19 +0300 schrieb Michael S. Tsirkin: > > UIO has the same property, doesn't it? Multiple users can > access device memory through sysfs. Indeed, that's a similar problem. I haven't tried it (yet), but this particular problem can maybe circumvented by using mmap with the MAP_PRIVATE flag. Doing so is the responsibility of the driver programmer (like Hans already said). Even if that mmap trick does not work, it is pretty much sure that a BAR is already used by another program, if a related kernel driver is loaded. In that case the kernel has a chance to avoid such BAR race conditions by not giving the possibility to map them to the userspace. Nevertheless, I'm pretty sure that the possibility via sysfs to access BARs, which are already managed by a kernel driver, opens the door for denial of service attacks. On the other hand, I'm quite a newbie on this topic and maybe I don't see the big picture here. Therefore it is up to you guys to make the right decision (if needed). -- Gruß Dominic -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: UIO: missing resource mapping
Am Freitag, den 13.07.2012, 21:19 +0300 schrieb Michael S. Tsirkin: UIO has the same property, doesn't it? Multiple users can access device memory through sysfs. Indeed, that's a similar problem. I haven't tried it (yet), but this particular problem can maybe circumvented by using mmap with the MAP_PRIVATE flag. Doing so is the responsibility of the driver programmer (like Hans already said). Even if that mmap trick does not work, it is pretty much sure that a BAR is already used by another program, if a related kernel driver is loaded. In that case the kernel has a chance to avoid such BAR race conditions by not giving the possibility to map them to the userspace. Nevertheless, I'm pretty sure that the possibility via sysfs to access BARs, which are already managed by a kernel driver, opens the door for denial of service attacks. On the other hand, I'm quite a newbie on this topic and maybe I don't see the big picture here. Therefore it is up to you guys to make the right decision (if needed). -- Gruß Dominic -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: UIO: missing resource mapping
Am Freitag, den 13.07.2012, 16:18 +0200 schrieb Hans J. Koch: > If somebody maps the card's memory through the UIO driver and somebody > else also maps it using sysfs, that is possible. What happens if both > write to the same hardware registers is a different topic. Writing a > driver implies some responsibility, even if it's done in userspace. That's true and I would not complain. I'm just saying that there is the possibility to disturb a kernel driver by mapping memory etc. I think it is worth to considerate moving the BAR mapping code to UIO. Especially, when I look at the discussions about what effects mappable DMA memory can have on the system security. -- Gruß Dominic -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: UIO: missing resource mapping
Am Freitag, den 13.07.2012, 16:22 +0300 schrieb Michael S. Tsirkin: > Could you give an example of the problem? How do you bind > both UIO and another driver to the same device? Sorry, I'm looking on it from the user-space perspective. Maybe I'm wrong, but I can give you an example : lspci -v ... 03:00.0 Network controller: Broadcom Corporation BCM4331 802.11a/b/g/n (rev 02) Subsystem: Apple Inc. AirPort Extreme Flags: bus master, fast devsel, latency 0, IRQ 17 Memory at b060 (64-bit, non-prefetchable) [size=16K] Capabilities: [40] Power Management version 3 Capabilities: [58] Vendor Specific Information: Len=78 Capabilities: [48] MSI: Enable- Count=1/1 Maskable- 64bit+ Capabilities: [d0] Express Endpoint, MSI 00 Capabilities: [100] Advanced Error Reporting Capabilities: [13c] Virtual Channel Capabilities: [160] Device Serial Number 85-f2-6d-ff-ff-42-68-a8 Capabilities: [16c] Power Budgeting Kernel driver in use: bcma-pci-bridge ... This Device has one 64 Bit Bar. When I look at the related sysfs entry ... ls /sys/bus/pci/devices/:03:00.0 ... --w--- 1 root root 4.0K Jul 13 16:35 reset -r--r--r-- 1 root root 4.0K Jul 12 21:43 resource -rw--- 1 root root 16K Jul 13 16:35 resource0 lrwxrwxrwx 1 root root0 Jul 12 23:41 subsystem -> ../../../../bus/pci ... ... I can see that it should be possible to map resource0 and directly write into a BAR which is already managed by a kernel drivers. Moving this functionality to UIO would only generate those resource files, if the device is handled by UIO and therefore intended to be managed from the user-space. -- Gruß Dominic -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: UIO: missing resource mapping
Am Freitag, den 13.07.2012, 02:16 +0300 schrieb Michael S. Tsirkin: > My concern was people will ask for more and more stuff that pci > sysfs already has. > If we do add these is there a way to not duplicate code from pci? I have some concerns about the placing for the BAR mapping code inside the kernel. The point is, that sysfs currently makes it possible to map BARs of all card which are handled by any driver. This is fine in case of UIO, because it is intended that a user-space program maps BARs, but it is also possible to map BARs that are already handle by a kernel driver. It i therefore possible to jam the system by confusing sysfs entries. I don't know which implications this has, but I would move the BAR mapping capabilities completely to UIO. This should ensure that only BARs can be mapped, which are handled by UIO and no other kernel-space driver. -- Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 69 79844114 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: UIO: missing resource mapping
Am Freitag, den 13.07.2012, 02:16 +0300 schrieb Michael S. Tsirkin: My concern was people will ask for more and more stuff that pci sysfs already has. If we do add these is there a way to not duplicate code from pci? I have some concerns about the placing for the BAR mapping code inside the kernel. The point is, that sysfs currently makes it possible to map BARs of all card which are handled by any driver. This is fine in case of UIO, because it is intended that a user-space program maps BARs, but it is also possible to map BARs that are already handle by a kernel driver. It i therefore possible to jam the system by confusing sysfs entries. I don't know which implications this has, but I would move the BAR mapping capabilities completely to UIO. This should ensure that only BARs can be mapped, which are handled by UIO and no other kernel-space driver. -- Gruß Dominic Frankfurt Institute for Advanced Studies (FIAS) Ruth-Moufang-Straße 1 D-60438 Frankfurt am Main Germany Phone: +49 69 79844114 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: UIO: missing resource mapping
Am Freitag, den 13.07.2012, 16:22 +0300 schrieb Michael S. Tsirkin: Could you give an example of the problem? How do you bind both UIO and another driver to the same device? Sorry, I'm looking on it from the user-space perspective. Maybe I'm wrong, but I can give you an example : lspci -v ... 03:00.0 Network controller: Broadcom Corporation BCM4331 802.11a/b/g/n (rev 02) Subsystem: Apple Inc. AirPort Extreme Flags: bus master, fast devsel, latency 0, IRQ 17 Memory at b060 (64-bit, non-prefetchable) [size=16K] Capabilities: [40] Power Management version 3 Capabilities: [58] Vendor Specific Information: Len=78 ? Capabilities: [48] MSI: Enable- Count=1/1 Maskable- 64bit+ Capabilities: [d0] Express Endpoint, MSI 00 Capabilities: [100] Advanced Error Reporting Capabilities: [13c] Virtual Channel Capabilities: [160] Device Serial Number 85-f2-6d-ff-ff-42-68-a8 Capabilities: [16c] Power Budgeting ? Kernel driver in use: bcma-pci-bridge ... This Device has one 64 Bit Bar. When I look at the related sysfs entry ... ls /sys/bus/pci/devices/:03:00.0 ... --w--- 1 root root 4.0K Jul 13 16:35 reset -r--r--r-- 1 root root 4.0K Jul 12 21:43 resource -rw--- 1 root root 16K Jul 13 16:35 resource0 lrwxrwxrwx 1 root root0 Jul 12 23:41 subsystem - ../../../../bus/pci ... ... I can see that it should be possible to map resource0 and directly write into a BAR which is already managed by a kernel drivers. Moving this functionality to UIO would only generate those resource files, if the device is handled by UIO and therefore intended to be managed from the user-space. -- Gruß Dominic -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: UIO: missing resource mapping
Am Freitag, den 13.07.2012, 16:18 +0200 schrieb Hans J. Koch: If somebody maps the card's memory through the UIO driver and somebody else also maps it using sysfs, that is possible. What happens if both write to the same hardware registers is a different topic. Writing a driver implies some responsibility, even if it's done in userspace. That's true and I would not complain. I'm just saying that there is the possibility to disturb a kernel driver by mapping memory etc. I think it is worth to considerate moving the BAR mapping code to UIO. Especially, when I look at the discussions about what effects mappable DMA memory can have on the system security. -- Gruß Dominic -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
