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 <pbonz...@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
signature.asc
Description: OpenPGP digital signature
