[v1] TYPE_SERIAL cleanup

[PATCH 08/12] hw/char/serial: Avoid implicit conversion when tracing

Posted by Bernhard Beschow 1 month, 1 week ago

On 64 bit targets, the MemoryRegion API passes an address and a value as
uint64_t, so use that for tracing. Keep the uint8_t for reading since
this is what the device model produces. On targets with less than 64
bits, uint64_t is wide enough to avoid narrowing.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/char/trace-events | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/char/trace-events b/hw/char/trace-events
index 9e74be2c14..a3fcc77287 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -5,8 +5,8 @@ parallel_ioport_read(const char *desc, uint16_t addr, uint8_t value) "read [%s]
 parallel_ioport_write(const char *desc, uint16_t addr, uint8_t value) "write [%s] addr 0x%02x val 0x%02x"
 
 # serial.c
-serial_read(uint16_t addr, uint8_t value) "read addr 0x%02x val 0x%02x"
-serial_write(uint16_t addr, uint8_t value) "write addr 0x%02x val 0x%02x"
+serial_read(uint64_t addr, uint8_t value) "[0x%02" PRIx64 "] -> 0x%02" PRIx8
+serial_write(uint64_t addr, uint64_t value) "[0x%02" PRIx64 "] <- 0x%02" PRIx64
 serial_update_parameters(uint64_t baudrate, char parity, int data_bits, int stop_bits) "baudrate=%"PRIu64" parity='%c' data=%d stop=%d"
 
 # virtio-serial-bus.c
-- 
2.53.0

Re: [PATCH 08/12] hw/char/serial: Avoid implicit conversion when tracing

Posted by Peter Maydell 1 month, 1 week ago

On Mon, 2 Mar 2026 at 22:03, Bernhard Beschow <shentey@gmail.com> wrote:
>
> On 64 bit targets, the MemoryRegion API passes an address and a value as
> uint64_t, so use that for tracing. Keep the uint8_t for reading since
> this is what the device model produces. On targets with less than 64
> bits, uint64_t is wide enough to avoid narrowing.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/char/trace-events | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/char/trace-events b/hw/char/trace-events
> index 9e74be2c14..a3fcc77287 100644
> --- a/hw/char/trace-events
> +++ b/hw/char/trace-events
> @@ -5,8 +5,8 @@ parallel_ioport_read(const char *desc, uint16_t addr, uint8_t value) "read [%s]
>  parallel_ioport_write(const char *desc, uint16_t addr, uint8_t value) "write [%s] addr 0x%02x val 0x%02x"
>
>  # serial.c
> -serial_read(uint16_t addr, uint8_t value) "read addr 0x%02x val 0x%02x"
> -serial_write(uint16_t addr, uint8_t value) "write addr 0x%02x val 0x%02x"
> +serial_read(uint64_t addr, uint8_t value) "[0x%02" PRIx64 "] -> 0x%02" PRIx8
> +serial_write(uint64_t addr, uint64_t value) "[0x%02" PRIx64 "] <- 0x%02" PRIx64
>  serial_update_parameters(uint64_t baudrate, char parity, int data_bits, int stop_bits) "baudrate=%"PRIu64" parity='%c' data=%d stop=%d"

We know that the value being written fits in one byte, though,
because we just asserted that size == 1 before trace_serial_write().

-- PMM

Re: [PATCH 08/12] hw/char/serial: Avoid implicit conversion when tracing

Posted by Bernhard Beschow 1 month, 1 week ago


Am 3. März 2026 11:34:01 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Mon, 2 Mar 2026 at 22:03, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> On 64 bit targets, the MemoryRegion API passes an address and a value as
>> uint64_t, so use that for tracing. Keep the uint8_t for reading since
>> this is what the device model produces. On targets with less than 64
>> bits, uint64_t is wide enough to avoid narrowing.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>  hw/char/trace-events | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/char/trace-events b/hw/char/trace-events
>> index 9e74be2c14..a3fcc77287 100644
>> --- a/hw/char/trace-events
>> +++ b/hw/char/trace-events
>> @@ -5,8 +5,8 @@ parallel_ioport_read(const char *desc, uint16_t addr, uint8_t value) "read [%s]
>>  parallel_ioport_write(const char *desc, uint16_t addr, uint8_t value) "write [%s] addr 0x%02x val 0x%02x"
>>
>>  # serial.c
>> -serial_read(uint16_t addr, uint8_t value) "read addr 0x%02x val 0x%02x"
>> -serial_write(uint16_t addr, uint8_t value) "write addr 0x%02x val 0x%02x"
>> +serial_read(uint64_t addr, uint8_t value) "[0x%02" PRIx64 "] -> 0x%02" PRIx8
>> +serial_write(uint64_t addr, uint64_t value) "[0x%02" PRIx64 "] <- 0x%02" PRIx64
>>  serial_update_parameters(uint64_t baudrate, char parity, int data_bits, int stop_bits) "baudrate=%"PRIu64" parity='%c' data=%d stop=%d"
>
>We know that the value being written fits in one byte, though,
>because we just asserted that size == 1 before trace_serial_write().

Well, we as humans know it but the compiler doesn't, so it will perform implicit conversion. The format string will still print it as a byte. Any issues with that?

Best regards,
Bernhard

>
>-- PMM