Drivers/Net/MarvellYukon: Don't re-use DMA buffers
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>
[ardb: replaced struct assignment with gBS->CopyMem()]
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
diff --git a/Drivers/Net/MarvellYukonDxe/if_msk.c b/Drivers/Net/MarvellYukonDxe/if_msk.c
index d2102dc..2b505c3 100644
--- a/Drivers/Net/MarvellYukonDxe/if_msk.c
+++ b/Drivers/Net/MarvellYukonDxe/if_msk.c
@@ -581,13 +581,6 @@
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 @@
struct msk_rxdesc *rxd;
INTN cons;
INTN rxlen;
+ MSK_DMA_BUF m;
DEBUG ((EFI_D_NET, "Marvell Yukon: rxeof\n"));
@@ -1871,26 +1865,41 @@
break;
}
+ rxd = &sc_if->msk_cdata.msk_rxdesc[cons];
+
+ gBS->CopyMem (&m, &rxd->rx_m, sizeof(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, rxd->rx_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 = NULL;
InsertTailList (&mSoftc->ReceiveQueueHead, &m_link->Link);
} else {
@@ -1899,6 +1908,8 @@
} 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);
