Re: [Qemu-devel] [RFC] Memory API

[Anthony Liguori <anthony@codemonkey.ws>]

On 05/18/2011 09:05 AM, Jan Kiszka wrote:
> On 2011-05-18 15:12, Avi Kivity wrote:
>> The current memory APIs (cpu_register_io_memory,
>> cpu_register_physical_memory) suffer from a number of drawbacks:
>>
>> - lack of centralized bookkeeping
>>     - a cpu_register_physical_memory() that overlaps an existing region
>> will overwrite the preexisting region; but a following
>> cpu_unregister_physical_memory() will not restore things to status quo
>> ante.
>
> Restoring is not the problem. The problem is that the current API
> deletes or truncates regions implicitly by overwriting. That makes
> tracking the layout hard, and it is also error-prone as the device that
> accidentally overlaps with some other device won't receive a
> notification of this potential conflict.
>
> Such implicite truncation or deletion must be avoided in a new API,
> forcing the users to explicitly reference an existing region when
> dropping or modifying it. But your API goes in the right direction.
>
>>     - coalescing and dirty logging are all cleared on unregistering, so
>> the client has to re-issue them when re-registering
>> - lots of opaques
>> - no nesting
>>     - if a region has multiple subregions that need different handling
>> (different callbacks, RAM interspersed with MMIO) then client code has
>> to deal with that manually
>>     - we can't interpose code between a device and global memory handling
>
> I would add another drawback:
>
>   - Inability to identify the origin of a region accesses and handle them
>     differently based on the source.
>
>     That is at least problematic for the x86 APIC which is CPU local. Our
>     current way do deal with it is, well, very creative and falls to
>     dust if a guest actually tries to remap the APIC.

This is about registration.  Right now you can only register IO 
intercepts in the chipset, not on a per-CPU basis.  We could just as 
easily have:

CPUState {
     MemoryRegion apic_region;
};

per_cpu_register_memory(env, &env->apic_region);

To make that work.

We need to revamp registration.  We should be able to register at the 
following levels:

1) per-CPU  (new API)

2) chipset  (effectively the current cpu_register_physical_memory)

3) per-BUS  (pci_register_bar())

Regards,

Anthony Liguori