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

[Avi Kivity]

On 05/18/2011 06:14 PM, Anthony Liguori wrote:
> 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);

Right.  Or all memory per-cpu, with two sub-regions:

 - global memory
 - overlaid apic memory

for this, we need to have well defined semantics for overlap (perhaps a 
priority argument to memory_region_add_subregion).

> 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())

The important thing is that all of these take a MemoryRegion argument, 
so we don't have to duplicate the coalescing, logging, and other APIs.

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