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 5CA571A1E06 for ; Fri, 2 Sep 2016 07:18:14 -0700 (PDT) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (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 CAF55C000403; Fri, 2 Sep 2016 14:18:13 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-76.phx2.redhat.com [10.3.116.76]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u82EIBvi002520; Fri, 2 Sep 2016 10:18:12 -0400 To: Leif Lindholm References: <1472634453-27246-1-git-send-email-ard.biesheuvel@linaro.org> <20160831124845.GU4715@bivouac.eciton.net> <20160902130128.GY4715@bivouac.eciton.net> Cc: Ard Biesheuvel , edk2-devel@ml01.01.org, "Wu, Hao A" From: Laszlo Ersek Message-ID: <9addcb0f-84ac-b572-3507-85678ece2342@redhat.com> Date: Fri, 2 Sep 2016 16:18:11 +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: <20160902130128.GY4715@bivouac.eciton.net> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 02 Sep 2016 14:18:13 +0000 (UTC) Subject: Re: [PATCH 0/3] ArmPkg: introduce IsZeroGuid() and IsZeroBuffer() 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: Fri, 02 Sep 2016 14:18:14 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 09/02/16 15:01, Leif Lindholm wrote: > On Fri, Sep 02, 2016 at 01:33:21PM +0200, Laszlo Ersek wrote: >>> Maybe this is a good point at which to move these into MdePkg, in the >>> hope that the ARM versions won't be overlooked in future API >>> revisions? >> >> I strongly suggest / request that your (good) suggestion be implemented >> as a separate endeavor. Moving this stuff into MdePkg is definitely >> justified, but it will almost certainly take a good chunk of time. >> Meanwhile the ArmVirtQemu builds remain broken. >> >> I suggest to go ahead and commit patches #2 and #3 as well, and swiftly >> at that. And, in order to keep ourselves honest about the longer term >> goal, I propose to file a bug for the code movement in our Bugzilla >> instance. (The affected packages should be MdePkg + ArmPkg.) > > Surely if we're doing a panic fix, that should be to revert > 313831d-72388f9? I preferred a quick fix that was also not destructive. Patches #2 and #3 here can be considered the initial, basic implementation of the new interfaces for ARM/AARCH64, which could (and should) have been part of the original series (that introduced the breakage by missing them). IOW, #2 and #3 could be construed as the continuation of that initial series, for ARM/AARCH64. > If we're not doing that, let's do the thing that will reduce the > likelihood of this breaking again. > In the interest of speeding this up, I would propose to wait with > Ard's latest set (which deserves and is likely to see some more > discussion) and go ahead with: > - Nuking BaseMemoryLibVstm > - Moving BaseMemoryLibStm to MdePkg > - Updating platforms in edk2 to reflect new location of > BaseMemoryLibStm > - Add the new functions to BaseMemoryLibStm > > This will interfere with nothing else under MdePkg, so could hopefully > be merged today anyway. > > Would that be an acceptable compromise? I'd just like the tree to resume building. Doing anything at all under MdePkg will require MdePkg reviewers to ACK the changes, which will introduce delays. (Not because those reviewers are very slow, but because MdePkg changes are always scrutinized, rightfully, with a fine toothed comb.) I wouldn't mind reverting the initial series either, except that it'd require reviews for MdePkg and SecurityPkg changes... Same problem. I'm quite sad that Hao missed grepping the tree for all INF files with 'LIBRARY_CLASS *= *BaseMemoryLib', when posting the original stuff :( $ git grep -l 'LIBRARY_CLASS *= *BaseMemoryLib' -- '*inf' ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf <--------- MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf MdePkg/Library/BaseMemoryLibMmx/BaseMemoryLibMmx.inf MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf MdePkg/Library/BaseMemoryLibOptPei/BaseMemoryLibOptPei.inf MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf MdePkg/Library/BaseMemoryLibSse2/BaseMemoryLibSse2.inf MdePkg/Library/PeiMemoryLib/PeiMemoryLib.inf MdePkg/Library/UefiMemoryLib/UefiMemoryLib.inf Sigh. Thanks Laszlo > > Regards, > > Leif > >> For patches #2 and #3: >> >> Reviewed-by: Laszlo Ersek >> >> Can we please commit these patches today? >> >> Thanks, >> Laszlo >> >>> >>> / >>> Leif >>> >>>> Ard Biesheuvel (3): >>>> ArmPkg: remove BaseMemoryLibVstm implementation of BaseMemoryLib >>>> ArmPkg/BaseMemoryLibStm: implement new IsZeroGuid() API function >>>> ArmPkg/BaseMemoryLibStm: implement new IsZeroBuffer() API function >>>> >>>> ArmPkg/ArmPkg.dsc | 2 - >>>> ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf | 1 + >>>> ArmPkg/Library/{BaseMemoryLibVstm/ZeroMemWrapper.c => BaseMemoryLibStm/IsZeroBufferWrapper.c} | 28 ++- >>>> ArmPkg/Library/BaseMemoryLibStm/MemLibGeneric.c | 29 +++ >>>> ArmPkg/Library/BaseMemoryLibStm/MemLibGuid.c | 29 +++ >>>> ArmPkg/Library/BaseMemoryLibStm/MemLibInternals.h | 17 ++ >>>> ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.S | 112 --------- >>>> ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.asm | 114 --------- >>>> ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.S | 76 ------ >>>> ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.asm | 78 ------ >>>> ArmPkg/Library/BaseMemoryLibVstm/BaseMemoryLibVstm.inf | 70 ------ >>>> ArmPkg/Library/BaseMemoryLibVstm/CompareMemWrapper.c | 66 ----- >>>> ArmPkg/Library/BaseMemoryLibVstm/CopyMem.c | 62 ----- >>>> ArmPkg/Library/BaseMemoryLibVstm/CopyMemWrapper.c | 63 ----- >>>> ArmPkg/Library/BaseMemoryLibVstm/MemLibGeneric.c | 264 -------------------- >>>> ArmPkg/Library/BaseMemoryLibVstm/MemLibGuid.c | 132 ---------- >>>> ArmPkg/Library/BaseMemoryLibVstm/MemLibInternals.h | 234 ----------------- >>>> ArmPkg/Library/BaseMemoryLibVstm/ScanMem16Wrapper.c | 67 ----- >>>> ArmPkg/Library/BaseMemoryLibVstm/ScanMem32Wrapper.c | 66 ----- >>>> ArmPkg/Library/BaseMemoryLibVstm/ScanMem64Wrapper.c | 67 ----- >>>> ArmPkg/Library/BaseMemoryLibVstm/ScanMem8Wrapper.c | 99 -------- >>>> ArmPkg/Library/BaseMemoryLibVstm/SetMem.c | 53 ---- >>>> ArmPkg/Library/BaseMemoryLibVstm/SetMem16Wrapper.c | 64 ----- >>>> ArmPkg/Library/BaseMemoryLibVstm/SetMem32Wrapper.c | 64 ----- >>>> ArmPkg/Library/BaseMemoryLibVstm/SetMem64Wrapper.c | 64 ----- >>>> ArmPkg/Library/BaseMemoryLibVstm/SetMemWrapper.c | 91 ------- >>>> 26 files changed, 91 insertions(+), 1921 deletions(-) >>>> rename ArmPkg/Library/{BaseMemoryLibVstm/ZeroMemWrapper.c => BaseMemoryLibStm/IsZeroBufferWrapper.c} (53%) >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.S >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.asm >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.S >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.asm >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/BaseMemoryLibVstm.inf >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/CompareMemWrapper.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/CopyMem.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/CopyMemWrapper.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/MemLibGeneric.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/MemLibGuid.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/MemLibInternals.h >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem16Wrapper.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem32Wrapper.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem64Wrapper.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem8Wrapper.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMem.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMem16Wrapper.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMem32Wrapper.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMem64Wrapper.c >>>> delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMemWrapper.c >>>> >>>> -- >>>> 2.7.4 >>>> >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel >>> >>