From patchwork Tue Aug 30 17:26:26 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 75010 Delivered-To: patch@linaro.org Received: by 10.140.29.52 with SMTP id a49csp2261321qga; Tue, 30 Aug 2016 10:27:23 -0700 (PDT) X-Received: by 10.107.143.209 with SMTP id r200mr274590iod.109.1472578043317; Tue, 30 Aug 2016 10:27:23 -0700 (PDT) Return-Path: Received: from lists.linaro.org (lists.linaro.org. [54.225.227.206]) by mx.google.com with ESMTP id 86si198158iom.19.2016.08.30.10.27.17; Tue, 30 Aug 2016 10:27:23 -0700 (PDT) Received-SPF: pass (google.com: domain of linaro-uefi-bounces@lists.linaro.org designates 54.225.227.206 as permitted sender) client-ip=54.225.227.206; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linaro-uefi-bounces@lists.linaro.org designates 54.225.227.206 as permitted sender) smtp.mailfrom=linaro-uefi-bounces@lists.linaro.org; dmarc=pass (p=NONE dis=NONE) header.from=linaro.org Received: by lists.linaro.org (Postfix, from userid 109) id A62D6608CB; Tue, 30 Aug 2016 17:26:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on ip-10-142-244-252 X-Spam-Level: X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL autolearn=disabled version=3.4.0 Received: from [127.0.0.1] (localhost [127.0.0.1]) by lists.linaro.org (Postfix) with ESMTP id 8BBE960819; Tue, 30 Aug 2016 17:26:47 +0000 (UTC) X-Original-To: linaro-uefi@lists.linaro.org Delivered-To: linaro-uefi@lists.linaro.org Received: by lists.linaro.org (Postfix, from userid 109) id E706A60D2F; Tue, 30 Aug 2016 17:26:44 +0000 (UTC) Received: from mail-wm0-f41.google.com (mail-wm0-f41.google.com [74.125.82.41]) by lists.linaro.org (Postfix) with ESMTPS id 2C27360D29 for ; Tue, 30 Aug 2016 17:26:40 +0000 (UTC) Received: by mail-wm0-f41.google.com with SMTP id c133so233824wmd.1 for ; Tue, 30 Aug 2016 10:26:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=38t36yvFN2gCPZzJcEiTxxvJ5QNr81Ow8CyEd2k5XYI=; b=GoJ+L0qP2ub4trEUME7qyO8Fbxo5xlOa63IGSWoSME46K7TfY1JDPK/V+pYVypDk1q DhtcnMV4gUykmiZNDjaQei66cMxzCvrBflXlm6MFis7O7agsS2GyK1qv4+XsTNPF+0aS pLZkzEW2mXKS1tgXVoFkhBBA4qaDEdQcfa2J095A6OSvtQ0Ox81VzqUlBqYg25/lM2E4 +ARZrdLdvw3G08+mrxDCKjgzoCDuJsc9WXtcQjx/eRzPaa9SvZhW6RSVt7+0amvZWnGf Dlqs4fPc3FJIAevItgVa4eQCFEiOmZjRHNVs/zIhBrQt2TDwNGEM/jhw2aq0HUMj4x0U arlA== X-Gm-Message-State: AE9vXwO+NG8MyS+RMM6WPZ1zRxEUYy8tNr0S3/jyGsPFRHNvNDVGvu7aOB1KK83n7lDYqpD2rg4= X-Received: by 10.194.221.202 with SMTP id qg10mr4293108wjc.180.1472577999214; Tue, 30 Aug 2016 10:26:39 -0700 (PDT) Received: from localhost.localdomain ([160.165.12.73]) by smtp.gmail.com with ESMTPSA id e65sm4908377wmg.3.2016.08.30.10.26.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 30 Aug 2016 10:26:38 -0700 (PDT) From: Ard Biesheuvel To: linaro-uefi@lists.linaro.org, alan@softiron.co.uk Date: Tue, 30 Aug 2016 18:26:26 +0100 Message-Id: <1472577986-7907-1-git-send-email-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.7.4 Subject: [Linaro-uefi] [RFC/RFT PATCH] Drivers/Net/MarvellYukonDxe: remove redundant copy on RX path X-BeenThere: linaro-uefi@lists.linaro.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: linaro-uefi-bounces@lists.linaro.org Sender: "Linaro-uefi" The RX path in MarvellYukonDxe uses DMA buffers allocated via EFI_PCI_IO_PROTOCOL::AllocateBuffer(), and copies the contents of each into a temporary buffer, whose contents are copied yet another time before arriving at the caller of EFI_SIMPLE_NETWORK_PROTOCOL::Receive() This is inefficient for several reasons: - the streaming bus master DMA operations used for RX do not require the use of buffers allocated via EFI_PCI_IO_PROTOCOL::AllocateBuffer(), which may return uncached memory on platforms with non-coherent DMA - EFI_PCI_IO_PROTOCOL::AllocateBuffer() performs page based allocations, which means the UEFI memory map is modified for every packet received, now that the improper reuse of DMA buffers has been fixed. So drop the call to EFI_PCI_IO_PROTOCOL::AllocateBuffer(), and instead, perform a pool allocation whose ownership is transferred from the RX ring to the RX linked list, which removes the need for a copy. Also, since pool allocations decay to page based allocations for sizes that equal or exceed EFI_PAGE_SIZE, reduce MAX_SUPPORTED_PACKET_SIZE to the Ethernet default of 1500 bytes. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel --- It builds, but does it run? I have no clue, since I don't own a Juno or a Softiron 1000. Comments and testing appreciated. Thanks. Drivers/Net/MarvellYukonDxe/if_msk.c | 26 ++++++-------------- Drivers/Net/MarvellYukonDxe/if_msk.h | 2 +- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/Drivers/Net/MarvellYukonDxe/if_msk.c b/Drivers/Net/MarvellYukonDxe/if_msk.c index ecd716a66b55..d49c8e92a69a 100644 --- a/Drivers/Net/MarvellYukonDxe/if_msk.c +++ b/Drivers/Net/MarvellYukonDxe/if_msk.c @@ -595,7 +595,7 @@ msk_newbuf ( rxd = &sc_if->msk_cdata.msk_rxdesc[idx]; - Status = mPciIo->AllocateBuffer (mPciIo, AllocateAnyPages, EfiBootServicesData, EFI_SIZE_TO_PAGES (Length), &Buffer, 0); + Status = gBS->AllocatePool (EfiBootServicesData, Length, &Buffer); if (EFI_ERROR (Status)) { return Status; } @@ -603,8 +603,7 @@ msk_newbuf ( Status = mPciIo->Map (mPciIo, EfiPciIoOperationBusMasterWrite, Buffer, &Length, &PhysAddr, &Mapping); if (EFI_ERROR (Status)) { - Length = MAX_SUPPORTED_PACKET_SIZE; - mPciIo->FreeBuffer (mPciIo, EFI_SIZE_TO_PAGES (Length), Buffer); + gBS->FreePool (Buffer); return Status; } @@ -1630,7 +1629,7 @@ msk_txrx_dma_free ( mPciIo->Unmap (mPciIo, rxd->rx_m.DmaMapping); // Free Rx buffers as we own these if(rxd->rx_m.Buf != NULL) { - mPciIo->FreeBuffer (mPciIo, EFI_SIZE_TO_PAGES (rxd->rx_m.Length), rxd->rx_m.Buf); + gBS->FreePool (rxd->rx_m.Buf); rxd->rx_m.Buf = NULL; } gBS->SetMem (&(rxd->rx_m), sizeof (MSK_DMA_BUF), 0); @@ -1933,23 +1932,14 @@ msk_rxeof ( if (!EFI_ERROR (Status)) { gBS->SetMem (m_link, sizeof (MSK_LINKED_SYSTEM_BUF), 0); m_link->Signature = RX_MBUF_SIGNATURE; - Status = gBS->AllocatePool (EfiBootServicesData, - len, - (VOID**) &m_link->SystemBuf.Buf); - if(!EFI_ERROR (Status)) { - gBS->CopyMem (m_link->SystemBuf.Buf, m.Buf, len); - m_link->SystemBuf.Length = len; - - InsertTailList (&mSoftc->ReceiveQueueHead, &m_link->Link); - } else { - DEBUG ((EFI_D_NET, "Marvell Yukon: failed to allocate DMA buffer. Dropping Frame\n")); - gBS->FreePool (m_link); - } + m_link->SystemBuf.Buf = m.Buf; + m_link->SystemBuf.Length = len; + + InsertTailList (&mSoftc->ReceiveQueueHead, &m_link->Link); } else { DEBUG ((EFI_D_NET, "Marvell Yukon: failed to allocate receive buffer link. Dropping Frame\n")); + gBS->FreePool (m.Buf); } - - mPciIo->FreeBuffer (mPciIo, EFI_SIZE_TO_PAGES (m.Length), m.Buf); } while (0); MSK_RX_INC (sc_if->msk_cdata.msk_rx_cons, MSK_RX_RING_CNT); diff --git a/Drivers/Net/MarvellYukonDxe/if_msk.h b/Drivers/Net/MarvellYukonDxe/if_msk.h index a5025a39ecf3..66513d3c54c1 100644 --- a/Drivers/Net/MarvellYukonDxe/if_msk.h +++ b/Drivers/Net/MarvellYukonDxe/if_msk.h @@ -15,7 +15,7 @@ #include -#define MAX_SUPPORTED_PACKET_SIZE EFI_PAGE_SIZE +#define MAX_SUPPORTED_PACKET_SIZE (1500) EFI_STATUS mskc_probe (EFI_PCI_IO_PROTOCOL *PciIo);