Re: [PATCH 5/6] usb/ohci: Add trace points for register access

[Peter Maydell]

On Tue, 31 Jan 2023 at 19:39, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> To help debugging add trace points that print values read from or
> written to the device's registers.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/usb/hcd-ohci.c   | 19 +++++++++++++++++++
>  hw/usb/trace-events |  4 ++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index c5306378cc..c5bec4e4d7 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -235,6 +235,15 @@ struct ohci_iso_td {
>
>  #define OHCI_HRESET_FSBIR       (1 << 0)
>
> +static const char *ohci_reg_names[] = {
> +    "HcRevision", "HcControl", "HcCommandStatus", "HcInterruptStatus",
> +    "HcInterruptEnable", "HcInterruptDisable", "HcHCCA", "HcPeriodCurrentED",
> +    "HcControlHeadED", "HcControlCurrentED", "HcBulkHeadED", 
> "HcBulkCurrentED",
> +    "HcDoneHead", "HcFmInterval", "HcFmRemaining", "HcFmNumber",
> +    "HcPeriodicStart", "HcLSThreshold", "HcRhDescriptorA", "HcRhDescriptorB",
> +    "HcRhStatus"
> +};

> @@ -1582,6 +1593,10 @@ static uint64_t ohci_mem_read(void *opaque,
>              trace_usb_ohci_mem_read_bad_offset(addr);
>              retval = 0xffffffff;
>          }
> +        if (addr != 0xc || retval) {
> +            trace_usb_ohci_mem_read(size, ohci_reg_names[addr >> 2],
> +                                    addr, addr >> 2, retval);
> +        }

It's a bit tricky to be sure that this doesn't index off the
end of the reg names array. Could you wrap the array access in a
function that has a check that the index is in bounds?

Otherwise looks good.

thanks
-- PMM