[PATCH] MdeModulePkg: add ARM/AARCH64 requirements to .dsc

[PATCH] MdeModulePkg: add ARM/AARCH64 requirements to .dsc

[Leif Lindholm]

Some LibraryClasses entries are missing to enable standalone builds
of MdeModulePkg components for ARM/AARCH64. Add those.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---

Tying back to the conversation in
https://lists.01.org/pipermail/edk2-devel/2016-August/000733.html

If we are not going to refactor the DxeIpl code, would this be an
acceptable fix to enable standalone HelloWorld builds on ARM/AARCH64?

BaseStackCheckLib is unrelated to the DxeIpl issue, but needed in
practice on ARM and in theory on AARCH64.

 MdeModulePkg/MdeModulePkg.dsc | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 05120c7..3eb55db 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -160,6 +160,9 @@ [LibraryClasses.common.UEFI_APPLICATION]
   DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
 
 [LibraryClasses.ARM, LibraryClasses.AARCH64]
+  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
+  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+
   #
   # It is not possible to prevent ARM compiler calls to generic intrinsic functions.
   # This library provides the instrinsic functions generated by a given compiler.
@@ -167,6 +170,12 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
   #
   NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
 
+  #
+  # Since software stack checking may be heuristically enabled by the compiler
+  # include BaseStackCheckLib unconditionally.
+  #
+  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
+
 [LibraryClasses.EBC]
   LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
 
-- 
2.9.3

Re: [PATCH] MdeModulePkg: add ARM/AARCH64 requirements to .dsc

[Ard Biesheuvel]

On 26 September 2016 at 18:15, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> Some LibraryClasses entries are missing to enable standalone builds
> of MdeModulePkg components for ARM/AARCH64. Add those.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>
> Tying back to the conversation in
> https://lists.01.org/pipermail/edk2-devel/2016-August/000733.html
>
> If we are not going to refactor the DxeIpl code, would this be an
> acceptable fix to enable standalone HelloWorld builds on ARM/AARCH64?
>
> BaseStackCheckLib is unrelated to the DxeIpl issue, but needed in
> practice on ARM and in theory on AARCH64.
>
>  MdeModulePkg/MdeModulePkg.dsc | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
> index 05120c7..3eb55db 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -160,6 +160,9 @@ [LibraryClasses.common.UEFI_APPLICATION]
>    DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
>
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> +  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +
>    #
>    # It is not possible to prevent ARM compiler calls to generic intrinsic functions.
>    # This library provides the instrinsic functions generated by a given compiler.
> @@ -167,6 +170,12 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
>    #
>    NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
>
> +  #
> +  # Since software stack checking may be heuristically enabled by the compiler
> +  # include BaseStackCheckLib unconditionally.
> +  #
> +  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> +
>  [LibraryClasses.EBC]
>    LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
>
> --
> 2.9.3
>

Re: [PATCH] MdeModulePkg: add ARM/AARCH64 requirements to .dsc

[Leif Lindholm]

Andrew, Mike - are you OK with me committing this?

On 27 September 2016 at 03:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 26 September 2016 at 18:15, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> Some LibraryClasses entries are missing to enable standalone builds
>> of MdeModulePkg components for ARM/AARCH64. Add those.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
>> ---
>>
>> Tying back to the conversation in
>> https://lists.01.org/pipermail/edk2-devel/2016-August/000733.html
>>
>> If we are not going to refactor the DxeIpl code, would this be an
>> acceptable fix to enable standalone HelloWorld builds on ARM/AARCH64?
>>
>> BaseStackCheckLib is unrelated to the DxeIpl issue, but needed in
>> practice on ARM and in theory on AARCH64.
>>
>>  MdeModulePkg/MdeModulePkg.dsc | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
>> index 05120c7..3eb55db 100644
>> --- a/MdeModulePkg/MdeModulePkg.dsc
>> +++ b/MdeModulePkg/MdeModulePkg.dsc
>> @@ -160,6 +160,9 @@ [LibraryClasses.common.UEFI_APPLICATION]
>>    DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
>>
>>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
>> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
>> +  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
>> +
>>    #
>>    # It is not possible to prevent ARM compiler calls to generic intrinsic functions.
>>    # This library provides the instrinsic functions generated by a given compiler.
>> @@ -167,6 +170,12 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
>>    #
>>    NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
>>
>> +  #
>> +  # Since software stack checking may be heuristically enabled by the compiler
>> +  # include BaseStackCheckLib unconditionally.
>> +  #
>> +  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
>> +
>>  [LibraryClasses.EBC]
>>    LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
>>
>> --
>> 2.9.3
>>

Re: [PATCH] MdeModulePkg: add ARM/AARCH64 requirements to .dsc

[Kinney, Michael D]

Leif,

Yes.  Please commit.

I see this was rb 3 weeks ago, and was not picked up by the MdeModulePkg maintainers.

Are you aware of any tools to help us notice if a commit like this is missed so
we can ping the maintainers?

Mike

> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Wednesday, October 19, 2016 11:58 PM
> To: Andrew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: edk2-devel-01 <edk2-devel@lists.01.org>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: Re: [PATCH] MdeModulePkg: add ARM/AARCH64 requirements to .dsc
> 
> Andrew, Mike - are you OK with me committing this?
> 
> On 27 September 2016 at 03:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 26 September 2016 at 18:15, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> Some LibraryClasses entries are missing to enable standalone builds
> >> of MdeModulePkg components for ARM/AARCH64. Add those.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
> > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> >> ---
> >>
> >> Tying back to the conversation in
> >> https://lists.01.org/pipermail/edk2-devel/2016-August/000733.html
> >>
> >> If we are not going to refactor the DxeIpl code, would this be an
> >> acceptable fix to enable standalone HelloWorld builds on ARM/AARCH64?
> >>
> >> BaseStackCheckLib is unrelated to the DxeIpl issue, but needed in
> >> practice on ARM and in theory on AARCH64.
> >>
> >>  MdeModulePkg/MdeModulePkg.dsc | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
> >> index 05120c7..3eb55db 100644
> >> --- a/MdeModulePkg/MdeModulePkg.dsc
> >> +++ b/MdeModulePkg/MdeModulePkg.dsc
> >> @@ -160,6 +160,9 @@ [LibraryClasses.common.UEFI_APPLICATION]
> >>    DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
> >>
> >>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> >> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> >> +  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> >> +
> >>    #
> >>    # It is not possible to prevent ARM compiler calls to generic intrinsic
> functions.
> >>    # This library provides the instrinsic functions generated by a given compiler.
> >> @@ -167,6 +170,12 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
> >>    #
> >>    NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> >>
> >> +  #
> >> +  # Since software stack checking may be heuristically enabled by the compiler
> >> +  # include BaseStackCheckLib unconditionally.
> >> +  #
> >> +  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> >> +
> >>  [LibraryClasses.EBC]
> >>    LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> >>
> >> --
> >> 2.9.3
> >>

Re: [PATCH] MdeModulePkg: add ARM/AARCH64 requirements to .dsc

[Leif Lindholm]

On 19 October 2016 at 23:56, Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
> Yes.  Please commit.

Thanks - pushed as b752e8a3fb685ae04155bf6a34a9aac83810613f

> I see this was rb 3 weeks ago, and was not picked up by the MdeModulePkg maintainers.
>
> Are you aware of any tools to help us notice if a commit like this is missed so
> we can ping the maintainers?

Not really. It'd be tricky to pick up on which patchsets should go in
and which shouldn't.
Although I'd be happy to hear suggestions from others for tools to
help maintainers keep track.

/
    Leif

> Mike
>
>> -----Original Message-----
>> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
>> Sent: Wednesday, October 19, 2016 11:58 PM
>> To: Andrew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
>> Cc: edk2-devel-01 <edk2-devel@lists.01.org>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star
>> <star.zeng@intel.com>
>> Subject: Re: [PATCH] MdeModulePkg: add ARM/AARCH64 requirements to .dsc
>>
>> Andrew, Mike - are you OK with me committing this?
>>
>> On 27 September 2016 at 03:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > On 26 September 2016 at 18:15, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> >> Some LibraryClasses entries are missing to enable standalone builds
>> >> of MdeModulePkg components for ARM/AARCH64. Add those.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.0
>> >> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>> >
>> > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >
>> >> ---
>> >>
>> >> Tying back to the conversation in
>> >> https://lists.01.org/pipermail/edk2-devel/2016-August/000733.html
>> >>
>> >> If we are not going to refactor the DxeIpl code, would this be an
>> >> acceptable fix to enable standalone HelloWorld builds on ARM/AARCH64?
>> >>
>> >> BaseStackCheckLib is unrelated to the DxeIpl issue, but needed in
>> >> practice on ARM and in theory on AARCH64.
>> >>
>> >>  MdeModulePkg/MdeModulePkg.dsc | 9 +++++++++
>> >>  1 file changed, 9 insertions(+)
>> >>
>> >> diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
>> >> index 05120c7..3eb55db 100644
>> >> --- a/MdeModulePkg/MdeModulePkg.dsc
>> >> +++ b/MdeModulePkg/MdeModulePkg.dsc
>> >> @@ -160,6 +160,9 @@ [LibraryClasses.common.UEFI_APPLICATION]
>> >>    DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
>> >>
>> >>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
>> >> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
>> >> +  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
>> >> +
>> >>    #
>> >>    # It is not possible to prevent ARM compiler calls to generic intrinsic
>> functions.
>> >>    # This library provides the instrinsic functions generated by a given compiler.
>> >> @@ -167,6 +170,12 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
>> >>    #
>> >>    NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
>> >>
>> >> +  #
>> >> +  # Since software stack checking may be heuristically enabled by the compiler
>> >> +  # include BaseStackCheckLib unconditionally.
>> >> +  #
>> >> +  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
>> >> +
>> >>  [LibraryClasses.EBC]
>> >>    LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
>> >>
>> >> --
>> >> 2.9.3
>> >>

Re: [PATCH] MdeModulePkg: add ARM/AARCH64 requirements to .dsc

[Laszlo Ersek]

On 10/20/16 10:17, Leif Lindholm wrote:
> On 19 October 2016 at 23:56, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
>> Yes.  Please commit.
> 
> Thanks - pushed as b752e8a3fb685ae04155bf6a34a9aac83810613f
> 
>> I see this was rb 3 weeks ago, and was not picked up by the MdeModulePkg maintainers.
>>
>> Are you aware of any tools to help us notice if a commit like this is missed so
>> we can ping the maintainers?
> 
> Not really. It'd be tricky to pick up on which patchsets should go in
> and which shouldn't.
> Although I'd be happy to hear suggestions from others for tools to
> help maintainers keep track.

Wait, is someone asking for my opinion? :)

So here's how I track patches. I use two methods that have worked for me
over a long time. I'm also aware of a third method.

The third method is running a patchwork server. I've never seen this
method work outside of Red Hat, that is, a downstream environment where
some people are actually tasked with handling patches. Quite a few
upstream projects use patchwork, but none I know does a great job with
it. I could be wrong, so corrections are welcome -- I'm not naming
project names here on purpose.


The first method that I personally use is starring / tagging / marking
emails in Thunderbird. It works exceedingly well. I can track patches
that I have to review / commit, patches I posted and wait for review
for, and any kind of discussion really. I can (and do) easily track
messages that are older than 6 months. Thunderbird has a good local
metadata search feature, so I can quickly round up all my tagged
messages, sort them based on this or that column (like date or mailing
list folder), and choose what needs kicking or work.

In a partly reviewed series, Thunderbird tags (or the IMAP "F" flag I
think) can be applied on the patch level as well.


The second method is Bugzilla (obviously). Bugzilla has awesome searches
(saved searches as well). People just need some discipline in using it:

- report bugs for issues (might be overkill sometimes, but it varies
with the attention threshold of the people of the specific project what
issues permanent tracker BZs should be reported for)

- religiously cross-link mailing list threads with the BZ -- if a new
thread that is related to the issue is started on the list, immediately
follow up with the BZ URL on-list, *plus* add a new BZ comment with the
URL of the thread starter (from the mailing list archive)

- whenever a patch or patch series is posted, be sure to reference the
BZ (by URL) in the commit message (Ref: ... or Fixes: ... or Bugzilla:
...), and equally importantly, link the blurb message of the series into
the BZ as well, in a new comment

- When the series is committed & pushed, capture the commit range in the
BZ, in a new comment, before the BZ is closed. (Also, close the BZ.)

For both methods, get into the habit of searching your mailbox for
tagged messages every week (at the rarest) and running your saved
Bugzilla searches. Send reminders as necessary.


The most important thing about both methods is that incoming email
("events") have to be triaged immediately. It is not necessary to act
upon the final R-b immediately when it arrives -- that is, when that R-b
appears, the maintainer need not commit the patch or series immediately.
He or she *does* have to update the status in his mailbox or in Bugzilla
immediately, so that later searches can turn up the ready-to-commit
series. Think of this as hardirq vs. softirq (bottom half): you need to
handle new messages in your mailbox with a hardirq handler, for triaging
and tagging. The actual work can happen later, in the softirq handler.
If the hardirq handler drops interrupts, there's nothing for the softirq
handler to act upon later.

("you" == "generic you", obviously)

Thunderbird, Bugzilla and Patchwork are clearly not the only possible
tools, but tooling does make a difference -- both tooling and discipline
are necessary. For example, GitHub, Outlook, GMail are all quite
inappropriate for mailing list-based distributed development. (You could
argue that many people are drawn to GitHub's web-based proprietary
workflow -- or I could mention Gerrit to I guess? -- exactly because
their crappy mail user agents don't let them interact with each other
sensibly on lists.)

... Thanks, this felt good. ;)

Laszlo