Re: [Qemu-devel] [PATCH 08/20] memory: store MemoryRegionSection pointers in phys_map

[Peter Maydell]

On 7 March 2012 17:49, Peter Maydell <address@hidden> wrote:
> git bisect blames this commit (5312bd8b3) for causing a Linux kernel
> on spitz to produce a bunch of pxa2xx_i2c warnings that weren't
> being emitted before:

What seems to happen here is that we register a memory region
(this is for the second i2c device in hw/pxa2xx.c):

    memory_region_init_io(&s->iomem, &pxa2xx_i2c_ops, s,
                          "pxa2xx-i2x", s->region_size);

where region_size is 0x100. We then map it at 0x40f00100
(via sysbus_mmio_map). This used to result in our read and write
functions being called with offsets from the start of the page,
so in this case for the register at 0x90 into the device the
passed in addr would be 0x190. There is some hackery in pxa2xx_i2c_init
to work out what the offset is from the start of the region
when we map the device, we pass it in as a qdev 'offset'
property, and then read/write can fix things up to get the
actual register offset.

With this commit read and write functions are now passed the actual
offset from the start of the device region, ie 0x90. So the hackery
ends up doing fixing up it doesn't need to do, and generates negative
offsets which cause the diagnostic messages.

So it seems like the new behaviour is more like the right thing,
but was it an intentional change? Should we just drop the offset
hackery as a workaround for a now-fixed bug?

Are we running into the "mapping devices at non-page-offsets isn't
supported" issue here? <optimism>Is that now supported after this
patch series?</>

(I think the other devices I know of which include workarounds
for being passed relative-to-page-base addresses handle it by
masking out the high bits of the address, eg arm11mpcore.c,
so they weren't broken by this commit.)

-- PMM