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

[Stefan Weil <weil@mail.berlios.de>]

Am 18.05.2011 17:08, schrieb Anthony Liguori:
> On 05/18/2011 08:12 AM, 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.
>> - 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
>>
>> To fix that, I propose an new API to replace the existing one:
>>
>>
>> #ifndef MEMORY_H
>> #define MEMORY_H
>>
>> typedef struct MemoryRegionOps MemoryRegionOps;
>> typedef struct MemoryRegion MemoryRegion;
>>
>> typedef uint32_t (*MemoryReadFunc)(MemoryRegion *mr, target_phys_addr_t
>> addr);
>> typedef void (*MemoryWriteFunc)(MemoryRegion *mr, target_phys_addr_t 
>> addr,
>> uint32_t data);
>
>
> The API should be 64-bit and needs to have a size associated with it.
>
>> struct MemoryRegionOps {
>> MemoryReadFunc readb, readw, readl;
>> MemoryWriteFunc writeb, writew, writel;
>> };
>
> I'm not a fan of having per-access type function pointers.

Do you think of something like these declaration:

typedef uint64_t (*MemoryReadFunc)(MemoryRegion *mr, target_phys_addr_t 
addr, size_t size);
typedef void (*MemoryWriteFunc)(MemoryRegion *mr, target_phys_addr_t 
addr, uint64_t data, size_tsize);

For 32 bit host / target, this would mean some unnecessary overhead.

What about passing values by address:

typedef void (*MemoryReadFunc)(MemoryRegion *mr, target_phys_addr_t 
addr, void *data, size_t size);
typedef void (*MemoryWriteFunc)(MemoryRegion *mr, target_phys_addr_t 
addr, const void *data, size_t size);

If we keep per-access type function pointers, they should use individual 
prototypes
for the different access types:

typedef uint8_t  (*MemoryReadbFunc)(MemoryRegion *mr, target_phys_addr_t 
addr);
typedef uint16_t (*MemoryReadwFunc)(MemoryRegion *mr, target_phys_addr_t 
addr);
typedef uint32_t (*MemoryReadlFunc)(MemoryRegion *mr, target_phys_addr_t 
addr);
typedef uint64_t (*MemoryReadllFunc)(MemoryRegion *mr, 
target_phys_addr_t addr);
...

Regards,
Stefan W.