Re: [Qemu-devel] sub-page-sized mmio regions and address passed to read/write fns

[Avi Kivity <avi@redhat.com>]

On 12/02/2011 04:49 PM, Peter Maydell wrote:
> Hi; I was working on a refactoring of the ARM 11MPCore/A9MP private
> peripherals and encountered something odd. Rather than having a single
> large mmio region, I tried splitting into several regions, like this:
>
>     memory_region_init(&s->container, "a9mp-priv-container", 0x2000);
>     memory_region_init_io(&s->scu_iomem, &a9_scu_ops, s, "a9mp-scu", 0x100);
>     memory_region_init_io(&s->gic_cpu_iomem, &a9_gic_cpu_ops, s,
>                           "a9mp-gic-cpu", 0x100);
>     memory_region_init_io(&s->ptimer_iomem, &a9_ptimer_ops, s,
>                           "a9mp-ptimer", 0x100);
>     memory_region_add_subregion(&s->container, 0, &s->scu_iomem);
>     memory_region_add_subregion(&s->container, 0x100, &s->gic_cpu_iomem);
>     memory_region_add_subregion(&s->container, 0x600, &s->ptimer_iomem);
>     memory_region_add_subregion(&s->container, 0x1000, &s->gic.iomem);
>     sysbus_init_mmio_region(dev, &s->container);

Good practice IMO, will become more important when we introduce a
Register class.

> However what I found is that the addresses passed to the read/write
> functions aren't what I would expect. For instance if the board
> maps the container at address 0x1e000000, then a read from 0x1e000100
> goes to the functions given by a9_gic_cpu_ops, as it should. However,
> the offset parameter that the read function is passed is not 0x0
> (offset from the start of the a9mp-gic-cpu region) but 0x100 (offset
> from the start of the page, I think).
>
> Is this expected behaviour? I certainly wasn't expecting it...

A while ago this was the behaviour across the board.  Then 8da3ff1809747
changed addresses to be relative, but apparently missed the subpage case.

> I looked through the code that's getting called for reads, and
> it looks to me like exec.c:subpage_readlen() is causing this.
> We look up the subpage_t based on the address within the page,
> but we don't then adjust the address we pass to io_mem_read
> (except by region_offset, which I take from the comment at the
> top of cpu_register_physical_memory_log() to be for something
> else.)
>

I think you can use subpage_t's region_offset array for this (adding
into it, of course, so the original value remains).

-- 
error compiling committee.c: too many arguments to function