Cord Amfmgm <dmamf...@gmail.com> writes: > On Tue, May 28, 2024 at 11:32 AM Peter Maydell <peter.mayd...@linaro.org> > wrote: > > On Tue, 28 May 2024 at 16:37, Cord Amfmgm <dmamf...@gmail.com> wrote: > > > > On Tue, May 28, 2024 at 9:03 AM Peter Maydell <peter.mayd...@linaro.org> > wrote: > >> > >> On Mon, 20 May 2024 at 23:24, Cord Amfmgm <dmamf...@gmail.com> wrote: > >> > On Mon, May 20, 2024 at 12:05 PM Peter Maydell > <peter.mayd...@linaro.org> wrote: <snip> > >> > And here's an example buffer of length 0 -- you probably already know > what I'm going to do here: > >> > > >> > char buf[0]; > >> > char * CurrentBufferPointer = &buf[0]; > >> > char * BufferEnd = &buf[-1]; // "address of the last byte in the buffer" > >> > // The OHCI Host Controller than advances CurrentBufferPointer like > this: CurrentBufferPointer += 0 > >> > // After the transfer: > >> > // CurrentBufferPointer = &buf[0]; > >> > // BufferEnd = &buf[-1]; > >> > >> Right, but why do you think this is valid, rather than > >> being a guest software bug? My reading of the spec is that it's > >> pretty clear about how to say "zero length buffer", and this > >> isn't it. > >> > >> Is there some real-world guest OS that programs the OHCI > >> controller this way that we're trying to accommodate? > > > > > > qemu versions 4.2 and before allowed this behavior. > > So? That might just mean we had a bug and we fixed it. > 4.2 is a very old version of QEMU and nobody seems to have > complained in the four years since we released 5.0 about this, > which suggests that generally guest OS drivers don't try > to send zero-length buffers in this way. > > > I don't think it's valid to ask for a *popular* guest OS as a > proof-of-concept because I'm not an expert on those. > > I didn't ask for "popular"; I asked for "real-world". > What is the actual guest code you're running that falls over > because of the behaviour change? > > More generally, why do you want this behaviour to be > changed? Reasonable reasons might include: > * we're out of spec based on reading the documentation > * you're trying to run some old Windows VM/QNX/etc image, > and it doesn't work any more > * all the real hardware we tested behaves this way > > But don't necessarily include: > * something somebody wrote and only tested on QEMU happens to > assume the old behaviour rather than following the hw spec > > QEMU occasionally works around guest OS bugs, but only as > when we really have to. It's usually better to fix the > bug in the guest. > > It's not, and I've already demonstrated that real hardware is consistent with > the fix in this patch. > > Please check your tone.
I don't think that is a particularly helpful comment for someone who is taking the time to review your patches. Reading through the thread I didn't see anything that said this is how real HW behaves but I may well have missed it. However you have a number of review comments to address so I suggest you spin a v2 of the series to address them and outline the reason to accept an out of spec transaction. -- Alex Bennée Virtualisation Tech Lead @ Linaro
