From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c0c::233; helo=mail-wr0-x233.google.com; envelope-from=roman.bacik@broadcom.com; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x233.google.com (mail-wr0-x233.google.com [IPv6:2a00:1450:400c:c0c::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 22BD221DF8098 for ; Mon, 14 May 2018 11:02:16 -0700 (PDT) Received: by mail-wr0-x233.google.com with SMTP id o4-v6so13303209wrm.0 for ; Mon, 14 May 2018 11:02:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=from:references:in-reply-to:mime-version:thread-index:date :message-id:subject:to:cc; bh=7ivUua4aOVcYR6Zq+Sle4AvN2BCAK1/4HGF+rcbbtOk=; b=gjTDw3lx83ZHa4tu+qcXbgV5MYV+XJSFjP/MjhGDBqA+zH18XQ/1roHAUUOdTE+lHS 4YBMEJNEYlW9M6qLdIFok7UVV3vxcpxFvIe0DFrBjtny9Brh1uIKlNJtwiIeRWsYs86x Iu1nBgDysSaBwgrwSjqzqw+tTkZIYTJGErSBc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:references:in-reply-to:mime-version :thread-index:date:message-id:subject:to:cc; bh=7ivUua4aOVcYR6Zq+Sle4AvN2BCAK1/4HGF+rcbbtOk=; b=e3uNVtiHFIuE69KixccXaIXIRYesOfXHKfQM2RsedUW4I+gPTpn/HDLpyLYUwHht0D 7WQh7mzNFRhbYjmjNicbm0FB++DVJYjSF3afxWXCkP7zO6nAQXYbePTeBn3Y0apgKP1t 9TrSOxYiMHFNukuyhkiIU+kjv6eEBPRd7bov4nUuMzjuSXZsy18T7qrVTVMHFgUZZ/6H 8zJDXsPg9/ZnegYJIS+v6ed6cgtjNs6FjwPQMoaW1O9HZY+KVYVxgMn0RhZC20UE4ikM DSanmrk2Pfeyhz4j9a90thKJz6MOd7ZILleIeYxlwutp1gcX0Vd9EEMN20iN8ils6XrJ e0hA== X-Gm-Message-State: ALKqPwcM9AgOw02DRetmzdtORcTqraVI0SHQug2ecuEQ4Vq+t8Q7hWDW 3BYf2BbXDpNWSW3ACPuUMLDQxAwQCAfubEKCUjZCxw== X-Google-Smtp-Source: AB8JxZq2U46PnXhQ3rNXIj/tzfYQkcRalSt4CjE8N3mMkffE92CVM2M9hkBRfF3GYgt2jJHjf0lsZA6JUteeTcqqcTY= X-Received: by 2002:adf:a617:: with SMTP id k23-v6mr8328861wrc.200.1526320934839; Mon, 14 May 2018 11:02:14 -0700 (PDT) From: Roman Bacik References: <51c562dc1e3275caf4a87939912e46d5@mail.gmail.com> <7a06065337081fd8f1c160dced72d708@mail.gmail.com> In-Reply-To: MIME-Version: 1.0 X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQHTOO3jGNEM57qjhAzKusnUj2uRYQHDCluqAW5//0gBCzZ6igKxHHx+o/l1wGA= Date: Mon, 14 May 2018 11:02:13 -0700 Message-ID: <6a91ca05d2b47d12623064eb7c337ba3@mail.gmail.com> To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, Ruiyu Ni , Vladimir Olovyannikov Subject: Re: [PATCH] Enable using device address when programming BARs X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 May 2018 18:02:17 -0000 Content-Type: text/plain; charset="UTF-8" > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Monday, May 14, 2018 10:35 AM > To: Roman Bacik > Cc: edk2-devel@lists.01.org; Ruiyu Ni; Vladimir Olovyannikov > Subject: Re: [edk2] [PATCH] Enable using device address when programming > BARs > > On 14 May 2018 at 19:28, Roman Bacik wrote: > > Ard, > > > > Thank you very much for your comment. > > > >> -----Original Message----- > >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > >> Sent: Sunday, May 13, 2018 3:25 AM > >> To: Roman Bacik > >> Cc: edk2-devel@lists.01.org; Ruiyu Ni; Vladimir Olovyannikov > >> Subject: Re: [edk2] [PATCH] Enable using device address when > >> programming BARs > >> > >> On 9 May 2018 at 22:17, Roman Bacik > wrote: > >> > I will upload v2 with the corrected subject - add package name > >> > MdeModulePkg/Bus . > >> > > >> > >> I don't think this is the correct approach. Please use the address > >> translation support that has been added recently to PciHostBridgeDxe > >> and PciHostBridgeLib. > >> > > > > Would you like to see this change: > > > > Address = Base + Node->Offset; > > + if (UseDeviceAddress) > > + Address = TO_DEVICE_ADDRESS(Address, -Base); > > > > Instead of: > > > > - Address = Base + Node->Offset; > > + Address = UseDeviceAddress? Node->Offset: Base + Node->Offset; > > > > No. > > Programming BARs should always involve device addresses, never CPU > addresses. If you wire up the MMIO translation support correctly, the > existing code will already do what you want. > Our code has base address 0x60000000 and offset 0x200. We need Address=0x200 to be passed to PciIo->Pci.Write() and Address=0x60000200 to be stored in Node->PciDev->PciBar[Node->Bar].BaseAddress. The code as is does not seem to do that unless PciIo->Pci.Write() itself changes the content of the Address from 0x200 to 0x60000200 for us. So if we modify MMIO and the existing code gets the correct Address 0x200 being passed to PciIo->Pci.Write() then it means Address = 0x200 will be stored into Node->PciDev->PciBar[Node->Bar].BaseAddress, which would be wrong in our case. It appears that regardless how we change MMIO translation the existing code would not work for us as is. Or I may be missing something. > >> > > >> > > >> > *From:* Roman Bacik [mailto:roman.bacik@broadcom.com] > >> > *Sent:* Thursday, May 3, 2018 3:55 PM > >> > *To:* edk2-devel@lists.01.org > >> > *Cc:* Ruiyu Ni; Vladimir Olovyannikov > >> > *Subject:* [edk2] [PATCH] Enable using device address when > >> > programming BARs > >> > > >> > > >> > > >> > Some SoCs require to use device address when BARs are programmed: > >> > https://bugzilla.tianocore.org/show_bug.cgi?id=948 > >> > > >> > Cc: Ruiyu Ni > >> > Cc: Vladimir Olovyannikov > >> > Contributed-under: TianoCore Contribution Agreement 1.1 > >> > Signed-off-by: Roman Bacik > >> > --- > >> > MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 + > >> > MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c | 8 +++++--- > >> > MdeModulePkg/MdeModulePkg.dec | 3 +++ > >> > MdeModulePkg/MdeModulePkg.dsc | 1 + > >> > 4 files changed, 10 insertions(+), 3 deletions(-) > >> > > >> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > >> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > >> > index 97608bfcf245..1368e5068574 100644 > >> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > >> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > >> > @@ -110,6 +110,7 @@ > >> > gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport ## > >> CONSUMES > >> > gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport ## > >> CONSUMES > >> > gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration > ## > >> > SOMETIMES_CONSUMES > >> > + gEfiMdeModulePkgTokenSpaceGuid.PcdUseDeviceAddress ## > >> CONSUMES > >> > > >> > [UserExtensions.TianoCore."ExtraFiles"] > >> > PciBusDxeExtra.uni > >> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > >> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > >> > index 2f713fcee95e..a23bd1e258ef 100644 > >> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > >> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > >> > @@ -1269,6 +1269,7 @@ ProgramBar ( > >> > EFI_PCI_IO_PROTOCOL *PciIo; > >> > UINT64 Address; > >> > UINT32 Address32; > >> > + BOOLEAN UseDeviceAddress; > >> > > >> > ASSERT (Node->Bar < PCI_MAX_BAR); > >> > > >> > @@ -1282,8 +1283,9 @@ ProgramBar ( > >> > > >> > Address = 0; > >> > PciIo = &(Node->PciDev->PciIo); > >> > + UseDeviceAddress = FeaturePcdGet (PcdUseDeviceAddress); > >> > > >> > - Address = Base + Node->Offset; > >> > + Address = UseDeviceAddress? Node->Offset: Base + Node->Offset; > >> > > >> > // > >> > // Indicate pci bus driver has allocated @@ -1308,7 +1310,7 @@ > >> > ProgramBar ( > >> > &Address > >> > ); > >> > > >> > - Node->PciDev->PciBar[Node->Bar].BaseAddress = Address; > >> > + Node->PciDev->PciBar[Node->Bar].BaseAddress = > UseDeviceAddress? > >> > + Base + > >> > Address: Address; > >> > > >> > break; > >> > > >> > @@ -1335,7 +1337,7 @@ ProgramBar ( > >> > &Address32 > >> > ); > >> > > >> > - Node->PciDev->PciBar[Node->Bar].BaseAddress = Address; > >> > + Node->PciDev->PciBar[Node->Bar].BaseAddress = > UseDeviceAddress? > >> > + Base + > >> > Address: Address; > >> > > >> > break; > >> > > >> > diff --git a/MdeModulePkg/MdeModulePkg.dec > >> > b/MdeModulePkg/MdeModulePkg.dec index > cc397185f7b9..58425ee0d57f > >> > 100644 > >> > --- a/MdeModulePkg/MdeModulePkg.dec > >> > +++ b/MdeModulePkg/MdeModulePkg.dec > >> > @@ -1005,6 +1005,9 @@ > >> > # @Prompt Enable UEFI Stack Guard. > >> > > >> > > >> > gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0 > >> x300010 > >> > 55 > >> > > >> > + ## Indicates whether the device address should be used for BAR > >> > programming > >> > + > >> > > >> > > >> > gEfiMdeModulePkgTokenSpaceGuid.PcdUseDeviceAddress|FALSE|BOOLEA > >> N|0x300 > >> > 01056 > >> > + > >> > [PcdsFixedAtBuild, PcdsPatchableInModule] > >> > ## Dynamic type PCD can be registered callback function for Pcd > >> > setting action. > >> > # PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum > >> > number of callback function diff --git > >> a/MdeModulePkg/MdeModulePkg.dsc > >> > b/MdeModulePkg/MdeModulePkg.dsc index > >> ec24a50c7d0a..39b397cb13d9 > >> > 100644 > >> > --- a/MdeModulePkg/MdeModulePkg.dsc > >> > +++ b/MdeModulePkg/MdeModulePkg.dsc > >> > @@ -200,6 +200,7 @@ > >> > > >> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizeNonPopulateCapsule|0x0 > >> > > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizePopulateCapsule|0x0 > >> > > >> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxPeiPerformanceLogEntries|28 > >> > + gEfiMdeModulePkgTokenSpaceGuid.PcdUseDeviceAddress|FALSE > >> > > >> > [PcdsFixedAtBuild.IPF] > >> > > >> > gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000 > >> > -- > >> > 1.9.1 > >> > _______________________________________________ > >> > edk2-devel mailing list > >> > edk2-devel@lists.01.org > >> > https://lists.01.org/mailman/listinfo/edk2-devel > > > > Thanks, > > > > Roman