Re: hw/usb/hcd-ohci: Fix #1510, #303: pid not IN or OUT

[Peter Maydell]

On Mon, 20 May 2024 at 23:24, Cord Amfmgm <dmamfmgm@gmail.com> wrote:
> On Mon, May 20, 2024 at 12:05 PM Peter Maydell <peter.maydell@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?

thanks
-- PMM