| Message ID | 20230214131441.101760-7-shentey@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Q35 PCI host fixes and QOM cleanup | expand |
On Tue, Feb 14, 2023 at 02:14:35PM +0100, Bernhard Beschow wrote: > Although not used there, the attributes for Q35's "pci-hole64-size" and > "short_root_bus" properties currently reside in its child device. This > causes the default values to be overwritten during the child's > object_initialize() phase. pls add explanation why this is a problem. > Avoid this by moving both attributes into the > host device. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > include/hw/pci-host/q35.h | 5 +++-- > hw/pci-host/q35.c | 20 +++++--------------- > 2 files changed, 8 insertions(+), 17 deletions(-) > > diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h > index fcbe57b42d..93e41ffbee 100644 > --- a/include/hw/pci-host/q35.h > +++ b/include/hw/pci-host/q35.h > @@ -54,8 +54,6 @@ struct MCHPCIState { > Range pci_hole; > uint64_t below_4g_mem_size; > uint64_t above_4g_mem_size; > - uint64_t pci_hole64_size; > - uint32_t short_root_bus; > uint16_t ext_tseg_mbytes; > }; > > @@ -64,7 +62,10 @@ struct Q35PCIHost { > PCIExpressHost parent_obj; > /*< public >*/ > > + uint64_t pci_hole64_size; > + uint32_t short_root_bus; > bool pci_hole64_fix; > + > MCHPCIState mch; > }; > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > index 0e198f97a7..03aa08dae5 100644 > --- a/hw/pci-host/q35.c > +++ b/hw/pci-host/q35.c > @@ -76,7 +76,7 @@ static const char *q35_host_root_bus_path(PCIHostState *host_bridge, > Q35PCIHost *s = Q35_HOST_DEVICE(host_bridge); > > /* For backwards compat with old device paths */ > - if (s->mch.short_root_bus) { > + if (s->short_root_bus) { > return "0000"; > } > return "0000:00"; > @@ -161,27 +161,19 @@ static void q35_host_get_pci_hole64_end(Object *obj, Visitor *v, > > pci_bus_get_w64_range(h->bus, &w64); > value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1; > - hole64_end = ROUND_UP(hole64_start + s->mch.pci_hole64_size, 1ULL << 30); > + hole64_end = ROUND_UP(hole64_start + s->pci_hole64_size, 1ULL << 30); > if (s->pci_hole64_fix && value < hole64_end) { > value = hole64_end; > } > visit_type_uint64(v, name, &value, errp); > } > > -/* > - * NOTE: setting defaults for the mch.* fields in this table > - * doesn't work, because mch is a separate QOM object that is > - * zeroed by the object_initialize(&s->mch, ...) call inside > - * q35_host_initfn(). The default values for those > - * properties need to be initialized manually by > - * q35_host_initfn() after the object_initialize() call. > - */ > static Property q35_host_props[] = { > DEFINE_PROP_UINT64(PCIE_HOST_MCFG_BASE, Q35PCIHost, parent_obj.base_addr, > MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT), > DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, Q35PCIHost, > - mch.pci_hole64_size, Q35_PCI_HOST_HOLE64_SIZE_DEFAULT), > - DEFINE_PROP_UINT32("short_root_bus", Q35PCIHost, mch.short_root_bus, 0), > + pci_hole64_size, Q35_PCI_HOST_HOLE64_SIZE_DEFAULT), > + DEFINE_PROP_UINT32("short_root_bus", Q35PCIHost, short_root_bus, 0), > DEFINE_PROP_SIZE(PCI_HOST_BELOW_4G_MEM_SIZE, Q35PCIHost, > mch.below_4g_mem_size, 0), > DEFINE_PROP_SIZE(PCI_HOST_ABOVE_4G_MEM_SIZE, Q35PCIHost, > @@ -218,9 +210,7 @@ static void q35_host_initfn(Object *obj) > object_initialize_child(OBJECT(s), "mch", &s->mch, TYPE_MCH_PCI_DEVICE); > qdev_prop_set_int32(DEVICE(&s->mch), "addr", PCI_DEVFN(0, 0)); > qdev_prop_set_bit(DEVICE(&s->mch), "multifunction", false); > - /* mch's object_initialize resets the default value, set it again */ > - qdev_prop_set_uint64(DEVICE(s), PCI_HOST_PROP_PCI_HOLE64_SIZE, > - Q35_PCI_HOST_HOLE64_SIZE_DEFAULT); > + > object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "uint32", > q35_host_get_pci_hole_start, > NULL, NULL, NULL); > -- > 2.39.1
diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h index fcbe57b42d..93e41ffbee 100644 --- a/include/hw/pci-host/q35.h +++ b/include/hw/pci-host/q35.h @@ -54,8 +54,6 @@ struct MCHPCIState { Range pci_hole; uint64_t below_4g_mem_size; uint64_t above_4g_mem_size; - uint64_t pci_hole64_size; - uint32_t short_root_bus; uint16_t ext_tseg_mbytes; }; @@ -64,7 +62,10 @@ struct Q35PCIHost { PCIExpressHost parent_obj; /*< public >*/ + uint64_t pci_hole64_size; + uint32_t short_root_bus; bool pci_hole64_fix; + MCHPCIState mch; }; diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 0e198f97a7..03aa08dae5 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -76,7 +76,7 @@ static const char *q35_host_root_bus_path(PCIHostState *host_bridge, Q35PCIHost *s = Q35_HOST_DEVICE(host_bridge); /* For backwards compat with old device paths */ - if (s->mch.short_root_bus) { + if (s->short_root_bus) { return "0000"; } return "0000:00"; @@ -161,27 +161,19 @@ static void q35_host_get_pci_hole64_end(Object *obj, Visitor *v, pci_bus_get_w64_range(h->bus, &w64); value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1; - hole64_end = ROUND_UP(hole64_start + s->mch.pci_hole64_size, 1ULL << 30); + hole64_end = ROUND_UP(hole64_start + s->pci_hole64_size, 1ULL << 30); if (s->pci_hole64_fix && value < hole64_end) { value = hole64_end; } visit_type_uint64(v, name, &value, errp); } -/* - * NOTE: setting defaults for the mch.* fields in this table - * doesn't work, because mch is a separate QOM object that is - * zeroed by the object_initialize(&s->mch, ...) call inside - * q35_host_initfn(). The default values for those - * properties need to be initialized manually by - * q35_host_initfn() after the object_initialize() call. - */ static Property q35_host_props[] = { DEFINE_PROP_UINT64(PCIE_HOST_MCFG_BASE, Q35PCIHost, parent_obj.base_addr, MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT), DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, Q35PCIHost, - mch.pci_hole64_size, Q35_PCI_HOST_HOLE64_SIZE_DEFAULT), - DEFINE_PROP_UINT32("short_root_bus", Q35PCIHost, mch.short_root_bus, 0), + pci_hole64_size, Q35_PCI_HOST_HOLE64_SIZE_DEFAULT), + DEFINE_PROP_UINT32("short_root_bus", Q35PCIHost, short_root_bus, 0), DEFINE_PROP_SIZE(PCI_HOST_BELOW_4G_MEM_SIZE, Q35PCIHost, mch.below_4g_mem_size, 0), DEFINE_PROP_SIZE(PCI_HOST_ABOVE_4G_MEM_SIZE, Q35PCIHost, @@ -218,9 +210,7 @@ static void q35_host_initfn(Object *obj) object_initialize_child(OBJECT(s), "mch", &s->mch, TYPE_MCH_PCI_DEVICE); qdev_prop_set_int32(DEVICE(&s->mch), "addr", PCI_DEVFN(0, 0)); qdev_prop_set_bit(DEVICE(&s->mch), "multifunction", false); - /* mch's object_initialize resets the default value, set it again */ - qdev_prop_set_uint64(DEVICE(s), PCI_HOST_PROP_PCI_HOLE64_SIZE, - Q35_PCI_HOST_HOLE64_SIZE_DEFAULT); + object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "uint32", q35_host_get_pci_hole_start, NULL, NULL, NULL);
Although not used there, the attributes for Q35's "pci-hole64-size" and "short_root_bus" properties currently reside in its child device. This causes the default values to be overwritten during the child's object_initialize() phase. Avoid this by moving both attributes into the host device. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- include/hw/pci-host/q35.h | 5 +++-- hw/pci-host/q35.c | 20 +++++--------------- 2 files changed, 8 insertions(+), 17 deletions(-)