public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel@lists.01.org, Laszlo Ersek <lersek@redhat.com>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Liming Gao <liming.gao@intel.com>
Subject: Re: [PATCH 1/3] MdePkg/Misc: Move ARM* BaseMemoryLibStm to MdePkg
Date: Fri, 2 Sep 2016 16:23:51 +0100	[thread overview]
Message-ID: <20160902152351.GB4715@bivouac.eciton.net> (raw)
In-Reply-To: <6C8DC7F6-2459-4CD6-A556-3B17A1FBBF5F@linaro.org>

On Fri, Sep 02, 2016 at 04:02:44PM +0100, Ard Biesheuvel wrote:
> > On 2 sep. 2016, at 15:29, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > 
> > Recent changes to the BaseMemoryLib implementations in MdePkg,
> > and other changes dependent on these, left all ARM* platforms
> > unbuildable. To avoid this sort of thing in the future, move
> > the ARM* BaseMemoryLib implementations to the same locations
> > as the other ones.
> 
> Can't we just get rid of the stm version, and move everything to the
> generic version? Copying all this crap into MdePkg does not seem
> like an improvement to me at all.

This isn't a proposed alternative to your patchset, this is an
alternative to reverting all of the other BaseMemoryLib changes.
I'm happy to throw it away next week if we can get something better in
by then.

> The AArch64 optdxe changes i proposed are arguably as harmless,

Yes, but that leaves ARM broken.

> since they don't affect any other arch either. And the stm aarch64
> versions are plain c to begin with

And I want to discuss that next week. Against the backdrop of a
working master branch.

/
    Leif

> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> > ---
> > ArmPkg/ArmPkg.dsc                                                | 1 -
> > ArmVirtPkg/ArmVirt.dsc.inc                                       | 2 +-
> > BeagleBoardPkg/BeagleBoardPkg.dsc                                | 2 +-
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/AArch64/CopyMem.c    | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/AArch64/SetMem.c     | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/Arm/CopyMem.S        | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/Arm/CopyMem.asm      | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/Arm/SetMem.S         | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/Arm/SetMem.asm       | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/CompareMemWrapper.c  | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/CopyMem.c            | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/CopyMemWrapper.c     | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/MemLibGeneric.c      | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/MemLibGuid.c         | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/MemLibInternals.h    | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/ScanMem16Wrapper.c   | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/ScanMem32Wrapper.c   | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/ScanMem64Wrapper.c   | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/ScanMem8Wrapper.c    | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/SetMem.c             | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/SetMem16Wrapper.c    | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/SetMem32Wrapper.c    | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/SetMem64Wrapper.c    | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/SetMemWrapper.c      | 0
> > {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/ZeroMemWrapper.c     | 0
> > MdePkg/MdePkg.dsc                                                | 1 +
> > 27 files changed, 3 insertions(+), 3 deletions(-)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/AArch64/CopyMem.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/AArch64/SetMem.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/Arm/CopyMem.S (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/Arm/CopyMem.asm (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/Arm/SetMem.S (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/Arm/SetMem.asm (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/CompareMemWrapper.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/CopyMem.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/CopyMemWrapper.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/MemLibGeneric.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/MemLibGuid.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/MemLibInternals.h (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/ScanMem16Wrapper.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/ScanMem32Wrapper.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/ScanMem64Wrapper.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/ScanMem8Wrapper.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/SetMem.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/SetMem16Wrapper.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/SetMem32Wrapper.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/SetMem64Wrapper.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/SetMemWrapper.c (100%)
> > rename {ArmPkg => MdePkg}/Library/BaseMemoryLibStm/ZeroMemWrapper.c (100%)
> > 
> > diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> > index 6f9fc66..09cbb85 100644
> > --- a/ArmPkg/ArmPkg.dsc
> > +++ b/ArmPkg/ArmPkg.dsc
> > @@ -112,7 +112,6 @@ [Components.common]
> >   ArmPkg/Library/ArmDisassemblerLib/ArmDisassemblerLib.inf
> >   ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf
> >   ArmPkg/Library/ArmLib/Null/NullArmLib.inf
> > -  ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
> >   ArmPkg/Library/BdsLib/BdsLib.inf
> >   ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> >   ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
> > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> > index 6c6a52b..338c22c 100644
> > --- a/ArmVirtPkg/ArmVirt.dsc.inc
> > +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> > @@ -69,7 +69,7 @@ [LibraryClasses.common]
> > 
> >   # 1/123 faster than Stm or Vstm version
> >   #BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> > -  BaseMemoryLib|ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
> > +  BaseMemoryLib|MdePkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
> > 
> >   # Networking Requirements
> >   NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> > diff --git a/BeagleBoardPkg/BeagleBoardPkg.dsc b/BeagleBoardPkg/BeagleBoardPkg.dsc
> > index b4a645b..7bf7d52 100644
> > --- a/BeagleBoardPkg/BeagleBoardPkg.dsc
> > +++ b/BeagleBoardPkg/BeagleBoardPkg.dsc
> > @@ -55,7 +55,7 @@ [LibraryClasses.common]
> >   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> > 
> >   BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> > -  BaseMemoryLib|ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
> > +  BaseMemoryLib|MdePkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
> > 
> >   EfiResetSystemLib|BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.inf
> > 
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/AArch64/CopyMem.c b/MdePkg/Library/BaseMemoryLibStm/AArch64/CopyMem.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/AArch64/CopyMem.c
> > rename to MdePkg/Library/BaseMemoryLibStm/AArch64/CopyMem.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/AArch64/SetMem.c b/MdePkg/Library/BaseMemoryLibStm/AArch64/SetMem.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/AArch64/SetMem.c
> > rename to MdePkg/Library/BaseMemoryLibStm/AArch64/SetMem.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/Arm/CopyMem.S b/MdePkg/Library/BaseMemoryLibStm/Arm/CopyMem.S
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/Arm/CopyMem.S
> > rename to MdePkg/Library/BaseMemoryLibStm/Arm/CopyMem.S
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/Arm/CopyMem.asm b/MdePkg/Library/BaseMemoryLibStm/Arm/CopyMem.asm
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/Arm/CopyMem.asm
> > rename to MdePkg/Library/BaseMemoryLibStm/Arm/CopyMem.asm
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/Arm/SetMem.S b/MdePkg/Library/BaseMemoryLibStm/Arm/SetMem.S
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/Arm/SetMem.S
> > rename to MdePkg/Library/BaseMemoryLibStm/Arm/SetMem.S
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/Arm/SetMem.asm b/MdePkg/Library/BaseMemoryLibStm/Arm/SetMem.asm
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/Arm/SetMem.asm
> > rename to MdePkg/Library/BaseMemoryLibStm/Arm/SetMem.asm
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf b/MdePkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
> > rename to MdePkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/CompareMemWrapper.c b/MdePkg/Library/BaseMemoryLibStm/CompareMemWrapper.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/CompareMemWrapper.c
> > rename to MdePkg/Library/BaseMemoryLibStm/CompareMemWrapper.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/CopyMem.c b/MdePkg/Library/BaseMemoryLibStm/CopyMem.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/CopyMem.c
> > rename to MdePkg/Library/BaseMemoryLibStm/CopyMem.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/CopyMemWrapper.c b/MdePkg/Library/BaseMemoryLibStm/CopyMemWrapper.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/CopyMemWrapper.c
> > rename to MdePkg/Library/BaseMemoryLibStm/CopyMemWrapper.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/MemLibGeneric.c b/MdePkg/Library/BaseMemoryLibStm/MemLibGeneric.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/MemLibGeneric.c
> > rename to MdePkg/Library/BaseMemoryLibStm/MemLibGeneric.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/MemLibGuid.c b/MdePkg/Library/BaseMemoryLibStm/MemLibGuid.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/MemLibGuid.c
> > rename to MdePkg/Library/BaseMemoryLibStm/MemLibGuid.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/MemLibInternals.h b/MdePkg/Library/BaseMemoryLibStm/MemLibInternals.h
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/MemLibInternals.h
> > rename to MdePkg/Library/BaseMemoryLibStm/MemLibInternals.h
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/ScanMem16Wrapper.c b/MdePkg/Library/BaseMemoryLibStm/ScanMem16Wrapper.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/ScanMem16Wrapper.c
> > rename to MdePkg/Library/BaseMemoryLibStm/ScanMem16Wrapper.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/ScanMem32Wrapper.c b/MdePkg/Library/BaseMemoryLibStm/ScanMem32Wrapper.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/ScanMem32Wrapper.c
> > rename to MdePkg/Library/BaseMemoryLibStm/ScanMem32Wrapper.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/ScanMem64Wrapper.c b/MdePkg/Library/BaseMemoryLibStm/ScanMem64Wrapper.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/ScanMem64Wrapper.c
> > rename to MdePkg/Library/BaseMemoryLibStm/ScanMem64Wrapper.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/ScanMem8Wrapper.c b/MdePkg/Library/BaseMemoryLibStm/ScanMem8Wrapper.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/ScanMem8Wrapper.c
> > rename to MdePkg/Library/BaseMemoryLibStm/ScanMem8Wrapper.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/SetMem.c b/MdePkg/Library/BaseMemoryLibStm/SetMem.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/SetMem.c
> > rename to MdePkg/Library/BaseMemoryLibStm/SetMem.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/SetMem16Wrapper.c b/MdePkg/Library/BaseMemoryLibStm/SetMem16Wrapper.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/SetMem16Wrapper.c
> > rename to MdePkg/Library/BaseMemoryLibStm/SetMem16Wrapper.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/SetMem32Wrapper.c b/MdePkg/Library/BaseMemoryLibStm/SetMem32Wrapper.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/SetMem32Wrapper.c
> > rename to MdePkg/Library/BaseMemoryLibStm/SetMem32Wrapper.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/SetMem64Wrapper.c b/MdePkg/Library/BaseMemoryLibStm/SetMem64Wrapper.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/SetMem64Wrapper.c
> > rename to MdePkg/Library/BaseMemoryLibStm/SetMem64Wrapper.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/SetMemWrapper.c b/MdePkg/Library/BaseMemoryLibStm/SetMemWrapper.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/SetMemWrapper.c
> > rename to MdePkg/Library/BaseMemoryLibStm/SetMemWrapper.c
> > diff --git a/ArmPkg/Library/BaseMemoryLibStm/ZeroMemWrapper.c b/MdePkg/Library/BaseMemoryLibStm/ZeroMemWrapper.c
> > similarity index 100%
> > rename from ArmPkg/Library/BaseMemoryLibStm/ZeroMemWrapper.c
> > rename to MdePkg/Library/BaseMemoryLibStm/ZeroMemWrapper.c
> > diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
> > index ab57689..75a8f1a 100644
> > --- a/MdePkg/MdePkg.dsc
> > +++ b/MdePkg/MdePkg.dsc
> > @@ -177,6 +177,7 @@ [Components.EBC]
> >   MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> > 
> > [Components.ARM, Components.AARCH64]
> > +  MdePkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
> >   MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> > 
> > [BuildOptions]
> > -- 
> > 2.9.3
> > 


  reply	other threads:[~2016-09-02 15:23 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-02 14:29 [PATCH 0/3] MdePkg/Misc: fix Arm BaseMemoryLib Leif Lindholm
2016-09-02 14:29 ` [PATCH 1/3] MdePkg/Misc: Move ARM* BaseMemoryLibStm to MdePkg Leif Lindholm
2016-09-02 15:02   ` Ard Biesheuvel
2016-09-02 15:23     ` Leif Lindholm [this message]
2016-09-02 16:07       ` Ard Biesheuvel
2016-09-02 18:05         ` Leif Lindholm
2016-09-02 18:11           ` Ard Biesheuvel
2016-09-02 18:32             ` Leif Lindholm
2016-09-02 18:45               ` Ard Biesheuvel
2016-09-02 19:18                 ` Leif Lindholm
2016-09-02 19:24                   ` Laszlo Ersek
2016-09-02 14:29 ` [PATCH 2/3] MdePkg/BaseMemoryLibStm: implement new IsZeroGuid() API function Leif Lindholm
2016-09-02 14:29 ` [PATCH 3/3] MdePkg/BaseMemoryLibStm: implement new IsZeroBuffer() " Leif Lindholm
2016-09-02 14:48 ` [PATCH 0/3] MdePkg/Misc: fix Arm BaseMemoryLib Laszlo Ersek
2016-09-02 14:53   ` Laszlo Ersek
2016-09-02 16:02     ` Leif Lindholm
2016-09-02 15:49 ` Kinney, Michael D
2016-09-02 15:57   ` Leif Lindholm
2016-09-02 15:59     ` Andrew Fish

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160902152351.GB4715@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox