[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] isa: Avoid using obsolete memory_region_set_off

From: Jan Kiszka
Subject: Re: [Qemu-devel] [PATCH] isa: Avoid using obsolete memory_region_set_offset for old portio
Date: Mon, 19 Sep 2011 14:48:45 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv: Gecko/20080226 SUSE/ Thunderbird/ Mnenhy/

On 2011-09-19 14:37, Avi Kivity wrote:
> 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).

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.

>> >  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.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]