On 2013-05-25 09:47, Paolo Bonzini wrote:
> 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 <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 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.
This works for me now:
diff --git a/exec.c b/exec.c
index 1610604..3d36bc7 100644
--- a/exec.c
+++ b/exec.c
@@ -210,13 +210,15 @@ MemoryRegionSection *address_space_translate(AddressSpace
*as, hwaddr addr,
IOMMUTLBEntry iotlb;
MemoryRegionSection *section;
hwaddr len = *plen;
+ Int128 diff;
for (;;) {
section = address_space_lookup_region(as, addr);
/* Compute offset within MemoryRegionSection */
addr -= section->offset_within_address_space;
- len = MIN(section->size - addr, len);
+ diff = int128_sub(section->mr->size, int128_make64(addr));
+ len = MIN(int128_get64(diff), len);
/* Compute offset within MemoryRegion */
addr += section->offset_within_region;
Jan
signature.asc
Description: OpenPGP digital signature
