Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Avi Kivity]

On 04/30/2012 03:17 PM, Mark Cave-Ayland wrote:
> On 30/04/12 09:41, Avi Kivity wrote:
>
>> Yes.  I think it's even possible to do this now, you can create an mmio
>> region for the bus and add subregions to it.  All subregions
>> automatically overlap the container region.
>>
>> Simply replace
>>
>>    memory_region_init(&bus->address_space, ...)
>>    memory_region_add_subregion(&bus->address_space, addr,&dev->mmio)
>>
>> with
>>
>>    memory_region_init_io(&bus->address_space,&bus_nodev_ops, bus, ...)
>>    memory_region_add_subregion(&bus->address_space, addr,&dev->mmio) //
>> unchanged
>>
>> This was never used so it hasn't been tested, but the code was written
>> with it in mind.  I didn't want to document this so we could back out of
>> it, but if it's useful, let's use it.
>
> Hi Avi,
>
> Well I've attempted to code something like this, but I've hit a
> problem with devices that aren't just used by SPARC, such as the disk
> controller - it seems that the sysbus API can only MMIO map devices
> into the "root" MemoryRegion :(
>
> The SBUS I am trying to model is actually attached to the physical
> address bus, but effectively the top 4 bits of the address control the
> multiplexing of the on-board RAM and the individual per-slot address
> lines. Effectively what I'm trying to model based on a 128MB default
> memory setting is something like this:
>
> Memory
> 0x000000000 - 0x007ffffff - Normal RAM
>
>   SBus
>   0x20000000 - 0x02fffffff - SBus Slot 0
>   0x30000000 - 0x03fffffff - SBus Slot 1
>   0x40000000 - 0x04fffffff - SBus Slot 2
>   0x50000000 - 0x05fffffff - SBus Slot 3
>     0x50200000 - 0x5020000f - TCX DAC
>     0x50300000 - 0x5030081b - TCH THC8
>     0x50301000 - 0x50301fff - TCX THC24
>     0x50700000 - 0x50700fff - TCX TEC
>
>   0x600000000 - 0x06fffffff - SBus Slot 4
>   0x700000000 - 0x07fffffff - SBus Slot 5
>
> Using your example above, I believe I can create a simple memory
> hierarchy to represent this and catch unknown operations in a
> bus_nodev_ops section no problem. The issue is that several shared
> peripherals such as ESP have the following in their init function:
>
> void esp_init(target_phys_addr_t espaddr, int it_shift,
>               ESPDMAMemoryReadWriteFunc dma_memory_read,
>               ESPDMAMemoryReadWriteFunc dma_memory_write,
>               void *dma_opaque, qemu_irq irq, qemu_irq *reset,
>               qemu_irq *dma_enable)
> {
>     ...
>     ...
>     sysbus_mmio_map(s, 0, espaddr);
>     ...
> }
>
> and sysbus_mmio_map() looks like this:
>
> void sysbus_mmio_map(SysBusDevice *dev, int n, target_phys_addr_t addr)
> {
>     ...
>     ...
>     memory_region_add_subregion(get_system_memory(),
>                                 addr,
>                                 dev->mmio[n].memory);
> }
>
> My understanding based upon this is that it would be impossible to
> register a different parent MemoryRegion without duplicating the init
> function for all shared devices which seems undesirable :(

What are the requirements?  You need a different catch-all mmio handler
for each slot?  Or would one catch-all mmio handler for all sysbus
devices suffice?

>
> The only solution I can think of is to make sysbus_mmio_map() more
> intelligent so that instead of blindly adding the device to the "root"
> MemoryRegion, it traverses down the MemoryRegion hierarchy starting
> from the root to the furtherest leaf node it can find based upon the
> specified address and then adds the new subregion there. Hence if I
> add my SBus memory regions first before call the various peripheral
> init functions, everything should end up in the correct part of the
> memory tree.
>

This solution attempts to reconstruct the memory hierarchy from the
address, instead of maintaining it in the device tree.

> I believe this should preserve compatibility for existing sysbus API
> users with just a single "root" MemoryRegion, however due to lack of
> any documentation related to sysbus I'm not sure if this would break
> other platforms or maybe even violate one of its key design features?

IMO the best fix is to unsysbus the device and qomify it instead.  This
way we're 100% flexible in how we can attach it.

-- 
error compiling committee.c: too many arguments to function