Re: [edk2] [PATCH 2/2] ArmPkg/ArmMmuLib ARM: fix thinko in second level page table handling

2019-01-13 Thread Ard Biesheuvel 
On Fri, 11 Jan 2019 at 19:07, Leif Lindholm  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 
>
> Reviewed-by: Leif Lindholm 
>

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

Re: [edk2] [PATCH 2/2] ArmPkg/ArmMmuLib ARM: fix thinko in second level page table handling

2019-01-11 Thread Leif Lindholm 
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 

Reviewed-by: Leif Lindholm 

> ---
>  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

[edk2] [PATCH 2/2] ArmPkg/ArmMmuLib ARM: fix thinko in second level page table handling

2019-01-04 Thread Ard Biesheuvel 
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 
---
 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