public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2] ArmVirtPkg: clear PcdPerformanceLibraryPropertyMask PCD
@ 2017-02-27 15:09 Ard Biesheuvel
  2017-02-27 16:07 ` Laszlo Ersek
  0 siblings, 1 reply; 2+ messages in thread
From: Ard Biesheuvel @ 2017-02-27 15:09 UTC (permalink / raw)
  To: edk2-devel, lersek; +Cc: Ard Biesheuvel

The only observeable effect of having PcdPerformanceLibraryPropertyMask
set to 1 is that a EfiReservedMemory region of 4 pages is allocated right
below the 4 GB mark. This region is out of bounds for the OS, which means
it is not even allowed to map it, to avoid speculative loads from it.

On Linux, this may prevent the kernel from using a 1 GB block mapping for
this region, and instead it has to carve up the block as follows:

  0xffffffff80000000-0xffffffffbe000000         992M PMD CON BLK
  0xffffffffbe000000-0xffffffffbfe00000          30M PMD     BLK
  0xffffffffbfe00000-0xffffffffbfff0000        1984K PTE CON
  0xffffffffbfff0000-0xffffffffbfffc000          48K PTE

where it would otherwise use a single 1 GB mapping (*), i.e.,

  0xffffffff80000000-0xffffffffc0000000           1G PGD

To clarify, the latter is a single 8 byte entry in the top level page
table, whereas in the former case, we have two additional levels of
paging, requiring two extra 4 KB pages (on a 4 KB pagesize kernel).

The real cost, however, is the TLB footprint, which goes up from a
single entry to a number between 90 and 1020, depending on whether
contiguous hints are honoured by the hardware.

So let's remove PcdPerformanceLibraryPropertyMask until we find a reason
why we need it.

(*) provided that no other allocations were deliberately located right
    below the 4 GB mark, and that we are running with more than 3 GB of
    memory, in which case most allocations will be over 4 GB, given EDK2's
    default top-down allocation policy.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2: remove PCD altogether rather than explicitly set it to its default
    value of 0

 ArmVirtPkg/ArmVirt.dsc.inc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 8e3d298723f1..cc09d38910a2 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -295,7 +295,6 @@ [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|1000000
   gEfiMdePkgTokenSpaceGuid.PcdSpinLockTimeout|10000000
   gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF
-  gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|1
   gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
   gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320
 
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] ArmVirtPkg: clear PcdPerformanceLibraryPropertyMask PCD
  2017-02-27 15:09 [PATCH v2] ArmVirtPkg: clear PcdPerformanceLibraryPropertyMask PCD Ard Biesheuvel
@ 2017-02-27 16:07 ` Laszlo Ersek
  0 siblings, 0 replies; 2+ messages in thread
From: Laszlo Ersek @ 2017-02-27 16:07 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel

On 02/27/17 16:09, Ard Biesheuvel wrote:
> The only observeable effect of having PcdPerformanceLibraryPropertyMask
> set to 1 is that a EfiReservedMemory region of 4 pages is allocated right
> below the 4 GB mark. This region is out of bounds for the OS, which means
> it is not even allowed to map it, to avoid speculative loads from it.
> 
> On Linux, this may prevent the kernel from using a 1 GB block mapping for
> this region, and instead it has to carve up the block as follows:
> 
>   0xffffffff80000000-0xffffffffbe000000         992M PMD CON BLK
>   0xffffffffbe000000-0xffffffffbfe00000          30M PMD     BLK
>   0xffffffffbfe00000-0xffffffffbfff0000        1984K PTE CON
>   0xffffffffbfff0000-0xffffffffbfffc000          48K PTE
> 
> where it would otherwise use a single 1 GB mapping (*), i.e.,
> 
>   0xffffffff80000000-0xffffffffc0000000           1G PGD
> 
> To clarify, the latter is a single 8 byte entry in the top level page
> table, whereas in the former case, we have two additional levels of
> paging, requiring two extra 4 KB pages (on a 4 KB pagesize kernel).
> 
> The real cost, however, is the TLB footprint, which goes up from a
> single entry to a number between 90 and 1020, depending on whether
> contiguous hints are honoured by the hardware.
> 
> So let's remove PcdPerformanceLibraryPropertyMask until we find a reason
> why we need it.
> 
> (*) provided that no other allocations were deliberately located right
>     below the 4 GB mark, and that we are running with more than 3 GB of
>     memory, in which case most allocations will be over 4 GB, given EDK2's
>     default top-down allocation policy.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> v2: remove PCD altogether rather than explicitly set it to its default
>     value of 0
> 
>  ArmVirtPkg/ArmVirt.dsc.inc | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 8e3d298723f1..cc09d38910a2 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -295,7 +295,6 @@ [PcdsFixedAtBuild.common]
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|1000000
>    gEfiMdePkgTokenSpaceGuid.PcdSpinLockTimeout|10000000
>    gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF
> -  gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|1
>    gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
>    gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320
>  
> 

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-02-27 16:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-27 15:09 [PATCH v2] ArmVirtPkg: clear PcdPerformanceLibraryPropertyMask PCD Ard Biesheuvel
2017-02-27 16:07 ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox