On Fri, 31 May 2024 at 19:16, Cord Amfmgm <dmamf...@gmail.com> wrote: > On Fri, May 31, 2024 at 9:03 AM Peter Maydell <peter.mayd...@linaro.org> > wrote: >> What I would like to see is what we could classify under >> "rationale", which is to say "what prompted us to make this >> change?". In my experience it's important to record this >> (including in the commit message). There are of course >> many cases in QEMU's git history where we failed to do that, >> but in general I think it's a good standard to meet. (I >> am also erring on the side of caution in reviewing this >> particular patch, because I don't know the relevant standards >> or this bit of the code very well.)
> Thanks, in responding to that, I'm breaking down my responses into the > following answers: > > Q1: (summarizing) What prompted us to make this change? > > A1: I'm summarizing what I wrote in previous emails and the commit message - > > * Bring qemu closer to actual hw with a neatly packaged test case to > demonstrate the behavior > * I interpret the spec (Table 4-2) as saying the "be = cbp - 1" is valid, in > addition to setting "cbp = 0" > * I interpret it that way due to a comment in an old linux kernel version in > the 2.x range, ohci-hcd.c file. It said (paraphrasing) some misbehaving ohci > controllers would fetch physical memory at 0 when cbp = 0 was in the Transfer > Descriptor Interesting. Do you have a more specific version for that? I had a look at various 2.x Linux OHCI drivers but they all seem to do zero-length packets "by the spec" with CBP=BE=0. eg 2.6.39.4: https://elixir.bootlin.com/linux/v2.6.39.4/source/drivers/usb/host/ohci-q.c#L539 and there's no hardware-quirk handling to do it differently on some controllers. (The USB OHCI driver seems to have gone through a couple of rewrites in the 2.x kernel timeframe; the earlier 2.3.51 version does the TD fill here: https://elixir.bootlin.com/linux/2.3.51/source/drivers/usb/usb-ohci.c#L812 and again it handles zero length as BE=CBP=0.) > But I only have a test case I created myself, and am not an expert on > computer history here. I think "be liberal in what you accept, by > conservative in what you send" applies when I don't know which historical > OSes, if any, need this behavior. I think the behavior of actual hardware > weighs more heavily since that *is* available and testable. Are there > additional checks that would expand on what's known about actual ohci hw? The other side of the argument is "if in doubt and you don't know of any concrete problem being caused, don't change working code". If there are any historical OSes that rely on being able to send zero packets with be=cbp-1 for some nonzero be, and anybody wants to run them on QEMU, then presumably they can send us a bug report saying "XYZ's USB support doesn't work". That nobody has ever does this is evidence on the side of "there is no such OS out there, everybody writing an OHCI driver read the spec and made their driver send zero length packets the way the spec very clearly says you should". Please correct me if I'm wrong, but my interpretation of your helpful explanation above is that this is essentially a theoretical problem rather than one that's caused you a problem you need to fix. thanks -- PMM
