|
From: | Michael Nawrocki |
Subject: | Re: [Qemu-devel] [PATCH] Enable 8-byte wide MMIO for 16550 serial devices |
Date: | Mon, 6 Nov 2017 11:08:51 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 11/02/2017 04:04 PM, Paolo Bonzini wrote:
----- Original Message -----From: "Philippe Mathieu-Daudé" <address@hidden> To: "Paolo Bonzini" <address@hidden>, "Mike Nawrocki" <address@hidden>, "Avi Kivity" <address@hidden>, "Peter Maydell" <address@hidden>, "Konrad Frederic" <address@hidden> Cc: address@hidden, address@hidden Sent: Thursday, November 2, 2017 8:28:09 PM Subject: Re: [Qemu-devel] [PATCH] Enable 8-byte wide MMIO for 16550 serial devices 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?The mm version doesn't have .impl.max_access_size=1. Thanks, Paolo
Sorry for the late reply on this. I implemented it with the shifting mask initially to mirror the original code, but it should definitely work with "value &= 255". Looking at serial_ioport_write, I think this might also fix a potential issue where the upper bits of the divider variable could be set if any bits 8-15 of "val" are set (and addr = 0).
I'll go ahead and push up a v2 patch with this fix. Thanks, Mike
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,
[Prev in Thread] | Current Thread | [Next in Thread] |