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

[Jan Kiszka]

On 2011-05-18 17:23, Avi Kivity wrote:
> On 05/18/2011 06:11 PM, Jan Kiszka wrote:
>> On 2011-05-18 16:36, Avi Kivity wrote:
>>>>  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.
>>>>
>>>>  However, I'm unsure if that can easily be addressed. As long as only x86
>>>>  is affected, it's tricky to ask for a big infrastructure to handle this
>>>>  special case. Maybe there some other use cases, don't know.
>>>
>>>  We could implement it with a per-cpu MemoryRegion, with each cpu's
>>>  MemoryRegion populated by a different APIC sub-region.
>>
>> The tricky part is wiring this up efficiently for TCG, ie. in QEMU's
>> softmmu. I played with passing the issuing CPUState (or NULL for
>> devices) down the MMIO handler chain. Not totally beautiful as
>> decentralized dispatching was still required, but at least only
>> moderately invasive. Maybe your API allows for cleaning up the
>> management and dispatching part, need to rethink...
> 
> My suggestion is opposite - have a different MemoryRegion for each (e.g. 
> CPUState::memory).  Then the TLBs will resolve to a different ram_addr_t 
> for the same physical address, for the local APIC range.

Need to recheck, but I think I dropped that idea due to invasiveness of
the implementation. Not a very good argument, but without a clear
picture how useful this per-cpu registration is beyond x86, that was
more important.

> 
>>>
>>>  There is nothing we can do with a return code.  You can't fail an mmio
>>>  that causes overlapping physical memory map.
>>
>> We must fail such requests to make progress with the API. That may
>> happen either on caller side or in cpu_register_memory_region itself
>> (hwerror). Otherwise the new API will just be a shiny new facade for on
>> old and still fragile building.
> 
>>>>
>>>>>   void cpu_unregister_memory_region(MemoryRegion *mr);
>>>
>>>  Instead, we need cpu_unregister_memory_region() to restore any
>>>  previously hidden ranges.
>>
>> I disagree. Both approaches, rejecting overlaps or restoring them, imply
>> subtle semantical changes that exiting device models have to deal with.
>> We can't use any of both without some review and conversion work. So
>> better head for the clearer and, thus, cleaner approach.
> 
> We need to head for the more hardware-like approach.  What happens when 
> you program overlapping BARs?  I imagine the result is 
> implementation-defined, but ends up with one region decoded in 
> preference to the other.  There is simply no way to reject an 
> overlapping mapping.

But there is also now simple way to allow them. At least not without
exposing control about their ordering AND allowing to hook up managing
code (e.g. of the PCI bridge or the chipset) that controls registrations.

...
>> See [1]: We really need to get rid of slot management on
>> CPUPhysMemoryClient side. Your API provides a perfect opportunity to
>> establish the infrastructure of slot tracking at a central place. We can
>> then switch from reporting cpu_registering_memory events to reporting
>> coalesced changes to slots, those slot that also the core uses. So a new
>> CPUPhysMemoryClient API needs to be considered in this API change as
>> well - or we change twice in the end.
> 
> The kvm memory slots (and hopefully future qemu memory slots) are a 
> flattened view of the MemoryRegion tree.  There is no 1:1 mapping.

We need a flatted view of your memory regions during runtime as well. No
major difference here. If we share that view with PhysMemClients, they
can drop most of their creative slot tracking algorithms, focusing on
the real differences.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux