Re: [Qemu-devel] [RFC][PATCH 10/15] memory: Rework sub-page handling

[Paolo Bonzini]

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.

valgrind is not happy with this patch either:

static int subsection_register(PhysSection *psection, uint32_t start,
                               uint32_t end, uint16_t section)
{
    int idx, eidx;

    if (start >= TARGET_PAGE_SIZE || end >= TARGET_PAGE_SIZE)
        return -1;
    idx = SUBSECTION_IDX(start);
    eidx = SUBSECTION_IDX(end);
    if (memory_region_is_ram(phys_sections[section].section.mr)) {
        MemoryRegionSection new_section = phys_sections[section].section;
        new_section.mr = &io_mem_subpage_ram;
        section = phys_section_add(&new_section);
    }
    for (; idx <= eidx; idx++) {
        psection->sub_section[idx] = section;
    }

    return 0;
}

The phys_section_add might invalidate psection.  If we can drop subpage
RAM, that would fix it.  But similarly here:

    subsection_register(psection, start, end, phys_section_add(section));

The phys_section_add might invalidate psection and the fix is a bit
more involved.

Paolo

> 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...
> 
> thanks
> -- PMM
> 
>