[Qemu-devel] [PATCH v2 0/6] pci: keep window properties up to date

[Qemu-devel] [PATCH v2 0/6] pci: keep window properties up to date

[Michael S. Tsirkin]

w32/w64 properties that we report in QOM are
currently static, but this is wrong:
guest firmware can select its own windows:
optimal placement for w64 is guest-dependent, further,
for Q35, w32 is affected by the MCFG base and size.

This detects the actual window configuration used by guest
and reports it in QOM.

Changes from v1:
Fix up issues noted by Igor:
    - ignore addresses below 1<<32 when getting W64
    - don't change W64 - instead calculate it in
      a local variable. This way users are forced
      to use the property API so they won't get
      an out of date range by accessing w64 directly.

Note: maybe we should get rid of the pci_info struct completely, it doesn't
seem to be all that useful anymore.

However, this is better addressed by a separate patch.

Michael S. Tsirkin (6):
  q35: make pci window address/size match guest cfg
  range: add Range to typedefs
  range: add min/max operations on ranges
  pci: add helper to retrieve the 64-bit range
  q35: use 64 bit window programmed by guest
  piix: use 64 bit window programmed by guest

 include/hw/pci/pci.h    |  1 +
 include/qemu/range.h    | 19 ++++++++++++++++++-
 include/qemu/typedefs.h |  1 +
 hw/pci-host/piix.c      | 14 ++++++++++----
 hw/pci-host/q35.c       | 24 ++++++++++++++++++++----
 hw/pci/pci.c            | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 100 insertions(+), 9 deletions(-)

-- 
MST

[Qemu-devel] [PATCH v2 1/6] q35: make pci window address/size match guest cfg

[Michael S. Tsirkin]

For Q35, MMCFG address and size are guest configurable.
Update w32 property to make it behave accordingly.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/q35.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 5473504..72f6b72 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -214,6 +214,16 @@ static void mch_update_pciexbar(MCHPCIState *mch)
     }
     addr = pciexbar & addr_mask;
     pcie_host_mmcfg_update(pehb, enable, addr, length);
+    /* Leave enough space for the MCFG BAR */
+    /*
+     * TODO: this matches current bios behaviour, but it's not a power of two,
+     * which means an MTRR can't cover it exactly.
+     */
+    if (enable) {
+        mch->pci_info.w32.begin = addr + length;
+    } else {
+        mch->pci_info.w32.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
+    }
 }
 
 /* PAM */
-- 
MST

[Qemu-devel] [PATCH v2 2/6] range: add Range to typedefs

[Michael S. Tsirkin]

will help simplify header dependencies.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/qemu/range.h    | 2 +-
 include/qemu/typedefs.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/qemu/range.h b/include/qemu/range.h
index b76cc0d..4a0780d 100644
--- a/include/qemu/range.h
+++ b/include/qemu/range.h
@@ -2,6 +2,7 @@
 #define QEMU_RANGE_H
 
 #include <inttypes.h>
+#include <qemu/typedefs.h>
 
 /*
  * Operations on 64 bit address ranges.
@@ -15,7 +16,6 @@ struct Range {
     uint64_t begin; /* First byte of the range, or 0 if empty. */
     uint64_t end;   /* 1 + the last byte. 0 if range empty or ends at ~0x0LL. */
 };
-typedef struct Range Range;
 
 /* Get last byte of a range from offset + length.
  * Undefined for ranges that wrap around 0. */
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 3205540..a4c1b84 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -68,5 +68,6 @@ typedef struct QEMUSGList QEMUSGList;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct FWCfgState FWCfgState;
 typedef struct PcGuestInfo PcGuestInfo;
+typedef struct Range Range;
 
 #endif /* QEMU_TYPEDEFS_H */
-- 
MST

[Qemu-devel] [PATCH v2 3/6] range: add min/max operations on ranges

[Michael S. Tsirkin]

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/qemu/range.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/qemu/range.h b/include/qemu/range.h
index 4a0780d..1c688ca 100644
--- a/include/qemu/range.h
+++ b/include/qemu/range.h
@@ -17,6 +17,23 @@ struct Range {
     uint64_t end;   /* 1 + the last byte. 0 if range empty or ends at ~0x0LL. */
 };
 
+static inline void range_extend(Range *range, Range *extend_by)
+{
+    if (!extend_by->begin && !extend_by->end) {
+        return;
+    }
+    if (!range->begin && !range->end) {
+        *range = *extend_by;
+        return;
+    }
+    if (range->begin > extend_by->begin) {
+        range->begin = extend_by->begin;
+    }
+    if (range->end - 1 < extend_by->end - 1) {
+        range->end = extend_by->end;
+    }
+}
+
 /* Get last byte of a range from offset + length.
  * Undefined for ranges that wrap around 0. */
 static inline uint64_t range_get_last(uint64_t offset, uint64_t len)
-- 
MST

Re: [Qemu-devel] [PATCH v2 3/6] range: add min/max operations on ranges

[Michael S. Tsirkin]

On Sun, Sep 15, 2013 at 11:46:44AM +0300, Michael S. Tsirkin wrote:
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Actually I applied the following on top - not going to
repost as that just adds a comment.

diff --git a/include/qemu/range.h b/include/qemu/range.h
index 1c688ca..aae9720 100644
--- a/include/qemu/range.h
+++ b/include/qemu/range.h
@@ -29,6 +29,7 @@ static inline void range_extend(Range *range, Range *extend_by)
     if (range->begin > extend_by->begin) {
         range->begin = extend_by->begin;
     }
+    /* Compare last byte in case region ends at ~0x0LL */
     if (range->end - 1 < extend_by->end - 1) {
         range->end = extend_by->end;
     }

[Qemu-devel] [PATCH v2 4/6] pci: add helper to retrieve the 64-bit range

[Michael S. Tsirkin]

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/pci/pci.h |  1 +
 hw/pci/pci.c         | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 37979aa..4b90e5d 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -397,6 +397,7 @@ const char *pci_root_bus_path(PCIDevice *dev);
 PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn);
 int pci_qdev_find_device(const char *id, PCIDevice **pdev);
 PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root, const char *devaddr);
+void pci_bus_get_w64_range(PCIBus *bus, Range *range);
 
 int pci_parse_devaddr(const char *addr, int *domp, int *busp,
                       unsigned int *slotp, unsigned int *funcp);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index ad1c1ca..52cbab7 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2257,6 +2257,56 @@ void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque)
     bus->iommu_opaque = opaque;
 }
 
+static void pci_dev_get_w64(PCIBus *b, PCIDevice *dev, void *opaque)
+{
+    Range *range = opaque;
+    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+    uint16_t cmd = pci_get_word(dev->config + PCI_COMMAND);
+    int r;
+
+    if (!(cmd & PCI_COMMAND_MEMORY)) {
+        return;
+    }
+
+    if (pc->is_bridge) {
+        pcibus_t base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+        pcibus_t limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+
+        base = MAX(base, 0x1ULL << 32);
+
+        if (limit >= base) {
+            Range pref_range;
+            pref_range.begin = base;
+            pref_range.end = limit + 1;
+            range_extend(range, &pref_range);
+        }
+    }
+    for (r = 0; r < PCI_NUM_REGIONS; ++r) {
+        PCIIORegion *region = &dev->io_regions[r];
+        Range region_range;
+
+        if (!region->size ||
+            (region->type & PCI_BASE_ADDRESS_SPACE_IO) ||
+            !(region->type & PCI_BASE_ADDRESS_MEM_TYPE_64)) {
+            continue;
+        }
+        region_range.begin = pci_get_quad(dev->config + pci_bar(dev, r));
+        region_range.end = region_range.begin + region->size;
+
+        region_range.begin = MAX(region_range.begin, 0x1ULL << 32);
+
+        if (region_range.end - 1 >= region_range.begin) {
+            range_extend(range, &region_range);
+        }
+    }
+}
+
+void pci_bus_get_w64_range(PCIBus *bus, Range *range)
+{
+    range->begin = range->end = 0;
+    pci_for_each_device_under_bus(bus, pci_dev_get_w64, range);
+}
+
 static const TypeInfo pci_device_type_info = {
     .name = TYPE_PCI_DEVICE,
     .parent = TYPE_DEVICE,
-- 
MST

[Qemu-devel] [PATCH v2 5/6] q35: use 64 bit window programmed by guest

[Michael S. Tsirkin]

Detect the 64 bit window programmed by firmware
and configure properties accordingly.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/q35.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 72f6b72..23dbeea 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -89,18 +89,24 @@ static void q35_host_get_pci_hole64_start(Object *obj, Visitor *v,
                                           void *opaque, const char *name,
                                           Error **errp)
 {
-    Q35PCIHost *s = Q35_HOST_DEVICE(obj);
+    PCIHostState *h = PCI_HOST_BRIDGE(obj);
+    Range w64;
+
+    pci_bus_get_w64_range(h->bus, &w64);
 
-    visit_type_uint64(v, &s->mch.pci_info.w64.begin, name, errp);
+    visit_type_uint64(v, &w64.begin, name, errp);
 }
 
 static void q35_host_get_pci_hole64_end(Object *obj, Visitor *v,
                                         void *opaque, const char *name,
                                         Error **errp)
 {
-    Q35PCIHost *s = Q35_HOST_DEVICE(obj);
+    PCIHostState *h = PCI_HOST_BRIDGE(obj);
+    Range w64;
+
+    pci_bus_get_w64_range(h->bus, &w64);
 
-    visit_type_uint64(v, &s->mch.pci_info.w64.end, name, errp);
+    visit_type_uint64(v, &w64.end, name, errp);
 }
 
 static Property mch_props[] = {
-- 
MST

[Qemu-devel] [PATCH v2 6/6] piix: use 64 bit window programmed by guest

[Michael S. Tsirkin]

Detect the 64 bit window programmed by firmware
and configure properties accordingly.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/piix.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 221d82b..c041149 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -235,18 +235,24 @@ static void i440fx_pcihost_get_pci_hole64_start(Object *obj, Visitor *v,
                                                 void *opaque, const char *name,
                                                 Error **errp)
 {
-    I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
+    PCIHostState *h = PCI_HOST_BRIDGE(obj);
+    Range w64;
+
+    pci_bus_get_w64_range(h->bus, &w64);
 
-    visit_type_uint64(v, &s->pci_info.w64.begin, name, errp);
+    visit_type_uint64(v, &w64.begin, name, errp);
 }
 
 static void i440fx_pcihost_get_pci_hole64_end(Object *obj, Visitor *v,
                                               void *opaque, const char *name,
                                               Error **errp)
 {
-    I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
+    PCIHostState *h = PCI_HOST_BRIDGE(obj);
+    Range w64;
+
+    pci_bus_get_w64_range(h->bus, &w64);
 
-    visit_type_uint64(v, &s->pci_info.w64.end, name, errp);
+    visit_type_uint64(v, &w64.end, name, errp);
 }
 
 static void i440fx_pcihost_initfn(Object *obj)
-- 
MST

Re: [Qemu-devel] [PATCH v2 0/6] pci: keep window properties up to date

[Igor Mammedov]

On Sun, 15 Sep 2013 11:46:36 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> w32/w64 properties that we report in QOM are
> currently static, but this is wrong:
> guest firmware can select its own windows:
> optimal placement for w64 is guest-dependent, further,
> for Q35, w32 is affected by the MCFG base and size.
> 
> This detects the actual window configuration used by guest
> and reports it in QOM.
> 
> Changes from v1:
> Fix up issues noted by Igor:
>     - ignore addresses below 1<<32 when getting W64
>     - don't change W64 - instead calculate it in
>       a local variable. This way users are forced
>       to use the property API so they won't get
>       an out of date range by accessing w64 directly.
> 
> Note: maybe we should get rid of the pci_info struct completely, it doesn't
> seem to be all that useful anymore.
> 
> However, this is better addressed by a separate patch.
Provided cleanup will be in ACPI tables series, then from memory hotplug pov,
Acked-By: Igor Mammedov <imammedo@redhat.com>

> Michael S. Tsirkin (6):
>   q35: make pci window address/size match guest cfg
>   range: add Range to typedefs
>   range: add min/max operations on ranges
>   pci: add helper to retrieve the 64-bit range
>   q35: use 64 bit window programmed by guest
>   piix: use 64 bit window programmed by guest
> 
>  include/hw/pci/pci.h    |  1 +
>  include/qemu/range.h    | 19 ++++++++++++++++++-
>  include/qemu/typedefs.h |  1 +
>  hw/pci-host/piix.c      | 14 ++++++++++----
>  hw/pci-host/q35.c       | 24 ++++++++++++++++++++----
>  hw/pci/pci.c            | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 100 insertions(+), 9 deletions(-)
>