On Fri, Jun 21, 2024 at 10:21 AM Peter Maydell <peter.mayd...@linaro.org> wrote:
> 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 > That tweak looks great. Thank you for your patience working with me to get this patch into a good shape. This was my first attempt to contribute to qemu - really appreciate it.
