On 07/09/20 03:55, Philippe Mathieu-Daudé wrote:
> When a SoC has multiple UARTs (some configured differently),
> it is hard to associate events to their UART.
>
> To be able to distinct trace events between various instances,
> add an 'id' field. Update the trace format accordingly.
>
> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> ---
> include/hw/char/serial.h | 1 +
> hw/char/serial.c | 7 ++++---
> hw/char/trace-events | 6 +++---
> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
> index 3d2a5b27e87..3ee2d096a85 100644
> --- a/include/hw/char/serial.h
> +++ b/include/hw/char/serial.h
> @@ -75,6 +75,7 @@ typedef struct SerialState {
> uint64_t char_transmit_time; /* time to transmit a char in ticks */
> int poll_msl;
>
> + uint8_t id;
> QEMUTimer *modem_status_poll;
> MemoryRegion io;
> } SerialState;
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index ade89fadb44..e5a6b939f13 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -177,7 +177,7 @@ static void serial_update_parameters(SerialState *s)
> ssp.stop_bits = stop_bits;
> s->char_transmit_time = (NANOSECONDS_PER_SECOND / speed) * frame_size;
> qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp);
> - trace_serial_update_parameters(speed, parity, data_bits, stop_bits);
> + trace_serial_update_parameters(s->id, speed, parity, data_bits,
> stop_bits);
> }
>
> static void serial_update_msl(SerialState *s)
> @@ -333,7 +333,7 @@ static void serial_ioport_write(void *opaque, hwaddr
> addr, uint64_t val,
> SerialState *s = opaque;
>
> assert(size == 1 && addr < 8);
> - trace_serial_write(addr, val);
> + trace_serial_write(s->id, addr, val);
> switch(addr) {
> default:
> case 0:
> @@ -550,7 +550,7 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr
> addr, unsigned size)
> ret = s->scr;
> break;
> }
> - trace_serial_read(addr, ret);
> + trace_serial_read(s->id, addr, ret);
> return ret;
> }
>
> @@ -1013,6 +1013,7 @@ static const TypeInfo serial_io_info = {
> };
>
> static Property serial_properties[] = {
> + DEFINE_PROP_UINT8("id", SerialState, id, 0),
> DEFINE_PROP_CHR("chardev", SerialState, chr),
> DEFINE_PROP_UINT32("baudbase", SerialState, baudbase, 115200),
> DEFINE_PROP_BOOL("wakeup", SerialState, wakeup, false),
> diff --git a/hw/char/trace-events b/hw/char/trace-events
> index cd36b63f39d..40800c9334c 100644
> --- a/hw/char/trace-events
> +++ b/hw/char/trace-events
> @@ -5,9 +5,9 @@ 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_update_parameters(uint64_t baudrate, char parity, int data_bits, int
> stop_bits) "baudrate=%"PRIu64" parity='%c' data=%d stop=%d"
> +serial_read(uint8_t id, uint8_t addr, uint8_t value) "id#%u read addr 0x%x
> val 0x%02x"
> +serial_write(uint8_t id, uint8_t addr, uint8_t value) "id#%u write addr 0x%x
> val 0x%02x"
> +serial_update_parameters(uint8_t id, uint64_t baudrate, char parity, int
> data_bits, int stop_bits) "id#%u baudrate=%"PRIu64" parity=%c data=%d stop=%d"
>
> # virtio-serial-bus.c
> virtio_serial_send_control_event(unsigned int port, uint16_t event, uint16_t
> value) "port %u, event %u, value %u"
>
I'm not sure about making this one a one-off for serial.c. You could
add the SerialState* too, for example. I have queued the other six though.
Paolo
