i440fx_init() is a legacy init function. The previous patches worked towards
TYPE_I440FX_PCI_HOST_BRIDGE to be instantiated the QOM way. Do this now by
transforming the parameters passed to i440fx_init() into property assignments.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/pci-host/i440fx.h | 10 ----------
hw/i386/pc_piix.c | 30 +++++++++++++++++++++---------
hw/pci-host/i440fx.c | 34 +++++-----------------------------
3 files changed, 26 insertions(+), 48 deletions(-)
diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
index 2d7bae5a45..c988f70890 100644
--- a/include/hw/pci-host/i440fx.h
+++ b/include/hw/pci-host/i440fx.h
@@ -34,14 +34,4 @@ struct PCII440FXState {
#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "igd-passthrough-i440FX"
-PCIBus *i440fx_init(const char *pci_type,
- DeviceState *dev,
- MemoryRegion *address_space_mem,
- MemoryRegion *address_space_io,
- ram_addr_t below_4g_mem_size,
- ram_addr_t above_4g_mem_size,
- MemoryRegion *pci_memory,
- MemoryRegion *ram_memory);
-
-
#endif
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 87bee368fc..1df309b8e2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -126,7 +126,7 @@ static void pc_init1(MachineState *machine,
MemoryRegion *rom_memory;
ram_addr_t lowmem;
uint64_t hole64_size;
- DeviceState *i440fx_host;
+ Object *i440fx_host;
/*
* Calculate ram split, for memory below and above 4G. It's a bit
@@ -201,8 +201,8 @@ static void pc_init1(MachineState *machine,
pci_memory = g_new(MemoryRegion, 1);
memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
rom_memory = pci_memory;
- i440fx_host = qdev_new(host_type);
- hole64_size = object_property_get_uint(OBJECT(i440fx_host),
+ i440fx_host = OBJECT(qdev_new(host_type));
+ hole64_size = object_property_get_uint(i440fx_host,
PCI_HOST_PROP_PCI_HOLE64_SIZE,
&error_abort);
} else {
@@ -243,12 +243,24 @@ static void pc_init1(MachineState *machine,
PIIX3State *piix3;
PCIDevice *pci_dev;
- pci_bus = i440fx_init(pci_type,
- i440fx_host,
- system_memory, system_io,
- x86ms->below_4g_mem_size,
- x86ms->above_4g_mem_size,
- pci_memory, ram_memory);
+ object_property_add_child(OBJECT(machine), "i440fx", i440fx_host);
+ object_property_set_link(i440fx_host, PCI_HOST_PROP_RAM_MEM,
+ OBJECT(ram_memory), &error_fatal);
+ object_property_set_link(i440fx_host, PCI_HOST_PROP_PCI_MEM,
+ OBJECT(pci_memory), &error_fatal);
+ object_property_set_link(i440fx_host, PCI_HOST_PROP_SYSTEM_MEM,
+ OBJECT(system_memory), &error_fatal);
+ object_property_set_link(i440fx_host, PCI_HOST_PROP_IO_MEM,
+ OBJECT(system_io), &error_fatal);
+ object_property_set_uint(i440fx_host, PCI_HOST_BELOW_4G_MEM_SIZE,
+ x86ms->below_4g_mem_size, &error_fatal);
+ object_property_set_uint(i440fx_host, PCI_HOST_ABOVE_4G_MEM_SIZE,
+ x86ms->above_4g_mem_size, &error_fatal);
+ object_property_set_str(i440fx_host, I440FX_HOST_PROP_PCI_TYPE,
+ pci_type, &error_fatal);
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(i440fx_host), &error_fatal);
+
+ pci_bus = PCI_BUS(qdev_get_child_bus(DEVICE(i440fx_host), "pci.0"));
pci_bus_map_irqs(pci_bus,
xen_enabled() ? xen_pci_slot_get_pirq
: pc_pci_slot_get_pirq);
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index e8e66afc11..62d6287681 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -249,9 +249,14 @@ static void i440fx_pcihost_initfn(Object *obj)
static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
{
+ ERRP_GUARD();
I440FXState *s = I440FX_PCI_HOST_BRIDGE(dev);
PCIHostState *phb = PCI_HOST_BRIDGE(dev);
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+ PCIBus *b;
+ PCIDevice *d;
+ PCII440FXState *f;
+ unsigned i;
memory_region_add_subregion(s->io_memory, 0xcf8, &phb->conf_mem);
sysbus_init_ioports(sbd, 0xcf8, 4);
@@ -262,37 +267,10 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
/* register i440fx 0xcf8 port as coalesced pio */
memory_region_set_flush_coalesced(&phb->data_mem);
memory_region_add_coalescing(&phb->conf_mem, 0, 4);
-}
-
-PCIBus *i440fx_init(const char *pci_type,
- DeviceState *dev,
- MemoryRegion *address_space_mem,
- MemoryRegion *address_space_io,
- ram_addr_t below_4g_mem_size,
- ram_addr_t above_4g_mem_size,
- MemoryRegion *pci_address_space,
- MemoryRegion *ram_memory)
-{
- I440FXState *s = I440FX_PCI_HOST_BRIDGE(dev);
- PCIHostState *phb = PCI_HOST_BRIDGE(dev);
- PCIBus *b;
- PCIDevice *d;
- PCII440FXState *f;
- unsigned i;
-
- s->system_memory = address_space_mem;
- s->io_memory = address_space_io;
- s->pci_address_space = pci_address_space;
- s->ram_memory = ram_memory;
- s->below_4g_mem_size = below_4g_mem_size;
- s->above_4g_mem_size = above_4g_mem_size;
- s->pci_type = (char *)pci_type;
b = pci_root_bus_new(dev, NULL, s->pci_address_space,
s->io_memory, 0, TYPE_PCI_BUS);
phb->bus = b;
- object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev));
- sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
d = pci_create_simple(b, 0, s->pci_type);
f = I440FX_PCI_DEVICE(d);
@@ -336,8 +314,6 @@ PCIBus *i440fx_init(const char *pci_type,
d->config[I440FX_COREBOOT_RAM_SIZE] = ram_size;
i440fx_update_memory_mappings(f);
-
- return b;
}
static void i440fx_class_init(ObjectClass *klass, void *data)
--
2.41.0
Hi Bernhard,
On 28/6/23 21:52, Bernhard Beschow wrote:
> i440fx_init() is a legacy init function. The previous patches worked towards
> TYPE_I440FX_PCI_HOST_BRIDGE to be instantiated the QOM way. Do this now by
> transforming the parameters passed to i440fx_init() into property assignments.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> include/hw/pci-host/i440fx.h | 10 ----------
> hw/i386/pc_piix.c | 30 +++++++++++++++++++++---------
> hw/pci-host/i440fx.c | 34 +++++-----------------------------
> 3 files changed, 26 insertions(+), 48 deletions(-)
>
> diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
> index 2d7bae5a45..c988f70890 100644
> --- a/include/hw/pci-host/i440fx.h
> +++ b/include/hw/pci-host/i440fx.h
> @@ -34,14 +34,4 @@ struct PCII440FXState {
>
> #define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "igd-passthrough-i440FX"
>
> -PCIBus *i440fx_init(const char *pci_type,
> - DeviceState *dev,
> - MemoryRegion *address_space_mem,
> - MemoryRegion *address_space_io,
> - ram_addr_t below_4g_mem_size,
> - ram_addr_t above_4g_mem_size,
> - MemoryRegion *pci_memory,
> - MemoryRegion *ram_memory);
> -
> -
> #endif
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 87bee368fc..1df309b8e2 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -126,7 +126,7 @@ static void pc_init1(MachineState *machine,
> MemoryRegion *rom_memory;
> ram_addr_t lowmem;
> uint64_t hole64_size;
> - DeviceState *i440fx_host;
> + Object *i440fx_host;
>
> /*
> * Calculate ram split, for memory below and above 4G. It's a bit
> @@ -201,8 +201,8 @@ static void pc_init1(MachineState *machine,
> pci_memory = g_new(MemoryRegion, 1);
> memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
> rom_memory = pci_memory;
> - i440fx_host = qdev_new(host_type);
> - hole64_size = object_property_get_uint(OBJECT(i440fx_host),
> + i440fx_host = OBJECT(qdev_new(host_type));
[*]
> + hole64_size = object_property_get_uint(i440fx_host,
> PCI_HOST_PROP_PCI_HOLE64_SIZE,
> &error_abort);
> } else {
> @@ -243,12 +243,24 @@ static void pc_init1(MachineState *machine,
> PIIX3State *piix3;
> PCIDevice *pci_dev;
>
> - pci_bus = i440fx_init(pci_type,
> - i440fx_host,
> - system_memory, system_io,
> - x86ms->below_4g_mem_size,
> - x86ms->above_4g_mem_size,
> - pci_memory, ram_memory);
> + object_property_add_child(OBJECT(machine), "i440fx", i440fx_host);
I'd keep the object_property_add_child() close to qdev_new() in [*].
Matter of taste...
> + object_property_set_link(i440fx_host, PCI_HOST_PROP_RAM_MEM,
> + OBJECT(ram_memory), &error_fatal);
> + object_property_set_link(i440fx_host, PCI_HOST_PROP_PCI_MEM,
> + OBJECT(pci_memory), &error_fatal);
> + object_property_set_link(i440fx_host, PCI_HOST_PROP_SYSTEM_MEM,
> + OBJECT(system_memory), &error_fatal);
> + object_property_set_link(i440fx_host, PCI_HOST_PROP_IO_MEM,
> + OBJECT(system_io), &error_fatal);
> + object_property_set_uint(i440fx_host, PCI_HOST_BELOW_4G_MEM_SIZE,
> + x86ms->below_4g_mem_size, &error_fatal);
> + object_property_set_uint(i440fx_host, PCI_HOST_ABOVE_4G_MEM_SIZE,
> + x86ms->above_4g_mem_size, &error_fatal);
> + object_property_set_str(i440fx_host, I440FX_HOST_PROP_PCI_TYPE,
> + pci_type, &error_fatal);
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(i440fx_host), &error_fatal);
> +
> + pci_bus = PCI_BUS(qdev_get_child_bus(DEVICE(i440fx_host), "pci.0"));
> pci_bus_map_irqs(pci_bus,
> xen_enabled() ? xen_pci_slot_get_pirq
> : pc_pci_slot_get_pirq);
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index e8e66afc11..62d6287681 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -249,9 +249,14 @@ static void i440fx_pcihost_initfn(Object *obj)
>
> static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
> {
> + ERRP_GUARD();
Unrelated change?
Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> I440FXState *s = I440FX_PCI_HOST_BRIDGE(dev);
> PCIHostState *phb = PCI_HOST_BRIDGE(dev);
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + PCIBus *b;
> + PCIDevice *d;
> + PCII440FXState *f;
> + unsigned i;
>
> memory_region_add_subregion(s->io_memory, 0xcf8, &phb->conf_mem);
> sysbus_init_ioports(sbd, 0xcf8, 4);
> @@ -262,37 +267,10 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
> /* register i440fx 0xcf8 port as coalesced pio */
> memory_region_set_flush_coalesced(&phb->data_mem);
> memory_region_add_coalescing(&phb->conf_mem, 0, 4);
> -}
> -
> -PCIBus *i440fx_init(const char *pci_type,
> - DeviceState *dev,
> - MemoryRegion *address_space_mem,
> - MemoryRegion *address_space_io,
> - ram_addr_t below_4g_mem_size,
> - ram_addr_t above_4g_mem_size,
> - MemoryRegion *pci_address_space,
> - MemoryRegion *ram_memory)
> -{
> - I440FXState *s = I440FX_PCI_HOST_BRIDGE(dev);
> - PCIHostState *phb = PCI_HOST_BRIDGE(dev);
> - PCIBus *b;
> - PCIDevice *d;
> - PCII440FXState *f;
> - unsigned i;
> -
> - s->system_memory = address_space_mem;
> - s->io_memory = address_space_io;
> - s->pci_address_space = pci_address_space;
> - s->ram_memory = ram_memory;
> - s->below_4g_mem_size = below_4g_mem_size;
> - s->above_4g_mem_size = above_4g_mem_size;
> - s->pci_type = (char *)pci_type;
>
> b = pci_root_bus_new(dev, NULL, s->pci_address_space,
> s->io_memory, 0, TYPE_PCI_BUS);
> phb->bus = b;
> - object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev));
> - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>
> d = pci_create_simple(b, 0, s->pci_type);
> f = I440FX_PCI_DEVICE(d);
> @@ -336,8 +314,6 @@ PCIBus *i440fx_init(const char *pci_type,
> d->config[I440FX_COREBOOT_RAM_SIZE] = ram_size;
>
> i440fx_update_memory_mappings(f);
> -
> - return b;
> }
>
> static void i440fx_class_init(ObjectClass *klass, void *data)
Am 29. Juni 2023 07:50:10 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Hi Bernhard,
Hi Phil,
>
>On 28/6/23 21:52, Bernhard Beschow wrote:
>> i440fx_init() is a legacy init function. The previous patches worked towards
>> TYPE_I440FX_PCI_HOST_BRIDGE to be instantiated the QOM way. Do this now by
>> transforming the parameters passed to i440fx_init() into property assignments.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> include/hw/pci-host/i440fx.h | 10 ----------
>> hw/i386/pc_piix.c | 30 +++++++++++++++++++++---------
>> hw/pci-host/i440fx.c | 34 +++++-----------------------------
>> 3 files changed, 26 insertions(+), 48 deletions(-)
>>
>> diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
>> index 2d7bae5a45..c988f70890 100644
>> --- a/include/hw/pci-host/i440fx.h
>> +++ b/include/hw/pci-host/i440fx.h
>> @@ -34,14 +34,4 @@ struct PCII440FXState {
>> #define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "igd-passthrough-i440FX"
>> -PCIBus *i440fx_init(const char *pci_type,
>> - DeviceState *dev,
>> - MemoryRegion *address_space_mem,
>> - MemoryRegion *address_space_io,
>> - ram_addr_t below_4g_mem_size,
>> - ram_addr_t above_4g_mem_size,
>> - MemoryRegion *pci_memory,
>> - MemoryRegion *ram_memory);
>> -
>> -
>> #endif
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 87bee368fc..1df309b8e2 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -126,7 +126,7 @@ static void pc_init1(MachineState *machine,
>> MemoryRegion *rom_memory;
>> ram_addr_t lowmem;
>> uint64_t hole64_size;
>> - DeviceState *i440fx_host;
>> + Object *i440fx_host;
>> /*
>> * Calculate ram split, for memory below and above 4G. It's a bit
>> @@ -201,8 +201,8 @@ static void pc_init1(MachineState *machine,
>> pci_memory = g_new(MemoryRegion, 1);
>> memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
>> rom_memory = pci_memory;
>> - i440fx_host = qdev_new(host_type);
>> - hole64_size = object_property_get_uint(OBJECT(i440fx_host),
>> + i440fx_host = OBJECT(qdev_new(host_type));
>
>[*]
>
>> + hole64_size = object_property_get_uint(i440fx_host,
>> PCI_HOST_PROP_PCI_HOLE64_SIZE,
>> &error_abort);
>> } else {
>> @@ -243,12 +243,24 @@ static void pc_init1(MachineState *machine,
>> PIIX3State *piix3;
>> PCIDevice *pci_dev;
>> - pci_bus = i440fx_init(pci_type,
>> - i440fx_host,
>> - system_memory, system_io,
>> - x86ms->below_4g_mem_size,
>> - x86ms->above_4g_mem_size,
>> - pci_memory, ram_memory);
>> + object_property_add_child(OBJECT(machine), "i440fx", i440fx_host);
>
>I'd keep the object_property_add_child() close to qdev_new() in [*].
>Matter of taste...
Okay. I'd add a dedicated patch before this one since it has value in its own (removal of qdev_get_machine() usage *and* doing what you propose).
Best regards,
Bernhard
>
>> + object_property_set_link(i440fx_host, PCI_HOST_PROP_RAM_MEM,
>> + OBJECT(ram_memory), &error_fatal);
>> + object_property_set_link(i440fx_host, PCI_HOST_PROP_PCI_MEM,
>> + OBJECT(pci_memory), &error_fatal);
>> + object_property_set_link(i440fx_host, PCI_HOST_PROP_SYSTEM_MEM,
>> + OBJECT(system_memory), &error_fatal);
>> + object_property_set_link(i440fx_host, PCI_HOST_PROP_IO_MEM,
>> + OBJECT(system_io), &error_fatal);
>> + object_property_set_uint(i440fx_host, PCI_HOST_BELOW_4G_MEM_SIZE,
>> + x86ms->below_4g_mem_size, &error_fatal);
>> + object_property_set_uint(i440fx_host, PCI_HOST_ABOVE_4G_MEM_SIZE,
>> + x86ms->above_4g_mem_size, &error_fatal);
>> + object_property_set_str(i440fx_host, I440FX_HOST_PROP_PCI_TYPE,
>> + pci_type, &error_fatal);
>> + sysbus_realize_and_unref(SYS_BUS_DEVICE(i440fx_host), &error_fatal);
>> +
>> + pci_bus = PCI_BUS(qdev_get_child_bus(DEVICE(i440fx_host), "pci.0"));
>> pci_bus_map_irqs(pci_bus,
>> xen_enabled() ? xen_pci_slot_get_pirq
>> : pc_pci_slot_get_pirq);
>> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
>> index e8e66afc11..62d6287681 100644
>> --- a/hw/pci-host/i440fx.c
>> +++ b/hw/pci-host/i440fx.c
>> @@ -249,9 +249,14 @@ static void i440fx_pcihost_initfn(Object *obj)
>> static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
>> {
>> + ERRP_GUARD();
>
>Unrelated change?
>
>Otherwise:
>Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>> I440FXState *s = I440FX_PCI_HOST_BRIDGE(dev);
>> PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> + PCIBus *b;
>> + PCIDevice *d;
>> + PCII440FXState *f;
>> + unsigned i;
>> memory_region_add_subregion(s->io_memory, 0xcf8, &phb->conf_mem);
>> sysbus_init_ioports(sbd, 0xcf8, 4);
>> @@ -262,37 +267,10 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
>> /* register i440fx 0xcf8 port as coalesced pio */
>> memory_region_set_flush_coalesced(&phb->data_mem);
>> memory_region_add_coalescing(&phb->conf_mem, 0, 4);
>> -}
>> -
>> -PCIBus *i440fx_init(const char *pci_type,
>> - DeviceState *dev,
>> - MemoryRegion *address_space_mem,
>> - MemoryRegion *address_space_io,
>> - ram_addr_t below_4g_mem_size,
>> - ram_addr_t above_4g_mem_size,
>> - MemoryRegion *pci_address_space,
>> - MemoryRegion *ram_memory)
>> -{
>> - I440FXState *s = I440FX_PCI_HOST_BRIDGE(dev);
>> - PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>> - PCIBus *b;
>> - PCIDevice *d;
>> - PCII440FXState *f;
>> - unsigned i;
>> -
>> - s->system_memory = address_space_mem;
>> - s->io_memory = address_space_io;
>> - s->pci_address_space = pci_address_space;
>> - s->ram_memory = ram_memory;
>> - s->below_4g_mem_size = below_4g_mem_size;
>> - s->above_4g_mem_size = above_4g_mem_size;
>> - s->pci_type = (char *)pci_type;
>> b = pci_root_bus_new(dev, NULL, s->pci_address_space,
>> s->io_memory, 0, TYPE_PCI_BUS);
>> phb->bus = b;
>> - object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev));
>> - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>> d = pci_create_simple(b, 0, s->pci_type);
>> f = I440FX_PCI_DEVICE(d);
>> @@ -336,8 +314,6 @@ PCIBus *i440fx_init(const char *pci_type,
>> d->config[I440FX_COREBOOT_RAM_SIZE] = ram_size;
>> i440fx_update_memory_mappings(f);
>> -
>> - return b;
>> }
>> static void i440fx_class_init(ObjectClass *klass, void *data)
>
© 2016 - 2026 Red Hat, Inc.