[PATCH v5 0/3] Red Hat PCI bridge resource reserve capability (was: Allow RedHat PCI bridges reserve more buses than necessary during init)

[Aleksandr Bezzubikov]

Now PCI bridges get a bus range number on a system init,
basing on currently plugged devices. That's why when one wants to hotplug another bridge,
it needs his child bus, which the parent is unable to provide (speaking about virtual device).
The suggested workaround is to have vendor-specific capability in Red Hat PCI bridges
that contains number of additional bus to reserve (as well as IO/MEM/PREF space limit hints) 
on BIOS PCI init.
So this capability is intended only for pure QEMU->SeaBIOS usage.

Considering all aforesaid, this series is directly connected with
QEMU series "Generic PCIE-PCI Bridge".

Although the new PCI capability is supposed to contain various limits along with
bus number to reserve, now only its full layout is proposed. And
only bus_reserve field is used in QEMU and BIOS. Limits usage
is still a subject for implementation as now
the main goal of this series to provide necessary support from the 
firmware side to PCIE-PCI bridge hotplug.

Changes v4->v5:
1. Rename capability-related #defines
2. Move capability IO/MEM/PREF fields values usage to the regions creation stage (addresses Marcel's comment)
3. The capability layout change: separate pref_mem into pref_mem_32 and pref_mem_64 fields (QEMU side has the same changes) (addresses Laszlo's comment)
4. Extract the capability lookup and check to the separate function (addresses Marcel's comment)
    - despite of Marcel's comment do not extract field check for -1 since it increases code length
      and doesn't look nice because of different field types 
5. Fix the capability's comment (addresses Marcel's comment)
6. Fix the 3rd patch message

Changes v3->v4:
1. Use all QEMU PCI capability fields - addresses Michael's comment
2. Changes of the capability layout (QEMU side has the same changes):
    - change reservation fields types: bus_res - uint32_t, others - uint64_t
    - interpret -1 value as 'ignore'

Changes v2->v3:
1. Merge commit 2 (Red Hat vendor ID) into commit 4 - addresses Marcel's comment,
    and add Generic PCIE Root Port device ID - addresses Michael's comment.
2. Changes of the capability layout  (QEMU side has the same changes):
    - add 'type' field to distinguish multiple 
    	RedHat-specific capabilities - addresses Michael's comment
    - do not mimiс PCI Config space register layout, but use mutually exclusive differently
    	sized fields for IO and prefetchable memory limits - addresses Laszlo's comment
    - use defines instead of structure and offsetof - addresses Michael's comment
3. Interpret 'bus_reserve' field as a minimum necessary
     range to reserve - addresses Gerd's comment
4. pci_find_capability moved to pci.c - addresses Kevin's comment
5. Move capability layout header to src/fw/dev-pci.h - addresses Kevin's comment
6. Add the capability documentation - addresses Michael's comment
7. Add capability length and bus_reserve field sanity checks - addresses Michael's comment

Changes v1->v2:
1. New #define for Red Hat vendor added (addresses Konrad's comment).
2. Refactored pci_find_capability function (addresses Marcel's comment).
3. Capability reworked:
    - data type added;
    - reserve space in a structure for IO, memory and 
      prefetchable memory limits.

Aleksandr Bezzubikov (3):
  pci: refactor pci_find_capapibilty to get bdf as the first argument
    instead of the whole pci_device
  pci: add QEMU-specific PCI capability structure
  pci: enable RedHat PCI bridges to reserve additional resource on PCI
    init

src/fw/dev-pci.h    |  52 +++++++++++++++++++++
 src/fw/pciinit.c    | 127 ++++++++++++++++++++++++++++++++++++++++++++++------
 src/hw/pci.c        |  25 +++++++++++
 src/hw/pci.h        |   1 +
 src/hw/pci_ids.h    |   3 ++
 src/hw/pcidevice.c  |  24 ----------
 src/hw/pcidevice.h  |   1 -
 src/hw/virtio-pci.c |   6 +--
 8 files changed, 197 insertions(+), 42 deletions(-)
 create mode 100644 src/fw/dev-pci.h

[PATCH v5 1/3] pci: refactor pci_find_capapibilty to get bdf as the first argument instead of the whole pci_device

[Aleksandr Bezzubikov]

Refactor pci_find_capability function to get bdf instead of
a whole pci_device* as the only necessary field for this function
is still bdf.

Signed-off-by: Aleksandr Bezzubikov 
---
 src/fw/pciinit.c    |  4 ++--
 src/hw/pci.c        | 25 +++++++++++++++++++++++++
 src/hw/pci.h        |  1 +
 src/hw/pcidevice.c  | 24 ------------------------
 src/hw/pcidevice.h  |  1 -
 src/hw/virtio-pci.c |  6 +++---
 6 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index 08221e6..864954f 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -762,7 +762,7 @@ static int pci_bus_hotplug_support(struct pci_bus *bus, u8 pcie_cap)
         return downstream_port && slot_implemented;
     }

-    shpc_cap = pci_find_capability(bus->bus_dev, PCI_CAP_ID_SHPC, 0);
+    shpc_cap = pci_find_capability(bus->bus_dev->bdf, PCI_CAP_ID_SHPC, 0);
     return !!shpc_cap;
 }

@@ -844,7 +844,7 @@ static int pci_bios_check_devices(struct pci_bus *busses)
              */
             parent = &busses[0];
         int type;
-        u8 pcie_cap = pci_find_capability(s->bus_dev, PCI_CAP_ID_EXP, 0);
+        u8 pcie_cap = pci_find_capability(s->bus_dev->bdf, PCI_CAP_ID_EXP, 0);
         int hotplug_support = pci_bus_hotplug_support(s, pcie_cap);
         for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
             u64 align = (type == PCI_REGION_TYPE_IO) ?
diff --git a/src/hw/pci.c b/src/hw/pci.c
index 8e3d617..50d9d2d 100644
--- a/src/hw/pci.c
+++ b/src/hw/pci.c
@@ -58,6 +58,30 @@ pci_config_maskw(u16 bdf, u32 addr, u16 off, u16 on)
     pci_config_writew(bdf, addr, val);
 }

+u8 pci_find_capability(u16 bdf, u8 cap_id, u8 cap)
+{
+    int i;
+    u16 status = pci_config_readw(bdf, PCI_STATUS);
+
+    if (!(status & PCI_STATUS_CAP_LIST))
+        return 0;
+
+    if (cap == 0) {
+        /* find first */
+        cap = pci_config_readb(bdf, PCI_CAPABILITY_LIST);
+    } else {
+        /* find next */
+        cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
+    }
+    for (i = 0; cap && i <= 0xff; i++) {
+        if (pci_config_readb(bdf, cap + PCI_CAP_LIST_ID) == cap_id)
+            return cap;
+        cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
+    }
+
+    return 0;
+}
+
 // Helper function for foreachbdf() macro - return next device
 int
 pci_next(int bdf, int bus)
@@ -107,3 +131,4 @@ pci_reboot(void)
     outb(v|6, PORT_PCI_REBOOT); /* Actually do the reset */
     udelay(50);
 }
+
diff --git a/src/hw/pci.h b/src/hw/pci.h
index ee6e196..2e30e28 100644
--- a/src/hw/pci.h
+++ b/src/hw/pci.h
@@ -39,6 +39,7 @@ u32 pci_config_readl(u16 bdf, u32 addr);
 u16 pci_config_readw(u16 bdf, u32 addr);
 u8 pci_config_readb(u16 bdf, u32 addr);
 void pci_config_maskw(u16 bdf, u32 addr, u16 off, u16 on);
+u8 pci_find_capability(u16 bdf, u8 cap_id, u8 cap);
 int pci_next(int bdf, int bus);
 int pci_probe_host(void);
 void pci_reboot(void);
diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c
index cfebf66..8853cf7 100644
--- a/src/hw/pcidevice.c
+++ b/src/hw/pcidevice.c
@@ -134,30 +134,6 @@ pci_find_init_device(const struct pci_device_id *ids, void *arg)
     return NULL;
 }

-u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap)
-{
-    int i;
-    u16 status = pci_config_readw(pci->bdf, PCI_STATUS);
-
-    if (!(status & PCI_STATUS_CAP_LIST))
-        return 0;
-
-    if (cap == 0) {
-        /* find first */
-        cap = pci_config_readb(pci->bdf, PCI_CAPABILITY_LIST);
-    } else {
-        /* find next */
-        cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT);
-    }
-    for (i = 0; cap && i <= 0xff; i++) {
-        if (pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_ID) == cap_id)
-            return cap;
-        cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT);
-    }
-
-    return 0;
-}
-
 // Enable PCI bus-mastering (ie, DMA) support on a pci device
 void
 pci_enable_busmaster(struct pci_device *pci)
diff --git a/src/hw/pcidevice.h b/src/hw/pcidevice.h
index 354b549..225d545 100644
--- a/src/hw/pcidevice.h
+++ b/src/hw/pcidevice.h
@@ -69,7 +69,6 @@ int pci_init_device(const struct pci_device_id *ids
                     , struct pci_device *pci, void *arg);
 struct pci_device *pci_find_init_device(const struct pci_device_id *ids
                                         , void *arg);
-u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap);
 void pci_enable_busmaster(struct pci_device *pci);
 u16 pci_enable_iobar(struct pci_device *pci, u32 addr);
 void *pci_enable_membar(struct pci_device *pci, u32 addr);
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c
index e5c2c33..96f9c6b 100644
--- a/src/hw/virtio-pci.c
+++ b/src/hw/virtio-pci.c
@@ -19,7 +19,7 @@
 #include "malloc.h" // free
 #include "output.h" // dprintf
 #include "pci.h" // pci_config_readl
-#include "pcidevice.h" // pci_find_capability
+#include "pcidevice.h" // struct pci_device
 #include "pci_regs.h" // PCI_BASE_ADDRESS_0
 #include "string.h" // memset
 #include "virtio-pci.h"
@@ -381,7 +381,7 @@ fail:

void vp_init_simple(struct vp_device *vp, struct pci_device *pci)
 {
-    u8 cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, 0);
+    u8 cap = pci_find_capability(pci->bdf, PCI_CAP_ID_VNDR, 0);
     struct vp_cap *vp_cap;
     const char *mode;
     u32 offset, base, mul;
@@ -479,7 +479,7 @@ void vp_init_simple(struct vp_device *vp, struct pci_device *pci)
                     vp_cap->cap, type, vp_cap->bar, addr, offset, mode);
         }

-        cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, cap);
+        cap = pci_find_capability(pci->bdf, PCI_CAP_ID_VNDR, cap);
     }

if (vp->common.cap && vp->notify.cap && vp->isr.cap && vp->device.cap) {

[PATCH v5 1/3] pci: refactor pci_find_capapibilty to get bdf as the first argument instead of the whole pci_device

[Aleksandr Bezzubikov]

> From the previous versions:

Reviewed-by: Marcel Apfelbaum

2017-08-11 2:21 GMT+03:00 Aleksandr Bezzubikov :
> Refactor pci_find_capability function to get bdf instead of
> a whole pci_device* as the only necessary field for this function
> is still bdf.
> Signed-off-by: Aleksandr Bezzubikov
>
> src/fw/pciinit.c    |  4 ++--
>  src/hw/pci.c        | 25 +++++++++++++++++++++++++
>  src/hw/pci.h        |  1 +
>  src/hw/pcidevice.c  | 24 ------------------------
>  src/hw/pcidevice.h  |  1 -
>  src/hw/virtio-pci.c |  6 +++---
>  6 files changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 08221e6..864954f 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -762,7 +762,7 @@ static int pci_bus_hotplug_support(struct pci_bus *bus, u8 pcie_cap)
>          return downstream_port && slot_implemented;
>      }
> shpc_cap = pci_find_capability(bus->bus_dev, PCI_CAP_ID_SHPC, 0);
> shpc_cap = pci_find_capability(bus->bus_dev->bdf, PCI_CAP_ID_SHPC, 0);
> return !!shpc_cap;
>
> }
>
> @@ -844,7 +844,7 @@ static int pci_bios_check_devices(struct pci_bus *busses)
>               */
>              parent = &busses[0];
>          int type;
>    u8 pcie_cap = pci_find_capability(s->bus_dev, PCI_CAP_ID_EXP, 0);
>    u8 pcie_cap = pci_find_capability(s->bus_dev->bdf, PCI_CAP_ID_EXP, 0);
>    int hotplug_support = pci_bus_hotplug_support(s, pcie_cap);
>    for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
>        u64 align = (type == PCI_REGION_TYPE_IO) ?
>
> diff --git a/src/hw/pci.c b/src/hw/pci.c
> index 8e3d617..50d9d2d 100644
> --- a/src/hw/pci.c
> +++ b/src/hw/pci.c
> @@ -58,6 +58,30 @@ pci_config_maskw(u16 bdf, u32 addr, u16 off, u16 on)
>      pci_config_writew(bdf, addr, val);
>  }
>
> +u8 pci_find_capability(u16 bdf, u8 cap_id, u8 cap)
> +{
> int i;
> u16 status = pci_config_readw(bdf, PCI_STATUS);
> if (!(status & PCI_STATUS_CAP_LIST))
>    return 0;
> if (cap == 0) {
>    /* find first */
>    cap = pci_config_readb(bdf, PCI_CAPABILITY_LIST);
> } else {
>    /* find next */
>    cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
> }
> for (i = 0; cap && i <= 0xff; i++) {
>    if (pci_config_readb(bdf, cap + PCI_CAP_LIST_ID) == cap_id)
>        return cap;
>    cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
> }
> return 0;
>
> +}
>
> // Helper function for foreachbdf() macro - return next device
>  int
>  pci_next(int bdf, int bus)
> @@ -107,3 +131,4 @@ pci_reboot(void)
>      outb(v|6, PORT_PCI_REBOOT); /* Actually do the reset */
>      udelay(50);
>  }
>
> diff --git a/src/hw/pci.h b/src/hw/pci.h
> index ee6e196..2e30e28 100644
> --- a/src/hw/pci.h
> +++ b/src/hw/pci.h
> @@ -39,6 +39,7 @@ u32 pci_config_readl(u16 bdf, u32 addr);
>  u16 pci_config_readw(u16 bdf, u32 addr);
>  u8 pci_config_readb(u16 bdf, u32 addr);
>  void pci_config_maskw(u16 bdf, u32 addr, u16 off, u16 on);
> +u8 pci_find_capability(u16 bdf, u8 cap_id, u8 cap);
>  int pci_next(int bdf, int bus);
>  int pci_probe_host(void);
>  void pci_reboot(void);
> diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c
> index cfebf66..8853cf7 100644
> --- a/src/hw/pcidevice.c
> +++ b/src/hw/pcidevice.c
> @@ -134,30 +134,6 @@ pci_find_init_device(const struct pci_device_id *ids, void *arg)
>      return NULL;
>  }
>
> -u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap)
> -{
> int i;
> u16 status = pci_config_readw(pci->bdf, PCI_STATUS);
> if (!(status & PCI_STATUS_CAP_LIST))
>    return 0;
> if (cap == 0) {
>    /* find first */
>    cap = pci_config_readb(pci->bdf, PCI_CAPABILITY_LIST);
> } else {
>    /* find next */
>    cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT);
> }
> for (i = 0; cap && i <= 0xff; i++) {
>    if (pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_ID) == cap_id)
>        return cap;
>    cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT);
> }
> return 0;
>
> -}
>
> // Enable PCI bus-mastering (ie, DMA) support on a pci device
>  void
>  pci_enable_busmaster(struct pci_device *pci)
> diff --git a/src/hw/pcidevice.h b/src/hw/pcidevice.h
> index 354b549..225d545 100644
> --- a/src/hw/pcidevice.h
> +++ b/src/hw/pcidevice.h
> @@ -69,7 +69,6 @@ int pci_init_device(const struct pci_device_id *ids
>                      , struct pci_device *pci, void *arg);
>  struct pci_device *pci_find_init_device(const struct pci_device_id *ids
>                                          , void *arg);
> -u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap);
>  void pci_enable_busmaster(struct pci_device *pci);
>  u16 pci_enable_iobar(struct pci_device *pci, u32 addr);
>  void *pci_enable_membar(struct pci_device *pci, u32 addr);
> diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c
> index e5c2c33..96f9c6b 100644
> --- a/src/hw/virtio-pci.c
> +++ b/src/hw/virtio-pci.c
> @@ -19,7 +19,7 @@
>  #include "malloc.h" // free
>  #include "output.h" // dprintf
>  #include "pci.h" // pci_config_readl
> -#include "pcidevice.h" // pci_find_capability
> +#include "pcidevice.h" // struct pci_device
>  #include "pci_regs.h" // PCI_BASE_ADDRESS_0
>  #include "string.h" // memset
>  #include "virtio-pci.h"
> @@ -381,7 +381,7 @@ fail:
>
> void vp_init_simple(struct vp_device *vp, struct pci_device *pci)
>  {
> u8 cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, 0);
> u8 cap = pci_find_capability(pci->bdf, PCI_CAP_ID_VNDR, 0);
> struct vp_cap *vp_cap;
> const char *mode;
> u32 offset, base, mul;
>
> @@ -479,7 +479,7 @@ void vp_init_simple(struct vp_device *vp, struct pci_device *pci)
>                      vp_cap->cap, type, vp_cap->bar, addr, offset, mode);
>          }
>    cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, cap);
>    cap = pci_find_capability(pci->bdf, PCI_CAP_ID_VNDR, cap);
>
> }
>
> if (vp->common.cap && vp->notify.cap && vp->isr.cap && vp->device.cap) {

[PATCH v5 2/3] pci: add QEMU-specific PCI capability structure

[Aleksandr Bezzubikov]

On PCI init PCI bridge devices may need some
extra info about bus number to reserve, IO, memory and
prefetchable memory limits. QEMU can provide this
with special vendor-specific PCI capability.

This capability is intended to be used only
for Red Hat PCI bridges, i.e. QEMU cooperation.

Signed-off-by: Aleksandr Bezzubikov 
---
 src/fw/dev-pci.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 src/fw/dev-pci.h

diff --git a/src/fw/dev-pci.h b/src/fw/dev-pci.h
new file mode 100644
index 0000000..cf16b2e
--- /dev/null
+++ b/src/fw/dev-pci.h
@@ -0,0 +1,52 @@
+#ifndef _PCI_CAP_H
+#define _PCI_CAP_H
+
+#include "types.h"
+
+/*
+ *
+ * QEMU-specific vendor(Red Hat)-specific capability.
+ * It's intended to provide some hints for firmware to init PCI devices.
+ *
+ * Its structure is shown below:
+ *
+ * Header:
+ *
+ * u8 id;       Standard PCI Capability Header field
+ * u8 next;     Standard PCI Capability Header field
+ * u8 len;      Standard PCI Capability Header field
+ * u8 type;     Red Hat vendor-specific capability type
+ * Data:
+ *
+ * u32 bus_res;     minimum bus number to reserve;
+ *                  this is necessary for PCI Express Root Ports
+ *                  to support PCI bridges hotplug
+ * u64 io;          IO space to reserve
+ * u32 mem;         non-prefetchable memory to reserve
+ *
+ * this two fields are mutually exclusive:
+ * u32 prefetchable_mem_32;     prefetchable memory to reserve (32-bit MMIO)
+ * u64 prefetchable_mem_64;     prefetchable memory to reserve (64-bit MMIO)
+ *
+ *
+ * If any field value in Data section is 0xFF...F,
+ * it means that such kind of reservation is not needed and must be ignored.
+ *
+*/
+
+/* Offset of vendor-specific capability type field */
+#define PCI_CAP_REDHAT_TYPE_OFFSET  3
+
+/* List of valid Red Hat vendor-specific capability types */
+#define REDHAT_CAP_RESOURCE_RESERVE    1
+
+
+/* Offsets of RESOURCE_RESERVE capability fields */
+#define RES_RESERVE_BUS_RES        4
+#define RES_RESERVE_IO             8
+#define RES_RESERVE_MEM            16
+#define RES_RESERVE_PREF_MEM_32    20
+#define RES_RESERVE_PREF_MEM_64    24
+#define RES_RESERVE_CAP_SIZE       32
+
+#endif /* _PCI_CAP_H */

[PATCH v5 3/3] pci: enable RedHat PCI bridges to reserve additional resource on PCI init

[Aleksandr Bezzubikov]

In case of Red Hat Generic PCIE Root Port reserve additional buses
and/or IO/MEM/PREF space, which values are provided in a vendor-specific capability.

Signed-off-by: Aleksandr Bezzubikov 
---
 src/fw/dev-pci.h |   2 +-
 src/fw/pciinit.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++------
 src/hw/pci_ids.h |   3 ++
 3 files changed, 116 insertions(+), 14 deletions(-)

diff --git a/src/fw/dev-pci.h b/src/fw/dev-pci.h
index cf16b2e..99ccc12 100644
--- a/src/fw/dev-pci.h
+++ b/src/fw/dev-pci.h
@@ -38,7 +38,7 @@
 #define PCI_CAP_REDHAT_TYPE_OFFSET  3

/* List of valid Red Hat vendor-specific capability types */
-#define REDHAT_CAP_RESOURCE_RESERVE    1
+#define REDHAT_CAP_RESOURCE_RESERVE 1

/* Offsets of RESOURCE_RESERVE capability fields */
diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index 864954f..d9aef56 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -15,6 +15,7 @@
 #include "hw/pcidevice.h" // pci_probe_devices
 #include "hw/pci_ids.h" // PCI_VENDOR_ID_INTEL
 #include "hw/pci_regs.h" // PCI_COMMAND
+#include "fw/dev-pci.h" // REDHAT_CAP_RESOURCE_RESERVE
 #include "list.h" // struct hlist_node
 #include "malloc.h" // free
 #include "output.h" // dprintf
@@ -522,6 +523,32 @@ static void pci_bios_init_platform(void)
     }
 }

+static u8 pci_find_resource_reserve_capability(u16 bdf)
+{
+    if (pci_config_readw(bdf, PCI_VENDOR_ID) == PCI_VENDOR_ID_REDHAT &&
+        pci_config_readw(bdf, PCI_DEVICE_ID) ==
+                PCI_DEVICE_ID_REDHAT_ROOT_PORT) {
+        u8 cap = 0;
+        do {
+            cap = pci_find_capability(bdf, PCI_CAP_ID_VNDR, cap);
+        } while (cap &&
+                 pci_config_readb(bdf, cap + PCI_CAP_REDHAT_TYPE_OFFSET) !=
+                        REDHAT_CAP_RESOURCE_RESERVE);
+        if (cap) {
+            u8 cap_len = pci_config_readb(bdf, cap + PCI_CAP_FLAGS);
+            if (cap_len < RES_RESERVE_CAP_SIZE) {
+                dprintf(1, "PCI: QEMU resource reserve cap length %d is invalid\n",
+                        cap_len);
+            }
+        } else {
+            dprintf(1, "PCI: invalid QEMU resource reserve cap offset\n");
+        }
+        return cap;
+    } else {
+        dprintf(1, "PCI: QEMU resource reserve cap not found\n");
+        return 0;
+    }
+}

/****************************************************************
  * Bus initialization
@@ -578,9 +605,28 @@ pci_bios_init_bus_rec(int bus, u8 *pci_bus)
         pci_bios_init_bus_rec(secbus, pci_bus);

if (subbus != *pci_bus) {
+            u8 res_bus = 0;
+            u8 cap = pci_find_resource_reserve_capability(bdf);
+
+            if (cap) {
+                u32 tmp_res_bus = pci_config_readl(bdf,
+                        cap + RES_RESERVE_BUS_RES);
+                if (tmp_res_bus != (u32)-1) {
+                    res_bus = tmp_res_bus & 0xFF;
+                    if ((u8)(res_bus + secbus) < secbus ||
+                            (u8)(res_bus + secbus) < res_bus) {
+                        dprintf(1, "PCI: bus_reserve value %d is invalid\n",
+                                res_bus);
+                        res_bus = 0;
+                    }
+                }
+                res_bus = (*pci_bus > secbus + res_bus) ? *pci_bus
+                        : secbus + res_bus;
+            }
             dprintf(1, "PCI: subordinate bus = 0x%x -> 0x%x\n",
-                    subbus, *pci_bus);
-            subbus = *pci_bus;
+                    subbus, res_bus);
+            subbus = res_bus;
+            *pci_bus = res_bus;
         } else {
             dprintf(1, "PCI: subordinate bus = 0x%x\n", subbus);
         }
@@ -844,22 +890,74 @@ static int pci_bios_check_devices(struct pci_bus *busses)
              */
             parent = &busses[0];
         int type;
-        u8 pcie_cap = pci_find_capability(s->bus_dev->bdf, PCI_CAP_ID_EXP, 0);
+        u16 bdf = s->bus_dev->bdf;
+        u8 pcie_cap = pci_find_capability(bdf, PCI_CAP_ID_EXP, 0);
+        u8 qemu_cap = pci_find_resource_reserve_capability(bdf);
+
         int hotplug_support = pci_bus_hotplug_support(s, pcie_cap);
         for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
             u64 align = (type == PCI_REGION_TYPE_IO) ?
-                PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
+                    PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
             if (!pci_bridge_has_region(s->bus_dev, type))
                 continue;
-            if (pci_region_align(&s->r[type]) > align)
-                 align = pci_region_align(&s->r[type]);
-            u64 sum = pci_region_sum(&s->r[type]);
-            int resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO);
-            if (!sum && hotplug_support && !resource_optional)
-                sum = align; /* reserve min size for hot-plug */
-            u64 size = ALIGN(sum, align);
-            int is64 = pci_bios_bridge_region_is64(&s->r[type],
-                                            s->bus_dev, type);
+            u64 size;
+            int is64;
+            int qemu_cap_used = 0;
+            if (qemu_cap) {
+                u32 tmp_size;
+                u64 tmp_size_64;
+                switch(type) {
+                case PCI_REGION_TYPE_IO:
+                    tmp_size_64 = (pci_config_readl(bdf, qemu_cap + RES_RESERVE_IO) |
+                            (u64)pci_config_readl(bdf, qemu_cap + RES_RESERVE_IO + 4) << 32);
+                    if (tmp_size_64 != (u64)-1) {
+                        size = tmp_size_64;
+                        is64 = 0;
+                        qemu_cap_used = 1;
+                    }
+                    break;
+                case PCI_REGION_TYPE_MEM:
+                    tmp_size = pci_config_readl(bdf, qemu_cap + RES_RESERVE_MEM);
+                    if (tmp_size != (u32)-1) {
+                        size = tmp_size;
+                        is64 = 0;
+                        qemu_cap_used = 1;
+                    }
+                    break;
+                case PCI_REGION_TYPE_PREFMEM:
+                    tmp_size = pci_config_readl(bdf, qemu_cap + RES_RESERVE_PREF_MEM_32);
+                    tmp_size_64 = (pci_config_readl(bdf, qemu_cap + RES_RESERVE_PREF_MEM_64) |
+                            (u64)pci_config_readl(bdf, qemu_cap + RES_RESERVE_PREF_MEM_64 + 4) << 32);
+                    if (tmp_size != (u32)-1 && tmp_size_64 == (u64)-1) {
+                        size = tmp_size;
+                        is64 = 0;
+                        qemu_cap_used = 1;
+                    } else if (tmp_size == (u32)-1 && tmp_size_64 != (u64)-1) {
+                        size = tmp_size_64;
+                        is64 = 1;
+                        qemu_cap_used = 1;
+                    } else if (tmp_size != (u32)-1 && tmp_size_64 != (u64)-1) {
+                        dprintf(1, "PCI: resource reserve cap PREF32 and PREF64"
+                                " conflict\n");
+                    }
+                    break;
+                default:
+                    break;
+                }
+            }
+            if (!qemu_cap_used) {
+                if (pci_region_align(&s->r[type]) > align)
+                     align = pci_region_align(&s->r[type]);
+                u64 sum = pci_region_sum(&s->r[type]);
+                int resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO);
+                if (!sum && hotplug_support && !resource_optional)
+                    sum = align; /* reserve min size for hot-plug */
+                size = ALIGN(sum, align);
+                is64 = pci_bios_bridge_region_is64(&s->r[type],
+                                                s->bus_dev, type);
+            } else {
+                dprintf(1, "PCI: resource reserve cap used\n");
+            }
             // entry->bar is -1 if the entry represents a bridge region
             struct pci_region_entry *entry = pci_region_create_entry(
                 parent, s->bus_dev, -1, size, align, type, is64);
@@ -951,6 +1049,7 @@ pci_region_map_one_entry(struct pci_region_entry *entry, u64 addr)

u16 bdf = entry->dev->bdf;
     u64 limit = addr + entry->size - 1;
+
     if (entry->type == PCI_REGION_TYPE_IO) {
         pci_config_writeb(bdf, PCI_IO_BASE, addr >> PCI_IO_SHIFT);
         pci_config_writew(bdf, PCI_IO_BASE_UPPER16, 0);
diff --git a/src/hw/pci_ids.h b/src/hw/pci_ids.h
index 4ac73b4..38fa2ca 100644
--- a/src/hw/pci_ids.h
+++ b/src/hw/pci_ids.h
@@ -2263,6 +2263,9 @@
 #define PCI_DEVICE_ID_KORENIX_JETCARDF0	0x1600
 #define PCI_DEVICE_ID_KORENIX_JETCARDF1	0x16ff

+#define PCI_VENDOR_ID_REDHAT		0x1b36
+#define PCI_DEVICE_ID_REDHAT_ROOT_PORT	0x000C
+
 #define PCI_VENDOR_ID_TEKRAM		0x1de1
 #define PCI_DEVICE_ID_TEKRAM_DC290	0xdc29

[PATCH v5 3/3] pci: enable RedHat PCI bridges to reserve additional resource on PCI init

[Marcel Apfelbaum]

On 11/08/2017 2:21, Aleksandr Bezzubikov wrote:
> In case of Red Hat Generic PCIE Root Port reserve additional buses
> and/or IO/MEM/PREF space, which values are provided in a vendor-specific capability.

Hi Aleksandr,
> Signed-off-by: Aleksandr Bezzubikov
>
> src/fw/dev-pci.h |   2 +-
>   src/fw/pciinit.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++------
>   src/hw/pci_ids.h |   3 ++
>   3 files changed, 116 insertions(+), 14 deletions(-)
>
> diff --git a/src/fw/dev-pci.h b/src/fw/dev-pci.h
> index cf16b2e..99ccc12 100644
> --- a/src/fw/dev-pci.h
> +++ b/src/fw/dev-pci.h
> @@ -38,7 +38,7 @@
>   #define PCI_CAP_REDHAT_TYPE_OFFSET  3
>
> /* List of valid Red Hat vendor-specific capability types */
> -#define REDHAT_CAP_RESOURCE_RESERVE    1
> +#define REDHAT_CAP_RESOURCE_RESERVE 1

Do you need the above chunk? If not, please
get rid if it.
> /* Offsets of RESOURCE_RESERVE capability fields */
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 864954f..d9aef56 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -15,6 +15,7 @@
>   #include "hw/pcidevice.h" // pci_probe_devices
>   #include "hw/pci_ids.h" // PCI_VENDOR_ID_INTEL
>   #include "hw/pci_regs.h" // PCI_COMMAND
> +#include "fw/dev-pci.h" // REDHAT_CAP_RESOURCE_RESERVE
>   #include "list.h" // struct hlist_node
>   #include "malloc.h" // free
>   #include "output.h" // dprintf
> @@ -522,6 +523,32 @@ static void pci_bios_init_platform(void)
>       }
>   }
>
> +static u8 pci_find_resource_reserve_capability(u16 bdf)
> +{
> if (pci_config_readw(bdf, PCI_VENDOR_ID) == PCI_VENDOR_ID_REDHAT &&
>    pci_config_readw(bdf, PCI_DEVICE_ID) ==
>            PCI_DEVICE_ID_REDHAT_ROOT_PORT) {
>    u8 cap = 0;
>    do {
>        cap = pci_find_capability(bdf, PCI_CAP_ID_VNDR, cap);
>    } while (cap &&
>             pci_config_readb(bdf, cap + PCI_CAP_REDHAT_TYPE_OFFSET) !=
>                    REDHAT_CAP_RESOURCE_RESERVE);
>    if (cap) {
>        u8 cap_len = pci_config_readb(bdf, cap + PCI_CAP_FLAGS);
>        if (cap_len < RES_RESERVE_CAP_SIZE) {
>            dprintf(1, "PCI: QEMU resource reserve cap length %d is invalid\n",
>                    cap_len);
>        }
>    } else {
>        dprintf(1, "PCI: invalid QEMU resource reserve cap offset\n");
>    }
>    return cap;
> } else {
>    dprintf(1, "PCI: QEMU resource reserve cap not found\n");
>    return 0;
> }
>
> +}
>
> /****************************************************************
> Bus initialization
>
> @@ -578,9 +605,28 @@ pci_bios_init_bus_rec(int bus, u8 *pci_bus)
>           pci_bios_init_bus_rec(secbus, pci_bus);
>       if (subbus != *pci_bus) {
>        u8 res_bus = 0;
>        u8 cap = pci_find_resource_reserve_capability(bdf);
>        if (cap) {
>            u32 tmp_res_bus = pci_config_readl(bdf,
>                    cap + RES_RESERVE_BUS_RES);
>            if (tmp_res_bus != (u32)-1) {
>                res_bus = tmp_res_bus & 0xFF;
>                if ((u8)(res_bus + secbus) < secbus ||
>                        (u8)(res_bus + secbus) < res_bus) {
>                    dprintf(1, "PCI: bus_reserve value %d is invalid\n",
>                            res_bus);
>                    res_bus = 0;
>                }
>            }
>            res_bus = (*pci_bus > secbus + res_bus) ? *pci_bus
>                    : secbus + res_bus;
>        }
>         dprintf(1, "PCI: subordinate bus = 0x%x -> 0x%x\n",
>                subbus, *pci_bus);
>        subbus = *pci_bus;
>                subbus, res_bus);
>        subbus = res_bus;
>        *pci_bus = res_bus;
>     } else {
>         dprintf(1, "PCI: subordinate bus = 0x%x\n", subbus);
>     }
>
> @@ -844,22 +890,74 @@ static int pci_bios_check_devices(struct pci_bus *busses)
>                */
>               parent = &busses[0];
>           int type;
>    u8 pcie_cap = pci_find_capability(s->bus_dev->bdf, PCI_CAP_ID_EXP, 0);
>    u16 bdf = s->bus_dev->bdf;
>    u8 pcie_cap = pci_find_capability(bdf, PCI_CAP_ID_EXP, 0);
>    u8 qemu_cap = pci_find_resource_reserve_capability(bdf);
>     int hotplug_support = pci_bus_hotplug_support(s, pcie_cap);
>     for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
>         u64 align = (type == PCI_REGION_TYPE_IO) ?
>            PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
>                PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;

The above chunk is also not needed.
>           if (!pci_bridge_has_region(s->bus_dev, type))
>               continue;
>        if (pci_region_align(&s->r[type]) > align)
>             align = pci_region_align(&s->r[type]);
>        u64 sum = pci_region_sum(&s->r[type]);
>        int resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO);
>        if (!sum && hotplug_support && !resource_optional)
>            sum = align; /* reserve min size for hot-plug */
>        u64 size = ALIGN(sum, align);
>        int is64 = pci_bios_bridge_region_is64(&s->r[type],
>                                        s->bus_dev, type);
>        u64 size;
>        int is64;

I think 'is64' flag should be computed as before even if
we have QEMU hints.
The same about alignment. Please see below (1).
>        int qemu_cap_used = 0;
>        if (qemu_cap) {
>            u32 tmp_size;
>            u64 tmp_size_64;
>            switch(type) {
>            case PCI_REGION_TYPE_IO:
>                tmp_size_64 = (pci_config_readl(bdf, qemu_cap + RES_RESERVE_IO) |
>                        (u64)pci_config_readl(bdf, qemu_cap + RES_RESERVE_IO + 4) << 32);
>                if (tmp_size_64 != (u64)-1) {
>                    size = tmp_size_64;
>                    is64 = 0;
>                    qemu_cap_used = 1;
>                }
>                break;
>            case PCI_REGION_TYPE_MEM:
>                tmp_size = pci_config_readl(bdf, qemu_cap + RES_RESERVE_MEM);
>                if (tmp_size != (u32)-1) {
>                    size = tmp_size;
>                    is64 = 0;
>                    qemu_cap_used = 1;
>                }
>                break;
>            case PCI_REGION_TYPE_PREFMEM:
>                tmp_size = pci_config_readl(bdf, qemu_cap + RES_RESERVE_PREF_MEM_32);
>                tmp_size_64 = (pci_config_readl(bdf, qemu_cap + RES_RESERVE_PREF_MEM_64) |
>                        (u64)pci_config_readl(bdf, qemu_cap + RES_RESERVE_PREF_MEM_64 + 4) << 32);
>                if (tmp_size != (u32)-1 && tmp_size_64 == (u64)-1) {
>                    size = tmp_size;
>                    is64 = 0;
>                    qemu_cap_used = 1;
>                } else if (tmp_size == (u32)-1 && tmp_size_64 != (u64)-1) {
>                    size = tmp_size_64;
>                    is64 = 1;
>                    qemu_cap_used = 1;
>                } else if (tmp_size != (u32)-1 && tmp_size_64 != (u64)-1) {
>                    dprintf(1, "PCI: resource reserve cap PREF32 and PREF64"
>                            " conflict\n");
>                }
>                break;
>            default:
>                break;
>            }
>        }
>        if (!qemu_cap_used) {
>            if (pci_region_align(&s->r[type]) > align)
>                 align = pci_region_align(&s->r[type]);
>            u64 sum = pci_region_sum(&s->r[type]);

Even if qemu_cap_used  you still need to compute the sum and take
      max (sum, cap_res_size)
in case QEMU hint is smaller than what devices attached to bridge need.
so you use:
      size = ALIGN(max(sum,cap_size), align)
>            int resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO);
>            if (!sum && hotplug_support && !resource_optional)
>                sum = align; /* reserve min size for hot-plug */
>            size = ALIGN(sum, align);
>            is64 = pci_bios_bridge_region_is64(&s->r[type],
>                                            s->bus_dev, type);

(1) I think you need the last 2 lines for both cases.
>        } else {
>            dprintf(1, "PCI: resource reserve cap used\n");
>        }
>         // entry->bar is -1 if the entry represents a bridge region
>         struct pci_region_entry *entry = pci_region_create_entry(
>             parent, s->bus_dev, -1, size, align, type, is64);
>
> @@ -951,6 +1049,7 @@ pci_region_map_one_entry(struct pci_region_entry *entry, u64 addr)
>   u16 bdf = entry->dev->bdf;
>   u64 limit = addr + entry->size - 1;

Get rid of this too, please.
>   if (entry->type == PCI_REGION_TYPE_IO) {
>       pci_config_writeb(bdf, PCI_IO_BASE, addr >> PCI_IO_SHIFT);
>       pci_config_writew(bdf, PCI_IO_BASE_UPPER16, 0);
>
> diff --git a/src/hw/pci_ids.h b/src/hw/pci_ids.h
> index 4ac73b4..38fa2ca 100644
> --- a/src/hw/pci_ids.h
> +++ b/src/hw/pci_ids.h
> @@ -2263,6 +2263,9 @@
>   #define PCI_DEVICE_ID_KORENIX_JETCARDF0	0x1600
>   #define PCI_DEVICE_ID_KORENIX_JETCARDF1	0x16ff
>
> +#define PCI_VENDOR_ID_REDHAT		0x1b36
> +#define PCI_DEVICE_ID_REDHAT_ROOT_PORT	0x000C
> #define PCI_VENDOR_ID_TEKRAM		0x1de1
> #define PCI_DEVICE_ID_TEKRAM_DC290	0xdc29

Other than taking into account both the capability and the devices
behind the bridge, the patch looks ready.

Thanks for all the work you put into it,
Marcel

[PATCH v5 3/3] pci: enable RedHat PCI bridges to reserve additional resource on PCI init

[Marcel Apfelbaum]

On 11/08/2017 2:21, Aleksandr Bezzubikov wrote:
> In case of Red Hat Generic PCIE Root Port reserve additional buses
> and/or IO/MEM/PREF space, which values are provided in a vendor-specific capability.
> Signed-off-by: Aleksandr Bezzubikov
>
> src/fw/dev-pci.h |   2 +-
>   src/fw/pciinit.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++------
>   src/hw/pci_ids.h |   3 ++
>   3 files changed, 116 insertions(+), 14 deletions(-)
>
> diff --git a/src/fw/dev-pci.h b/src/fw/dev-pci.h
> index cf16b2e..99ccc12 100644
> --- a/src/fw/dev-pci.h
> +++ b/src/fw/dev-pci.h
> @@ -38,7 +38,7 @@
>   #define PCI_CAP_REDHAT_TYPE_OFFSET  3
>
> /* List of valid Red Hat vendor-specific capability types */
> -#define REDHAT_CAP_RESOURCE_RESERVE    1
> +#define REDHAT_CAP_RESOURCE_RESERVE 1
>
> /* Offsets of RESOURCE_RESERVE capability fields */
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 864954f..d9aef56 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -15,6 +15,7 @@
>   #include "hw/pcidevice.h" // pci_probe_devices
>   #include "hw/pci_ids.h" // PCI_VENDOR_ID_INTEL
>   #include "hw/pci_regs.h" // PCI_COMMAND
> +#include "fw/dev-pci.h" // REDHAT_CAP_RESOURCE_RESERVE
>   #include "list.h" // struct hlist_node
>   #include "malloc.h" // free
>   #include "output.h" // dprintf
> @@ -522,6 +523,32 @@ static void pci_bios_init_platform(void)
>       }
>   }
>
> +static u8 pci_find_resource_reserve_capability(u16 bdf)
> +{
> if (pci_config_readw(bdf, PCI_VENDOR_ID) == PCI_VENDOR_ID_REDHAT &&
>    pci_config_readw(bdf, PCI_DEVICE_ID) ==
>            PCI_DEVICE_ID_REDHAT_ROOT_PORT) {
>    u8 cap = 0;
>    do {
>        cap = pci_find_capability(bdf, PCI_CAP_ID_VNDR, cap);
>    } while (cap &&
>             pci_config_readb(bdf, cap + PCI_CAP_REDHAT_TYPE_OFFSET) !=
>                    REDHAT_CAP_RESOURCE_RESERVE);
>    if (cap) {
>        u8 cap_len = pci_config_readb(bdf, cap + PCI_CAP_FLAGS);
>        if (cap_len < RES_RESERVE_CAP_SIZE) {
>            dprintf(1, "PCI: QEMU resource reserve cap length %d is invalid\n",
>                    cap_len);
>        }
>    } else {
>        dprintf(1, "PCI: invalid QEMU resource reserve cap offset\n");
>    }
>    return cap;
> } else {
>    dprintf(1, "PCI: QEMU resource reserve cap not found\n");
>    return 0;
> }
>
> +}
>
> /****************************************************************
> Bus initialization
>
> @@ -578,9 +605,28 @@ pci_bios_init_bus_rec(int bus, u8 *pci_bus)
>           pci_bios_init_bus_rec(secbus, pci_bus);
>       if (subbus != *pci_bus) {
>        u8 res_bus = 0;
>        u8 cap = pci_find_resource_reserve_capability(bdf);
>        if (cap) {
>            u32 tmp_res_bus = pci_config_readl(bdf,
>                    cap + RES_RESERVE_BUS_RES);
>            if (tmp_res_bus != (u32)-1) {
>                res_bus = tmp_res_bus & 0xFF;
>                if ((u8)(res_bus + secbus) < secbus ||
>                        (u8)(res_bus + secbus) < res_bus) {
>                    dprintf(1, "PCI: bus_reserve value %d is invalid\n",
>                            res_bus);
>                    res_bus = 0;
>                }
>            }
>            res_bus = (*pci_bus > secbus + res_bus) ? *pci_bus
>                    : secbus + res_bus;
>        }
>         dprintf(1, "PCI: subordinate bus = 0x%x -> 0x%x\n",
>                subbus, *pci_bus);
>        subbus = *pci_bus;
>                subbus, res_bus);
>        subbus = res_bus;
>        *pci_bus = res_bus;
>     } else {
>         dprintf(1, "PCI: subordinate bus = 0x%x\n", subbus);
>     }
>
> @@ -844,22 +890,74 @@ static int pci_bios_check_devices(struct pci_bus *busses)
>                */
>               parent = &busses[0];
>           int type;
>    u8 pcie_cap = pci_find_capability(s->bus_dev->bdf, PCI_CAP_ID_EXP, 0);
>    u16 bdf = s->bus_dev->bdf;
>    u8 pcie_cap = pci_find_capability(bdf, PCI_CAP_ID_EXP, 0);
>    u8 qemu_cap = pci_find_resource_reserve_capability(bdf);
>     int hotplug_support = pci_bus_hotplug_support(s, pcie_cap);
>     for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
>         u64 align = (type == PCI_REGION_TYPE_IO) ?
>            PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
>                PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
>         if (!pci_bridge_has_region(s->bus_dev, type))
>             continue;
>        if (pci_region_align(&s->r[type]) > align)
>             align = pci_region_align(&s->r[type]);
>        u64 sum = pci_region_sum(&s->r[type]);
>        int resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO);
>        if (!sum && hotplug_support && !resource_optional)
>            sum = align; /* reserve min size for hot-plug */
>        u64 size = ALIGN(sum, align);
>        int is64 = pci_bios_bridge_region_is64(&s->r[type],
>                                        s->bus_dev, type);
>        u64 size;
>        int is64;
>        int qemu_cap_used = 0;
>        if (qemu_cap) {
>            u32 tmp_size;
>            u64 tmp_size_64;
>            switch(type) {
>            case PCI_REGION_TYPE_IO:
>                tmp_size_64 = (pci_config_readl(bdf, qemu_cap + RES_RESERVE_IO) |
>                        (u64)pci_config_readl(bdf, qemu_cap + RES_RESERVE_IO + 4) << 32);
>                if (tmp_size_64 != (u64)-1) {
>                    size = tmp_size_64;
>                    is64 = 0;
>                    qemu_cap_used = 1;
>                }
>                break;
>            case PCI_REGION_TYPE_MEM:
>                tmp_size = pci_config_readl(bdf, qemu_cap + RES_RESERVE_MEM);
>                if (tmp_size != (u32)-1) {
>                    size = tmp_size;
>                    is64 = 0;
>                    qemu_cap_used = 1;
>                }
>                break;
>            case PCI_REGION_TYPE_PREFMEM:
>                tmp_size = pci_config_readl(bdf, qemu_cap + RES_RESERVE_PREF_MEM_32);
>                tmp_size_64 = (pci_config_readl(bdf, qemu_cap + RES_RESERVE_PREF_MEM_64) |
>                        (u64)pci_config_readl(bdf, qemu_cap + RES_RESERVE_PREF_MEM_64 + 4) << 32);
>                if (tmp_size != (u32)-1 && tmp_size_64 == (u64)-1) {
>                    size = tmp_size;
>                    is64 = 0;
>                    qemu_cap_used = 1;
>                } else if (tmp_size == (u32)-1 && tmp_size_64 != (u64)-1) {
>                    size = tmp_size_64;
>                    is64 = 1;
>                    qemu_cap_used = 1;
>                } else if (tmp_size != (u32)-1 && tmp_size_64 != (u64)-1) {
>                    dprintf(1, "PCI: resource reserve cap PREF32 and PREF64"
>                            " conflict\n");
>                }
>                break;
>            default:
>                break;
>            }
>        }
>        if (!qemu_cap_used) {
>            if (pci_region_align(&s->r[type]) > align)
>                 align = pci_region_align(&s->r[type]);
>            u64 sum = pci_region_sum(&s->r[type]);
>            int resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO);
>            if (!sum && hotplug_support && !resource_optional)
>                sum = align; /* reserve min size for hot-plug */
>            size = ALIGN(sum, align);
>            is64 = pci_bios_bridge_region_is64(&s->r[type],
>                                            s->bus_dev, type);
>        } else {
>            dprintf(1, "PCI: resource reserve cap used\n");

Forgot about this one. Adding debug info that the cap was used is
a great idea, but I think it misses extra info: What was used?

I would expect to see:
    - qemu resource reserve cap mem/io/pref...: 20000
or something like this.

Thanks,
Marcel
>        }
>         // entry->bar is -1 if the entry represents a bridge region
>         struct pci_region_entry *entry = pci_region_create_entry(
>             parent, s->bus_dev, -1, size, align, type, is64);
>
> @@ -951,6 +1049,7 @@ pci_region_map_one_entry(struct pci_region_entry *entry, u64 addr)
>   u16 bdf = entry->dev->bdf;
>   u64 limit = addr + entry->size - 1;
> if (entry->type == PCI_REGION_TYPE_IO) {
>     pci_config_writeb(bdf, PCI_IO_BASE, addr >> PCI_IO_SHIFT);
>     pci_config_writew(bdf, PCI_IO_BASE_UPPER16, 0);
>
> diff --git a/src/hw/pci_ids.h b/src/hw/pci_ids.h
> index 4ac73b4..38fa2ca 100644
> --- a/src/hw/pci_ids.h
> +++ b/src/hw/pci_ids.h
> @@ -2263,6 +2263,9 @@
>   #define PCI_DEVICE_ID_KORENIX_JETCARDF0	0x1600
>   #define PCI_DEVICE_ID_KORENIX_JETCARDF1	0x16ff
>
> +#define PCI_VENDOR_ID_REDHAT		0x1b36
> +#define PCI_DEVICE_ID_REDHAT_ROOT_PORT	0x000C
> #define PCI_VENDOR_ID_TEKRAM		0x1de1
> #define PCI_DEVICE_ID_TEKRAM_DC290	0xdc29