On Mon, Apr 27, 2009 at 2:56 PM, Avi Kivity <avi@xxxxxxxxxx> wrote:
> Pantelis Koukousoulas wrote:
>>>
>>> Or maybe
>>>
>>> modprobe virtio-pci claim=0x10f2 claim=0x10f7
>>>
>>
>> How about claim=0x10f2,0x10f7 instead so that it can be implemented as
>> a standard module array parameter?
>>
>
> Even better.
Ok, since a day has passed with no further comments, I 'll dare to
assume everyone
is happy with this solution. So, here is an implementation. I 've
tested locally with
my driver that uses 0x10f5 and it seems to work.
I am both attaching and inlining the patch because I 'm sure gmail
will mess it up
but I have no access to any other mailer at this time.
Feel free to rewrite any part that is too ugly.
Pantelis
From: Pantelis Koukousoulas <pktoss@xxxxxxxxx>
Date: Mon, 27 Apr 2009 20:49:20 +0300
Subject: [PATCH] Fix virtio_pci handling of PCI IDs
Currently virtio_pci does not claim the PCI IDs in the
"experimental range" 0x10f0-0x10ff. This means that
developers wanting to to the "right thing" and use
one of these IDs find that their drivers don't load.
In the end, this encourages developers to just hijack
an ID from the low end of the managed range (0x1000-0x103f).
Also, the choice of only claiming part of the available
managed range (0x1000-0x10ef) might seem arbitrary or
a typo to someone reading the code, since there is
no comment to explain/justify it.
After discussion of the problem in kvm-devel, this patch
attempts to fix these problems.
For the managed range we just add a comment to explain that
the reason for only claiming part of the range was
future-proofing against potential ABI breakage.
For the experimental range we implement a module parameter
to allow developers to claim the IDs they want individually.
E.g., to develop 2 virtio devices with IDs 0x10f3 and 0x10f5
you just add:
options virtio_pci claim=0x10f3,0x10f5
to e.g., /etc/modprobe.d/virtio.conf and you are set. This
way should also be ABI breakage-proof while still allowing
private development of up to 16 devices by the same organization
simultaneously (i.e., the full experimental range).
Gerd Hoffmann suggested a module parameter and Avi Kivity
suggested the claim= syntax.
Signed-off-by: Pantelis Koukousoulas <pktoss@xxxxxxxxx>
---
drivers/virtio/virtio_pci.c | 33 ++++++++++++++++++++++++++++++---
1 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 330aacb..9337a1d 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -30,6 +30,10 @@ MODULE_DESCRIPTION("virtio-pci");
MODULE_LICENSE("GPL");
MODULE_VERSION("1");
+static int claim[16];
+module_param_array(claim, int, NULL, 0444);
+MODULE_PARM_DESC(claim, "Claimed PCI IDs in the 0x10f0-0x10ff range");
+
/* Our device structure */
struct virtio_pci_device
{
@@ -318,6 +322,30 @@ static void virtio_pci_release_dev(struct device *_d)
kfree(vp_dev);
}
+/*
+ * We only claim devices >= 0x1000 and <= 0x103f from the managed
+ * range: leave the rest. This allows for potential breaking of the
+ * ABI in the future. We also allow explicit selective claiming of
+ * IDs in the experimental range 0x10f0 - 0x10ff via a module param.
+ */
+static inline int is_valid_virtio_pci_id(short id)
+{
+ int i;
+
+ if (id < 0x1000 || (id > 0x103f && id < 0x10f0) || id > 0x10ff)
+ return 0;
+
+ if (id > 0x10f0) { /* 0x10f0 - 0x10ff case: experimental range id */
+ for (i = 0; i < ARRAY_SIZE(claim); i++) {
+ if (id == claim[i])
+ return 1;
+ }
+ return 0;
+ }
+
+ return 1;
+}
+
/* the PCI probing function */
static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
const struct pci_device_id *id)
@@ -325,9 +353,8 @@ static int __devinit virtio_pci_probe(struct
pci_dev *pci_dev,
struct virtio_pci_device *vp_dev;
int err;
- /* We only own devices >= 0x1000 and <= 0x103f: leave the rest. */
- if (pci_dev->device < 0x1000 || pci_dev->device > 0x103f)
- return -ENODEV;
+ if (!is_valid_virtio_pci_id(pci_dev->device))
+ return -ENODEV;
if (pci_dev->revision != VIRTIO_PCI_ABI_VERSION) {
printk(KERN_ERR "virtio_pci: expected ABI version %d, got %d\n",
--
1.5.6.3
From daa4ba5078dd05f9e58d3f9a2327e5d60f345150 Mon Sep 17 00:00:00 2001
From: Pantelis Koukousoulas <pktoss@xxxxxxxxx>
Date: Mon, 27 Apr 2009 20:49:20 +0300
Subject: [PATCH] Fix virtio_pci handling of PCI IDs
Currently virtio_pci does not claim the PCI IDs in the
"experimental range" 0x10f0-0x10ff. This means that
developers wanting to to the "right thing" and use
one of these IDs find that their drivers don't load.
In the end, this encourages developers to just hijack
an ID from the low end of the managed range (0x1000-0x103f).
Also, the choice of only claiming part of the available
managed range (0x1000-0x10ef) might seem arbitrary or
a typo to someone reading the code, since there is
no comment to explain/justify it.
After discussion of the problem in kvm-devel, this patch
attempts to fix these problems.
For the managed range we just add a comment to explain that
the reason for only claiming part of the range was
future-proofing against potential ABI breakage.
For the experimental range we implement a module parameter
to allow developers to claim the IDs they want individually.
E.g., to develop 2 virtio devices with IDs 0x10f3 and 0x10f5
you just add:
options virtio_pci claim=0x10f3,0x10f5
to e.g., /etc/modprobe.d/virtio.conf and you are set. This
way should also be ABI breakage-proof while still allowing
private development of up to 16 devices by the same organization
simultaneously (i.e., the full experimental range).
Gerd Hoffmann suggested a module parameter and Avi Kivity
suggested the claim= syntax.
Signed-off-by: Pantelis Koukousoulas <pktoss@xxxxxxxxx>
---
drivers/virtio/virtio_pci.c | 33 ++++++++++++++++++++++++++++++---
1 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 330aacb..9337a1d 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -30,6 +30,10 @@ MODULE_DESCRIPTION("virtio-pci");
MODULE_LICENSE("GPL");
MODULE_VERSION("1");
+static int claim[16];
+module_param_array(claim, int, NULL, 0444);
+MODULE_PARM_DESC(claim, "Claimed PCI IDs in the 0x10f0-0x10ff range");
+
/* Our device structure */
struct virtio_pci_device
{
@@ -318,6 +322,30 @@ static void virtio_pci_release_dev(struct device *_d)
kfree(vp_dev);
}
+/*
+ * We only claim devices >= 0x1000 and <= 0x103f from the managed
+ * range: leave the rest. This allows for potential breaking of the
+ * ABI in the future. We also allow explicit selective claiming of
+ * IDs in the experimental range 0x10f0 - 0x10ff via a module param.
+ */
+static inline int is_valid_virtio_pci_id(short id)
+{
+ int i;
+
+ if (id < 0x1000 || (id > 0x103f && id < 0x10f0) || id > 0x10ff)
+ return 0;
+
+ if (id > 0x10f0) { /* 0x10f0 - 0x10ff case: experimental range id */
+ for (i = 0; i < ARRAY_SIZE(claim); i++) {
+ if (id == claim[i])
+ return 1;
+ }
+ return 0;
+ }
+
+ return 1;
+}
+
/* the PCI probing function */
static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
const struct pci_device_id *id)
@@ -325,9 +353,8 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
struct virtio_pci_device *vp_dev;
int err;
- /* We only own devices >= 0x1000 and <= 0x103f: leave the rest. */
- if (pci_dev->device < 0x1000 || pci_dev->device > 0x103f)
- return -ENODEV;
+ if (!is_valid_virtio_pci_id(pci_dev->device))
+ return -ENODEV;
if (pci_dev->revision != VIRTIO_PCI_ABI_VERSION) {
printk(KERN_ERR "virtio_pci: expected ABI version %d, got %d\n",
--
1.5.6.3
