Re: [Qemu-devel] MMIO address changes

[takasi-y]

Hi,

> I've just committed a patch that changes the MMIO callback interface for 
> devices.  Instead of being passed an absolute address these are now passed an 
> offset from the start[1] of the memory region that was registered.
Good! It is just what I wanted.

> I've tried to be fairly thorough with the changes, and tested what I can. 
> However it's possible I missed or broke something, so please test your 
> favourite targets.
Mmmmm, sh/r2d is broken. I'll post a fix later.
But, before that, could you please tell me if this modification below
 is good and acceptable ?

diff --git a/exec.c b/exec.c
--- a/exec.c
+++ b/exec.c
@@ -2295,6 +2295,7 @@ void cpu_register_physical_memory_offset(target_phys_addr_
t start_addr,
                 p->region_offset = 0;
             } else {
                 p->phys_offset = phys_offset;
+                p->region_offset = region_offset;
                 if ((phys_offset & ~TARGET_PAGE_MASK) <= IO_MEM_ROM ||
                     (phys_offset & IO_MEM_ROMD))
                     phys_offset += TARGET_PAGE_SIZE;

You can see the difference when you register a region overriding
 other(perhaps bigger) region.

hw/sh7750.c hits this by registering
 0xfc000000 .. 0xffffffff for CPU control registers,
then
 0xffe00000 .. 0xffe00028
 0xffe80000 .. 0xffe80028
 0xffd00000 .. 0xffd01000
 0xffd80000 .. 0xffd81000
 ...
for peripheral modules.

I can divide the first region to some pieces to avoid overlapping.
That is one solution, and is not bad to do (even without this issue), though.
Ah, in addition,
I'd like to know if registering overlapping region is valid or no.

> [1] It's actually the offset from the start of the first page of that region. 
> In practice this difference doesn't matter, and makes the implementation a 
> lot simpler.
Simpler is better.
But, I'd like to have a guide to write a code when it does matter.

Now, at least hw/sm501.c has been already broken by this case.
It registers palette register array at
 base + MMIO_BASE_OFFSET + SM501_DC + SM501_DC_PANEL_PALETTE,
 = 0x10000000 + 0x3e00000 + 0x080000 + 0x400
so, palette index offsets 0x400.

Of cource I can fix it by
 addr -= (MMIO_BASE_OFFSET + SM501_DC + 
SM501_DC_PANEL_PALETTE)&~TARGET_PAGE_MASK;
before accessing the register. Looks not so smart...
Are there any idea?

Cheers,
/yoshii