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.