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

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1989 bytes --]

On 2011-09-18 17:57, Avi Kivity wrote:
> On 09/18/2011 03:54 PM, Jan Kiszka wrote:
>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>
>> We can express the offset of old portio completely via
>> MemoryRegionPortio::offset by splitting up regions of different offsets
>> and adjusting those offsets appropriately.
> 
> Please split into two patches - core and isa.

They depend on each other.

> 
>> +    /* Copy the sub-list and null-terminate it.  */
>> +    pio = g_new(MemoryRegionPortio, count + 1);
>> +    memcpy(pio, pio_init, sizeof(MemoryRegionPortio) * count);
>> +    memset(pio + count, 0, sizeof(MemoryRegionPortio));
> 
> Wish: g_copy(pio, pio_init, count);  // aka std::copy()
> 
>> @@ -396,12 +395,12 @@ static void memory_region_iorange_read(IORange
>> *iorange,
>>
>>           *data = ((uint64_t)1<<  (width * 8)) - 1;
>>           if (mrp) {
>> -            *data = mrp->read(mr->opaque, offset + mr->offset);
>> +            *data = mrp->read(mr->opaque, offset + mrp->offset);
>>           } else if (width == 2) {
>>               mrp = find_portio(mr, offset, 1, false);
>>               assert(mrp);
>> -            *data = mrp->read(mr->opaque, offset + mr->offset) |
>> -                    (mrp->read(mr->opaque, offset + mr->offset +
>> 1)<<  8);
>> +            *data = mrp->read(mr->opaque, offset + mrp->offset) |
>> +                    (mrp->read(mr->opaque, offset + mrp->offset +
>> 1)<<  8);
>>           }
>>           return;
>>       }
> 
> So long as mr->offset exists, you need to take it into account.

Only fair.

>  And I
> don't want to remove memory_region_set_offset() until everything (that
> can potentially use it, at least) has been converted.

IMO it's easier to fix those potential users before converting them. You
need to review them anyway to decide if an offset might be needed, and
which one precisely.

Are you aware of any candidates? For PIO, there should be none now.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]