[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v1 19/22] memory: per-AddressSpace dispatch
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [RFC v1 19/22] memory: per-AddressSpace dispatch |
Date: |
Thu, 04 Oct 2012 09:13:16 -0500 |
User-agent: |
Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
Avi Kivity <address@hidden> writes:
> Currently we use a global radix tree to dispatch memory access. This only
> works with a single address space; to support multiple address spaces we
> make the radix tree a member of AddressSpace (via an intermediate structure
> AddressSpaceDispatch to avoid exposing too many internals).
>
> A side effect is that address_space_io also gains a dispatch table. When
> we remove all the pre-memory-API I/O registrations, we can use that for
> dispatching I/O and get rid of the original I/O dispatch.
>
> Signed-off-by: Avi Kivity <address@hidden>
> ---
> cputlb.c | 3 +-
> cputlb.h | 3 +-
> exec.c | 176
> +++++++++++++++++++++++++++++++++---------------------
> memory-internal.h | 22 ++++++-
> memory.c | 1 +
> memory.h | 62 +++++++++++++++++++
> 6 files changed, 196 insertions(+), 71 deletions(-)
>
> diff --git a/cputlb.c b/cputlb.c
> index 0627f32..9027557 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -21,6 +21,7 @@
> #include "cpu.h"
> #include "exec-all.h"
> #include "memory.h"
> +#include "exec-memory.h"
>
> #include "cputlb.h"
>
> @@ -251,7 +252,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
> if (size != TARGET_PAGE_SIZE) {
> tlb_add_large_page(env, vaddr, size);
> }
> - section = phys_page_find(paddr >> TARGET_PAGE_BITS);
> + section = phys_page_find(address_space_memory.dispatch, paddr >>
> TARGET_PAGE_BITS);
> #if defined(DEBUG_TLB)
> printf("tlb_set_page: vaddr=" TARGET_FMT_lx " paddr=0x" TARGET_FMT_plx
> " prot=%x idx=%d pd=0x%08lx\n",
> diff --git a/cputlb.h b/cputlb.h
> index 2dc2c96..d537b77 100644
> --- a/cputlb.h
> +++ b/cputlb.h
> @@ -26,7 +26,8 @@ void tlb_unprotect_code_phys(CPUArchState *env, ram_addr_t
> ram_addr,
> target_ulong vaddr);
> void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start,
> uintptr_t length);
> -MemoryRegionSection *phys_page_find(target_phys_addr_t index);
> +MemoryRegionSection *phys_page_find(struct AddressSpaceDispatch *d,
> + target_phys_addr_t index);
> void cpu_tlb_reset_dirty_all(ram_addr_t start1, ram_addr_t length);
> void tlb_set_dirty(CPUArchState *env, target_ulong vaddr);
> extern int tlb_flush_count;
> diff --git a/exec.c b/exec.c
> index 7a76efa..42f9ad1 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -187,7 +187,6 @@
> static void *l1_map[V_L1_SIZE];
>
> #if !defined(CONFIG_USER_ONLY)
> -typedef struct PhysPageEntry PhysPageEntry;
>
> static MemoryRegionSection *phys_sections;
> static unsigned phys_sections_nb, phys_sections_nb_alloc;
> @@ -196,22 +195,12 @@
> static uint16_t phys_section_rom;
> static uint16_t phys_section_watch;
>
> -struct PhysPageEntry {
> - uint16_t is_leaf : 1;
> - /* index into phys_sections (is_leaf) or phys_map_nodes (!is_leaf) */
> - uint16_t ptr : 15;
> -};
> -
> /* Simple allocator for PhysPageEntry nodes */
> static PhysPageEntry (*phys_map_nodes)[L2_SIZE];
> static unsigned phys_map_nodes_nb, phys_map_nodes_nb_alloc;
>
> #define PHYS_MAP_NODE_NIL (((uint16_t)~0) >> 1)
>
> -/* This is a multi-level map on the physical address space.
> - The bottom level has pointers to MemoryRegionSections. */
> -static PhysPageEntry phys_map = { .ptr = PHYS_MAP_NODE_NIL, .is_leaf = 0 };
> -
> static void io_mem_init(void);
> static void memory_map_init(void);
>
> @@ -459,18 +448,19 @@ static void phys_page_set_level(PhysPageEntry *lp,
> target_phys_addr_t *index,
> }
> }
>
> -static void phys_page_set(target_phys_addr_t index, target_phys_addr_t nb,
> +static void phys_page_set(AddressSpaceDispatch *d,
> + target_phys_addr_t index, target_phys_addr_t nb,
> uint16_t leaf)
> {
> /* Wildly overreserve - it doesn't matter much. */
> phys_map_node_reserve(3 * P_L2_LEVELS);
>
> - phys_page_set_level(&phys_map, &index, &nb, leaf, P_L2_LEVELS - 1);
> + phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS - 1);
> }
>
> -MemoryRegionSection *phys_page_find(target_phys_addr_t index)
> +MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d,
> target_phys_addr_t index)
> {
> - PhysPageEntry lp = phys_map;
> + PhysPageEntry lp = d->phys_map;
> PhysPageEntry *p;
> int i;
> uint16_t s_index = phys_section_unassigned;
> @@ -1472,7 +1462,7 @@ void tb_invalidate_phys_addr(target_phys_addr_t addr)
> ram_addr_t ram_addr;
> MemoryRegionSection *section;
>
> - section = phys_page_find(addr >> TARGET_PAGE_BITS);
> + section = phys_page_find(address_space_memory.dispatch, addr >>
> TARGET_PAGE_BITS);
> if (!(memory_region_is_ram(section->mr)
> || (section->mr->rom_device && section->mr->readable))) {
> return;
> @@ -2218,9 +2208,9 @@ static void destroy_l2_mapping(PhysPageEntry *lp,
> unsigned level)
> lp->ptr = PHYS_MAP_NODE_NIL;
> }
>
> -static void destroy_all_mappings(void)
> +static void destroy_all_mappings(AddressSpaceDispatch *d)
> {
> - destroy_l2_mapping(&phys_map, P_L2_LEVELS - 1);
> + destroy_l2_mapping(&d->phys_map, P_L2_LEVELS - 1);
> phys_map_nodes_reset();
> }
>
> @@ -2240,12 +2230,12 @@ static void phys_sections_clear(void)
> phys_sections_nb = 0;
> }
>
> -static void register_subpage(MemoryRegionSection *section)
> +static void register_subpage(AddressSpaceDispatch *d, MemoryRegionSection
> *section)
> {
> subpage_t *subpage;
> target_phys_addr_t base = section->offset_within_address_space
> & TARGET_PAGE_MASK;
> - MemoryRegionSection *existing = phys_page_find(base >> TARGET_PAGE_BITS);
> + MemoryRegionSection *existing = phys_page_find(d, base >>
> TARGET_PAGE_BITS);
> MemoryRegionSection subsection = {
> .offset_within_address_space = base,
> .size = TARGET_PAGE_SIZE,
> @@ -2257,7 +2247,7 @@ static void register_subpage(MemoryRegionSection
> *section)
> if (!(existing->mr->subpage)) {
> subpage = subpage_init(base);
> subsection.mr = &subpage->iomem;
> - phys_page_set(base >> TARGET_PAGE_BITS, 1,
> + phys_page_set(d, base >> TARGET_PAGE_BITS, 1,
> phys_section_add(&subsection));
> } else {
> subpage = container_of(existing->mr, subpage_t, iomem);
> @@ -2268,7 +2258,7 @@ static void register_subpage(MemoryRegionSection
> *section)
> }
>
>
> -static void register_multipage(MemoryRegionSection *section)
> +static void register_multipage(AddressSpaceDispatch *d, MemoryRegionSection
> *section)
> {
> target_phys_addr_t start_addr = section->offset_within_address_space;
> ram_addr_t size = section->size;
> @@ -2278,13 +2268,13 @@ static void register_multipage(MemoryRegionSection
> *section)
> assert(size);
>
> addr = start_addr;
> - phys_page_set(addr >> TARGET_PAGE_BITS, size >> TARGET_PAGE_BITS,
> + phys_page_set(d, addr >> TARGET_PAGE_BITS, size >> TARGET_PAGE_BITS,
> section_index);
> }
>
> -void cpu_register_physical_memory_log(MemoryRegionSection *section,
> - bool readonly)
> +static void mem_add(MemoryListener *listener, MemoryRegionSection *section)
> {
> + AddressSpaceDispatch *d = container_of(listener, AddressSpaceDispatch,
> listener);
> MemoryRegionSection now = *section, remain = *section;
>
> if ((now.offset_within_address_space & ~TARGET_PAGE_MASK)
> @@ -2292,7 +2282,7 @@ void
> cpu_register_physical_memory_log(MemoryRegionSection *section,
> now.size = MIN(TARGET_PAGE_ALIGN(now.offset_within_address_space)
> - now.offset_within_address_space,
> now.size);
> - register_subpage(&now);
> + register_subpage(d, &now);
> remain.size -= now.size;
> remain.offset_within_address_space += now.size;
> remain.offset_within_region += now.size;
> @@ -2301,10 +2291,10 @@ void
> cpu_register_physical_memory_log(MemoryRegionSection *section,
> now = remain;
> if (remain.offset_within_region & ~TARGET_PAGE_MASK) {
> now.size = TARGET_PAGE_SIZE;
> - register_subpage(&now);
> + register_subpage(d, &now);
> } else {
> now.size &= TARGET_PAGE_MASK;
> - register_multipage(&now);
> + register_multipage(d, &now);
> }
> remain.size -= now.size;
> remain.offset_within_address_space += now.size;
> @@ -2312,7 +2302,7 @@ void
> cpu_register_physical_memory_log(MemoryRegionSection *section,
> }
> now = remain;
> if (now.size) {
> - register_subpage(&now);
> + register_subpage(d, &now);
> }
> }
>
> @@ -3163,11 +3153,17 @@ static void io_mem_init(void)
> "watch", UINT64_MAX);
> }
>
> +static void mem_begin(MemoryListener *listener)
> +{
> + AddressSpaceDispatch *d = container_of(listener, AddressSpaceDispatch,
> listener);
> +
> + destroy_all_mappings(d);
> + d->phys_map.ptr = PHYS_MAP_NODE_NIL;
> +}
> +
> static void core_begin(MemoryListener *listener)
> {
> - destroy_all_mappings();
> phys_sections_clear();
> - phys_map.ptr = PHYS_MAP_NODE_NIL;
> phys_section_unassigned = dummy_section(&io_mem_unassigned);
> phys_section_notdirty = dummy_section(&io_mem_notdirty);
> phys_section_rom = dummy_section(&io_mem_rom);
> @@ -3186,18 +3182,6 @@ static void tcg_commit(MemoryListener *listener)
> }
> }
>
> -static void core_region_add(MemoryListener *listener,
> - MemoryRegionSection *section)
> -{
> - cpu_register_physical_memory_log(section, section->readonly);
> -}
> -
> -static void core_region_nop(MemoryListener *listener,
> - MemoryRegionSection *section)
> -{
> - cpu_register_physical_memory_log(section, section->readonly);
> -}
> -
> static void core_log_global_start(MemoryListener *listener)
> {
> cpu_physical_memory_set_dirty_tracking(1);
> @@ -3229,11 +3213,9 @@ static void io_region_del(MemoryListener *listener,
> static MemoryListener core_memory_listener = {
> MEMORY_LISTENER_DEFAULT_OPS,
> .begin = core_begin,
> - .region_add = core_region_add,
> - .region_nop = core_region_nop,
> .log_global_start = core_log_global_start,
> .log_global_stop = core_log_global_stop,
> - .priority = 0,
> + .priority = 1,
> };
>
> static MemoryListener io_memory_listener = {
> @@ -3248,6 +3230,22 @@ static void io_region_del(MemoryListener *listener,
> .commit = tcg_commit,
> };
>
> +void address_space_init_dispatch(AddressSpace *as)
> +{
> + AddressSpaceDispatch *d = g_new(AddressSpaceDispatch, 1);
> +
> + d->phys_map = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .is_leaf = 0
> };
> + d->listener = (MemoryListener) {
> + MEMORY_LISTENER_DEFAULT_OPS,
> + .begin = mem_begin,
> + .region_add = mem_add,
> + .region_nop = mem_add,
> + .priority = 0,
> + };
I see you've become fond of this extension :-)
I'd personally avoid it... You're typing more than you need to.
BTW, if you make the earlier change I suggested, the
MEMORY_LISTENER_DEFAULT_OPS is no longer necessary.
Regards,
Anthony Liguori
> diff --git a/memory-internal.h b/memory-internal.h
> index 655f71f..a9d914e 100644
> --- a/memory-internal.h
> +++ b/memory-internal.h
> @@ -21,6 +21,26 @@
>
> #ifndef CONFIG_USER_ONLY
>
> +typedef struct PhysPageEntry PhysPageEntry;
> +
> +struct PhysPageEntry {
> + uint16_t is_leaf : 1;
> + /* index into phys_sections (is_leaf) or phys_map_nodes (!is_leaf) */
> + uint16_t ptr : 15;
> +};
> +
> +typedef struct AddressSpaceDispatch AddressSpaceDispatch;
> +
> +struct AddressSpaceDispatch {
> + /* This is a multi-level map on the physical address space.
> + * The bottom level has pointers to MemoryRegionSections.
> + */
> + PhysPageEntry phys_map;
> + MemoryListener listener;
> +};
> +
> +void address_space_init_dispatch(AddressSpace *as);
> +
> ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> MemoryRegion *mr);
> ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr);
> @@ -29,8 +49,6 @@ void qemu_ram_free_from_ptr(ram_addr_t addr);
>
> struct MemoryRegion;
> struct MemoryRegionSection;
> -void cpu_register_physical_memory_log(struct MemoryRegionSection *section,
> - bool readonly);
>
> void qemu_register_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size);
> void qemu_unregister_coalesced_mmio(target_phys_addr_t addr, ram_addr_t
> size);
> diff --git a/memory.c b/memory.c
> index f829d84..28a79ae 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1550,6 +1550,7 @@ void address_space_init(AddressSpace *as, MemoryRegion
> *root)
> QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
> as->name = NULL;
> memory_region_transaction_commit();
> + address_space_init_dispatch(as);
> }
>
> uint64_t io_mem_read(MemoryRegion *mr, target_phys_addr_t addr, unsigned
> size)
> diff --git a/memory.h b/memory.h
> index 6115f48..84f7439 100644
> --- a/memory.h
> +++ b/memory.h
> @@ -169,6 +169,7 @@ struct AddressSpace {
> struct FlatView *current_map;
> int ioeventfd_nb;
> struct MemoryRegionIoeventfd *ioeventfds;
> + struct AddressSpaceDispatch *dispatch;
> QTAILQ_ENTRY(AddressSpace) address_spaces_link;
> };
>
> @@ -830,6 +831,67 @@ void mtree_info(fprintf_function mon_printf, void *f);
> */
> void address_space_init(AddressSpace *as, MemoryRegion *root);
>
> +/**
> + * address_space_rw: read from or write to an address space.
> + *
> + * @as: #AddressSpace to be accessed
> + * @addr: address within that address space
> + * @buf: buffer with the data transferred
> + * @is_write: indicates the transfer direction
> + */
> +void address_space_rw(AddressSpace *as, target_phys_addr_t addr, uint8_t
> *buf,
> + int len, bool is_write);
> +
> +/**
> + * address_space_write: write to address space.
> + *
> + * @as: #AddressSpace to be accessed
> + * @addr: address within that address space
> + * @buf: buffer with the data transferred
> + */
> +void address_space_write(AddressSpace *as, target_phys_addr_t addr,
> + const uint8_t *buf, int len);
> +
> +/**
> + * address_space_read: read from an address space.
> + *
> + * @as: #AddressSpace to be accessed
> + * @addr: address within that address space
> + * @buf: buffer with the data transferred
> + */
> +void address_space_read(AddressSpace *as, target_phys_addr_t addr, uint8_t
> *buf, int len);
> +
> +/* address_space_map: map a physical memory region into a host virtual
> address
> + *
> + * May map a subset of the requested range, given by and returned in @plen.
> + * May return %NULL if resources needed to perform the mapping are exhausted.
> + * Use only for reads OR writes - not for read-modify-write operations.
> + * Use cpu_register_map_client() to know when retrying the map operation is
> + * likely to succeed.
> + *
> + * @as: #AddressSpace to be accessed
> + * @addr: address within that address space
> + * @plen: pointer to length of buffer; updated on return
> + * @is_write: indicates the transfer direction
> + */
> +void *address_space_map(AddressSpace *as, target_phys_addr_t addr,
> + target_phys_addr_t *plen, bool is_write);
> +
> +/* address_space_unmap: Unmaps a memory region previously mapped by
> address_space_map()
> + *
> + * Will also mark the memory as dirty if @is_write == %true. @access_len
> gives
> + * the amount of memory that was actually read or written by the caller.
> + *
> + * @as: #AddressSpace used
> + * @addr: address within that address space
> + * @len: buffer length as returned by address_space_map()
> + * @access_len: amount of data actually transferred
> + * @is_write: indicates the transfer direction
> + */
> +void address_space_unmap(AddressSpace *as, void *buffer, target_phys_addr_t
> len,
> + int is_write, target_phys_addr_t access_len);
> +
> +
> #endif
>
> #endif
> --
> 1.7.12
- Re: [Qemu-devel] [RFC v1 19/22] memory: per-AddressSpace dispatch, (continued)
- Re: [Qemu-devel] [RFC v1 19/22] memory: per-AddressSpace dispatch, Blue Swirl, 2012/10/04
- Re: [Qemu-devel] [RFC v1 19/22] memory: per-AddressSpace dispatch, Avi Kivity, 2012/10/04
- Re: [Qemu-devel] [RFC v1 19/22] memory: per-AddressSpace dispatch, Blue Swirl, 2012/10/04
- Re: [Qemu-devel] [RFC v1 19/22] memory: per-AddressSpace dispatch, Anthony Liguori, 2012/10/04
- Re: [Qemu-devel] [RFC v1 19/22] memory: per-AddressSpace dispatch, Peter Maydell, 2012/10/04
- Re: [Qemu-devel] [RFC v1 19/22] memory: per-AddressSpace dispatch, Avi Kivity, 2012/10/07
- Re: [Qemu-devel] [RFC v1 19/22] memory: per-AddressSpace dispatch, Blue Swirl, 2012/10/04
Re: [Qemu-devel] [RFC v1 19/22] memory: per-AddressSpace dispatch,
Anthony Liguori <=
[Qemu-devel] [RFC v1 17/22] memory: use AddressSpace for MemoryListener filtering, Avi Kivity, 2012/10/03
Re: [Qemu-devel] [RFC v1 17/22] memory: use AddressSpace for MemoryListener filtering, Anthony Liguori, 2012/10/04
[Qemu-devel] [RFC v1 18/22] s390: avoid reaching into memory core internals, Avi Kivity, 2012/10/03
Re: [Qemu-devel] [RFC v1 00/22] Integrate DMA into the memory API, Blue Swirl, 2012/10/03