[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC][PATCH 10/15] memory: Rework sub-page handling
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [RFC][PATCH 10/15] memory: Rework sub-page handling |
Date: |
Tue, 07 May 2013 14:35:09 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 |
Il 06/05/2013 22:46, Peter Maydell ha scritto:
> On 6 May 2013 15:26, Jan Kiszka <address@hidden> wrote:
>> Simplify the sub-page handling by implementing it directly in the
>> dispatcher instead of using a redirection memory region. We extend the
>> phys_sections entries to optionally hold a pointer to the sub-section
>> table that used to reside in the subpage_t structure. IOW, we add one
>> optional dispatch level below the existing radix tree.
>>
>> address_space_lookup_region is extended to take this additional level
>> into account. This direct dispatching to that target memory region will
>> also be helpful when we want to add per-region locking control.
>
> This patch seems to break vexpress-a9. Test case if you want it:
> http://staging.people.linaro.org/~peter.maydell/vexpress-3.8.tar.gz
> (125MB) Edit the 'runme' script to fix up the paths to kernel/initrd/dtb
> and then run it; before this patch it boots, afterwards it doesn't
> even manage to start the kernel.
>
> My guess is you've broken subregion-sized mmio regions somehow
> (and/or regions which are larger than a page in size but start
> or finish at a non-page-aligned address), and probably in particular
> the arm_gic regions that a9mpcore maps...
I think we just found out what all the subpage stuff is for. :)
When I added address_space_translate(), the trickiest conversion to the
new API was in tlb_set_page. Hence I added a "you never know"-style assert:
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);
Now, remember that address_space_translate clamps sz on output to the
size of the section. And here's what happens:
(gdb) p sz
$4 = 256
(gdb) p *(section->mr)
$5 = {ops = 0x7ffff82d33c0, iommu_ops = 0x0, opaque = 0x7ffff91e6b50,
parent = 0x7ffff9069400, owner = 0x0, size = {lo = 256,
hi = 0}, addr = 0, destructor = 0x7ffff7e824d0
<memory_region_destructor_none>, ram_addr = 18446744073709551615,
terminates =
true, romd_mode = true, ram = false, readonly = false, enabled =
true, rom_device = false, warning_printed = false,
flush_coalesced_mmio = false, alias = 0x0, alias_offset = 0, priority
= 0, may_overlap = false, subregions = {tqh_first = 0x0,
tqh_last = 0x7ffff91e8ee8}, subregions_link = {tqe_next = 0x0,
tqe_prev = 0x7ffff91e64f8}, coalesced = {tqh_first = 0x0,
tqh_last = 0x7ffff91e8f08}, name = 0x7ffff906bb60 "a9-scu",
dirty_log_mask = 0 '\000', ioeventfd_nb = 0, ioeventfds = 0x0,
iommu_target_as = 0x0}
The TLB would only store this region instead of the whole page, and
subsequent access past the first 256 bytes would be emulated incorrectly.
For the record, I attach the patch I was using to fix Jan's.
Paolo
0001-subpage-fix.patch
Description: Text Data
- [Qemu-devel] [RFC][PATCH 00/15] Refactor portio dispatching, Jan Kiszka, 2013/05/06
- [Qemu-devel] [RFC][PATCH 03/15] wdt_ib700: replace register_ioport*, Jan Kiszka, 2013/05/06
- [Qemu-devel] [RFC][PATCH 14/15] ioport: Remove unused old dispatching services, Jan Kiszka, 2013/05/06
- [Qemu-devel] [RFC][PATCH 06/15] vt82c686: replace register_ioport*, Jan Kiszka, 2013/05/06
- [Qemu-devel] [RFC][PATCH 02/15] applesmc: replace register_ioport*, Jan Kiszka, 2013/05/06
- [Qemu-devel] [RFC][PATCH 10/15] memory: Rework sub-page handling, Jan Kiszka, 2013/05/06
[Qemu-devel] [RFC][PATCH 08/15] isa: implement isa_is_ioport_assigned via memory_region_find, Jan Kiszka, 2013/05/06
[Qemu-devel] [RFC][PATCH 11/15] memory: Allow unaligned address_space_rw, Jan Kiszka, 2013/05/06