| Message ID | 1507568462-28775-5-git-send-email-mw@semihalf.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Mon, Oct 09, 2017 at 07:00:53PM +0200, Marcin Wojtas wrote: > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > To prevent cache coherency issues when chainloading via U-Boot, clean > and invalidate the FV image in the caches before re-enabling the MMU. Is this only relevant for chainloading (which is not the expected normal usage) or is it also important for warm-reset - for example for capsule update (at least from within OS)? If the former, I would prefer for this to be conditionalised, and not included by default. If the latter, please update the commit message. / Leif > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > --- > Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S | 15 +++++++++++++++ > Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf | 3 +++ > 2 files changed, 18 insertions(+) > > diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S > index 72f8cfc..7544361 100644 > --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S > +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S > @@ -17,6 +17,21 @@ > > ASM_FUNC(ArmPlatformPeiBootAction) > mov x29, xzr > + > + MOV32 (x0, FixedPcdGet64 (PcdFvBaseAddress)) > + MOV32 (x3, FixedPcdGet32 (PcdFvSize)) > + add x3, x3, x0 > + > + mrs x1, ctr_el0 > + and x1, x1, #0xf // Dminline > + mov x2, #4 > + lsl x1, x2, x1 // by-VA stride for D-cache maintenance > + > +0:dc civac, x0 > + add x0, x0, x1 > + cmp x0, x3 > + b.lt 0b > + > ret > > //UINTN > diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf > index 2e198c3..6966683 100644 > --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf > +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf > @@ -67,5 +67,8 @@ > gArmTokenSpaceGuid.PcdArmPrimaryCoreMask > gArmTokenSpaceGuid.PcdArmPrimaryCore > > + gArmTokenSpaceGuid.PcdFvBaseAddress > + gArmTokenSpaceGuid.PcdFvSize > + > [Ppis] > gArmMpCoreInfoPpiGuid > -- > 1.8.3.1 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
2017-10-10 16:43 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: > On Mon, Oct 09, 2017 at 07:00:53PM +0200, Marcin Wojtas wrote: >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> To prevent cache coherency issues when chainloading via U-Boot, clean >> and invalidate the FV image in the caches before re-enabling the MMU. > > Is this only relevant for chainloading (which is not the expected > normal usage) or is it also important for warm-reset - for example for > capsule update (at least from within OS)? Initially it was done for chainloading purpose - I don't use it anymore, but just thought the patch itself is worth keeping. About capsule update - I haven't tried it, it's been not the top priority for me recently. > > If the former, I would prefer for this to be conditionalised, and not > included by default. How can we detect, that uefi is being chain-loaded? > > If the latter, please update the commit message. > I'm considering keeping this patch aside, until it may become necessary for capsule update, as I cannot guarantee now it's needed at all. What's your recommendation? Best regards, Marcin > / > Leif > >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >> --- >> Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S | 15 +++++++++++++++ >> Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf | 3 +++ >> 2 files changed, 18 insertions(+) >> >> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S >> index 72f8cfc..7544361 100644 >> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S >> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S >> @@ -17,6 +17,21 @@ >> >> ASM_FUNC(ArmPlatformPeiBootAction) >> mov x29, xzr >> + >> + MOV32 (x0, FixedPcdGet64 (PcdFvBaseAddress)) >> + MOV32 (x3, FixedPcdGet32 (PcdFvSize)) >> + add x3, x3, x0 >> + >> + mrs x1, ctr_el0 >> + and x1, x1, #0xf // Dminline >> + mov x2, #4 >> + lsl x1, x2, x1 // by-VA stride for D-cache maintenance >> + >> +0:dc civac, x0 >> + add x0, x0, x1 >> + cmp x0, x3 >> + b.lt 0b >> + >> ret >> >> //UINTN >> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf >> index 2e198c3..6966683 100644 >> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf >> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf >> @@ -67,5 +67,8 @@ >> gArmTokenSpaceGuid.PcdArmPrimaryCoreMask >> gArmTokenSpaceGuid.PcdArmPrimaryCore >> >> + gArmTokenSpaceGuid.PcdFvBaseAddress >> + gArmTokenSpaceGuid.PcdFvSize >> + >> [Ppis] >> gArmMpCoreInfoPpiGuid >> -- >> 1.8.3.1 >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Oct 10, 2017 at 04:50:04PM +0200, Marcin Wojtas wrote: > 2017-10-10 16:43 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: > > On Mon, Oct 09, 2017 at 07:00:53PM +0200, Marcin Wojtas wrote: > >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> > >> To prevent cache coherency issues when chainloading via U-Boot, clean > >> and invalidate the FV image in the caches before re-enabling the MMU. > > > > Is this only relevant for chainloading (which is not the expected > > normal usage) or is it also important for warm-reset - for example for > > capsule update (at least from within OS)? > > Initially it was done for chainloading purpose - I don't use it > anymore, but just thought the patch itself is worth keeping. About > capsule update - I haven't tried it, it's been not the top priority > for me recently. > > > If the former, I would prefer for this to be conditionalised, and not > > included by default. > > How can we detect, that uefi is being chain-loaded? Oh, I meant compile time. Hence "not included by default". It has been a useful debug feature, but I don't think anyone is expecting to be routinely run either EDK2 on top of U-Boot or U-Boot on top of EDK2 on this platform. > > If the latter, please update the commit message. > > I'm considering keeping this patch aside, until it may become > necessary for capsule update, as I cannot guarantee now it's needed at > all. What's your recommendation? I'll wait to see what Ard has to say. So yes, it may make sense to move it out of the series for now. Regards, Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 10 October 2017 at 16:29, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Tue, Oct 10, 2017 at 04:50:04PM +0200, Marcin Wojtas wrote: >> 2017-10-10 16:43 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>: >> > On Mon, Oct 09, 2017 at 07:00:53PM +0200, Marcin Wojtas wrote: >> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> >> >> To prevent cache coherency issues when chainloading via U-Boot, clean >> >> and invalidate the FV image in the caches before re-enabling the MMU. >> > >> > Is this only relevant for chainloading (which is not the expected >> > normal usage) or is it also important for warm-reset - for example for >> > capsule update (at least from within OS)? >> >> Initially it was done for chainloading purpose - I don't use it >> anymore, but just thought the patch itself is worth keeping. About >> capsule update - I haven't tried it, it's been not the top priority >> for me recently. >> >> > If the former, I would prefer for this to be conditionalised, and not >> > included by default. >> >> How can we detect, that uefi is being chain-loaded? > > Oh, I meant compile time. Hence "not included by default". > > It has been a useful debug feature, but I don't think anyone is > expecting to be routinely run either EDK2 on top of U-Boot or U-Boot > on top of EDK2 on this platform. > >> > If the latter, please update the commit message. >> >> I'm considering keeping this patch aside, until it may become >> necessary for capsule update, as I cannot guarantee now it's needed at >> all. What's your recommendation? > > I'll wait to see what Ard has to say. > So yes, it may make sense to move it out of the series for now. > We don't need this patch, I think. I don't remember the details, but the FV should simply be cleaned to the PoC before entering UEFI. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S index 72f8cfc..7544361 100644 --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S @@ -17,6 +17,21 @@ ASM_FUNC(ArmPlatformPeiBootAction) mov x29, xzr + + MOV32 (x0, FixedPcdGet64 (PcdFvBaseAddress)) + MOV32 (x3, FixedPcdGet32 (PcdFvSize)) + add x3, x3, x0 + + mrs x1, ctr_el0 + and x1, x1, #0xf // Dminline + mov x2, #4 + lsl x1, x2, x1 // by-VA stride for D-cache maintenance + +0:dc civac, x0 + add x0, x0, x1 + cmp x0, x3 + b.lt 0b + ret //UINTN diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf index 2e198c3..6966683 100644 --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf @@ -67,5 +67,8 @@ gArmTokenSpaceGuid.PcdArmPrimaryCoreMask gArmTokenSpaceGuid.PcdArmPrimaryCore + gArmTokenSpaceGuid.PcdFvBaseAddress + gArmTokenSpaceGuid.PcdFvSize + [Ppis] gArmMpCoreInfoPpiGuid