Patchwork [v2] pci: Error on PCI capability collisions

login
register
mail settings
Submitter Jan Kiszka
Date Aug. 24, 2011, 12:29 p.m.
Message ID <4E54EEAA.7020707@siemens.com>
Download mbox | patch
Permalink /patch/111332/
State New
Headers show

Comments

Jan Kiszka - Aug. 24, 2011, 12:29 p.m.
Nothing good can happen when we overlap capabilities. This may happen
when plugging in assigned devices or when devices models contain bugs.
Detect the overlap and report it.

Based on qemu-kvm commit by Alex Williamson.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/pci.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

Changes in v2:
 - properly find the conflicting CAP
 - adjust error code
Michael S. Tsirkin - Aug. 24, 2011, 12:53 p.m.
On Wed, Aug 24, 2011 at 02:29:30PM +0200, Jan Kiszka wrote:
> Nothing good can happen when we overlap capabilities. This may happen
> when plugging in assigned devices or when devices models contain bugs.
> Detect the overlap and report it.
> 
> Based on qemu-kvm commit by Alex Williamson.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Applied, thanks everyone.

> ---
>  hw/pci.c |   34 ++++++++++++++++++++++++++++++++++
>  1 files changed, 34 insertions(+), 0 deletions(-)
> 
> Changes in v2:
>  - properly find the conflicting CAP
>  - adjust error code
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 6124790..634e789 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1811,6 +1811,25 @@ static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
>      return next;
>  }
>  
> +static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset)
> +{
> +    uint8_t next, prev, found = 0;
> +
> +    if (!(pdev->used[offset])) {
> +        return 0;
> +    }
> +
> +    assert(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST);
> +
> +    for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]);
> +         prev = next + PCI_CAP_LIST_NEXT) {
> +        if (next <= offset && next > found) {
> +            found = next;
> +        }
> +    }
> +    return found;
> +}
> +
>  /* Patch the PCI vendor and device ids in a PCI rom image if necessary.
>     This is needed for an option rom which is used for more than one device. */
>  static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size)
> @@ -1952,11 +1971,26 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
>                         uint8_t offset, uint8_t size)
>  {
>      uint8_t *config;
> +    int i, overlapping_cap;
> +
>      if (!offset) {
>          offset = pci_find_space(pdev, size);
>          if (!offset) {
>              return -ENOSPC;
>          }
> +    } else {
> +        for (i = offset; i < offset + size; i++) {
> +            overlapping_cap = pci_find_capability_at_offset(pdev, i);
> +            if (overlapping_cap) {
> +                fprintf(stderr, "ERROR: %04x:%02x:%02x.%x "
> +                        "Attempt to add PCI capability %x at offset "
> +                        "%x overlaps existing capability %x at offset %x\n",
> +                        pci_find_domain(pdev->bus), pci_bus_num(pdev->bus),
> +                        PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
> +                        cap_id, offset, overlapping_cap, i);
> +                return -EINVAL;
> +            }
> +        }
>      }
>  
>      config = pdev->config + offset;
> -- 
> 1.7.3.4

Patch

diff --git a/hw/pci.c b/hw/pci.c
index 6124790..634e789 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1811,6 +1811,25 @@  static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
     return next;
 }
 
+static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset)
+{
+    uint8_t next, prev, found = 0;
+
+    if (!(pdev->used[offset])) {
+        return 0;
+    }
+
+    assert(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST);
+
+    for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]);
+         prev = next + PCI_CAP_LIST_NEXT) {
+        if (next <= offset && next > found) {
+            found = next;
+        }
+    }
+    return found;
+}
+
 /* Patch the PCI vendor and device ids in a PCI rom image if necessary.
    This is needed for an option rom which is used for more than one device. */
 static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size)
@@ -1952,11 +1971,26 @@  int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
                        uint8_t offset, uint8_t size)
 {
     uint8_t *config;
+    int i, overlapping_cap;
+
     if (!offset) {
         offset = pci_find_space(pdev, size);
         if (!offset) {
             return -ENOSPC;
         }
+    } else {
+        for (i = offset; i < offset + size; i++) {
+            overlapping_cap = pci_find_capability_at_offset(pdev, i);
+            if (overlapping_cap) {
+                fprintf(stderr, "ERROR: %04x:%02x:%02x.%x "
+                        "Attempt to add PCI capability %x at offset "
+                        "%x overlaps existing capability %x at offset %x\n",
+                        pci_find_domain(pdev->bus), pci_bus_num(pdev->bus),
+                        PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
+                        cap_id, offset, overlapping_cap, i);
+                return -EINVAL;
+            }
+        }
     }
 
     config = pdev->config + offset;