Sorry for the late answer. On 7/13/20 6:15 PM, Peter Maydell wrote: > On Mon, 13 Jul 2020 at 15:19, Cédric Le Goater <[email protected]> wrote: >> On 7/10/20 1:33 PM, Peter Maydell wrote: >>> Andrew, Cedric: do you have the datasheet for this device? Do you >>> know if we should also be flagging the error back to the >>> guest somehow? >> >> zero is the only invalid size of a transmit buffer and the specs does >> not have any special information on which bit to raise in that case. > > I found a datasheet which might or might not be the equivalent > bit of hardware -- does your datasheet have a note on the > TXBUF_SIZE field of a tx descriptor that says "When the size is 0, > the descriptor would be discarded" ? (Though I found another > random doc that just says it's illegal...)
I only have : TXBUF SIZE: Transmit buffer size in byte The transmit buffer size can not be zero. >> I think FTGMAC100_INT_NO_NPTXBUF (transmit buffer unavailable) is our >> best option and we should add an extra 'len == 0' test in front of >> the dma_memory_read() call to raise it. A zero length is not considered >> bogus by dma_memory_read() it seems. > > My best guess at "what the hardware does" here would be: > * TXBUF_SIZE in a tx descriptor can be anything: the h/w > would happily allow you to assemble a tx packet with a > whole series of 1-byte sized buffers, each with its own > tx descriptor yes. > * zero-byte tx descriptors might just be marked "done" and > skipped over since they have no actual data yes. > * any checking on max/min lengths would be done > only on the accumulated total-packet length (we do this > this way already for the frame-too-big check) yes. > * I suspect "transmit buffer unavailable" means "the ethernet > controller needs more data but the next tx descriptor > is still marked as owned by the guest" -- this is certainly > what we currently do with it, and that doesn't seem like > the best thing to signal for the "tx packet too small" > case. It's possible that the hardware simply sends out a > runt packet of some form if the software tells it to do > that. My vote would be for handling it with XPKT_LOST, > the same way we do for over-large frames. XPKT_LOST means that packets are lost due to late/excessive collision. I have used this status for large frames because not other bits made sense. > This probably > is not what the hardware does but at least it's a > coherent thing that the guest might be expecting to have > happen for a tx attempt and it matches the fact that we > really are not going to put it on the 'wire'. I agree. > > Side note: I suspect that any failures from > dma_memory_read() and dma_memory_write() should be > reported as AHB_ERR (currently we have a mix of > ignoring them or using NO_NPTXBUF). AHB_ERR is not used in the Aspeed implementation. I checked with them. But I think it makes sense for other implementations, so we can add this status for Linux. NO_NPTXBUF means a lack a transmit descriptors. XPKT_LOST is our best choice then. Thanks, C. > (It would in theory be possible to test some of these edge > cases on real hardware, but that kind of bare-metal test > case is usually a pain to put together and way overkill > for this situation, so I don't think we should bother.) > >> Is address zero considered bogus ? >> If not, we need to check that also. > > Writes to address 0 are fine, it is not a special physical address. > > thanks > -- PMM >
