On 3/10/21 6:06 PM, Peter Xu wrote: > Phil, > > On Tue, Mar 09, 2021 at 10:54:18PM +0100, Philippe Mathieu-Daudé wrote: >> +Peter/Mark/Edgar for SoC modelling >> >> On 3/9/21 10:39 AM, Philippe Mathieu-Daudé wrote: >>> Hi Peter, >>> >>> On 3/9/21 12:40 AM, Peter Xu wrote:> On Sat, Mar 06, 2021 at 12:54:13AM >>> +0100, Philippe Mathieu-Daudé wrote: >>>>> @@ -3188,14 +3188,15 @@ void mtree_info(bool flatview, bool >>>>> dispatch_tree, bool owner, bool disabled) >>>>> >>>>> QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { >>>>> qemu_printf("address-space: %s\n", as->name); >>>>> - mtree_print_mr(as->root, 1, 0, &ml_head, owner, disabled); >>>>> + mtree_print_mr(as->root, 1, 0, as->root->addr, >>>> >>>> Root MR of any address space should have mr->addr==0, right? >>>> >>>> I'm slightly confused on what this patch wanted to do if so, since then >>>> "base" >>>> will always be zero.. Or am I wrong? >>> >>> That is what I am expecting too... Maybe the problem is elsewhere >>> when I create the address space... The simpler way to >>> figure it out is add an assertion. I haven't figure out my >>> issue yet, I'll follow up later with a proof-of-concept series >>> which triggers the assertion. >> >> To trigger I simply use: >> >> mydevice_realize() >> { >> memory_region_init(&mr, obj, name, size); >> >> address_space_init(&as, &mr, name); > > Could I ask why you need to set this sysbus mmio region as root MR of as? > What's AS used for here? > > Btw, normally I see these regions should be initialized with > memory_region_init_io(), since normally that MR will need a MemoryRegionOps > bound to it to trap MMIOs, iiuc.
This is the pattern for buses: static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, const char *name, int devfn, Error **errp) { ... memory_region_init(&pci_dev->bus_master_container_region, OBJECT(pci_dev), "bus master container", UINT64_MAX); address_space_init(&pci_dev->bus_master_as, &pci_dev->bus_master_container_region, pci_dev->name); .... AUXBus *aux_bus_init(DeviceState *parent, const char *name) { AUXBus *bus; Object *auxtoi2c; bus = AUX_BUS(qbus_create(TYPE_AUX_BUS, parent, name)); auxtoi2c = object_new_with_props(TYPE_AUXTOI2C, OBJECT(bus), "i2c", &error_abort, NULL); bus->bridge = AUXTOI2C(auxtoi2c); /* Memory related. */ bus->aux_io = g_malloc(sizeof(*bus->aux_io)); memory_region_init(bus->aux_io, OBJECT(bus), "aux-io", 1 * MiB); address_space_init(&bus->aux_addr_space, bus->aux_io, "aux-io"); return bus; } static void artist_realizefn(DeviceState *dev, Error **errp) { ... memory_region_init(&s->mem_as_root, OBJECT(dev), "artist", ~0ull); address_space_init(&s->as, &s->mem_as_root, "artist"); PCI host: PCIBus *gt64120_register(qemu_irq *pic) { ... memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", 4 * GiB); address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem"); Also: static void pnv_xive_realize(DeviceState *dev, Error **errp) { ... /* * The XiveSource and XiveENDSource objects are realized with the * maximum allowed HW configuration. The ESB MMIO regions will be * resized dynamically when the controller is configured by the FW * to limit accesses to resources not provisioned. */ memory_region_init(&xive->ipi_mmio, OBJECT(xive), "xive-vc-ipi", PNV9_XIVE_VC_SIZE); address_space_init(&xive->ipi_as, &xive->ipi_mmio, "xive-vc-ipi"); memory_region_init(&xive->end_mmio, OBJECT(xive), "xive-vc-end", PNV9_XIVE_VC_SIZE); address_space_init(&xive->end_as, &xive->end_mmio, "xive-vc-end"); And: static void memory_map_init(void) { ... memory_region_init(system_memory, NULL, "system", UINT64_MAX); address_space_init(&address_space_memory, system_memory, "memory"); Or: static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn) { ... /* * Memory region relationships looks like (Address range shows * only lower 32 bits to make it short in length...): * * |-----------------+-------------------+----------| * | Name | Address range | Priority | * |-----------------+-------------------+----------+ * | amdvi_root | 00000000-ffffffff | 0 | * | amdvi_iommu | 00000000-ffffffff | 1 | * | amdvi_iommu_ir | fee00000-feefffff | 64 | * |-----------------+-------------------+----------| */ memory_region_init(&amdvi_dev_as->root, OBJECT(s), "amdvi_root", UINT64_MAX); address_space_init(&amdvi_dev_as->as, &amdvi_dev_as->root, name); memory_region_init_io(&amdvi_dev_as->iommu_ir, OBJECT(s), &amdvi_ir_ops, s, "amd_iommu_ir", AMDVI_INT_ADDR_SIZE); memory_region_add_subregion_overlap(&amdvi_dev_as->root, AMDVI_INT_ADDR_FIRST, &amdvi_dev_as->iommu_ir, 64); memory_region_add_subregion_overlap(&amdvi_dev_as->root, 0, MEMORY_REGION(&amdvi_dev_as->iommu), 1); I'll send a reproducer.