Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

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

Am 19.05.2011 16:12, schrieb Avi Kivity:
> The memory API separates the attributes of a memory region (its size, how
> reads or writes are handled, dirty logging, and coalescing) from where it
> is mapped and whether it is enabled. This allows a device to configure
> a memory region once, then hand it off to its parent bus to map it 
> according
> to the bus configuration.
>
> Hierarchical registration also allows a device to compose a region out of
> a number of sub-regions with different properties; for example some may be
> RAM while others may be MMIO.
>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
> memory.h | 142 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 142 insertions(+), 0 deletions(-)
> create mode 100644 memory.h
>
> diff --git a/memory.h b/memory.h
> new file mode 100644
> index 0000000..77c5951
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,142 @@
> +#ifndef MEMORY_H
> +#define MEMORY_H
> +
> +#include <stdint.h>
> +#include <stdbool.h>

stdbool.h is already included in qemu-common.h,
stdint.h (indirectly) too.

Therefore both include statements can be removed.

> +#include "qemu-common.h"
> +#include "cpu-common.h"
> +#include "targphys.h"
> +#include "qemu-queue.h"
> +
> +typedef struct MemoryRegionOps MemoryRegionOps;
> +typedef struct MemoryRegion MemoryRegion;
> +
> +/*
> + * Memory region callbacks
> + */
> +struct MemoryRegionOps {
> + /* Read from the memory region. @addr is relative to @mr; @size is
> + * in bytes. */
> + uint64_t (*read)(MemoryRegion *mr,
> + target_phys_addr_t addr,
> + unsigned size);
> + /* Write to the memory region. @addr is relative to @mr; @size is
> + * in bytes. */
> + void (*write)(MemoryRegion *mr,
> + target_phys_addr_t addr,
> + uint64_t data,
> + unsigned size);
> + /* Guest-visible constraints: */
> + struct {
> + /* If nonzero, specify bounds on access sizes beyond which a machine
> + * check is thrown.
> + */
> + unsigned min_access_size;
> + unsigned max_access_size;
> + /* If true, unaligned accesses are supported. Otherwise unaligned
> + * accesses throw machine checks.
> + */
> + bool unaligned;
> + } valid;
> + /* Internal implementation constraints: */
> + struct {
> + /* If nonzero, specifies the minimum size implemented. Smaller sizes
> + * will be rounded upwards and a partial result will be returned.
> + */
> + unsigned min_access_size;
> + /* If nonzero, specifies the maximum size implemented. Larger sizes
> + * will be done as a series of accesses with smaller sizes.
> + */
> + unsigned max_access_size;
> + /* If true, unaligned accesses are supported. Otherwise all accesses
> + * are converted to (possibly multiple) naturally aligned accesses.
> + */
> + bool unaligned;
> + } impl;
> +};
> +
> +typedef struct CoalescedMemoryRange CoalescedMemoryRange;
> +
> +struct CoalescedMemoryRange {
> + target_phys_addr_t start;
> + target_phys_addr_t size;
> + QTAILQ_ENTRY(coalesced_ranges) link;
> +};
> +
> +struct MemoryRegion {
> + /* All fields are private - violators will be prosecuted */

Is it possible to move this private declaration into the implementation
file (or a private header file if the declaration is needed by more than
one file)?

> + const MemoryRegionOps *ops;
> + MemoryRegion *parent;
> + target_phys_addr_t size;
> + target_phys_addr_t addr;
> + ram_addr_t ram_addr;
> + unsigned priority;
> + bool may_overlap;
> + QTAILQ_HEAD(subregions, MemoryRegion) subregions;
> + QTAILQ_ENTRY(subregions) subregions_link;
> + QTAILQ_HEAD(coalesced_ranges, CoalescedMemoryRange) coalesced;
> +};
> +
> +/* Initialize a memory region
> + *
> + * The region typically acts as a container for other memory regions.
> + */
> +void memory_region_init(MemoryRegion *mr,
> + target_phys_addr_t size);
> +/* Initialize an I/O memory region. Accesses into the region will be
> + * cause the callbacks in @ops to be called.
> + *
> + * if @size is nonzero, subregions will be clipped to @size.
> + */
> +void memory_region_init_io(MemoryRegion *mr,
> + const MemoryRegionOps *ops,
> + target_phys_addr_t size);
> +/* Initialize an I/O memory region. Accesses into the region will be
> + * modify memory directly.
> + */
> +void memory_region_init_ram(MemoryRegion *mr,
> + target_phys_addr_t size);
> +/* Initialize a RAM memory region. Accesses into the region will be
> + * modify memory in @ptr directly.
> + */
> +void memory_region_init_ram_ptr(MemoryRegion *mr,
> + target_phys_addr_t size,
> + void *ptr);
> +/* Destroy a memory region. The memory becomes inaccessible. */
> +void memory_region_destroy(MemoryRegion *mr);
> +/* Sets an offset to be added to MemoryRegionOps callbacks. */
> +void memory_region_set_offset(MemoryRegion *mr, target_phys_addr_t 
> offset);
> +/* Turn loggging on or off for specified client (display, migration) */
> +void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client);
> +/* Enable memory coalescing for the region. MMIO ->write callbacks may be
> + * delayed until a non-coalesced MMIO is issued.
> + */
> +void memory_region_set_coalescing(MemoryRegion *mr);
> +/* Enable memory coalescing for a sub-range of the region. MMIO ->write
> + * callbacks may be delayed until a non-coalesced MMIO is issued.
> + */
> +void memory_region_add_coalescing(MemoryRegion *mr,
> + target_phys_addr_t offset,
> + target_phys_addr_t size);
> +/* Disable MMIO coalescing for the region. */
> +void memory_region_clear_coalescing(MemoryRegion *mr);
> +
> +/* Add a sub-region at @offset. The sub-region may not overlap with other
> + * subregions (except for those explicitly marked as overlapping)
> + */
> +void memory_region_add_subregion(MemoryRegion *mr,
> + target_phys_addr_t offset,
> + MemoryRegion *subregion);
> +/* Add a sub-region at @offset. The sun-region may overlap other 
> subregions;
> + * conflicts are resolved by having a higher @priority hide a lower 
> @priority.
> + * Subregions without priority are taken as @priority 0.
> + */
> +void memory_region_add_subregion_overlap(MemoryRegion *mr,
> + target_phys_addr_t offset,
> + MemoryRegion *subregion,
> + unsigned priority);
> +/* Remove a subregion. */
> +void memory_region_del_subregion(MemoryRegion *mr,
> + MemoryRegion *subregion);
> +
> +#endif

Kind regards,
Stefan W.