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.
