Re: [Qemu-devel] [PATCH] isa: Avoid using obsolete memory_region_set_offset for old portio

[Avi Kivity <avi@redhat.com>]

On 09/19/2011 03:29 PM, Jan Kiszka wrote:
> On 2011-09-19 14:14, Avi Kivity wrote:
> >  On 09/18/2011 10:04 PM, Jan Kiszka wrote:
> >>  >
> >>  >   If you make the core patch add both mr->offset and mrp->offset, then
> >>  >   change isa to drop memory_region_set_offset(), instead adding the
> >>  delta
> >>  >   to mrp->offset, does that not work out?
> >>
> >>  Nope. The old API accepted arbitrary portio lists per memory region, the
> >>  new requires one region with a consistent offset per range. I should
> >>  have documented it...
> >
> >  What does "a consistent offset per range" mean?  You aren't actually
> >  changing the caller's ranges.
>
> I'm changing the way isa_register_portio_1 registers portios with the
> core: only one per offset. The new commit log says:
>
> "This implies that MemoryRegionPortio::offset is no longer used as
> offset within the memory region but just as a correction value for the
> offset passed to legacy handlers that expect absolute port addresses."


Ah:

-        /* If we see a hole, break the region.  */
+        /* If we see a new offset, break the region. */


But, sorry for being slow, I don't see why it requires a core update 
(other for adding mrp->offset).

>
> >  They all use the same handler, so you need to split e.g.
> >  sh7750_io_memory into six MemoryRegionsOps. Or use tricks with aliases -
> >  have one giant 4G region with one handler, and map six 4k aliases into
> >  the system address space.
>
> Looks more like 3 regions with one alias each. But we likely need to
> disentangle all that logic first. I would be surprised if there wasn't a
> more readable way to express it via the memory API.
>

Depends if you subscribe to the "blindly make it work exactly the same 
way" or "understand the details and rewrite it cleanly" brands of masochism.

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