On Wed, 12 Jun 2024 at 20:36, Alex Bennée <alex.ben...@linaro.org> wrote:
>
> Cord Amfmgm <dmamf...@gmail.com> writes:
>
> > On Wed, Jun 12, 2024 at 9:21 AM Alex Bennée <alex.ben...@linaro.org> wrote:
> >
> > David Hubbard <dmamf...@gmail.com> writes:
> >
> > > From: Cord Amfmgm <dmamf...@gmail.com>
> > >
> > > This changes the way the ohci emulation handles a Transfer Descriptor
> > with
> > > "Current Buffer Pointer" set to "Buffer End" + 1.
> > >
> > > The OHCI spec 4.3.1.2 Table 4-2 allows td.cbp to be one byte more than
> > td.be
> > > to signal the buffer has zero length. Currently qemu only accepts
> > zero-length
> > > Transfer Descriptors if the td.cbp is equal to 0, while actual OHCI
> > hardware
> > > accepts both cases.
> > >
> > > The qemu ohci emulation has a regression in ohci_service_td. Version 4.2
> > > and earlier matched the spec. (I haven't taken the time to bisect exactly
> > > where the logic was changed.)
> >
> > I find it hard to characterise this as a regression because we've
> > basically gone from no checks to actually doing bounds checks:
> >
> > 1328fe0c32 (hw: usb: hcd-ohci: check len and frame_number variables)
> >
> > The argument here seems to be that real hardware is laxer than the specs
> > in what it accepts.
> >
> <snip>
> >
> > With the updated commit message:
> >
> > Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
> >
> > Please forgive my lack of experience on this mailing list. I don't see a
> > suggested commit message from Alex but in case that
> > was what is being considered, here is one. Feedback welcome, also if this
> > is not what is wanted, please just say it.
> >
>
> Something along the lines of what you suggest here
Thanks; I've picked up this patch for target-arm.next (as with
your previous one for hcd-ohci, adjusting the Author and
Signed-off-by lines to both read David Hubbard).
I tweaked the commit message a little bit, so the middle part reads:
What this patch does is loosen the qemu ohci implementation to allow a
zero-length packet if td.be (Buffer End) is set to td.cbp - 1, and with a
non-zero td.cbp value.
The spec is unclear whether this is valid or not -- it is not the
clearly documented way to send a zero length TD (which is CBP=BE=0),
but it isn't specifically forbidden. Actual hw seems to be ok with it.
thanks
-- PMM
