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:48 PM, Jan Kiszka wrote:
> >
> >  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).
>
> So far we matched accesses in find_portio by considering the portio
> offset as well. If we want to replace the region offset with the portio
> one (which confines legacy to a legacy-only place), we need to make the
> portio offset a pure correction value on handler invocation and exclude
> it from any range matching. And that means an old_portio memory region
> can only describe one range starting exactly at MemoryRegion::addr.

Thanks for the explanation.  But I think you're trying to remove 
->offset by moving it somewhere else.  That's not removal, that's 
renaming, and it reduces functionality for other old_portio users.

If users need absolute addresses, then we should provide them via 
set_offset(), not pretend the need doesn't exist.

(btw, another way to emulate set_offset() is via aliases, as detailed in 
the other thread).

>
> >
> >>
> >>  >   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.
>
> We generally used to convert from APIv<n-1>  to APIv<n>  by adding legacy
> wrappers, rarely removing any of them. This doesn't scale, but - granted
> - it requires some masochism to make progress.
>

Wrappers reduce the risk of regression from a bad conversion by a tired 
coder.  But yes, they increase the amount of cruft immensely.

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