* [PATCH] sabre: use object_initialize_child() for iommu child object
@ 2020-10-21 11:43 Mark Cave-Ayland
2020-10-25 11:11 ` Mark Cave-Ayland
0 siblings, 1 reply; 4+ messages in thread
From: Mark Cave-Ayland @ 2020-10-21 11:43 UTC (permalink / raw)
To: qemu-devel, f4bug
Store the child object directly within the sabre object rather than using
link properties.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/pci-host/sabre.c | 10 ++++------
hw/sparc64/sun4u.c | 8 +-------
include/hw/pci-host/sabre.h | 2 +-
3 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
index f41a0cc301..aaa93acd6e 100644
--- a/hw/pci-host/sabre.c
+++ b/hw/pci-host/sabre.c
@@ -383,9 +383,10 @@ static void sabre_realize(DeviceState *dev, Error **errp)
pci_create_simple(phb->bus, 0, TYPE_SABRE_PCI_DEVICE);
/* IOMMU */
+ sysbus_realize(SYS_BUS_DEVICE(&s->iommu), &error_fatal);
memory_region_add_subregion_overlap(&s->sabre_config, 0x200,
- sysbus_mmio_get_region(SYS_BUS_DEVICE(s->iommu), 0), 1);
- pci_setup_iommu(phb->bus, sabre_pci_dma_iommu, s->iommu);
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->iommu), 0), 1);
+ pci_setup_iommu(phb->bus, sabre_pci_dma_iommu, &s->iommu);
/* APB secondary busses */
pci_dev = pci_new_multifunction(PCI_DEVFN(1, 0), true,
@@ -422,10 +423,7 @@ static void sabre_init(Object *obj)
s->pci_irq_in = 0ULL;
/* IOMMU */
- object_property_add_link(obj, "iommu", TYPE_SUN4U_IOMMU,
- (Object **) &s->iommu,
- qdev_prop_allow_set_link_before_realize,
- 0);
+ object_initialize_child(obj, "iommu", &s->iommu, TYPE_SUN4U_IOMMU);
/* sabre_config */
memory_region_init_io(&s->sabre_config, OBJECT(s), &sabre_config_ops, s,
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 2f8fc670cf..a33f1eccfd 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -562,7 +562,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
PCIBus *pci_bus, *pci_busA, *pci_busB;
PCIDevice *ebus, *pci_dev;
SysBusDevice *s;
- DeviceState *iommu, *dev;
+ DeviceState *dev;
FWCfgState *fw_cfg;
NICInfo *nd;
MACAddr macaddr;
@@ -571,10 +571,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
/* init CPUs */
cpu = sparc64_cpu_devinit(machine->cpu_type, hwdef->prom_addr);
- /* IOMMU */
- iommu = qdev_new(TYPE_SUN4U_IOMMU);
- sysbus_realize_and_unref(SYS_BUS_DEVICE(iommu), &error_fatal);
-
/* set up devices */
ram_init(0, machine->ram_size);
@@ -584,8 +580,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
sabre = SABRE(qdev_new(TYPE_SABRE));
qdev_prop_set_uint64(DEVICE(sabre), "special-base", PBM_SPECIAL_BASE);
qdev_prop_set_uint64(DEVICE(sabre), "mem-base", PBM_MEM_BASE);
- object_property_set_link(OBJECT(sabre), "iommu", OBJECT(iommu),
- &error_abort);
sysbus_realize_and_unref(SYS_BUS_DEVICE(sabre), &error_fatal);
/* sabre_config */
diff --git a/include/hw/pci-host/sabre.h b/include/hw/pci-host/sabre.h
index 01190241bb..05bf741cde 100644
--- a/include/hw/pci-host/sabre.h
+++ b/include/hw/pci-host/sabre.h
@@ -34,7 +34,7 @@ struct SabreState {
MemoryRegion pci_mmio;
MemoryRegion pci_ioport;
uint64_t pci_irq_in;
- IOMMUState *iommu;
+ IOMMUState iommu;
PCIBridge *bridgeA;
PCIBridge *bridgeB;
uint32_t pci_control[16];
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] sabre: use object_initialize_child() for iommu child object
2020-10-21 11:43 [PATCH] sabre: use object_initialize_child() for iommu child object Mark Cave-Ayland
@ 2020-10-25 11:11 ` Mark Cave-Ayland
2020-10-25 11:41 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 4+ messages in thread
From: Mark Cave-Ayland @ 2020-10-25 11:11 UTC (permalink / raw)
To: qemu-devel, f4bug
On 21/10/2020 12:43, Mark Cave-Ayland wrote:
> Store the child object directly within the sabre object rather than using
> link properties.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/pci-host/sabre.c | 10 ++++------
> hw/sparc64/sun4u.c | 8 +-------
> include/hw/pci-host/sabre.h | 2 +-
> 3 files changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
> index f41a0cc301..aaa93acd6e 100644
> --- a/hw/pci-host/sabre.c
> +++ b/hw/pci-host/sabre.c
> @@ -383,9 +383,10 @@ static void sabre_realize(DeviceState *dev, Error **errp)
> pci_create_simple(phb->bus, 0, TYPE_SABRE_PCI_DEVICE);
>
> /* IOMMU */
> + sysbus_realize(SYS_BUS_DEVICE(&s->iommu), &error_fatal);
> memory_region_add_subregion_overlap(&s->sabre_config, 0x200,
> - sysbus_mmio_get_region(SYS_BUS_DEVICE(s->iommu), 0), 1);
> - pci_setup_iommu(phb->bus, sabre_pci_dma_iommu, s->iommu);
> + sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->iommu), 0), 1);
> + pci_setup_iommu(phb->bus, sabre_pci_dma_iommu, &s->iommu);
>
> /* APB secondary busses */
> pci_dev = pci_new_multifunction(PCI_DEVFN(1, 0), true,
> @@ -422,10 +423,7 @@ static void sabre_init(Object *obj)
> s->pci_irq_in = 0ULL;
>
> /* IOMMU */
> - object_property_add_link(obj, "iommu", TYPE_SUN4U_IOMMU,
> - (Object **) &s->iommu,
> - qdev_prop_allow_set_link_before_realize,
> - 0);
> + object_initialize_child(obj, "iommu", &s->iommu, TYPE_SUN4U_IOMMU);
>
> /* sabre_config */
> memory_region_init_io(&s->sabre_config, OBJECT(s), &sabre_config_ops, s,
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 2f8fc670cf..a33f1eccfd 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -562,7 +562,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
> PCIBus *pci_bus, *pci_busA, *pci_busB;
> PCIDevice *ebus, *pci_dev;
> SysBusDevice *s;
> - DeviceState *iommu, *dev;
> + DeviceState *dev;
> FWCfgState *fw_cfg;
> NICInfo *nd;
> MACAddr macaddr;
> @@ -571,10 +571,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
> /* init CPUs */
> cpu = sparc64_cpu_devinit(machine->cpu_type, hwdef->prom_addr);
>
> - /* IOMMU */
> - iommu = qdev_new(TYPE_SUN4U_IOMMU);
> - sysbus_realize_and_unref(SYS_BUS_DEVICE(iommu), &error_fatal);
> -
> /* set up devices */
> ram_init(0, machine->ram_size);
>
> @@ -584,8 +580,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
> sabre = SABRE(qdev_new(TYPE_SABRE));
> qdev_prop_set_uint64(DEVICE(sabre), "special-base", PBM_SPECIAL_BASE);
> qdev_prop_set_uint64(DEVICE(sabre), "mem-base", PBM_MEM_BASE);
> - object_property_set_link(OBJECT(sabre), "iommu", OBJECT(iommu),
> - &error_abort);
> sysbus_realize_and_unref(SYS_BUS_DEVICE(sabre), &error_fatal);
>
> /* sabre_config */
> diff --git a/include/hw/pci-host/sabre.h b/include/hw/pci-host/sabre.h
> index 01190241bb..05bf741cde 100644
> --- a/include/hw/pci-host/sabre.h
> +++ b/include/hw/pci-host/sabre.h
> @@ -34,7 +34,7 @@ struct SabreState {
> MemoryRegion pci_mmio;
> MemoryRegion pci_ioport;
> uint64_t pci_irq_in;
> - IOMMUState *iommu;
> + IOMMUState iommu;
> PCIBridge *bridgeA;
> PCIBridge *bridgeB;
> uint32_t pci_control[16];
No further comments (and I'm happier that this is a better solution than having an
"optional" link property) so I've applied this to my qemu-sparc branch.
ATB,
Mark.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sabre: use object_initialize_child() for iommu child object
2020-10-25 11:11 ` Mark Cave-Ayland
@ 2020-10-25 11:41 ` Philippe Mathieu-Daudé
2020-10-27 9:33 ` Mark Cave-Ayland
0 siblings, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-25 11:41 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel
On 10/25/20 12:11 PM, Mark Cave-Ayland wrote:
> On 21/10/2020 12:43, Mark Cave-Ayland wrote:
>
>> Store the child object directly within the sabre object rather than using
>> link properties.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/pci-host/sabre.c | 10 ++++------
>> hw/sparc64/sun4u.c | 8 +-------
>> include/hw/pci-host/sabre.h | 2 +-
>> 3 files changed, 6 insertions(+), 14 deletions(-)
>>
>> diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
>> index f41a0cc301..aaa93acd6e 100644
>> --- a/hw/pci-host/sabre.c
>> +++ b/hw/pci-host/sabre.c
>> @@ -383,9 +383,10 @@ static void sabre_realize(DeviceState *dev, Error
>> **errp)
>> pci_create_simple(phb->bus, 0, TYPE_SABRE_PCI_DEVICE);
>> /* IOMMU */
>> + sysbus_realize(SYS_BUS_DEVICE(&s->iommu), &error_fatal);
>> memory_region_add_subregion_overlap(&s->sabre_config, 0x200,
>> - sysbus_mmio_get_region(SYS_BUS_DEVICE(s->iommu),
>> 0), 1);
>> - pci_setup_iommu(phb->bus, sabre_pci_dma_iommu, s->iommu);
>> + sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->iommu),
>> 0), 1);
>> + pci_setup_iommu(phb->bus, sabre_pci_dma_iommu, &s->iommu);
>> /* APB secondary busses */
>> pci_dev = pci_new_multifunction(PCI_DEVFN(1, 0), true,
>> @@ -422,10 +423,7 @@ static void sabre_init(Object *obj)
>> s->pci_irq_in = 0ULL;
>> /* IOMMU */
>> - object_property_add_link(obj, "iommu", TYPE_SUN4U_IOMMU,
>> - (Object **) &s->iommu,
>> - qdev_prop_allow_set_link_before_realize,
>> - 0);
>> + object_initialize_child(obj, "iommu", &s->iommu, TYPE_SUN4U_IOMMU);
>> /* sabre_config */
>> memory_region_init_io(&s->sabre_config, OBJECT(s),
>> &sabre_config_ops, s,
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index 2f8fc670cf..a33f1eccfd 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -562,7 +562,7 @@ static void sun4uv_init(MemoryRegion
>> *address_space_mem,
>> PCIBus *pci_bus, *pci_busA, *pci_busB;
>> PCIDevice *ebus, *pci_dev;
>> SysBusDevice *s;
>> - DeviceState *iommu, *dev;
>> + DeviceState *dev;
>> FWCfgState *fw_cfg;
>> NICInfo *nd;
>> MACAddr macaddr;
>> @@ -571,10 +571,6 @@ static void sun4uv_init(MemoryRegion
>> *address_space_mem,
>> /* init CPUs */
>> cpu = sparc64_cpu_devinit(machine->cpu_type, hwdef->prom_addr);
>> - /* IOMMU */
>> - iommu = qdev_new(TYPE_SUN4U_IOMMU);
>> - sysbus_realize_and_unref(SYS_BUS_DEVICE(iommu), &error_fatal);
>> -
>> /* set up devices */
>> ram_init(0, machine->ram_size);
>> @@ -584,8 +580,6 @@ static void sun4uv_init(MemoryRegion
>> *address_space_mem,
>> sabre = SABRE(qdev_new(TYPE_SABRE));
>> qdev_prop_set_uint64(DEVICE(sabre), "special-base",
>> PBM_SPECIAL_BASE);
>> qdev_prop_set_uint64(DEVICE(sabre), "mem-base", PBM_MEM_BASE);
>> - object_property_set_link(OBJECT(sabre), "iommu", OBJECT(iommu),
>> - &error_abort);
>> sysbus_realize_and_unref(SYS_BUS_DEVICE(sabre), &error_fatal);
>> /* sabre_config */
>> diff --git a/include/hw/pci-host/sabre.h b/include/hw/pci-host/sabre.h
>> index 01190241bb..05bf741cde 100644
>> --- a/include/hw/pci-host/sabre.h
>> +++ b/include/hw/pci-host/sabre.h
>> @@ -34,7 +34,7 @@ struct SabreState {
>> MemoryRegion pci_mmio;
>> MemoryRegion pci_ioport;
>> uint64_t pci_irq_in;
>> - IOMMUState *iommu;
>> + IOMMUState iommu;
>> PCIBridge *bridgeA;
>> PCIBridge *bridgeB;
>> uint32_t pci_control[16];
>
> No further comments (and I'm happier that this is a better solution than
> having an "optional" link property) so I've applied this to my
> qemu-sparc branch.
Sorry I had this patch tagged for review but am having trouble
managing that folder. This is certainly better, thanks for this
cleanup.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
>
> ATB,
>
> Mark.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sabre: use object_initialize_child() for iommu child object
2020-10-25 11:41 ` Philippe Mathieu-Daudé
@ 2020-10-27 9:33 ` Mark Cave-Ayland
0 siblings, 0 replies; 4+ messages in thread
From: Mark Cave-Ayland @ 2020-10-27 9:33 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
On 25/10/2020 11:41, Philippe Mathieu-Daudé wrote:
> On 10/25/20 12:11 PM, Mark Cave-Ayland wrote:
>> On 21/10/2020 12:43, Mark Cave-Ayland wrote:
>>
>>> Store the child object directly within the sabre object rather than using
>>> link properties.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>> hw/pci-host/sabre.c | 10 ++++------
>>> hw/sparc64/sun4u.c | 8 +-------
>>> include/hw/pci-host/sabre.h | 2 +-
>>> 3 files changed, 6 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
>>> index f41a0cc301..aaa93acd6e 100644
>>> --- a/hw/pci-host/sabre.c
>>> +++ b/hw/pci-host/sabre.c
>>> @@ -383,9 +383,10 @@ static void sabre_realize(DeviceState *dev, Error **errp)
>>> pci_create_simple(phb->bus, 0, TYPE_SABRE_PCI_DEVICE);
>>> /* IOMMU */
>>> + sysbus_realize(SYS_BUS_DEVICE(&s->iommu), &error_fatal);
>>> memory_region_add_subregion_overlap(&s->sabre_config, 0x200,
>>> - sysbus_mmio_get_region(SYS_BUS_DEVICE(s->iommu), 0), 1);
>>> - pci_setup_iommu(phb->bus, sabre_pci_dma_iommu, s->iommu);
>>> + sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->iommu), 0), 1);
>>> + pci_setup_iommu(phb->bus, sabre_pci_dma_iommu, &s->iommu);
>>> /* APB secondary busses */
>>> pci_dev = pci_new_multifunction(PCI_DEVFN(1, 0), true,
>>> @@ -422,10 +423,7 @@ static void sabre_init(Object *obj)
>>> s->pci_irq_in = 0ULL;
>>> /* IOMMU */
>>> - object_property_add_link(obj, "iommu", TYPE_SUN4U_IOMMU,
>>> - (Object **) &s->iommu,
>>> - qdev_prop_allow_set_link_before_realize,
>>> - 0);
>>> + object_initialize_child(obj, "iommu", &s->iommu, TYPE_SUN4U_IOMMU);
>>> /* sabre_config */
>>> memory_region_init_io(&s->sabre_config, OBJECT(s), &sabre_config_ops, s,
>>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>>> index 2f8fc670cf..a33f1eccfd 100644
>>> --- a/hw/sparc64/sun4u.c
>>> +++ b/hw/sparc64/sun4u.c
>>> @@ -562,7 +562,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>> PCIBus *pci_bus, *pci_busA, *pci_busB;
>>> PCIDevice *ebus, *pci_dev;
>>> SysBusDevice *s;
>>> - DeviceState *iommu, *dev;
>>> + DeviceState *dev;
>>> FWCfgState *fw_cfg;
>>> NICInfo *nd;
>>> MACAddr macaddr;
>>> @@ -571,10 +571,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>> /* init CPUs */
>>> cpu = sparc64_cpu_devinit(machine->cpu_type, hwdef->prom_addr);
>>> - /* IOMMU */
>>> - iommu = qdev_new(TYPE_SUN4U_IOMMU);
>>> - sysbus_realize_and_unref(SYS_BUS_DEVICE(iommu), &error_fatal);
>>> -
>>> /* set up devices */
>>> ram_init(0, machine->ram_size);
>>> @@ -584,8 +580,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>> sabre = SABRE(qdev_new(TYPE_SABRE));
>>> qdev_prop_set_uint64(DEVICE(sabre), "special-base", PBM_SPECIAL_BASE);
>>> qdev_prop_set_uint64(DEVICE(sabre), "mem-base", PBM_MEM_BASE);
>>> - object_property_set_link(OBJECT(sabre), "iommu", OBJECT(iommu),
>>> - &error_abort);
>>> sysbus_realize_and_unref(SYS_BUS_DEVICE(sabre), &error_fatal);
>>> /* sabre_config */
>>> diff --git a/include/hw/pci-host/sabre.h b/include/hw/pci-host/sabre.h
>>> index 01190241bb..05bf741cde 100644
>>> --- a/include/hw/pci-host/sabre.h
>>> +++ b/include/hw/pci-host/sabre.h
>>> @@ -34,7 +34,7 @@ struct SabreState {
>>> MemoryRegion pci_mmio;
>>> MemoryRegion pci_ioport;
>>> uint64_t pci_irq_in;
>>> - IOMMUState *iommu;
>>> + IOMMUState iommu;
>>> PCIBridge *bridgeA;
>>> PCIBridge *bridgeB;
>>> uint32_t pci_control[16];
>>
>> No further comments (and I'm happier that this is a better solution than having an
>> "optional" link property) so I've applied this to my qemu-sparc branch.
>
> Sorry I had this patch tagged for review but am having trouble
> managing that folder. This is certainly better, thanks for this
> cleanup.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Looks like this patch exposes another issue related to QOM lifecycle, so I'm dropping
it from my qemu-sparc branch for now.
ATB,
Mark.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-27 9:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21 11:43 [PATCH] sabre: use object_initialize_child() for iommu child object Mark Cave-Ayland
2020-10-25 11:11 ` Mark Cave-Ayland
2020-10-25 11:41 ` Philippe Mathieu-Daudé
2020-10-27 9:33 ` Mark Cave-Ayland
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.