From: Laszlo Ersek <lersek@redhat.com>
To: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
edk2-devel@ml01.01.org, "Wu, Hao A" <hao.a.wu@intel.com>
Subject: Re: [PATCH 0/3] ArmPkg: introduce IsZeroGuid() and IsZeroBuffer()
Date: Fri, 2 Sep 2016 16:18:11 +0200 [thread overview]
Message-ID: <9addcb0f-84ac-b572-3507-85678ece2342@redhat.com> (raw)
In-Reply-To: <20160902130128.GY4715@bivouac.eciton.net>
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 <lersek@redhat.com>
>>
>> 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
>>>
>>
next prev parent reply other threads:[~2016-09-02 14:18 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-31 9:07 [PATCH 0/3] ArmPkg: introduce IsZeroGuid() and IsZeroBuffer() Ard Biesheuvel
2016-08-31 9:07 ` [PATCH 1/3] ArmPkg: remove BaseMemoryLibVstm implementation of BaseMemoryLib Ard Biesheuvel
2016-09-01 12:58 ` Leif Lindholm
2016-08-31 9:07 ` [PATCH 2/3] ArmPkg/BaseMemoryLibStm: implement new IsZeroGuid() API function Ard Biesheuvel
2016-08-31 9:07 ` [PATCH 3/3] ArmPkg/BaseMemoryLibStm: implement new IsZeroBuffer() " Ard Biesheuvel
2016-08-31 12:48 ` [PATCH 0/3] ArmPkg: introduce IsZeroGuid() and IsZeroBuffer() Leif Lindholm
2016-09-02 11:33 ` Laszlo Ersek
2016-09-02 13:01 ` Leif Lindholm
2016-09-02 14:18 ` Laszlo Ersek [this message]
2016-09-02 14:40 ` Leif Lindholm
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=9addcb0f-84ac-b572-3507-85678ece2342@redhat.com \
--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