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

[Avi Kivity]

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