| Message ID | 1487960215-14052-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 02/24/17 19:16, 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 mappings 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 value between 90 and 1020, depending on whether > contiguous hints are honoured by the hardware. > > So let's set PcdPerformanceLibraryPropertyMask to zero until we find > a reason why we shouldn't. > > (*) 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> > --- > ArmVirtPkg/ArmVirt.dsc.inc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc > index 61d4a6642eb7..ca0a4d31a03d 100644 > --- a/ArmVirtPkg/ArmVirt.dsc.inc > +++ b/ArmVirtPkg/ArmVirt.dsc.inc > @@ -295,7 +295,7 @@ [PcdsFixedAtBuild.common] > gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|1000000 > gEfiMdePkgTokenSpaceGuid.PcdSpinLockTimeout|10000000 > gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF > - gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|1 > + gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0 > gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0 > gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320 > > The value 1 dates back to ancient commit 6f5872b1f401 ("ArmPlatformPkg/ArmVirtualizationPkg: Add ArmVirtualizationQemu platform", 2014-09-18). The default is zero in "MdePkg/MdePkg.dec": ## The mask is used to control PerformanceLib behavior.<BR><BR> # BIT0 - Enable Performance Measurement.<BR> # @Prompt Performance Measurement Property. # @Expression 0x80000002 | (gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask & 0xFE) == 0 gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0|UINT8|0x00000009 Shouldn't we just remove the line? Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 24 February 2017 at 23:17, Laszlo Ersek <lersek@redhat.com> wrote: > On 02/24/17 19:16, 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 mappings 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 value between 90 and 1020, depending on whether >> contiguous hints are honoured by the hardware. >> >> So let's set PcdPerformanceLibraryPropertyMask to zero until we find >> a reason why we shouldn't. >> >> (*) 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> >> --- >> ArmVirtPkg/ArmVirt.dsc.inc | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc >> index 61d4a6642eb7..ca0a4d31a03d 100644 >> --- a/ArmVirtPkg/ArmVirt.dsc.inc >> +++ b/ArmVirtPkg/ArmVirt.dsc.inc >> @@ -295,7 +295,7 @@ [PcdsFixedAtBuild.common] >> gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|1000000 >> gEfiMdePkgTokenSpaceGuid.PcdSpinLockTimeout|10000000 >> gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF >> - gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|1 >> + gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0 >> gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0 >> gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320 >> >> > > The value 1 dates back to ancient commit 6f5872b1f401 ("ArmPlatformPkg/ArmVirtualizationPkg: Add ArmVirtualizationQemu platform", 2014-09-18). > > The default is zero in "MdePkg/MdePkg.dec": > > ## The mask is used to control PerformanceLib behavior.<BR><BR> > # BIT0 - Enable Performance Measurement.<BR> > # @Prompt Performance Measurement Property. > # @Expression 0x80000002 | (gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask & 0xFE) == 0 > gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0|UINT8|0x00000009 > > Shouldn't we just remove the line? > Sure, even better! _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc index 61d4a6642eb7..ca0a4d31a03d 100644 --- a/ArmVirtPkg/ArmVirt.dsc.inc +++ b/ArmVirtPkg/ArmVirt.dsc.inc @@ -295,7 +295,7 @@ [PcdsFixedAtBuild.common] gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|1000000 gEfiMdePkgTokenSpaceGuid.PcdSpinLockTimeout|10000000 gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF - gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|1 + gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0 gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0 gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320
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 mappings 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 value between 90 and 1020, depending on whether contiguous hints are honoured by the hardware. So let's set PcdPerformanceLibraryPropertyMask to zero until we find a reason why we shouldn't. (*) 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> --- ArmVirtPkg/ArmVirt.dsc.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel