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: >> >> For the "zero length buffer" case, do you have a more detailed >> >> pointer to the bit of the spec that says that "cbp = be + 1" is a >> >> valid way to specify a zero length buffer? Table 4-2 in the copy I >> >> have says for CurrentBufferPointer "a value of 0 indicates >> >> a zero-length data packet or that all bytes have been transferred", >> >> and the sample host OS driver function QueueGeneralRequest() >> >> later in the spec handles a 0 length packet by setting >> >> TD->HcTD.CBP = TD->HcTD.BE = 0; >> >> (which our emulation's code does handle). >> > >> > >> > Reading the spec carefully, a CBP set to 0 should always mean the >> > zero-length buffer case (or that all bytes have been transferred, so the >> > buffer is now zero-length - the same thing). >> >> Yeah, I can see the argument for "we should treat all cbp == 0 as >> zero-length buffer, not just cbp == be == 0". >> >> > Table 4-2 is the correct reference, and this part is clear. It's the part >> > you quoted. "Contains the physical address of the next memory location >> > that will be accessed for transfer to/from the endpoint. A value of 0 >> > indicates a zero-length data packet or that all bytes have been >> > transferred." >> > >> > Table 4-2 has this additional nugget that may be confusingly worded, for >> > BufferEnd: "Contains physical address of the last byte in the buffer for >> > this TD" >> >> Mmm, but for a zero length buffer there is no "last byte" to >> have this be the physical address of. >> >> > 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. thanks -- PMM
