[edk2] [PATCH] ArmVirtPkg: increase memory preallocations to reduce region count

Discussion:

Ard Biesheuvel

2015-06-03 14:49:14 UTC

This updates the sizes of the preallocated regions so that the number
of distinct regions that exists is minimal at the time we exit boot
services.

For a typical run of the ArmVirtQemu platform, we get the following
utilization numbers:

Reserved : 4 Pages (16,384 Bytes)
LoaderCode: 210 Pages (860,160 Bytes)
LoaderData: 0 Pages (0 Bytes)
BS_Code : 355 Pages (1,454,080 Bytes)
BS_Data : 6,807 Pages (27,881,472 Bytes)
RT_Code : 112 Pages (458,752 Bytes)
RT_Data : 288 Pages (1,179,648 Bytes)
ACPI_Recl : 32 Pages (131,072 Bytes)
ACPI_NVS : 0 Pages (0 Bytes)
MMIO : 16,385 Pages (67,112,960 Bytes)
MMIO_Port : 0 Pages (0 Bytes)
PalCode : 0 Pages (0 Bytes)
Available : 123,264 Pages (504,889,344 Bytes)
--------------
Total Memory: 511 MB (536,854,528 Bytes)

Strangely enough, the allocation count of 20,000 pages for BS_Data
does not result in those regions being merged. For BS_Code, RT_Code
and RT_Data, the increased preallocation results in the following
reduction in the number of regions.

0x000040000000-0x00004000ffff [Loader Data | | | | | |WB|WT|WC|UC]
- 0x000040010000-0x0000b66bbfff [Conventional Memory| | | | | |WB|WT|WC|UC]
- 0x0000b66bc000-0x0000b66d0fff [Loader Data | | | | | |WB|WT|WC|UC]
- 0x0000b66d1000-0x0000b6f6bfff [Loader Code | | | | | |WB|WT|WC|UC]
- 0x0000b6f6c000-0x0000b6f6ffff [Reserved | | | | | |WB|WT|WC|UC]*
- 0x0000b6f70000-0x0000b6f8ffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
- 0x0000b6f90000-0x0000b6faffff [ACPI Reclaim Memory| | | | | |WB|WT|WC|UC]*
- 0x0000b6fb0000-0x0000b6fcffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
- 0x0000b6fd0000-0x0000b701ffff [Runtime Data |RUN| | | | |WB|WT|WC|UC]*
- 0x0000b7020000-0x0000b702ffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
- 0x0000b7030000-0x0000b70cffff [Runtime Data |RUN| | | | |WB|WT|WC|UC]*
- 0x0000b70d0000-0x0000b70dffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
+ 0x000040010000-0x0000b680bfff [Conventional Memory| | | | | |WB|WT|WC|UC]
+ 0x0000b680c000-0x0000b6820fff [Loader Data | | | | | |WB|WT|WC|UC]
+ 0x0000b6821000-0x0000b70bbfff [Loader Code | | | | | |WB|WT|WC|UC]
+ 0x0000b70bc000-0x0000b70bffff [Reserved | | | | | |WB|WT|WC|UC]*
+ 0x0000b70c0000-0x0000b70dffff [ACPI Reclaim Memory| | | | | |WB|WT|WC|UC]*
0x0000b70e0000-0x0000b9f87fff [Conventional Memory| | | | | |WB|WT|WC|UC]
0x0000b9f88000-0x0000bb921fff [Boot Data | | | | | |WB|WT|WC|UC]
0x0000bb922000-0x0000bb9bffff [Conventional Memory| | | | | |WB|WT|WC|UC]
0x0000bb9c0000-0x0000bbbb0fff [Boot Data | | | | | |WB|WT|WC|UC]
0x0000bbbb1000-0x0000bbbeffff [Conventional Memory| | | | | |WB|WT|WC|UC]
0x0000bbbf0000-0x0000bbf1ffff [Boot Data | | | | | |WB|WT|WC|UC]
- 0x0000bbf20000-0x0000bedfffff [Conventional Memory| | | | | |WB|WT|WC|UC]
- 0x0000bee00000-0x0000bf71ffff [Loader Data | | | | | |WB|WT|WC|UC]
- 0x0000bf720000-0x0000bf7ccfff [Conventional Memory| | | | | |WB|WT|WC|UC]
- 0x0000bf7cd000-0x0000bf92ffff [Boot Code | | | | | |WB|WT|WC|UC]
- 0x0000bf930000-0x0000bf93ffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
- 0x0000bf940000-0x0000bf95ffff [Conventional Memory| | | | | |WB|WT|WC|UC]
- 0x0000bf960000-0x0000bf97ffff [Runtime Data |RUN| | | | |WB|WT|WC|UC]*
+ 0x0000bbf20000-0x0000bebfffff [Conventional Memory| | | | | |WB|WT|WC|UC]
+ 0x0000bec00000-0x0000bf51ffff [Loader Data | | | | | |WB|WT|WC|UC]
+ 0x0000bf520000-0x0000bf65cfff [Conventional Memory| | | | | |WB|WT|WC|UC]
+ 0x0000bf65d000-0x0000bf7bffff [Boot Code | | | | | |WB|WT|WC|UC]
+ 0x0000bf7c0000-0x0000bf84ffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
+ 0x0000bf850000-0x0000bf86ffff [Conventional Memory| | | | | |WB|WT|WC|UC]
+ 0x0000bf870000-0x0000bf97ffff [Runtime Data |RUN| | | | |WB|WT|WC|UC]*
0x0000bf980000-0x0000bf997fff [Conventional Memory| | | | | |WB|WT|WC|UC]
0x0000bf998000-0x0000bf99afff [Boot Data | | | | | |WB|WT|WC|UC]
0x0000bf99b000-0x0000bf9aefff [Conventional Memory| | | | | |WB|WT|WC|UC]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <***@linaro.org>
---
ArmVirtPkg/ArmVirt.dsc.inc | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index fd20ff39a068..08fb18a0c11a 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -330,9 +330,9 @@
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType|0
- gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|50
- gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|20
- gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode|400
+ gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|300
+ gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|150
+ gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode|1000
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData|20000
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode|20
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData|0

--
1.9.1

------------------------------------------------------------------------------

Laszlo Ersek

2015-06-03 15:40:03 UTC

Permalink

Post by Ard Biesheuvel
This updates the sizes of the preallocated regions so that the number
of distinct regions that exists is minimal at the time we exit boot
services.
For a typical run of the ArmVirtQemu platform, we get the following
Reserved : 4 Pages (16,384 Bytes)
LoaderCode: 210 Pages (860,160 Bytes)
LoaderData: 0 Pages (0 Bytes)
BS_Code : 355 Pages (1,454,080 Bytes)
BS_Data : 6,807 Pages (27,881,472 Bytes)
RT_Code : 112 Pages (458,752 Bytes)
RT_Data : 288 Pages (1,179,648 Bytes)
ACPI_Recl : 32 Pages (131,072 Bytes)
ACPI_NVS : 0 Pages (0 Bytes)
MMIO : 16,385 Pages (67,112,960 Bytes)
MMIO_Port : 0 Pages (0 Bytes)
PalCode : 0 Pages (0 Bytes)
Available : 123,264 Pages (504,889,344 Bytes)
--------------
Total Memory: 511 MB (536,854,528 Bytes)
Strangely enough, the allocation count of 20,000 pages for BS_Data
does not result in those regions being merged. For BS_Code, RT_Code
and RT_Data, the increased preallocation results in the following
reduction in the number of regions.
0x000040000000-0x00004000ffff [Loader Data | | | | | |WB|WT|WC|UC]
- 0x000040010000-0x0000b66bbfff [Conventional Memory| | | | | |WB|WT|WC|UC]
- 0x0000b66bc000-0x0000b66d0fff [Loader Data | | | | | |WB|WT|WC|UC]
- 0x0000b66d1000-0x0000b6f6bfff [Loader Code | | | | | |WB|WT|WC|UC]
- 0x0000b6f6c000-0x0000b6f6ffff [Reserved | | | | | |WB|WT|WC|UC]*
- 0x0000b6f70000-0x0000b6f8ffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
- 0x0000b6f90000-0x0000b6faffff [ACPI Reclaim Memory| | | | | |WB|WT|WC|UC]*
- 0x0000b6fb0000-0x0000b6fcffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
- 0x0000b6fd0000-0x0000b701ffff [Runtime Data |RUN| | | | |WB|WT|WC|UC]*
- 0x0000b7020000-0x0000b702ffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
- 0x0000b7030000-0x0000b70cffff [Runtime Data |RUN| | | | |WB|WT|WC|UC]*
- 0x0000b70d0000-0x0000b70dffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
+ 0x000040010000-0x0000b680bfff [Conventional Memory| | | | | |WB|WT|WC|UC]
+ 0x0000b680c000-0x0000b6820fff [Loader Data | | | | | |WB|WT|WC|UC]
+ 0x0000b6821000-0x0000b70bbfff [Loader Code | | | | | |WB|WT|WC|UC]
+ 0x0000b70bc000-0x0000b70bffff [Reserved | | | | | |WB|WT|WC|UC]*
+ 0x0000b70c0000-0x0000b70dffff [ACPI Reclaim Memory| | | | | |WB|WT|WC|UC]*
0x0000b70e0000-0x0000b9f87fff [Conventional Memory| | | | | |WB|WT|WC|UC]
0x0000b9f88000-0x0000bb921fff [Boot Data | | | | | |WB|WT|WC|UC]
0x0000bb922000-0x0000bb9bffff [Conventional Memory| | | | | |WB|WT|WC|UC]
0x0000bb9c0000-0x0000bbbb0fff [Boot Data | | | | | |WB|WT|WC|UC]
0x0000bbbb1000-0x0000bbbeffff [Conventional Memory| | | | | |WB|WT|WC|UC]
0x0000bbbf0000-0x0000bbf1ffff [Boot Data | | | | | |WB|WT|WC|UC]
- 0x0000bbf20000-0x0000bedfffff [Conventional Memory| | | | | |WB|WT|WC|UC]
- 0x0000bee00000-0x0000bf71ffff [Loader Data | | | | | |WB|WT|WC|UC]
- 0x0000bf720000-0x0000bf7ccfff [Conventional Memory| | | | | |WB|WT|WC|UC]
- 0x0000bf7cd000-0x0000bf92ffff [Boot Code | | | | | |WB|WT|WC|UC]
- 0x0000bf930000-0x0000bf93ffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
- 0x0000bf940000-0x0000bf95ffff [Conventional Memory| | | | | |WB|WT|WC|UC]
- 0x0000bf960000-0x0000bf97ffff [Runtime Data |RUN| | | | |WB|WT|WC|UC]*
+ 0x0000bbf20000-0x0000bebfffff [Conventional Memory| | | | | |WB|WT|WC|UC]
+ 0x0000bec00000-0x0000bf51ffff [Loader Data | | | | | |WB|WT|WC|UC]
+ 0x0000bf520000-0x0000bf65cfff [Conventional Memory| | | | | |WB|WT|WC|UC]
+ 0x0000bf65d000-0x0000bf7bffff [Boot Code | | | | | |WB|WT|WC|UC]
+ 0x0000bf7c0000-0x0000bf84ffff [Runtime Code |RUN| | | | |WB|WT|WC|UC]*
+ 0x0000bf850000-0x0000bf86ffff [Conventional Memory| | | | | |WB|WT|WC|UC]
+ 0x0000bf870000-0x0000bf97ffff [Runtime Data |RUN| | | | |WB|WT|WC|UC]*
0x0000bf980000-0x0000bf997fff [Conventional Memory| | | | | |WB|WT|WC|UC]
0x0000bf998000-0x0000bf99afff [Boot Data | | | | | |WB|WT|WC|UC]
0x0000bf99b000-0x0000bf9aefff [Conventional Memory| | | | | |WB|WT|WC|UC]

Heh, this output was produced by kernel code that I wrote. (I've written
negligible amounts of upstream kernel code.) Good way to sell me this
patch! :)

Post by Ard Biesheuvel
Contributed-under: TianoCore Contribution Agreement 1.0
---
ArmVirtPkg/ArmVirt.dsc.inc | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index fd20ff39a068..08fb18a0c11a 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -330,9 +330,9 @@
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType|0
- gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|50
- gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|20
- gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode|400
+ gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|300
+ gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|150
+ gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode|1000
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData|20000
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode|20
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData|0

Reviewed-by: Laszlo Ersek <***@redhat.com>

------------------------------------------------------------------------------

Ard Biesheuvel

2015-06-03 16:14:54 UTC

Permalink

Post by Laszlo Ersek

Heh, this output was produced by kernel code that I wrote. (I've written
negligible amounts of upstream kernel code.) Good way to sell me this
patch! :)

Thanks, pushed as r17554

--
Ard.

------------------------------------------------------------------------------

Laszlo Ersek

2015-06-03 17:03:10 UTC

Permalink

Post by Ard Biesheuvel
Thanks, pushed as r17554

Congrats to your first (am I right?) SVN commit!

A small tip (feel free to ignore it): since for edk2 patches, the
tianocore contribution agreement is needed, it's useful to set up a
commit message template. Something like:

[commit]
template = /home/ardb/misc/tianocore.template

where the referenced file would say:

--------------

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <***@linaro.org>
-----------------

The "trick" I'm proposing here is that you include your S-o-b with the
template as well, since you "should" have a template anyway. (Or else,
pass --signoff to "git-commit".)

Additionally, ask git-format-patch not to generate your S-o-b's on the fly:

[format]
signoff = false

Your posted patches will look the same, but the difference will be that
your commit messages themselves will include your signoffs; the S-o-b
tags won't be generated *only* when formatting the patches.

Why is this good? Because the resultant signoffs-in-commits will be
consistent with patches that you apply from others (with git-am), and
because this way you can add my R-b tag *after* your S-o-b, via rebasing.

Just my two cents (in a wall of text, sorry), feel free to ignore.

Thanks
Laszlo

------------------------------------------------------------------------------

Ard Biesheuvel

2015-06-03 17:08:33 UTC

Permalink

Post by Laszlo Ersek

Post by Ard Biesheuvel
Thanks, pushed as r17554

Congrats to your first (am I right?) SVN commit!

Indeed!

Post by Laszlo Ersek
A small tip (feel free to ignore it): since for edk2 patches, the
tianocore contribution agreement is needed, it's useful to set up a
[commit]
template = /home/ardb/misc/tianocore.template
--------------
Contributed-under: TianoCore Contribution Agreement 1.0
-----------------

Yes, I am using that already

Post by Laszlo Ersek
The "trick" I'm proposing here is that you include your S-o-b with the
template as well, since you "should" have a template anyway. (Or else,
pass --signoff to "git-commit".)

On other projects, I am used to doing 'git commit -s' to add the
signoff. I never rely on git-format-patch to do that.

Post by Laszlo Ersek
[format]
signoff = false
Your posted patches will look the same, but the difference will be that
your commit messages themselves will include your signoffs; the S-o-b
tags won't be generated *only* when formatting the patches.
Why is this good? Because the resultant signoffs-in-commits will be
consistent with patches that you apply from others (with git-am), and
because this way you can add my R-b tag *after* your S-o-b, via rebasing.

I added it before deliberately, since the signoff that comes after it
then signifies that *I* was the one that added the R-b, not someone
that handled and signed off on the patch after me.

For instance, in kernel commits, you may see something like:

"""
Signed-off-by: Javier Martinez Canillas <***@collabora.co.uk>
Reviewed-by: Doug Anderson <***@chromium.org>
Signed-off-by: Kukjin Kim <***@kernel.org>
"""

(https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/commit/8cf5e6dc8dd55d0f1ad46ab4046c3a8a51d2136d)

where it is clearly the maintainer that picked up the R-b from the
list, and not the author that added it.

Post by Laszlo Ersek
Just my two cents (in a wall of text, sorry), feel free to ignore.

No, let's discuss ... :-)

------------------------------------------------------------------------------

Laszlo Ersek

2015-06-03 17:39:33 UTC

Permalink

Post by Ard Biesheuvel

Post by Laszlo Ersek

Post by Ard Biesheuvel
Thanks, pushed as r17554

Congrats to your first (am I right?) SVN commit!

Indeed!

Yes, I am using that already

Post by Laszlo Ersek
The "trick" I'm proposing here is that you include your S-o-b with the
template as well, since you "should" have a template anyway. (Or else,
pass --signoff to "git-commit".)

On other projects, I am used to doing 'git commit -s' to add the
signoff. I never rely on git-format-patch to do that.

I added it before deliberately, since the signoff that comes after it
then signifies that *I* was the one that added the R-b, not someone
that handled and signed off on the patch after me.
"""
"""
(https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/commit/8cf5e6dc8dd55d0f1ad46ab4046c3a8a51d2136d)
where it is clearly the maintainer that picked up the R-b from the
list, and not the author that added it.

Post by Laszlo Ersek
Just my two cents (in a wall of text, sorry), feel free to ignore.

No, let's discuss ... :-)

Haha, okay :)

Since in this case you are both original author and maintainer, I think
you should add your Signed-off-by both before and after my Reviewed-by,
in essence taking both Javier's and Kukjin's roles. /deadpan

In earnest tho, using your earlier (pre-maintainership) patches as an
example, let's say you posted a patch (first S-o-b), Olivier reviewed it
(R-b after first S-o-b), and (assume) I would apply it (second S-o-b,
presumably). In this case however I would simply not add my S-o-b,
because I had absolutely no input into it. Unless I forwarded that patch
(via formatting and posting it) or expected someone to pull my tree,
there would be no reason to add my S-o-b just for committing.

Anyway, I shan't obsess about this again ;)

Laszlo

------------------------------------------------------------------------------

Ard Biesheuvel

2015-06-04 06:07:15 UTC

Permalink

Post by Laszlo Ersek

Post by Ard Biesheuvel

Post by Laszlo Ersek

Post by Ard Biesheuvel
Thanks, pushed as r17554

Congrats to your first (am I right?) SVN commit!

Indeed!

Yes, I am using that already

Post by Laszlo Ersek
The "trick" I'm proposing here is that you include your S-o-b with the
template as well, since you "should" have a template anyway. (Or else,
pass --signoff to "git-commit".)

On other projects, I am used to doing 'git commit -s' to add the
signoff. I never rely on git-format-patch to do that.

I added it before deliberately, since the signoff that comes after it
then signifies that *I* was the one that added the R-b, not someone
that handled and signed off on the patch after me.
"""
"""
(https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/commit/8cf5e6dc8dd55d0f1ad46ab4046c3a8a51d2136d)
where it is clearly the maintainer that picked up the R-b from the
list, and not the author that added it.

Post by Laszlo Ersek
Just my two cents (in a wall of text, sorry), feel free to ignore.

No, let's discuss ... :-)

In the kernel, you *must* add your S-o-b if you are on the patch
delivery path (unless the patch gets merged from a pull request)

Post by Laszlo Ersek
Unless I forwarded that patch
(via formatting and posting it) or expected someone to pull my tree,
there would be no reason to add my S-o-b just for committing.
Anyway, I shan't obsess about this again ;)

Good.

------------------------------------------------------------------------------