From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (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 ADDA522423839 for ; Thu, 1 Mar 2018 02:11:26 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4DDED402291E; Thu, 1 Mar 2018 10:17:33 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-4.rdu2.redhat.com [10.10.120.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id DCC8B9C04F; Thu, 1 Mar 2018 10:17:30 +0000 (UTC) To: Heyi Guo , edk2-devel@lists.01.org Cc: Jordan Justen , Anthony Perard , Julien Grall , Ruiyu Ni , Ard Biesheuvel References: <1519887444-75510-1-git-send-email-heyi.guo@linaro.org> <1519887444-75510-3-git-send-email-heyi.guo@linaro.org> From: Laszlo Ersek Message-ID: <68122225-c3d3-14e6-053c-538db503e8f8@redhat.com> Date: Thu, 1 Mar 2018 11:17:30 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <1519887444-75510-3-git-send-email-heyi.guo@linaro.org> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Thu, 01 Mar 2018 10:17:33 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Thu, 01 Mar 2018 10:17:33 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH v5 2/6] OvmfPkg/PciHostBridgeLib: Init PCI aperture to 0 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Mar 2018 10:11:27 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hello Heyi, On 03/01/18 07:57, Heyi Guo wrote: > Use ZeroMem to initialize all fields in temporary > PCI_ROOT_BRIDGE_APERTURE variables to zero. This is not mandatory but > is helpful for future extension: when we add new fields to > PCI_ROOT_BRIDGE_APERTURE and the default value of these fields can > safely be zero, this code will not suffer from an additional > change. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Heyi Guo > > Cc: Jordan Justen > Cc: Anthony Perard > Cc: Julien Grall > Cc: Ruiyu Ni > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > --- > OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c | 4 ++++ > OvmfPkg/Library/PciHostBridgeLib/XenSupport.c | 5 +++++ > 2 files changed, 9 insertions(+) > > diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c > index ff837035caff..4a650a4c6df9 100644 > --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c > +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c > @@ -217,6 +217,10 @@ PciHostBridgeGetRootBridges ( > PCI_ROOT_BRIDGE_APERTURE Mem; > PCI_ROOT_BRIDGE_APERTURE MemAbove4G; > > + ZeroMem (&Io, sizeof (Io)); > + ZeroMem (&Mem, sizeof (Mem)); > + ZeroMem (&MemAbove4G, sizeof (MemAbove4G)); > + > if (PcdGetBool (PcdPciDisableBusEnumeration)) { > return ScanForRootBridges (Count); > } This is OK. (Although for a trivial perf improvement, you could move the ZeroMem() calls after the PcdGetBool() / return. Not necessary, up to you.) However: > diff --git a/OvmfPkg/Library/PciHostBridgeLib/XenSupport.c b/OvmfPkg/Library/PciHostBridgeLib/XenSupport.c > index 31c63ae19e0a..aaf101dfcb0e 100644 > --- a/OvmfPkg/Library/PciHostBridgeLib/XenSupport.c > +++ b/OvmfPkg/Library/PciHostBridgeLib/XenSupport.c > @@ -193,6 +193,11 @@ ScanForRootBridges ( > > *NumberOfRootBridges = 0; > RootBridges = NULL; > + ZeroMem (&Io, sizeof (Io)); > + ZeroMem (&Mem, sizeof (Mem)); > + ZeroMem (&MemAbove4G, sizeof (MemAbove4G)); > + ZeroMem (&PMem, sizeof (PMem)); > + ZeroMem (&PMemAbove4G, sizeof (PMemAbove4G)); > > // > // After scanning all the PCI devices on the PCI root bridge's primary bus, > these ZeroMem() calls are not in the correct place. Please move them into the "PrimaryBus" loop just underneath. That loop works like this: For each primary bus: (1) set all of the aperture variables to "nonexistent": Io.Base = Mem.Base = MemAbove4G.Base = PMem.Base = PMemAbove4G.Base = MAX_UINT64; Io.Limit = Mem.Limit = MemAbove4G.Limit = PMem.Limit = PMemAbove4G.Limit = 0; (2) accumulate the BARs of the devices on the bus into the aperture variables (3) call InitRootBridge() with the aperture variables That is, the ZeroMem() calls that you are adding have to be part of step (1). So, please replace the assignments Io.Base = Mem.Base = MemAbove4G.Base = PMem.Base = PMemAbove4G.Base = MAX_UINT64; Io.Limit = Mem.Limit = MemAbove4G.Limit = PMem.Limit = PMemAbove4G.Limit = 0; with ZeroMem (&Io, sizeof (Io)); ZeroMem (&Mem, sizeof (Mem)); ZeroMem (&MemAbove4G, sizeof (MemAbove4G)); ZeroMem (&PMem, sizeof (PMem)); ZeroMem (&PMemAbove4G, sizeof (PMemAbove4G)); Io.Base = Mem.Base = MemAbove4G.Base = PMem.Base = PMemAbove4G.Base = MAX_UINT64; Thanks! Laszlo