Change the receive buffers to be single-use only. This involves a few changes that work together: 1. Change msk_newbuf() to not attempt to re-use or free a buffer, 2. Change msk_rxeof() free the buffer when it is done with it, 3. Store a temporary copy of the received data for passing to the Receive Queue, 4. Call the required Flush() and Unmap() on the DMA buffer.
In addition this means failure to allocate a new buffer is a failure of msk_rxeof().
Note that this change makes the driver work the way the FreeBSD driver (from which this driver was derived) works, and this simply removes an optimization (the code in msk_newbuf() which re-uses the buffers. This removal of the optimization is done for two reasons: 1. The optimization failed to work for 64-bit DMA transfers; 2. The UEFI specification, version 2.6, section 13.4 requires calls to Flush() and Unmap() before reading a DMA write buffer from the CPU, which doesn't fit with the optimization as it existed.
Reverting back to the behavior as it was in the FreeBSD driver solves number 1 and 2 above, and makes this driver more consistent with something we know to be working. There is slightly more overhead, but it is more consistent with the UEFI standard.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Alan Ott alan@softiron.co.uk --- Drivers/Net/MarvellYukonDxe/if_msk.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/Drivers/Net/MarvellYukonDxe/if_msk.c b/Drivers/Net/MarvellYukonDxe/if_msk.c index d2102dc..ad7a1c2 100644 --- a/Drivers/Net/MarvellYukonDxe/if_msk.c +++ b/Drivers/Net/MarvellYukonDxe/if_msk.c @@ -581,13 +581,6 @@ msk_newbuf (
rxd = &sc_if->msk_cdata.msk_rxdesc[idx];
- if ((rxd->rx_m.Buf != NULL) && (rxd->rx_m.Length >= Length)) { - return EFI_ALREADY_STARTED; - } else if (rxd->rx_m.Buf != NULL) { - mPciIo->Unmap (mPciIo, rxd->rx_m.DmaMapping); - mPciIo->FreeBuffer (mPciIo, EFI_SIZE_TO_PAGES (rxd->rx_m.Length), rxd->rx_m.Buf); - } - Status = mPciIo->AllocateBuffer (mPciIo, AllocateAnyPages, EfiBootServicesData, EFI_SIZE_TO_PAGES (Length), &Buffer, 0); if (EFI_ERROR (Status)) { return Status; @@ -1848,6 +1841,7 @@ msk_rxeof ( struct msk_rxdesc *rxd; INTN cons; INTN rxlen; + MSK_DMA_BUF m;
DEBUG ((EFI_D_NET, "Marvell Yukon: rxeof\n"));
@@ -1871,26 +1865,40 @@ msk_rxeof ( break; }
+ rxd = &sc_if->msk_cdata.msk_rxdesc[cons]; + + m = rxd->rx_m; Status = msk_newbuf (sc_if, cons); if (EFI_ERROR (Status)) { + // This is a dropped packet, but we aren't counting drops // Reuse old buffer msk_discard_rxbuf (sc_if, cons); + break; + } + + Status = mPciIo->Flush (mPciIo); + if (EFI_ERROR (Status)) { + DEBUG ((EFI_D_NET, "Marvell Yukon: failed to Flush DMA\n")); } + + Status = mPciIo->Unmap (mPciIo, m.DmaMapping); + if (EFI_ERROR (Status)) { + DEBUG ((EFI_D_NET, "Marvell Yukon: failed to Unmap DMA\n")); + } + Status = gBS->AllocatePool (EfiBootServicesData, sizeof (MSK_LINKED_DMA_BUF), (VOID**) &m_link); if (!EFI_ERROR (Status)) { gBS->SetMem (m_link, sizeof (MSK_LINKED_DMA_BUF), 0); - rxd = &sc_if->msk_cdata.msk_rxdesc[cons]; - m_link->Signature = RX_MBUF_SIGNATURE; Status = gBS->AllocatePool (EfiBootServicesData, len, (VOID**) &m_link->DmaBuf.Buf); if(!EFI_ERROR (Status)) { - gBS->CopyMem (m_link->DmaBuf.Buf, rxd->rx_m.Buf, len); + gBS->CopyMem (m_link->DmaBuf.Buf, m.Buf, len); m_link->DmaBuf.Length = len; - m_link->DmaBuf.DmaMapping = rxd->rx_m.DmaMapping; + m_link->DmaBuf.DmaMapping = m.DmaMapping;
InsertTailList (&mSoftc->ReceiveQueueHead, &m_link->Link); } else { @@ -1899,6 +1907,8 @@ msk_rxeof ( } else { DEBUG ((EFI_D_NET, "Marvell Yukon: failed to allocate receive buffer link. Dropping Frame\n")); } + + mPciIo->FreeBuffer (mPciIo, EFI_SIZE_TO_PAGES (m.Length), m.Buf); } while (0);
MSK_INC (sc_if->msk_cdata.msk_rx_cons, MSK_RX_RING_CNT);