| Message ID | 20190104180432.24480-2-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 28ce4cb3590bc3aaa91c3be75429d4e8722415e2 |
| Headers | show |
| Series | [edk2,1/2] ArmPkg/ArmMmuLib ARM: add missing support for non-shareable cached mappings | expand |
On Fri, Jan 04, 2019 at 07:04:32PM +0100, Ard Biesheuvel wrote: > PopulateLevel2PageTable () is invoked for [parts of] mappings that > start or end on a non-1 MB aligned address (or both). The size of > the mapping depends on both the start address modulo 1 MB and the > length of the mapping, but the logic that calculates this size is > flawed: subtracting 'start address modulo 1 MB' could result in a > negative value for the remaining length, which is obviously wrong. > > So instead, take either RemainLength, or the rest of the 1 MB > block, whichever is smaller. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > --- > ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > index b237321a8d8b..3b3b20aa9b78 100644 > --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > @@ -294,8 +294,8 @@ FillTranslationTable ( > PhysicalBase += TT_DESCRIPTOR_SECTION_SIZE; > RemainLength -= TT_DESCRIPTOR_SECTION_SIZE; > } else { > - PageMapLength = MIN (RemainLength, TT_DESCRIPTOR_SECTION_SIZE) - > - (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE); > + PageMapLength = MIN (RemainLength, TT_DESCRIPTOR_SECTION_SIZE - > + (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE)); > > // Case: Physical address aligned on the Section Size (1MB) && the length > // does not fill a section > -- > 2.17.1 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Fri, 11 Jan 2019 at 19:07, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Fri, Jan 04, 2019 at 07:04:32PM +0100, Ard Biesheuvel wrote: > > PopulateLevel2PageTable () is invoked for [parts of] mappings that > > start or end on a non-1 MB aligned address (or both). The size of > > the mapping depends on both the start address modulo 1 MB and the > > length of the mapping, but the logic that calculates this size is > > flawed: subtracting 'start address modulo 1 MB' could result in a > > negative value for the remaining length, which is obviously wrong. > > > > So instead, take either RemainLength, or the rest of the 1 MB > > block, whichever is smaller. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > Series pushed as e3ad54faa855 ArmPkg/ArmMmuLib ARM: add missing support for non-shareable cached mappings 28ce4cb3590b ArmPkg/ArmMmuLib ARM: fix thinko in second level page table handling (with Eugene's tested-by added to the latter) Thanks > > --- > > ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > > index b237321a8d8b..3b3b20aa9b78 100644 > > --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > > +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > > @@ -294,8 +294,8 @@ FillTranslationTable ( > > PhysicalBase += TT_DESCRIPTOR_SECTION_SIZE; > > RemainLength -= TT_DESCRIPTOR_SECTION_SIZE; > > } else { > > - PageMapLength = MIN (RemainLength, TT_DESCRIPTOR_SECTION_SIZE) - > > - (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE); > > + PageMapLength = MIN (RemainLength, TT_DESCRIPTOR_SECTION_SIZE - > > + (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE)); > > > > // Case: Physical address aligned on the Section Size (1MB) && the length > > // does not fill a section > > -- > > 2.17.1 > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c index b237321a8d8b..3b3b20aa9b78 100644 --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c @@ -294,8 +294,8 @@ FillTranslationTable ( PhysicalBase += TT_DESCRIPTOR_SECTION_SIZE; RemainLength -= TT_DESCRIPTOR_SECTION_SIZE; } else { - PageMapLength = MIN (RemainLength, TT_DESCRIPTOR_SECTION_SIZE) - - (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE); + PageMapLength = MIN (RemainLength, TT_DESCRIPTOR_SECTION_SIZE - + (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE)); // Case: Physical address aligned on the Section Size (1MB) && the length // does not fill a section
PopulateLevel2PageTable () is invoked for [parts of] mappings that start or end on a non-1 MB aligned address (or both). The size of the mapping depends on both the start address modulo 1 MB and the length of the mapping, but the logic that calculates this size is flawed: subtracting 'start address modulo 1 MB' could result in a negative value for the remaining length, which is obviously wrong. So instead, take either RemainLength, or the rest of the 1 MB block, whichever is smaller. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.17.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel