commit b41d6cf38e27 (PCI: Check dynids driver_data value for validity)
requires all drivers to include an id table to try and match
driver_data. Before validating driver_data check driver has an id
table.
Cc: Jean Delvare <[email protected]>
Cc: Milton Miller <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
---
drivers/pci/pci-driver.c | 20 +++++++++++---------
1 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index b4cdd69..0a5edbe 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -48,7 +48,7 @@ store_new_id(struct device_driver *driver, const char *buf, size_t count)
subdevice=PCI_ANY_ID, class=0, class_mask=0;
unsigned long driver_data=0;
int fields=0;
- int retval;
+ int retval=0;
fields = sscanf(buf, "%x %x %x %x %x %x %lx",
&vendor, &device, &subvendor, &subdevice,
@@ -58,16 +58,18 @@ store_new_id(struct device_driver *driver, const char *buf, size_t count)
/* Only accept driver_data values that match an existing id_table
entry */
- retval = -EINVAL;
- while (ids->vendor || ids->subvendor || ids->class_mask) {
- if (driver_data == ids->driver_data) {
- retval = 0;
- break;
+ if (ids) {
+ retval = -EINVAL;
+ while (ids->vendor || ids->subvendor || ids->class_mask) {
+ if (driver_data == ids->driver_data) {
+ retval = 0;
+ break;
+ }
+ ids++;
}
- ids++;
+ if (retval) /* No match */
+ return retval;
}
- if (retval) /* No match */
- return retval;
dynid = kzalloc(sizeof(*dynid), GFP_KERNEL);
if (!dynid)
--
1.6.0.3
When doing device assignment with KVM there's currently nothing to
protect the device from having a driver in the host as well as the guest.
This trivial module just binds the pci device on the host to a stub
driver so that a real host driver can't bind to the device. It has no
pci id table, it supports only dynamic ids.
# echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/new_id
# echo -n 0000:00:19.0 > /sys/bus/pci/drivers/e1000e/unbind
# echo -n 0000:00:19.0 > /sys/bus/pci/drivers/pci-stub/bind
# ls -l /sys/bus/pci/devices/0000:00:19.0/driver
lrwxrwxrwx 1 root root 0 2008-11-25 19:10 /sys/bus/pci/devices/0000:00:19.0/driver -> ../../../bus/pci/drivers/pci-stub
Cc: "Kay, Allen M" <[email protected]>
Cc: "Nakajima, Jun" <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
---
drivers/pci/Kconfig | 9 +++++++++
drivers/pci/Makefile | 2 ++
drivers/pci/pci-stub.c | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 49 insertions(+), 0 deletions(-)
create mode 100644 drivers/pci/pci-stub.c
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index e1ca425..f6183df 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -42,6 +42,15 @@ config PCI_DEBUG
When in doubt, say N.
+config PCI_STUB
+ tristate "PCI Stub driver"
+ depends on PCI
+ help
+ Say Y or M here if you want be able to reserve a PCI device
+ when it is going to be assigned to a guest.
+
+ When in doubt, say N.
+
config HT_IRQ
bool "Interrupts on hypertransport devices"
default y
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index af3bfe2..3d07ce2 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -53,6 +53,8 @@ obj-$(CONFIG_HOTPLUG) += setup-bus.o
obj-$(CONFIG_PCI_SYSCALL) += syscall.o
+obj-$(CONFIG_PCI_STUB) += pci-stub.o
+
ifeq ($(CONFIG_PCI_DEBUG),y)
EXTRA_CFLAGS += -DDEBUG
endif
diff --git a/drivers/pci/pci-stub.c b/drivers/pci/pci-stub.c
new file mode 100644
index 0000000..8b19df6
--- /dev/null
+++ b/drivers/pci/pci-stub.c
@@ -0,0 +1,38 @@
+/* pci-stub - simple stub driver to reserve a pci device
+ *
+ * Copyright (C) 2008 Red Hat, Inc.
+ * Author:
+ * Chris Wright
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+
+static int pci_stub_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+ return 0;
+}
+
+static struct pci_driver stub_driver = {
+ .name = "pci-stub",
+ .id_table = NULL, /* only dynamic id's */
+ .probe = pci_stub_probe,
+};
+
+static int __init pci_stub_init(void)
+{
+ return pci_register_driver(&stub_driver);
+}
+
+static void __exit pci_stub_exit(void)
+{
+ pci_unregister_driver(&stub_driver);
+}
+
+module_init(pci_stub_init);
+module_exit(pci_stub_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Chris Wright <[email protected]>");
--
1.6.0.3
On Tue, Nov 25, 2008 at 07:38:39PM -0800, Chris Wright wrote:
> When doing device assignment with KVM there's currently nothing to
> protect the device from having a driver in the host as well as the guest.
> This trivial module just binds the pci device on the host to a stub
> driver so that a real host driver can't bind to the device. It has no
> pci id table, it supports only dynamic ids.
>
> # echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/new_id
> # echo -n 0000:00:19.0 > /sys/bus/pci/drivers/e1000e/unbind
> # echo -n 0000:00:19.0 > /sys/bus/pci/drivers/pci-stub/bind
> # ls -l /sys/bus/pci/devices/0000:00:19.0/driver
> lrwxrwxrwx 1 root root 0 2008-11-25 19:10 /sys/bus/pci/devices/0000:00:19.0/driver -> ../../../bus/pci/drivers/pci-stub
You might want to put this somewhere in the .c or documentation files
somewhere as well.
>
> Cc: "Kay, Allen M" <[email protected]>
> Cc: "Nakajima, Jun" <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>
> ---
> drivers/pci/Kconfig | 9 +++++++++
> drivers/pci/Makefile | 2 ++
> drivers/pci/pci-stub.c | 38 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 49 insertions(+), 0 deletions(-)
> create mode 100644 drivers/pci/pci-stub.c
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index e1ca425..f6183df 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -42,6 +42,15 @@ config PCI_DEBUG
>
> When in doubt, say N.
>
> +config PCI_STUB
> + tristate "PCI Stub driver"
> + depends on PCI
> + help
> + Say Y or M here if you want be able to reserve a PCI device
> + when it is going to be assigned to a guest.
"guest operating system"? Otherwise, just "guest" doesn't mean much
here in this context.
Other than that minor thing, looks great.
Acked-by: Greg Kroah-Hartman <[email protected]>
thanks,
greg k-h
* Greg KH ([email protected]) wrote:
> On Tue, Nov 25, 2008 at 07:38:39PM -0800, Chris Wright wrote:
> > When doing device assignment with KVM there's currently nothing to
> > protect the device from having a driver in the host as well as the guest.
> > This trivial module just binds the pci device on the host to a stub
> > driver so that a real host driver can't bind to the device. It has no
> > pci id table, it supports only dynamic ids.
> >
> > # echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/new_id
> > # echo -n 0000:00:19.0 > /sys/bus/pci/drivers/e1000e/unbind
> > # echo -n 0000:00:19.0 > /sys/bus/pci/drivers/pci-stub/bind
> > # ls -l /sys/bus/pci/devices/0000:00:19.0/driver
> > lrwxrwxrwx 1 root root 0 2008-11-25 19:10 /sys/bus/pci/devices/0000:00:19.0/driver -> ../../../bus/pci/drivers/pci-stub
>
> You might want to put this somewhere in the .c or documentation files
> somewhere as well.
OK, I'll add this to the .c file for now.
> > Cc: "Kay, Allen M" <[email protected]>
> > Cc: "Nakajima, Jun" <[email protected]>
> > Signed-off-by: Chris Wright <[email protected]>
> > ---
> > drivers/pci/Kconfig | 9 +++++++++
> > drivers/pci/Makefile | 2 ++
> > drivers/pci/pci-stub.c | 38 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 49 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/pci/pci-stub.c
> >
> > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> > index e1ca425..f6183df 100644
> > --- a/drivers/pci/Kconfig
> > +++ b/drivers/pci/Kconfig
> > @@ -42,6 +42,15 @@ config PCI_DEBUG
> >
> > When in doubt, say N.
> >
> > +config PCI_STUB
> > + tristate "PCI Stub driver"
> > + depends on PCI
> > + help
> > + Say Y or M here if you want be able to reserve a PCI device
> > + when it is going to be assigned to a guest.
>
> "guest operating system"? Otherwise, just "guest" doesn't mean much
> here in this context.
fixed
> Other than that minor thing, looks great.
>
> Acked-by: Greg Kroah-Hartman <[email protected]>
thanks,
-chris
When doing device assignment with KVM there's currently nothing to
protect the device from having a driver in the host as well as the guest.
This trivial module just binds the pci device on the host to a stub
driver so that a real host driver can't bind to the device. It has no
pci id table, it supports only dynamic ids.
# echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/new_id
# echo -n 0000:00:19.0 > /sys/bus/pci/drivers/e1000e/unbind
# echo -n 0000:00:19.0 > /sys/bus/pci/drivers/pci-stub/bind
# ls -l /sys/bus/pci/devices/0000:00:19.0/driver
lrwxrwxrwx 1 root root 0 2008-11-25 19:10 /sys/bus/pci/devices/0000:00:19.0/driver -> ../../../bus/pci/drivers/pci-stub
Cc: "Kay, Allen M" <[email protected]>
Cc: "Nakajima, Jun" <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
---
drivers/pci/Kconfig | 9 +++++++++
drivers/pci/Makefile | 2 ++
drivers/pci/pci-stub.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 58 insertions(+), 0 deletions(-)
create mode 100644 drivers/pci/pci-stub.c
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index e1ca425..2a4501d 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -42,6 +42,15 @@ config PCI_DEBUG
When in doubt, say N.
+config PCI_STUB
+ tristate "PCI Stub driver"
+ depends on PCI
+ help
+ Say Y or M here if you want be able to reserve a PCI device
+ when it is going to be assigned to a guest operating system.
+
+ When in doubt, say N.
+
config HT_IRQ
bool "Interrupts on hypertransport devices"
default y
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index af3bfe2..3d07ce2 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -53,6 +53,8 @@ obj-$(CONFIG_HOTPLUG) += setup-bus.o
obj-$(CONFIG_PCI_SYSCALL) += syscall.o
+obj-$(CONFIG_PCI_STUB) += pci-stub.o
+
ifeq ($(CONFIG_PCI_DEBUG),y)
EXTRA_CFLAGS += -DDEBUG
endif
diff --git a/drivers/pci/pci-stub.c b/drivers/pci/pci-stub.c
new file mode 100644
index 0000000..74fbec0
--- /dev/null
+++ b/drivers/pci/pci-stub.c
@@ -0,0 +1,47 @@
+/* pci-stub - simple stub driver to reserve a pci device
+ *
+ * Copyright (C) 2008 Red Hat, Inc.
+ * Author:
+ * Chris Wright
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ *
+ * Usage is simple, allocate a new id to the stub driver and bind the
+ * device to it. For example:
+ *
+ * # echo "8086 10f5" > /sys/bus/pci/drivers/pci-stub/new_id
+ * # echo -n 0000:00:19.0 > /sys/bus/pci/drivers/e1000e/unbind
+ * # echo -n 0000:00:19.0 > /sys/bus/pci/drivers/pci-stub/bind
+ * # ls -l /sys/bus/pci/devices/0000:00:19.0/driver
+ * .../0000:00:19.0/driver -> ../../../bus/pci/drivers/pci-stub
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+
+static int pci_stub_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+ return 0;
+}
+
+static struct pci_driver stub_driver = {
+ .name = "pci-stub",
+ .id_table = NULL, /* only dynamic id's */
+ .probe = pci_stub_probe,
+};
+
+static int __init pci_stub_init(void)
+{
+ return pci_register_driver(&stub_driver);
+}
+
+static void __exit pci_stub_exit(void)
+{
+ pci_unregister_driver(&stub_driver);
+}
+
+module_init(pci_stub_init);
+module_exit(pci_stub_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Chris Wright <[email protected]>");
--
1.6.0.3
Hi Chris,
On Tue, 25 Nov 2008 19:36:10 -0800, Chris Wright wrote:
> commit b41d6cf38e27 (PCI: Check dynids driver_data value for validity)
> requires all drivers to include an id table to try and match
> driver_data. Before validating driver_data check driver has an id
> table.
Sorry for missing this case.
> Cc: Jean Delvare <[email protected]>
> Cc: Milton Miller <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>
> ---
> drivers/pci/pci-driver.c | 20 +++++++++++---------
> 1 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index b4cdd69..0a5edbe 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -48,7 +48,7 @@ store_new_id(struct device_driver *driver, const char *buf, size_t count)
> subdevice=PCI_ANY_ID, class=0, class_mask=0;
> unsigned long driver_data=0;
> int fields=0;
> - int retval;
> + int retval=0;
>
> fields = sscanf(buf, "%x %x %x %x %x %x %lx",
> &vendor, &device, &subvendor, &subdevice,
> @@ -58,16 +58,18 @@ store_new_id(struct device_driver *driver, const char *buf, size_t count)
>
> /* Only accept driver_data values that match an existing id_table
> entry */
> - retval = -EINVAL;
> - while (ids->vendor || ids->subvendor || ids->class_mask) {
> - if (driver_data == ids->driver_data) {
> - retval = 0;
> - break;
> + if (ids) {
> + retval = -EINVAL;
> + while (ids->vendor || ids->subvendor || ids->class_mask) {
> + if (driver_data == ids->driver_data) {
> + retval = 0;
> + break;
> + }
> + ids++;
> }
> - ids++;
> + if (retval) /* No match */
> + return retval;
> }
> - if (retval) /* No match */
> - return retval;
>
> dynid = kzalloc(sizeof(*dynid), GFP_KERNEL);
> if (!dynid)
Do we really want to let the user add a new id to a driver which didn't
have any? Wouldn't it be safer to simply not create the new_id sysfs
file if driver->id_table is NULL? That's a one-line change:
Signed-off-by: Jean Delvare <[email protected]>
---
drivers/pci/pci-driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-2.6.28-rc6.orig/drivers/pci/pci-driver.c 2008-10-24 09:28:14.000000000 +0200
+++ linux-2.6.28-rc6/drivers/pci/pci-driver.c 2008-11-26 09:23:46.000000000 +0100
@@ -113,7 +113,7 @@ static int
pci_create_newid_file(struct pci_driver *drv)
{
int error = 0;
- if (drv->probe != NULL)
+ if (drv->probe != NULL && drv->id_table != NULL)
error = driver_create_file(&drv->driver, &driver_attr_new_id);
return error;
}
As a side note, I am curious what PCI driver we do have which has
driver->probe defined but no driver->id_table. Did you hit an actual
issue or are you fixing a theoretical one?
Thanks,
--
Jean Delvare
* Jean Delvare ([email protected]) wrote:
> As a side note, I am curious what PCI driver we do have which has
> driver->probe defined but no driver->id_table. Did you hit an actual
> issue or are you fixing a theoretical one?
Such a driver was patch 2/2.
http://lkml.org/lkml/2008/11/26/11
It's meant to have only dynamic ids, worked fine on 2.6.27 and oopsed
from NULL id-> deref on current git.
thanks,
-chris
Hi Chris,
On Wed, 26 Nov 2008 07:19:40 -0800, Chris Wright wrote:
> * Jean Delvare ([email protected]) wrote:
> > As a side note, I am curious what PCI driver we do have which has
> > driver->probe defined but no driver->id_table. Did you hit an actual
> > issue or are you fixing a theoretical one?
>
> Such a driver was patch 2/2.
>
> http://lkml.org/lkml/2008/11/26/11
>
> It's meant to have only dynamic ids, worked fine on 2.6.27 and oopsed
> from NULL id-> deref on current git.
Ah, OK, then your patch makes full sense and mine doesn't, sorry for
the noise. Feel free to add my Acked-by on your patch.
--
Jean Delvare
* Jean Delvare ([email protected]) wrote:
> Ah, OK, then your patch makes full sense and mine doesn't, sorry for
> the noise. Feel free to add my Acked-by on your patch.
Thanks Jean.
-chris
On Tuesday, November 25, 2008 7:36 pm Chris Wright wrote:
> commit b41d6cf38e27 (PCI: Check dynids driver_data value for validity)
> requires all drivers to include an id table to try and match
> driver_data. Before validating driver_data check driver has an id
> table.
>
> Cc: Jean Delvare <[email protected]>
> Cc: Milton Miller <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>
Applied these to my linux-next branch, thanks Chris.
--
Jesse Barnes, Intel Open Source Technology Center