| Message ID | 1458222900-7890-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | b5d89de167a683eeba3e2dbb6557f402b952cdc2 |
| Headers | show |
On Thu, Mar 17, 2016 at 02:55:00PM +0100, Ard Biesheuvel wrote: > The function ArmClearMemoryRegionReadOnly() was supposed to undo the > effect of ArmSetMemoryRegionReadOnly(), but instead, it sets the permissions > to EL0-no access, EL1-read-only. Since the EL0 bit should be 1 to align > with EL2/3 (where the bit is SBO), use TT_AP_RW_RW instead, which makes the > entry read-write for EL0 when executing at EL1, and read-write for all other > levels. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Happy with the RO->RW, confused by the NO->RW. I presume this is about consistency? Why do we need access for EL0? > --- > ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > index f967a6478840..b7d23c6f3286 100644 > --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > @@ -558,7 +558,7 @@ ArmClearMemoryRegionReadOnly ( > return SetMemoryRegionAttribute ( > BaseAddress, > Length, > - TT_AP_NO_RO, > + TT_AP_RW_RW, > ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK)); > } > > -- > 2.5.0 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 17 March 2016 at 16:31, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Thu, Mar 17, 2016 at 02:55:00PM +0100, Ard Biesheuvel wrote: >> The function ArmClearMemoryRegionReadOnly() was supposed to undo the >> effect of ArmSetMemoryRegionReadOnly(), but instead, it sets the permissions >> to EL0-no access, EL1-read-only. Since the EL0 bit should be 1 to align >> with EL2/3 (where the bit is SBO), use TT_AP_RW_RW instead, which makes the >> entry read-write for EL0 when executing at EL1, and read-write for all other >> levels. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Happy with the RO->RW, confused by the NO->RW. > I presume this is about consistency? > Why do we need access for EL0? > We don't. But when running on EL2 or EL3, the same bit needs to be set, so either we have two separate definitions, for EL1 and EL2/3, or we just set the bit for EL0 as well, which shouldn't matter anyway since we never return to EL0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Thu, Mar 17, 2016 at 04:40:38PM +0100, Ard Biesheuvel wrote: > On 17 March 2016 at 16:31, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Thu, Mar 17, 2016 at 02:55:00PM +0100, Ard Biesheuvel wrote: > >> The function ArmClearMemoryRegionReadOnly() was supposed to undo the > >> effect of ArmSetMemoryRegionReadOnly(), but instead, it sets the permissions > >> to EL0-no access, EL1-read-only. Since the EL0 bit should be 1 to align > >> with EL2/3 (where the bit is SBO), use TT_AP_RW_RW instead, which makes the > >> entry read-write for EL0 when executing at EL1, and read-write for all other > >> levels. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > > Happy with the RO->RW, confused by the NO->RW. > > I presume this is about consistency? > > Why do we need access for EL0? > > > > We don't. But when running on EL2 or EL3, the same bit needs to be > set, so either we have two separate definitions, for EL1 and EL2/3, or > we just set the bit for EL0 as well, which shouldn't matter anyway > since we never return to EL0 OK, that makes sense: Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c index f967a6478840..b7d23c6f3286 100644 --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c @@ -558,7 +558,7 @@ ArmClearMemoryRegionReadOnly ( return SetMemoryRegionAttribute ( BaseAddress, Length, - TT_AP_NO_RO, + TT_AP_RW_RW, ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK)); }
The function ArmClearMemoryRegionReadOnly() was supposed to undo the effect of ArmSetMemoryRegionReadOnly(), but instead, it sets the permissions to EL0-no access, EL1-read-only. Since the EL0 bit should be 1 to align with EL2/3 (where the bit is SBO), use TT_AP_RW_RW instead, which makes the entry read-write for EL0 when executing at EL1, and read-write for all other levels. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.5.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel