[PATCH] hw/usb/hcd-ehci.c: print diagnostics when cpage out of range

[PATCH] hw/usb/hcd-ehci.c: print diagnostics when cpage out of range

[Arnout Engelen]

Making it easier to diagnose what is going on when it happens

Signed-off-by: Arnout Engelen <arnout@bzzt.net>
---
 hw/usb/hcd-ehci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 33a8a377bd..8e2f6578c2 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1195,6 +1195,11 @@ static int ehci_init_transfer(EHCIPacket *p)
     while (bytes > 0) {
         if (cpage > 4) {
             fprintf(stderr, "cpage out of range (%u)\n", cpage);
+            bytes  = get_field(p->qtd.token, QTD_TOKEN_TBYTES);
+            offset = p->qtd.bufptr[0] & ~QTD_BUFPTR_MASK;
+            cpage  = get_field(p->qtd.token, QTD_TOKEN_CPAGE);
+            fprintf(stderr, "reading %u bytes from offset %u at page %u\n",
+                    bytes, offset, cpage);
             qemu_sglist_destroy(&p->sgl);
             return -1;
         }
-- 
2.35.3

Re: [PATCH] hw/usb/hcd-ehci.c: print diagnostics when cpage out of range

[Gerd Hoffmann]

  Hi,

>          if (cpage > 4) {
>              fprintf(stderr, "cpage out of range (%u)\n", cpage);
> +            bytes  = get_field(p->qtd.token, QTD_TOKEN_TBYTES);
> +            offset = p->qtd.bufptr[0] & ~QTD_BUFPTR_MASK;
> +            cpage  = get_field(p->qtd.token, QTD_TOKEN_CPAGE);
> +            fprintf(stderr, "reading %u bytes from offset %u at page %u\n",
> +                    bytes, offset, cpage);

I think we should either drop it (you've successfully debugged the
problem meanwhile, thanks for that), or turn it into a tracepoint.
Simply printing to stderr is deprecated.

take care,
  Gerd

Re: [PATCH] hw/usb/hcd-ehci.c: print diagnostics when cpage out of range

[Arnout Engelen]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

On Thu, Jun 9, 2022, at 12:09, Gerd Hoffmann wrote:
>   Hi,
> 
> >          if (cpage > 4) {
> >              fprintf(stderr, "cpage out of range (%u)\n", cpage);
> > +            bytes  = get_field(p->qtd.token, QTD_TOKEN_TBYTES);
> > +            offset = p->qtd.bufptr[0] & ~QTD_BUFPTR_MASK;
> > +            cpage  = get_field(p->qtd.token, QTD_TOKEN_CPAGE);
> > +            fprintf(stderr, "reading %u bytes from offset %u at page %u\n",
> > +                    bytes, offset, cpage);
> 
> I think we should either drop it (you've successfully debugged the
> problem meanwhile, thanks for that), or turn it into a tracepoint.
> Simply printing to stderr is deprecated.

Gotcha, I'm OK with dropping it. Thanks for the feedback & queue-ing the fix!


Kind regards,

Arnout

[-- Attachment #2: Type: text/html, Size: 1718 bytes --]