From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 222CA1A1E2D for ; Mon, 5 Sep 2016 05:04:35 -0700 (PDT) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A170F155E4; Mon, 5 Sep 2016 12:04:34 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-76.phx2.redhat.com [10.3.116.76]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u85C4Wug017810; Mon, 5 Sep 2016 08:04:33 -0400 To: Ard Biesheuvel , edk2-devel@ml01.01.org, feng.tian@intel.com, star.zeng@intel.com, liming.gao@intel.com References: <1473067049-16252-1-git-send-email-ard.biesheuvel@linaro.org> <1473067049-16252-7-git-send-email-ard.biesheuvel@linaro.org> Cc: leif.lindholm@linaro.org From: Laszlo Ersek Message-ID: <92560561-afa7-b228-0410-416b3b3aeba7@redhat.com> Date: Mon, 5 Sep 2016 14:04:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1473067049-16252-7-git-send-email-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 05 Sep 2016 12:04:34 +0000 (UTC) Subject: Re: [PATCH 6/7] MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that support it X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Sep 2016 12:04:35 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 09/05/16 11:17, Ard Biesheuvel wrote: > Currently, the EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute is completely > ignored by the PCI host bridge driver, which means that, on an implementation > that supports DMA above 4 GB, allocations above 4 GB may be provided to > devices that have not expressed support for it. > > So in addition to checking 'RootBridge->DmaAbove4G' to establish whether the > root bridge itself supports DMA above 4 GB, we must also take into account > the operation type (EfiPciOperationBusMaster{Read|Write|CommBuffer}64), > and the EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute, when mapping and > allocating DMA memory, respectively. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel > --- > MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > index b2d76d67afa2..8af131b0af37 100644 > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > @@ -1073,10 +1073,15 @@ RootBridgeIoMap ( > RootBridge = ROOT_BRIDGE_FROM_THIS (This); > > PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress; > - if (!RootBridge->DmaAbove4G && ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) { > + if ((!RootBridge->DmaAbove4G || > + (Operation != EfiPciOperationBusMasterRead64 && > + Operation != EfiPciOperationBusMasterWrite64 && > + Operation != EfiPciOperationBusMasterCommonBuffer64)) && > + ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) { > + > // > - // If the root bridge can not handle performing DMA above 4GB but > - // any part of the DMA transfer being mapped is above 4GB, then > + // If the root bridge or the device cannot handle performing DMA above > + // 4GB but any part of the DMA transfer being mapped is above 4GB, then > // map the DMA transfer to a buffer below 4GB. > // > > @@ -1308,7 +1313,8 @@ RootBridgeIoAllocateBuffer ( > RootBridge = ROOT_BRIDGE_FROM_THIS (This); > > AllocateType = AllocateAnyPages; > - if (!RootBridge->DmaAbove4G) { > + if (!RootBridge->DmaAbove4G || > + (Attributes & EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) { > // > // Limit allocations to memory below 4GB > // > Reviewed-by: Laszlo Ersek