Am 23. September 2024 10:28:35 UTC schrieb BALATON Zoltan <bala...@eik.bme.hu>:
>On Mon, 23 Sep 2024, Bernhard Beschow wrote:
>> Rather than accessing the attributes of TYPE_CCSR directly, use the
>> SysBusDevice
>> API which exists exactly for that purpose. Furthermore, registering the
>> memory
>> region with the SysBusDevice API makes it show up in QMP's `info qom-tree`
>> command.
>>
>> Signed-off-by: Bernhard Beschow <shen...@gmail.com>
>> ---
>> hw/pci-host/ppce500.c | 10 +++++-----
>> hw/ppc/e500.c | 8 ++++----
>> 2 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
>> index 95b983b2b3..97e5d47cec 100644
>> --- a/hw/pci-host/ppce500.c
>> +++ b/hw/pci-host/ppce500.c
>> @@ -16,7 +16,6 @@
>>
>> #include "qemu/osdep.h"
>> #include "hw/irq.h"
>> -#include "hw/ppc/e500-ccsr.h"
>> #include "hw/qdev-properties.h"
>> #include "migration/vmstate.h"
>> #include "hw/pci/pci_device.h"
>> @@ -419,11 +418,12 @@ static const VMStateDescription vmstate_ppce500_pci = {
>> static void e500_pcihost_bridge_realize(PCIDevice *d, Error **errp)
>> {
>> PPCE500PCIBridgeState *b = PPC_E500_PCI_BRIDGE(d);
>> - PPCE500CCSRState *ccsr = CCSR(container_get(qdev_get_machine(),
>> - "/e500-ccsr"));
>> + SysBusDevice *ccsr = SYS_BUS_DEVICE(container_get(qdev_get_machine(),
>> + "/e500-ccsr"));
>> + MemoryRegion *ccsr_space = sysbus_mmio_get_region(ccsr, 0);
>>
>> - memory_region_init_alias(&b->bar0, OBJECT(ccsr), "e500-pci-bar0",
>> &ccsr->ccsr_space,
>> - 0, int128_get64(ccsr->ccsr_space.size));
>> + memory_region_init_alias(&b->bar0, OBJECT(ccsr), "e500-pci-bar0",
>> + ccsr_space, 0, int128_get64(ccsr_space->size));
>
>I wonder if this really needs an alias or could the original memory region be
>registered as the PCI BAR region? Otherwise:
This hits an assertion when running Buildroot's qemu_ppc64_e5500_defconfig.
Therefore I'll keep the alias.
Best regards,
Bernhard
>
>Reviewed-by: BALATON Zoltan <bala...@eik.bme.hu>
>
>Regards,
>BALATON Zoltan
>
>> pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &b->bar0);
>> }
>>
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index e2a4f265a5..2225533e33 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -924,7 +924,6 @@ void ppce500_init(MachineState *machine)
>> DriveInfo *dinfo;
>> MemoryRegion *ccsr_addr_space;
>> SysBusDevice *s;
>> - PPCE500CCSRState *ccsr;
>> I2CBus *i2c;
>>
>> irqs = g_new0(IrqLines, smp_cpus);
>> @@ -980,10 +979,10 @@ void ppce500_init(MachineState *machine)
>> memory_region_add_subregion(address_space_mem, 0, machine->ram);
>>
>> dev = qdev_new("e500-ccsr");
>> + s = SYS_BUS_DEVICE(dev);
>> object_property_add_child(OBJECT(machine), "e500-ccsr", OBJECT(dev));
>> - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>> - ccsr = CCSR(dev);
>> - ccsr_addr_space = &ccsr->ccsr_space;
>> + sysbus_realize_and_unref(s, &error_fatal);
>> + ccsr_addr_space = sysbus_mmio_get_region(s, 0);
>> memory_region_add_subregion(address_space_mem, pmc->ccsrbar_base,
>> ccsr_addr_space);
>>
>> @@ -1270,6 +1269,7 @@ static void e500_ccsr_initfn(Object *obj)
>> PPCE500CCSRState *ccsr = CCSR(obj);
>> memory_region_init(&ccsr->ccsr_space, obj, "e500-ccsr",
>> MPC8544_CCSRBAR_SIZE);
>> + sysbus_init_mmio(SYS_BUS_DEVICE(ccsr), &ccsr->ccsr_space);
>> }
>>
>> static const TypeInfo e500_ccsr_info = {
>>
