Re: [Qemu-devel] [PATCH] Enable 8-byte wide MMIO for 16550 serial devices

[Philippe Mathieu-Daudé]

Hi Mike, Paolo.

>> diff --git a/hw/char/serial.c b/hw/char/serial.c
>> index 376bd2f240..c16c19a406 100644
>> --- a/hw/char/serial.c
>> +++ b/hw/char/serial.c
>> @@ -1005,7 +1005,7 @@ static void serial_mm_write(void *opaque, hwaddr addr,
>>                              uint64_t value, unsigned size)
>>  {
>>      SerialState *s = opaque;
>> -    value &= ~0u >> (32 - (size * 8));
>> +    value &= ~((uint64_t)0) >> (64 - (size * 8));
>>      serial_ioport_write(s, addr >> s->it_shift, value, 1);
>>  }
> 
> Couldn't this be simply "value &= 255"?  Otherwise looks good.

This is not incorrect, but I don't think this is the correct fix.
Isn't it what memory::access_with_adjusted_size() is supposed to do with
.impl.max_access_size = 1?

This looks like the same issue I tried to fix here:
http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02255.html

I'll try to find some time to respin the full series with tests.

>> @@ -1014,16 +1014,22 @@ static const MemoryRegionOps serial_mm_ops[3] = {
>>          .read = serial_mm_read,
>>          .write = serial_mm_write,
>>          .endianness = DEVICE_NATIVE_ENDIAN,
>> +        .valid.max_access_size = 8,
>> +        .impl.max_access_size = 8,
>>      },
>>      [DEVICE_LITTLE_ENDIAN] = {
>>          .read = serial_mm_read,
>>          .write = serial_mm_write,
>>          .endianness = DEVICE_LITTLE_ENDIAN,
>> +        .valid.max_access_size = 8,
>> +        .impl.max_access_size = 8,
>>      },
>>      [DEVICE_BIG_ENDIAN] = {
>>          .read = serial_mm_read,
>>          .write = serial_mm_write,
>>          .endianness = DEVICE_BIG_ENDIAN,
>> +        .valid.max_access_size = 8,
>> +        .impl.max_access_size = 8,