| Message ID | 1369133851-1894-18-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, May 21, 2013 at 6:57 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Using phys_page_find to translate an AddressSpace to a MemoryRegionSection > is unwieldy. It requires to pass the page index rather than the address, > and later memory_region_section_addr has to be called. Replace > memory_region_section_addr with a function that does all of it: call > phys_page_find, compute the offset within the region, and check how > big the current mapping is. This way, a large flat region can be written > with a single lookup rather than a page at a time. > > address_space_translate will also provide a single point where IOMMU > forwarding is implemented. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > cputlb.c | 20 ++--- > exec.c | 201 +++++++++++++++++++++++++++----------------------- > include/exec/cputlb.h | 12 ++- > include/exec/memory.h | 31 ++++---- > translate-all.c | 6 +- > 5 files changed, 143 insertions(+), 127 deletions(-) > > diff --git a/cputlb.c b/cputlb.c > index aba7e44..1f85da0 100644 > --- a/cputlb.c > +++ b/cputlb.c > @@ -248,13 +248,18 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr, > target_ulong code_address; > uintptr_t addend; > CPUTLBEntry *te; > - hwaddr iotlb; > + hwaddr iotlb, xlat, sz; > > assert(size >= TARGET_PAGE_SIZE); > if (size != TARGET_PAGE_SIZE) { > tlb_add_large_page(env, vaddr, size); > } > - section = phys_page_find(address_space_memory.dispatch, paddr >> TARGET_PAGE_BITS); > + > + sz = size; > + section = address_space_translate(&address_space_memory, paddr, &xlat, &sz, > + false); > + assert(sz >= TARGET_PAGE_SIZE); > + > #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", > @@ -269,15 +274,14 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr, > } > if (memory_region_is_ram(section->mr) || > memory_region_is_romd(section->mr)) { > - addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) > - + memory_region_section_addr(section, paddr); > + addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) + xlat; > } else { > addend = 0; > } > > code_address = address; > - iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, prot, > - &address); > + iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, xlat, > + prot, &address); > > index = (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); > env->iotlb[mmu_idx][index] = iotlb - vaddr; > @@ -300,9 +304,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr, > /* Write access calls the I/O callback. */ > te->addr_write = address | TLB_MMIO; > } else if (memory_region_is_ram(section->mr) > - && !cpu_physical_memory_is_dirty( > - section->mr->ram_addr > - + memory_region_section_addr(section, paddr))) { > + && !cpu_physical_memory_is_dirty(section->mr->ram_addr + xlat)) { > te->addr_write = address | TLB_NOTDIRTY; > } else { > te->addr_write = address; > diff --git a/exec.c b/exec.c > index 82da067..e5ee8ff 100644 > --- a/exec.c > +++ b/exec.c > @@ -182,7 +182,7 @@ static void phys_page_set(AddressSpaceDispatch *d, > phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS - 1); > } > > -MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index) > +static MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index) > { > PhysPageEntry lp = d->phys_map; > PhysPageEntry *p; > @@ -198,6 +198,22 @@ MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index) > return &phys_sections[lp.ptr]; > } > > +MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr, > + hwaddr *xlat, hwaddr *plen, > + bool is_write) > +{ > + MemoryRegionSection *section; > + > + section = phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS); > + /* Compute offset within MemoryRegionSection */ > + addr -= section->offset_within_address_space; > + *plen = MIN(section->size - addr, *plen); > + > + /* Compute offset within MemoryRegion */ > + *xlat = addr + section->offset_within_region; > + return section; > +} > + > bool memory_region_is_unassigned(MemoryRegion *mr) > { > return mr != &io_mem_ram && mr != &io_mem_rom > @@ -616,11 +632,11 @@ static int cpu_physical_memory_set_dirty_tracking(int enable) > } > > hwaddr memory_region_section_get_iotlb(CPUArchState *env, > - MemoryRegionSection *section, > - target_ulong vaddr, > - hwaddr paddr, > - int prot, > - target_ulong *address) > + MemoryRegionSection *section, > + target_ulong vaddr, > + hwaddr paddr, hwaddr xlat, > + int prot, > + target_ulong *address) > { > hwaddr iotlb; > CPUWatchpoint *wp; > @@ -628,7 +644,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env, > if (memory_region_is_ram(section->mr)) { > /* Normal RAM. */ > iotlb = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) > - + memory_region_section_addr(section, paddr); > + + xlat; > if (!section->readonly) { > iotlb |= phys_section_notdirty; > } else { > @@ -636,7 +652,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env, > } > } else { > iotlb = section - phys_sections; > - iotlb += memory_region_section_addr(section, paddr); > + iotlb += xlat; > } > > /* Make accesses to pages with watchpoints go via the > @@ -1888,24 +1904,18 @@ static void invalidate_and_set_dirty(hwaddr addr, > void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > int len, bool is_write) > { > - AddressSpaceDispatch *d = as->dispatch; > - int l; > + hwaddr l; > uint8_t *ptr; > uint32_t val; > - hwaddr page; > + hwaddr addr1; > MemoryRegionSection *section; > > while (len > 0) { > - page = addr & TARGET_PAGE_MASK; > - l = (page + TARGET_PAGE_SIZE) - addr; > - if (l > len) > - l = len; > - section = phys_page_find(d, page >> TARGET_PAGE_BITS); > + l = len; > + section = address_space_translate(as, addr, &addr1, &l, is_write); > Jan's "Rework sub-page handling" patch broke the tlb_set_page (anything else?) But I think we still need a function based on address_space_translate to wrap the subpage logic inside and ease the incoming RCU style. Regards, Pingfan > if (is_write) { > if (!memory_region_is_ram(section->mr)) { > - hwaddr addr1; > - addr1 = memory_region_section_addr(section, addr); > /* XXX: could force cpu_single_env to NULL to avoid > potential bugs */ > if (l >= 4 && ((addr1 & 3) == 0)) { > @@ -1925,9 +1935,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > l = 1; > } > } else if (!section->readonly) { > - ram_addr_t addr1; > - addr1 = memory_region_get_ram_addr(section->mr) > - + memory_region_section_addr(section, addr); > + addr1 += memory_region_get_ram_addr(section->mr); > /* RAM case */ > ptr = qemu_get_ram_ptr(addr1); > memcpy(ptr, buf, l); > @@ -1936,9 +1944,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > } else { > if (!(memory_region_is_ram(section->mr) || > memory_region_is_romd(section->mr))) { > - hwaddr addr1; > /* I/O case */ > - addr1 = memory_region_section_addr(section, addr); > if (l >= 4 && ((addr1 & 3) == 0)) { > /* 32 bit read access */ > val = io_mem_read(section->mr, addr1, 4); > @@ -1957,9 +1963,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, > } > } else { > /* RAM case */ > - ptr = qemu_get_ram_ptr(section->mr->ram_addr > - + memory_region_section_addr(section, > - addr)); > + ptr = qemu_get_ram_ptr(section->mr->ram_addr + addr1); > memcpy(buf, ptr, l); > } > } > @@ -1998,26 +2002,21 @@ void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf, > void cpu_physical_memory_write_rom(hwaddr addr, > const uint8_t *buf, int len) > { > - AddressSpaceDispatch *d = address_space_memory.dispatch; > - int l; > + hwaddr l; > uint8_t *ptr; > - hwaddr page; > + hwaddr addr1; > MemoryRegionSection *section; > > while (len > 0) { > - page = addr & TARGET_PAGE_MASK; > - l = (page + TARGET_PAGE_SIZE) - addr; > - if (l > len) > - l = len; > - section = phys_page_find(d, page >> TARGET_PAGE_BITS); > + l = len; > + section = address_space_translate(&address_space_memory, > + addr, &addr1, &l, true); > > if (!(memory_region_is_ram(section->mr) || > memory_region_is_romd(section->mr))) { > /* do nothing */ > } else { > - unsigned long addr1; > - addr1 = memory_region_get_ram_addr(section->mr) > - + memory_region_section_addr(section, addr); > + addr1 += memory_region_get_ram_addr(section->mr); > /* ROM/RAM case */ > ptr = qemu_get_ram_ptr(addr1); > memcpy(ptr, buf, l); > @@ -2077,18 +2076,12 @@ static void cpu_notify_map_clients(void) > > bool address_space_valid(AddressSpace *as, hwaddr addr, int len, bool is_write) > { > - AddressSpaceDispatch *d = as->dispatch; > MemoryRegionSection *section; > - int l; > - hwaddr page; > + hwaddr l, xlat; > > while (len > 0) { > - page = addr & TARGET_PAGE_MASK; > - l = (page + TARGET_PAGE_SIZE) - addr; > - if (l > len) { > - l = len; > - } > - section = phys_page_find(d, addr >> TARGET_PAGE_BITS); > + l = len; > + section = address_space_translate(as, addr, &xlat, &l, is_write); > if (section->mr == &io_mem_unassigned || > (is_write && section->mr->readonly)) { > return false; > @@ -2112,22 +2105,17 @@ void *address_space_map(AddressSpace *as, > hwaddr *plen, > bool is_write) > { > - AddressSpaceDispatch *d = as->dispatch; > hwaddr len = *plen; > hwaddr todo = 0; > - int l; > - hwaddr page; > + hwaddr l, xlat; > MemoryRegionSection *section; > ram_addr_t raddr = RAM_ADDR_MAX; > ram_addr_t rlen; > void *ret; > > while (len > 0) { > - page = addr & TARGET_PAGE_MASK; > - l = (page + TARGET_PAGE_SIZE) - addr; > - if (l > len) > - l = len; > - section = phys_page_find(d, page >> TARGET_PAGE_BITS); > + l = len; > + section = address_space_translate(as, addr, &xlat, &l, is_write); > > if (!(memory_region_is_ram(section->mr) && !section->readonly)) { > if (todo || bounce.buffer) { > @@ -2144,8 +2132,11 @@ void *address_space_map(AddressSpace *as, > return bounce.buffer; > } > if (!todo) { > - raddr = memory_region_get_ram_addr(section->mr) > - + memory_region_section_addr(section, addr); > + raddr = memory_region_get_ram_addr(section->mr) + xlat; > + } else { > + if (memory_region_get_ram_addr(section->mr) + xlat != raddr + todo) { > + break; > + } > } > > len -= l; > @@ -2211,14 +2202,19 @@ static inline uint32_t ldl_phys_internal(hwaddr addr, > uint8_t *ptr; > uint32_t val; > MemoryRegionSection *section; > + hwaddr l = 4; > + hwaddr addr1; > > - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); > + section = address_space_translate(&address_space_memory, addr, &addr1, &l, > + false); > + if (l < 4) { > + return unassigned_mem_read(NULL, addr, 4); > + } > > if (!(memory_region_is_ram(section->mr) || > memory_region_is_romd(section->mr))) { > /* I/O case */ > - addr = memory_region_section_addr(section, addr); > - val = io_mem_read(section->mr, addr, 4); > + val = io_mem_read(section->mr, addr1, 4); > #if defined(TARGET_WORDS_BIGENDIAN) > if (endian == DEVICE_LITTLE_ENDIAN) { > val = bswap32(val); > @@ -2232,7 +2228,7 @@ static inline uint32_t ldl_phys_internal(hwaddr addr, > /* RAM case */ > ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr) > & TARGET_PAGE_MASK) > - + memory_region_section_addr(section, addr)); > + + addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > val = ldl_le_p(ptr); > @@ -2270,28 +2266,33 @@ static inline uint64_t ldq_phys_internal(hwaddr addr, > uint8_t *ptr; > uint64_t val; > MemoryRegionSection *section; > + hwaddr l = 8; > + hwaddr addr1; > > - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); > + section = address_space_translate(&address_space_memory, addr, &addr1, &l, > + false); > + if (l < 8) { > + return unassigned_mem_read(NULL, addr, 8); > + } > > if (!(memory_region_is_ram(section->mr) || > memory_region_is_romd(section->mr))) { > /* I/O case */ > - addr = memory_region_section_addr(section, addr); > > /* XXX This is broken when device endian != cpu endian. > Fix and add "endian" variable check */ > #ifdef TARGET_WORDS_BIGENDIAN > - val = io_mem_read(section->mr, addr, 4) << 32; > - val |= io_mem_read(section->mr, addr + 4, 4); > + val = io_mem_read(section->mr, addr1, 4) << 32; > + val |= io_mem_read(section->mr, addr1 + 4, 4); > #else > - val = io_mem_read(section->mr, addr, 4); > - val |= io_mem_read(section->mr, addr + 4, 4) << 32; > + val = io_mem_read(section->mr, addr1, 4); > + val |= io_mem_read(section->mr, addr1 + 4, 4) << 32; > #endif > } else { > /* RAM case */ > ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr) > & TARGET_PAGE_MASK) > - + memory_region_section_addr(section, addr)); > + + addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > val = ldq_le_p(ptr); > @@ -2337,14 +2338,19 @@ static inline uint32_t lduw_phys_internal(hwaddr addr, > uint8_t *ptr; > uint64_t val; > MemoryRegionSection *section; > + hwaddr l = 2; > + hwaddr addr1; > > - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); > + section = address_space_translate(&address_space_memory, addr, &addr1, &l, > + false); > + if (l < 2) { > + return unassigned_mem_read(NULL, addr, 2); > + } > > if (!(memory_region_is_ram(section->mr) || > memory_region_is_romd(section->mr))) { > /* I/O case */ > - addr = memory_region_section_addr(section, addr); > - val = io_mem_read(section->mr, addr, 2); > + val = io_mem_read(section->mr, addr1, 2); > #if defined(TARGET_WORDS_BIGENDIAN) > if (endian == DEVICE_LITTLE_ENDIAN) { > val = bswap16(val); > @@ -2358,7 +2364,7 @@ static inline uint32_t lduw_phys_internal(hwaddr addr, > /* RAM case */ > ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr) > & TARGET_PAGE_MASK) > - + memory_region_section_addr(section, addr)); > + + addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > val = lduw_le_p(ptr); > @@ -2396,19 +2402,23 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val) > { > uint8_t *ptr; > MemoryRegionSection *section; > + hwaddr l = 4; > + hwaddr addr1; > > - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); > + section = address_space_translate(&address_space_memory, addr, &addr1, &l, > + true); > + if (l < 4) { > + unassigned_mem_write(NULL, addr, val, 4); > + return; > + } > > if (!memory_region_is_ram(section->mr) || section->readonly) { > - addr = memory_region_section_addr(section, addr); > if (memory_region_is_ram(section->mr)) { > section = &phys_sections[phys_section_rom]; > } > - io_mem_write(section->mr, addr, val, 4); > + io_mem_write(section->mr, addr1, val, 4); > } else { > - unsigned long addr1 = (memory_region_get_ram_addr(section->mr) > - & TARGET_PAGE_MASK) > - + memory_region_section_addr(section, addr); > + addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK; > ptr = qemu_get_ram_ptr(addr1); > stl_p(ptr, val); > > @@ -2430,11 +2440,17 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val, > { > uint8_t *ptr; > MemoryRegionSection *section; > + hwaddr l = 4; > + hwaddr addr1; > > - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); > + section = address_space_translate(&address_space_memory, addr, &addr1, &l, > + true); > + if (l < 4) { > + unassigned_mem_write(NULL, addr, val, 4); > + return; > + } > > if (!memory_region_is_ram(section->mr) || section->readonly) { > - addr = memory_region_section_addr(section, addr); > if (memory_region_is_ram(section->mr)) { > section = &phys_sections[phys_section_rom]; > } > @@ -2447,12 +2463,10 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val, > val = bswap32(val); > } > #endif > - io_mem_write(section->mr, addr, val, 4); > + io_mem_write(section->mr, addr1, val, 4); > } else { > - unsigned long addr1; > - addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) > - + memory_region_section_addr(section, addr); > /* RAM case */ > + addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK; > ptr = qemu_get_ram_ptr(addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > @@ -2497,11 +2511,17 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val, > { > uint8_t *ptr; > MemoryRegionSection *section; > + hwaddr l = 2; > + hwaddr addr1; > > - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); > + section = address_space_translate(&address_space_memory, addr, &addr1, &l, > + true); > + if (l < 2) { > + unassigned_mem_write(NULL, addr, val, 2); > + return; > + } > > if (!memory_region_is_ram(section->mr) || section->readonly) { > - addr = memory_region_section_addr(section, addr); > if (memory_region_is_ram(section->mr)) { > section = &phys_sections[phys_section_rom]; > } > @@ -2514,12 +2534,10 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val, > val = bswap16(val); > } > #endif > - io_mem_write(section->mr, addr, val, 2); > + io_mem_write(section->mr, addr1, val, 2); > } else { > - unsigned long addr1; > - addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) > - + memory_region_section_addr(section, addr); > /* RAM case */ > + addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK; > ptr = qemu_get_ram_ptr(addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > @@ -2622,9 +2640,10 @@ bool virtio_is_big_endian(void) > bool cpu_physical_memory_is_io(hwaddr phys_addr) > { > MemoryRegionSection *section; > + hwaddr l = 1; > > - section = phys_page_find(address_space_memory.dispatch, > - phys_addr >> TARGET_PAGE_BITS); > + section = address_space_translate(&address_space_memory, > + phys_addr, &phys_addr, &l, false); > > return !(memory_region_is_ram(section->mr) || > memory_region_is_romd(section->mr)); > diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h > index 733c885..e821660 100644 > --- a/include/exec/cputlb.h > +++ b/include/exec/cputlb.h > @@ -26,8 +26,6 @@ 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(struct AddressSpaceDispatch *d, > - hwaddr 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; > @@ -35,11 +33,11 @@ extern int tlb_flush_count; > /* exec.c */ > void tb_flush_jmp_cache(CPUArchState *env, target_ulong addr); > hwaddr memory_region_section_get_iotlb(CPUArchState *env, > - MemoryRegionSection *section, > - target_ulong vaddr, > - hwaddr paddr, > - int prot, > - target_ulong *address); > + MemoryRegionSection *section, > + target_ulong vaddr, > + hwaddr paddr, hwaddr xlat, > + int prot, > + target_ulong *address); > bool memory_region_is_unassigned(MemoryRegion *mr); > > #endif > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 2e5fd11..809a958 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -743,23 +743,6 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr, > hwaddr addr, uint64_t size); > > /** > - * memory_region_section_addr: get offset within MemoryRegionSection > - * > - * Returns offset within MemoryRegionSection > - * > - * @section: the memory region section being queried > - * @addr: address in address space > - */ > -static inline hwaddr > -memory_region_section_addr(MemoryRegionSection *section, > - hwaddr addr) > -{ > - addr -= section->offset_within_address_space; > - addr += section->offset_within_region; > - return addr; > -} > - > -/** > * address_space_sync_dirty_bitmap: synchronize the dirty log for all memory > * > * Synchronizes the dirty page log for an entire address space. > @@ -860,6 +843,20 @@ void address_space_write(AddressSpace *as, hwaddr addr, > */ > void address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len); > > +/* address_space_translate: translate an address range into an address space > + * into a MemoryRegionSection and an address range into that section > + * > + * @as: #AddressSpace to be accessed > + * @addr: address within that address space > + * @xlat: pointer to address within the returned memory region section's > + * #MemoryRegion. > + * @len: pointer to length > + * @is_write: indicates the transfer direction > + */ > +MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr, > + hwaddr *xlat, hwaddr *len, > + bool is_write); > + > /* address_space_valid: check for validity of an address space range > * > * Check whether memory is assigned to the given address space range. > diff --git a/translate-all.c b/translate-all.c > index 0d84b0d..7a7d537 100644 > --- a/translate-all.c > +++ b/translate-all.c > @@ -1355,15 +1355,15 @@ void tb_invalidate_phys_addr(hwaddr addr) > { > ram_addr_t ram_addr; > MemoryRegionSection *section; > + hwaddr l = 1; > > - section = phys_page_find(address_space_memory.dispatch, > - addr >> TARGET_PAGE_BITS); > + section = address_space_translate(&address_space_memory, addr, &addr, &l, false); > if (!(memory_region_is_ram(section->mr) > || memory_region_is_romd(section->mr))) { > return; > } > ram_addr = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) > - + memory_region_section_addr(section, addr); > + + addr; > tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0); > } > #endif /* TARGET_HAS_ICE && !defined(CONFIG_USER_ONLY) */ > -- > 1.8.1.4 > > >
Il 23/05/2013 09:09, liu ping fan ha scritto: >> > void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, >> > int len, bool is_write) >> > { >> > - AddressSpaceDispatch *d = as->dispatch; >> > - int l; >> > + hwaddr l; >> > uint8_t *ptr; >> > uint32_t val; >> > - hwaddr page; >> > + hwaddr addr1; >> > MemoryRegionSection *section; >> > >> > while (len > 0) { >> > - page = addr & TARGET_PAGE_MASK; >> > - l = (page + TARGET_PAGE_SIZE) - addr; >> > - if (l > len) >> > - l = len; >> > - section = phys_page_find(d, page >> TARGET_PAGE_BITS); >> > + l = len; >> > + section = address_space_translate(as, addr, &addr1, &l, is_write); >> > > Jan's "Rework sub-page handling" patch broke the tlb_set_page > (anything else?) But I think we still need a function based on > address_space_translate to wrap the subpage logic inside and ease the > incoming RCU style. The idea is that address_space_translate gets a ref to the MemoryRegion, and the ref is then released by the caller of address_space_translate. That means that the actual memory accesses can run outside the RCU critical section. But I'm not sure how that is related to subpage logic. Subpages are simply MemoryRegions that only exist in the phys page map, rather than within an AddressSpace. Their destruction will be delayed anyway by doing call_rcu on the old phys page map. Paolo
On Thu, May 23, 2013 at 5:59 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 23/05/2013 09:09, liu ping fan ha scritto: >>> > void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, >>> > int len, bool is_write) >>> > { >>> > - AddressSpaceDispatch *d = as->dispatch; >>> > - int l; >>> > + hwaddr l; >>> > uint8_t *ptr; >>> > uint32_t val; >>> > - hwaddr page; >>> > + hwaddr addr1; >>> > MemoryRegionSection *section; >>> > >>> > while (len > 0) { >>> > - page = addr & TARGET_PAGE_MASK; >>> > - l = (page + TARGET_PAGE_SIZE) - addr; >>> > - if (l > len) >>> > - l = len; >>> > - section = phys_page_find(d, page >> TARGET_PAGE_BITS); >>> > + l = len; >>> > + section = address_space_translate(as, addr, &addr1, &l, is_write); >>> > >> Jan's "Rework sub-page handling" patch broke the tlb_set_page >> (anything else?) But I think we still need a function based on >> address_space_translate to wrap the subpage logic inside and ease the >> incoming RCU style. > > The idea is that address_space_translate gets a ref to the MemoryRegion, > and the ref is then released by the caller of address_space_translate. > This will require subpage hold a reference to real mr. But I think it is needless. Maybe when later in your these series, we will see how to resolve it. BTW, do you target URCU patches to 1.6 Regards, Pingfan > That means that the actual memory accesses can run outside the RCU > critical section. > > But I'm not sure how that is related to subpage logic. Subpages are > simply MemoryRegions that only exist in the phys page map, rather than > within an AddressSpace. Their destruction will be delayed anyway by > doing call_rcu on the old phys page map. > > Paolo
Il 23/05/2013 15:06, liu ping fan ha scritto: >> The idea is that address_space_translate gets a ref to the MemoryRegion, >> and the ref is then released by the caller of address_space_translate. >> > This will require subpage hold a reference to real mr. Hmm, that's right. Jan, that could be a concern when you update your PIO series. > But I think it > is needless. Maybe when later in your these series, we will see how to > resolve it. BTW, do you target URCU patches to 1.6 Yes. Paolo
On 21 May 2013 11:57, Paolo Bonzini <pbonzini@redhat.com> wrote: > Using phys_page_find to translate an AddressSpace to a MemoryRegionSection > is unwieldy. It requires to pass the page index rather than the address, > and later memory_region_section_addr has to be called. Replace > memory_region_section_addr with a function that does all of it: call > phys_page_find, compute the offset within the region, and check how > big the current mapping is. This way, a large flat region can be written > with a single lookup rather than a page at a time. > > address_space_translate will also provide a single point where IOMMU > forwarding is implemented. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> 4 overlength lines in this patch. Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> -- PMM
On 2013-05-21 12:57, Paolo Bonzini wrote: > Using phys_page_find to translate an AddressSpace to a MemoryRegionSection > is unwieldy. It requires to pass the page index rather than the address, > and later memory_region_section_addr has to be called. Replace > memory_region_section_addr with a function that does all of it: call > phys_page_find, compute the offset within the region, and check how > big the current mapping is. This way, a large flat region can be written > with a single lookup rather than a page at a time. > > address_space_translate will also provide a single point where IOMMU > forwarding is implemented. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > cputlb.c | 20 ++--- > exec.c | 201 +++++++++++++++++++++++++++----------------------- > include/exec/cputlb.h | 12 ++- > include/exec/memory.h | 31 ++++---- > translate-all.c | 6 +- > 5 files changed, 143 insertions(+), 127 deletions(-) > > diff --git a/cputlb.c b/cputlb.c > index aba7e44..1f85da0 100644 > --- a/cputlb.c > +++ b/cputlb.c > @@ -248,13 +248,18 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr, > target_ulong code_address; > uintptr_t addend; > CPUTLBEntry *te; > - hwaddr iotlb; > + hwaddr iotlb, xlat, sz; > > assert(size >= TARGET_PAGE_SIZE); > if (size != TARGET_PAGE_SIZE) { > tlb_add_large_page(env, vaddr, size); > } > - section = phys_page_find(address_space_memory.dispatch, paddr >> TARGET_PAGE_BITS); > + > + sz = size; > + section = address_space_translate(&address_space_memory, paddr, &xlat, &sz, > + false); > + assert(sz >= TARGET_PAGE_SIZE); > + > #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", > @@ -269,15 +274,14 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr, > } > if (memory_region_is_ram(section->mr) || > memory_region_is_romd(section->mr)) { > - addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) > - + memory_region_section_addr(section, paddr); > + addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) + xlat; > } else { > addend = 0; > } > > code_address = address; > - iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, prot, > - &address); > + iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, xlat, > + prot, &address); > > index = (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); > env->iotlb[mmu_idx][index] = iotlb - vaddr; > @@ -300,9 +304,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr, > /* Write access calls the I/O callback. */ > te->addr_write = address | TLB_MMIO; > } else if (memory_region_is_ram(section->mr) > - && !cpu_physical_memory_is_dirty( > - section->mr->ram_addr > - + memory_region_section_addr(section, paddr))) { > + && !cpu_physical_memory_is_dirty(section->mr->ram_addr + xlat)) { > te->addr_write = address | TLB_NOTDIRTY; > } else { > te->addr_write = address; > diff --git a/exec.c b/exec.c > index 82da067..e5ee8ff 100644 > --- a/exec.c > +++ b/exec.c > @@ -182,7 +182,7 @@ static void phys_page_set(AddressSpaceDispatch *d, > phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS - 1); > } > > -MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index) > +static MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index) > { > PhysPageEntry lp = d->phys_map; > PhysPageEntry *p; > @@ -198,6 +198,22 @@ MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index) > return &phys_sections[lp.ptr]; > } > > +MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr, > + hwaddr *xlat, hwaddr *plen, > + bool is_write) > +{ > + MemoryRegionSection *section; > + > + section = phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS); > + /* Compute offset within MemoryRegionSection */ > + addr -= section->offset_within_address_space; > + *plen = MIN(section->size - addr, *plen); This limitation causes problems. Consider two overlapping memory regions A and B. A handles 4-byte accesses and is at least 4 bytes long, B only deals with a single byte. They overlap like this: B (prio 1): X A (prio 0): XXXX... ^access here with 4 bytes length Now if an access happens at the marked position, it is split into one 2-byte access to A, followed by a one-byte access to B and another one-byte access to A. But the right access emulation would be 4-bytes to A, no? I think we need to check against the length of the target mr here instead, but I'm not totally sure yet. A concrete scenario where this breaks popped up while reworking PIO dispatching. PCI accesses to 0xcf8 no longer worked: 0000000000000cf8-0000000000000cfb (prio 0, RW): pci-conf-idx 0000000000000cf9-0000000000000cf9 (prio 1, RW): piix3-reset-control Jan
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Il 25/05/2013 08:40, Jan Kiszka ha scritto: > On 2013-05-21 12:57, Paolo Bonzini wrote: >> Using phys_page_find to translate an AddressSpace to a >> MemoryRegionSection is unwieldy. It requires to pass the page >> index rather than the address, and later >> memory_region_section_addr has to be called. Replace >> memory_region_section_addr with a function that does all of it: >> call phys_page_find, compute the offset within the region, and >> check how big the current mapping is. This way, a large flat >> region can be written with a single lookup rather than a page at >> a time. >> >> address_space_translate will also provide a single point where >> IOMMU forwarding is implemented. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- cputlb.c >> | 20 ++--- exec.c | 201 >> +++++++++++++++++++++++++++----------------------- >> include/exec/cputlb.h | 12 ++- include/exec/memory.h | 31 >> ++++---- translate-all.c | 6 +- 5 files changed, 143 >> insertions(+), 127 deletions(-) >> >> diff --git a/cputlb.c b/cputlb.c index aba7e44..1f85da0 100644 >> --- a/cputlb.c +++ b/cputlb.c @@ -248,13 +248,18 @@ void >> tlb_set_page(CPUArchState *env, target_ulong vaddr, target_ulong >> code_address; uintptr_t addend; CPUTLBEntry *te; - hwaddr >> iotlb; + hwaddr iotlb, xlat, sz; >> >> assert(size >= TARGET_PAGE_SIZE); if (size != TARGET_PAGE_SIZE) >> { tlb_add_large_page(env, vaddr, size); } - section = >> phys_page_find(address_space_memory.dispatch, paddr >> >> TARGET_PAGE_BITS); + + sz = size; + section = >> address_space_translate(&address_space_memory, paddr, &xlat, >> &sz, + false); + >> assert(sz >= TARGET_PAGE_SIZE); + #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", @@ -269,15 +274,14 >> @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr, } if >> (memory_region_is_ram(section->mr) || >> memory_region_is_romd(section->mr)) { - addend = >> (uintptr_t)memory_region_get_ram_ptr(section->mr) - + >> memory_region_section_addr(section, paddr); + addend = >> (uintptr_t)memory_region_get_ram_ptr(section->mr) + xlat; } else >> { addend = 0; } >> >> code_address = address; - iotlb = >> memory_region_section_get_iotlb(env, section, vaddr, paddr, >> prot, - &address); + >> iotlb = memory_region_section_get_iotlb(env, section, vaddr, >> paddr, xlat, + prot, >> &address); >> >> index = (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); >> env->iotlb[mmu_idx][index] = iotlb - vaddr; @@ -300,9 +304,7 @@ >> void tlb_set_page(CPUArchState *env, target_ulong vaddr, /* Write >> access calls the I/O callback. */ te->addr_write = address | >> TLB_MMIO; } else if (memory_region_is_ram(section->mr) - >> && !cpu_physical_memory_is_dirty( - >> section->mr->ram_addr - + >> memory_region_section_addr(section, paddr))) { + >> && !cpu_physical_memory_is_dirty(section->mr->ram_addr + xlat)) >> { te->addr_write = address | TLB_NOTDIRTY; } else { >> te->addr_write = address; diff --git a/exec.c b/exec.c index >> 82da067..e5ee8ff 100644 --- a/exec.c +++ b/exec.c @@ -182,7 >> +182,7 @@ static void phys_page_set(AddressSpaceDispatch *d, >> phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS >> - 1); } >> >> -MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, >> hwaddr index) +static MemoryRegionSection >> *phys_page_find(AddressSpaceDispatch *d, hwaddr index) { >> PhysPageEntry lp = d->phys_map; PhysPageEntry *p; @@ -198,6 >> +198,22 @@ MemoryRegionSection >> *phys_page_find(AddressSpaceDispatch *d, hwaddr index) return >> &phys_sections[lp.ptr]; } >> >> +MemoryRegionSection *address_space_translate(AddressSpace *as, >> hwaddr addr, + hwaddr >> *xlat, hwaddr *plen, + >> bool is_write) +{ + MemoryRegionSection *section; + + >> section = phys_page_find(as->dispatch, addr >> >> TARGET_PAGE_BITS); + /* Compute offset within >> MemoryRegionSection */ + addr -= >> section->offset_within_address_space; + *plen = >> MIN(section->size - addr, *plen); > > This limitation causes problems. Consider two overlapping memory > regions A and B. A handles 4-byte accesses and is at least 4 bytes > long, B only deals with a single byte. They overlap like this: > > B (prio 1): X A (prio 0): XXXX... ^access here with 4 bytes > length > > Now if an access happens at the marked position, it is split into > one 2-byte access to A, followed by a one-byte access to B and > another one-byte access to A. But the right access emulation would > be 4-bytes to A, no? I guess it depends on the width of the bus. On a 16-bit computer the right thing to do would be to split the write in two, one two-byte access to A and one two-byte access to B. But as a first approximation the fix you suggest is the right one. Implementing bus width in TCG is tricky, so we should get by with the simple fix. This patch is not in the pull request I sent, so there is time to make it work. Is it simply - - *plen = MIN(section->size - addr, *plen); + addr += section->offset_within_memory_region; + *plen = MIN(section->mr->size - addr, *plen); or something like that? If you post it as a diff I can squash it in. > I think we need to check against the length of the target mr here > instead, but I'm not totally sure yet. > > A concrete scenario where this breaks popped up while reworking > PIO dispatching. PCI accesses to 0xcf8 no longer worked: > > 0000000000000cf8-0000000000000cfb (prio 0, RW): pci-conf-idx > 0000000000000cf9-0000000000000cf9 (prio 1, RW): > piix3-reset-control Paolo -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRoGybAAoJEBvWZb6bTYbyELUP/09bPNSKl39Tog0uyV+VJ3BR IGDsWfKHnIm6T2hC0S4BhVRaBkDyDAC6c8hJ7iy9084/pw61Cra1RrNMdSOzNHqo x4pC+Zw6hD9lK0ncRsB21w1n398yen7nk+RN4zf2LxxAnIiXrUuaG6SL5q3qSibz lXj1fKP+hG2yoxS7frMAJVjBEFIhBj/EY5u3pmd6+qREGxYaLBKkuXSnbjHdfG3s Q8OLJZ8bRH7o3c034yt+LvJ776PfhjzaggUmrYri7fZ2kU+SmUzqnkW9If2ekQsf MQ3ZQRVrVc5AaHLdQLyjSaSpzPcON5w4VpqU9SCisKi9rFdDOW6qHuZc6ZTbUpch wAbGeI0h1PA01o41milx0LU+Y7Qfl6wRzbB3CZEpgp8fFSR95LySwD1Zldt1Ft6E Zv8Mh0NVV4qvVEtp+HrTTh91kVz5W9OXnqFOz+wYrBlaVBRUxBLWSzQvuScnYv2B Rv51bcsq07bOZFR2BabC9zuRnHlCC9LqQAvP5i8AdvC1FsxIGPtK+FLc3QDeD+c7 Ndz5mZq6jgAeg0BQrBv7H++P6I2dw/lJkrb4zwGY68ckJ+Yo67NGbYvhM0/W1WF5 XVebDnb1F3+qWsVtSJWWoOHtW0HRFS+3il87/u4MKJc1BF+ceONrSJLBQyvSJDNf 9sPb/mhfVloOj7tWqWAk =Y4yR -----END PGP SIGNATURE-----
diff --git a/cputlb.c b/cputlb.c index aba7e44..1f85da0 100644 --- a/cputlb.c +++ b/cputlb.c @@ -248,13 +248,18 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr, target_ulong code_address; uintptr_t addend; CPUTLBEntry *te; - hwaddr iotlb; + hwaddr iotlb, xlat, sz; assert(size >= TARGET_PAGE_SIZE); if (size != TARGET_PAGE_SIZE) { tlb_add_large_page(env, vaddr, size); } - section = phys_page_find(address_space_memory.dispatch, paddr >> TARGET_PAGE_BITS); + + sz = size; + section = address_space_translate(&address_space_memory, paddr, &xlat, &sz, + false); + assert(sz >= TARGET_PAGE_SIZE); + #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", @@ -269,15 +274,14 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr, } if (memory_region_is_ram(section->mr) || memory_region_is_romd(section->mr)) { - addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) - + memory_region_section_addr(section, paddr); + addend = (uintptr_t)memory_region_get_ram_ptr(section->mr) + xlat; } else { addend = 0; } code_address = address; - iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, prot, - &address); + iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, xlat, + prot, &address); index = (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); env->iotlb[mmu_idx][index] = iotlb - vaddr; @@ -300,9 +304,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr, /* Write access calls the I/O callback. */ te->addr_write = address | TLB_MMIO; } else if (memory_region_is_ram(section->mr) - && !cpu_physical_memory_is_dirty( - section->mr->ram_addr - + memory_region_section_addr(section, paddr))) { + && !cpu_physical_memory_is_dirty(section->mr->ram_addr + xlat)) { te->addr_write = address | TLB_NOTDIRTY; } else { te->addr_write = address; diff --git a/exec.c b/exec.c index 82da067..e5ee8ff 100644 --- a/exec.c +++ b/exec.c @@ -182,7 +182,7 @@ static void phys_page_set(AddressSpaceDispatch *d, phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS - 1); } -MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index) +static MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index) { PhysPageEntry lp = d->phys_map; PhysPageEntry *p; @@ -198,6 +198,22 @@ MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr index) return &phys_sections[lp.ptr]; } +MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr, + hwaddr *xlat, hwaddr *plen, + bool is_write) +{ + MemoryRegionSection *section; + + section = phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS); + /* Compute offset within MemoryRegionSection */ + addr -= section->offset_within_address_space; + *plen = MIN(section->size - addr, *plen); + + /* Compute offset within MemoryRegion */ + *xlat = addr + section->offset_within_region; + return section; +} + bool memory_region_is_unassigned(MemoryRegion *mr) { return mr != &io_mem_ram && mr != &io_mem_rom @@ -616,11 +632,11 @@ static int cpu_physical_memory_set_dirty_tracking(int enable) } hwaddr memory_region_section_get_iotlb(CPUArchState *env, - MemoryRegionSection *section, - target_ulong vaddr, - hwaddr paddr, - int prot, - target_ulong *address) + MemoryRegionSection *section, + target_ulong vaddr, + hwaddr paddr, hwaddr xlat, + int prot, + target_ulong *address) { hwaddr iotlb; CPUWatchpoint *wp; @@ -628,7 +644,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env, if (memory_region_is_ram(section->mr)) { /* Normal RAM. */ iotlb = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) - + memory_region_section_addr(section, paddr); + + xlat; if (!section->readonly) { iotlb |= phys_section_notdirty; } else { @@ -636,7 +652,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env, } } else { iotlb = section - phys_sections; - iotlb += memory_region_section_addr(section, paddr); + iotlb += xlat; } /* Make accesses to pages with watchpoints go via the @@ -1888,24 +1904,18 @@ static void invalidate_and_set_dirty(hwaddr addr, void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, int len, bool is_write) { - AddressSpaceDispatch *d = as->dispatch; - int l; + hwaddr l; uint8_t *ptr; uint32_t val; - hwaddr page; + hwaddr addr1; MemoryRegionSection *section; while (len > 0) { - page = addr & TARGET_PAGE_MASK; - l = (page + TARGET_PAGE_SIZE) - addr; - if (l > len) - l = len; - section = phys_page_find(d, page >> TARGET_PAGE_BITS); + l = len; + section = address_space_translate(as, addr, &addr1, &l, is_write); if (is_write) { if (!memory_region_is_ram(section->mr)) { - hwaddr addr1; - addr1 = memory_region_section_addr(section, addr); /* XXX: could force cpu_single_env to NULL to avoid potential bugs */ if (l >= 4 && ((addr1 & 3) == 0)) { @@ -1925,9 +1935,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, l = 1; } } else if (!section->readonly) { - ram_addr_t addr1; - addr1 = memory_region_get_ram_addr(section->mr) - + memory_region_section_addr(section, addr); + addr1 += memory_region_get_ram_addr(section->mr); /* RAM case */ ptr = qemu_get_ram_ptr(addr1); memcpy(ptr, buf, l); @@ -1936,9 +1944,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, } else { if (!(memory_region_is_ram(section->mr) || memory_region_is_romd(section->mr))) { - hwaddr addr1; /* I/O case */ - addr1 = memory_region_section_addr(section, addr); if (l >= 4 && ((addr1 & 3) == 0)) { /* 32 bit read access */ val = io_mem_read(section->mr, addr1, 4); @@ -1957,9 +1963,7 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, } } else { /* RAM case */ - ptr = qemu_get_ram_ptr(section->mr->ram_addr - + memory_region_section_addr(section, - addr)); + ptr = qemu_get_ram_ptr(section->mr->ram_addr + addr1); memcpy(buf, ptr, l); } } @@ -1998,26 +2002,21 @@ void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf, void cpu_physical_memory_write_rom(hwaddr addr, const uint8_t *buf, int len) { - AddressSpaceDispatch *d = address_space_memory.dispatch; - int l; + hwaddr l; uint8_t *ptr; - hwaddr page; + hwaddr addr1; MemoryRegionSection *section; while (len > 0) { - page = addr & TARGET_PAGE_MASK; - l = (page + TARGET_PAGE_SIZE) - addr; - if (l > len) - l = len; - section = phys_page_find(d, page >> TARGET_PAGE_BITS); + l = len; + section = address_space_translate(&address_space_memory, + addr, &addr1, &l, true); if (!(memory_region_is_ram(section->mr) || memory_region_is_romd(section->mr))) { /* do nothing */ } else { - unsigned long addr1; - addr1 = memory_region_get_ram_addr(section->mr) - + memory_region_section_addr(section, addr); + addr1 += memory_region_get_ram_addr(section->mr); /* ROM/RAM case */ ptr = qemu_get_ram_ptr(addr1); memcpy(ptr, buf, l); @@ -2077,18 +2076,12 @@ static void cpu_notify_map_clients(void) bool address_space_valid(AddressSpace *as, hwaddr addr, int len, bool is_write) { - AddressSpaceDispatch *d = as->dispatch; MemoryRegionSection *section; - int l; - hwaddr page; + hwaddr l, xlat; while (len > 0) { - page = addr & TARGET_PAGE_MASK; - l = (page + TARGET_PAGE_SIZE) - addr; - if (l > len) { - l = len; - } - section = phys_page_find(d, addr >> TARGET_PAGE_BITS); + l = len; + section = address_space_translate(as, addr, &xlat, &l, is_write); if (section->mr == &io_mem_unassigned || (is_write && section->mr->readonly)) { return false; @@ -2112,22 +2105,17 @@ void *address_space_map(AddressSpace *as, hwaddr *plen, bool is_write) { - AddressSpaceDispatch *d = as->dispatch; hwaddr len = *plen; hwaddr todo = 0; - int l; - hwaddr page; + hwaddr l, xlat; MemoryRegionSection *section; ram_addr_t raddr = RAM_ADDR_MAX; ram_addr_t rlen; void *ret; while (len > 0) { - page = addr & TARGET_PAGE_MASK; - l = (page + TARGET_PAGE_SIZE) - addr; - if (l > len) - l = len; - section = phys_page_find(d, page >> TARGET_PAGE_BITS); + l = len; + section = address_space_translate(as, addr, &xlat, &l, is_write); if (!(memory_region_is_ram(section->mr) && !section->readonly)) { if (todo || bounce.buffer) { @@ -2144,8 +2132,11 @@ void *address_space_map(AddressSpace *as, return bounce.buffer; } if (!todo) { - raddr = memory_region_get_ram_addr(section->mr) - + memory_region_section_addr(section, addr); + raddr = memory_region_get_ram_addr(section->mr) + xlat; + } else { + if (memory_region_get_ram_addr(section->mr) + xlat != raddr + todo) { + break; + } } len -= l; @@ -2211,14 +2202,19 @@ static inline uint32_t ldl_phys_internal(hwaddr addr, uint8_t *ptr; uint32_t val; MemoryRegionSection *section; + hwaddr l = 4; + hwaddr addr1; - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); + section = address_space_translate(&address_space_memory, addr, &addr1, &l, + false); + if (l < 4) { + return unassigned_mem_read(NULL, addr, 4); + } if (!(memory_region_is_ram(section->mr) || memory_region_is_romd(section->mr))) { /* I/O case */ - addr = memory_region_section_addr(section, addr); - val = io_mem_read(section->mr, addr, 4); + val = io_mem_read(section->mr, addr1, 4); #if defined(TARGET_WORDS_BIGENDIAN) if (endian == DEVICE_LITTLE_ENDIAN) { val = bswap32(val); @@ -2232,7 +2228,7 @@ static inline uint32_t ldl_phys_internal(hwaddr addr, /* RAM case */ ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) - + memory_region_section_addr(section, addr)); + + addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: val = ldl_le_p(ptr); @@ -2270,28 +2266,33 @@ static inline uint64_t ldq_phys_internal(hwaddr addr, uint8_t *ptr; uint64_t val; MemoryRegionSection *section; + hwaddr l = 8; + hwaddr addr1; - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); + section = address_space_translate(&address_space_memory, addr, &addr1, &l, + false); + if (l < 8) { + return unassigned_mem_read(NULL, addr, 8); + } if (!(memory_region_is_ram(section->mr) || memory_region_is_romd(section->mr))) { /* I/O case */ - addr = memory_region_section_addr(section, addr); /* XXX This is broken when device endian != cpu endian. Fix and add "endian" variable check */ #ifdef TARGET_WORDS_BIGENDIAN - val = io_mem_read(section->mr, addr, 4) << 32; - val |= io_mem_read(section->mr, addr + 4, 4); + val = io_mem_read(section->mr, addr1, 4) << 32; + val |= io_mem_read(section->mr, addr1 + 4, 4); #else - val = io_mem_read(section->mr, addr, 4); - val |= io_mem_read(section->mr, addr + 4, 4) << 32; + val = io_mem_read(section->mr, addr1, 4); + val |= io_mem_read(section->mr, addr1 + 4, 4) << 32; #endif } else { /* RAM case */ ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) - + memory_region_section_addr(section, addr)); + + addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: val = ldq_le_p(ptr); @@ -2337,14 +2338,19 @@ static inline uint32_t lduw_phys_internal(hwaddr addr, uint8_t *ptr; uint64_t val; MemoryRegionSection *section; + hwaddr l = 2; + hwaddr addr1; - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); + section = address_space_translate(&address_space_memory, addr, &addr1, &l, + false); + if (l < 2) { + return unassigned_mem_read(NULL, addr, 2); + } if (!(memory_region_is_ram(section->mr) || memory_region_is_romd(section->mr))) { /* I/O case */ - addr = memory_region_section_addr(section, addr); - val = io_mem_read(section->mr, addr, 2); + val = io_mem_read(section->mr, addr1, 2); #if defined(TARGET_WORDS_BIGENDIAN) if (endian == DEVICE_LITTLE_ENDIAN) { val = bswap16(val); @@ -2358,7 +2364,7 @@ static inline uint32_t lduw_phys_internal(hwaddr addr, /* RAM case */ ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) - + memory_region_section_addr(section, addr)); + + addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: val = lduw_le_p(ptr); @@ -2396,19 +2402,23 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val) { uint8_t *ptr; MemoryRegionSection *section; + hwaddr l = 4; + hwaddr addr1; - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); + section = address_space_translate(&address_space_memory, addr, &addr1, &l, + true); + if (l < 4) { + unassigned_mem_write(NULL, addr, val, 4); + return; + } if (!memory_region_is_ram(section->mr) || section->readonly) { - addr = memory_region_section_addr(section, addr); if (memory_region_is_ram(section->mr)) { section = &phys_sections[phys_section_rom]; } - io_mem_write(section->mr, addr, val, 4); + io_mem_write(section->mr, addr1, val, 4); } else { - unsigned long addr1 = (memory_region_get_ram_addr(section->mr) - & TARGET_PAGE_MASK) - + memory_region_section_addr(section, addr); + addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK; ptr = qemu_get_ram_ptr(addr1); stl_p(ptr, val); @@ -2430,11 +2440,17 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val, { uint8_t *ptr; MemoryRegionSection *section; + hwaddr l = 4; + hwaddr addr1; - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); + section = address_space_translate(&address_space_memory, addr, &addr1, &l, + true); + if (l < 4) { + unassigned_mem_write(NULL, addr, val, 4); + return; + } if (!memory_region_is_ram(section->mr) || section->readonly) { - addr = memory_region_section_addr(section, addr); if (memory_region_is_ram(section->mr)) { section = &phys_sections[phys_section_rom]; } @@ -2447,12 +2463,10 @@ static inline void stl_phys_internal(hwaddr addr, uint32_t val, val = bswap32(val); } #endif - io_mem_write(section->mr, addr, val, 4); + io_mem_write(section->mr, addr1, val, 4); } else { - unsigned long addr1; - addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) - + memory_region_section_addr(section, addr); /* RAM case */ + addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK; ptr = qemu_get_ram_ptr(addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: @@ -2497,11 +2511,17 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val, { uint8_t *ptr; MemoryRegionSection *section; + hwaddr l = 2; + hwaddr addr1; - section = phys_page_find(address_space_memory.dispatch, addr >> TARGET_PAGE_BITS); + section = address_space_translate(&address_space_memory, addr, &addr1, &l, + true); + if (l < 2) { + unassigned_mem_write(NULL, addr, val, 2); + return; + } if (!memory_region_is_ram(section->mr) || section->readonly) { - addr = memory_region_section_addr(section, addr); if (memory_region_is_ram(section->mr)) { section = &phys_sections[phys_section_rom]; } @@ -2514,12 +2534,10 @@ static inline void stw_phys_internal(hwaddr addr, uint32_t val, val = bswap16(val); } #endif - io_mem_write(section->mr, addr, val, 2); + io_mem_write(section->mr, addr1, val, 2); } else { - unsigned long addr1; - addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) - + memory_region_section_addr(section, addr); /* RAM case */ + addr1 += memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK; ptr = qemu_get_ram_ptr(addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: @@ -2622,9 +2640,10 @@ bool virtio_is_big_endian(void) bool cpu_physical_memory_is_io(hwaddr phys_addr) { MemoryRegionSection *section; + hwaddr l = 1; - section = phys_page_find(address_space_memory.dispatch, - phys_addr >> TARGET_PAGE_BITS); + section = address_space_translate(&address_space_memory, + phys_addr, &phys_addr, &l, false); return !(memory_region_is_ram(section->mr) || memory_region_is_romd(section->mr)); diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h index 733c885..e821660 100644 --- a/include/exec/cputlb.h +++ b/include/exec/cputlb.h @@ -26,8 +26,6 @@ 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(struct AddressSpaceDispatch *d, - hwaddr 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; @@ -35,11 +33,11 @@ extern int tlb_flush_count; /* exec.c */ void tb_flush_jmp_cache(CPUArchState *env, target_ulong addr); hwaddr memory_region_section_get_iotlb(CPUArchState *env, - MemoryRegionSection *section, - target_ulong vaddr, - hwaddr paddr, - int prot, - target_ulong *address); + MemoryRegionSection *section, + target_ulong vaddr, + hwaddr paddr, hwaddr xlat, + int prot, + target_ulong *address); bool memory_region_is_unassigned(MemoryRegion *mr); #endif diff --git a/include/exec/memory.h b/include/exec/memory.h index 2e5fd11..809a958 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -743,23 +743,6 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr, hwaddr addr, uint64_t size); /** - * memory_region_section_addr: get offset within MemoryRegionSection - * - * Returns offset within MemoryRegionSection - * - * @section: the memory region section being queried - * @addr: address in address space - */ -static inline hwaddr -memory_region_section_addr(MemoryRegionSection *section, - hwaddr addr) -{ - addr -= section->offset_within_address_space; - addr += section->offset_within_region; - return addr; -} - -/** * address_space_sync_dirty_bitmap: synchronize the dirty log for all memory * * Synchronizes the dirty page log for an entire address space. @@ -860,6 +843,20 @@ void address_space_write(AddressSpace *as, hwaddr addr, */ void address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len); +/* address_space_translate: translate an address range into an address space + * into a MemoryRegionSection and an address range into that section + * + * @as: #AddressSpace to be accessed + * @addr: address within that address space + * @xlat: pointer to address within the returned memory region section's + * #MemoryRegion. + * @len: pointer to length + * @is_write: indicates the transfer direction + */ +MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr, + hwaddr *xlat, hwaddr *len, + bool is_write); + /* address_space_valid: check for validity of an address space range * * Check whether memory is assigned to the given address space range. diff --git a/translate-all.c b/translate-all.c index 0d84b0d..7a7d537 100644 --- a/translate-all.c +++ b/translate-all.c @@ -1355,15 +1355,15 @@ void tb_invalidate_phys_addr(hwaddr addr) { ram_addr_t ram_addr; MemoryRegionSection *section; + hwaddr l = 1; - section = phys_page_find(address_space_memory.dispatch, - addr >> TARGET_PAGE_BITS); + section = address_space_translate(&address_space_memory, addr, &addr, &l, false); if (!(memory_region_is_ram(section->mr) || memory_region_is_romd(section->mr))) { return; } ram_addr = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) - + memory_region_section_addr(section, addr); + + addr; tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0); } #endif /* TARGET_HAS_ICE && !defined(CONFIG_USER_ONLY) */
Using phys_page_find to translate an AddressSpace to a MemoryRegionSection is unwieldy. It requires to pass the page index rather than the address, and later memory_region_section_addr has to be called. Replace memory_region_section_addr with a function that does all of it: call phys_page_find, compute the offset within the region, and check how big the current mapping is. This way, a large flat region can be written with a single lookup rather than a page at a time. address_space_translate will also provide a single point where IOMMU forwarding is implemented. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- cputlb.c | 20 ++--- exec.c | 201 +++++++++++++++++++++++++++----------------------- include/exec/cputlb.h | 12 ++- include/exec/memory.h | 31 ++++---- translate-all.c | 6 +- 5 files changed, 143 insertions(+), 127 deletions(-)