| Message ID | 6d030bee2db08c2fe1b7569e692e8f66800b1e4c.1471369569.git.robin.murphy@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16/08/16 18:49, Robin Murphy wrote: > Since the non-secure copies of banked registers lack architecturally > defined reset values, there is no actual guarantee when entering in Hyp > from secure-only firmware that the Non-Secure PL1 state will look the > way that kernel entry (in particular the decompressor stub) expects. > So far, we've been getting away with it thanks to implementation details > of ARMv7 cores and/or bootloader behaviour, but for the sake of forwards > compatibility let's try to ensure that we have a minimally sane state > before dropping down into it. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com> M.
On Tue, Aug 16, 2016 at 06:49:18PM +0100, Robin Murphy wrote: > Since the non-secure copies of banked registers lack architecturally > defined reset values, there is no actual guarantee when entering in Hyp > from secure-only firmware that the Non-Secure PL1 state will look the > way that kernel entry (in particular the decompressor stub) expects. > So far, we've been getting away with it thanks to implementation details > of ARMv7 cores and/or bootloader behaviour, but for the sake of forwards > compatibility let's try to ensure that we have a minimally sane state > before dropping down into it. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org> > --- > > v2: Initialise SED/ITD to safe values as well. > > arch/arm/kernel/hyp-stub.S | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S > index 0b1e4a93d67e..15d073ae5da2 100644 > --- a/arch/arm/kernel/hyp-stub.S > +++ b/arch/arm/kernel/hyp-stub.S > @@ -142,6 +142,19 @@ ARM_BE8(orr r7, r7, #(1 << 25)) @ HSCTLR.EE > and r7, #0x1f @ Preserve HPMN > mcr p15, 4, r7, c1, c1, 1 @ HDCR > > + @ Make sure NS-SVC is initialised appropriately > + mrc p15, 0, r7, c1, c0, 0 @ SCTLR > + orr r7, #(1 << 5) @ CP15 barriers enabled > + bic r7, #(3 << 7) @ Clear SED/ITD for v8 (RES0 for v7) > + bic r7, #(3 << 19) @ WXN and UWXN disabled > + mcr p15, 0, r7, c1, c0, 0 @ SCTLR > + > + mrc p15, 0, r7, c0, c0, 0 @ MIDR > + mcr p15, 4, r7, c0, c0, 0 @ VPIDR > + > + mrc p15, 0, r7, c0, c0, 5 @ MPIDR > + mcr p15, 4, r7, c0, c0, 5 @ VMPIDR > + > #if !defined(ZIMAGE) && defined(CONFIG_ARM_ARCH_TIMER) > @ make CNTP_* and CNTPCT accessible from PL1 > mrc p15, 0, r7, c0, c1, 1 @ ID_PFR1 > -- > 2.8.1.dirty >
Hi Russell, On Tue, Aug 16, 2016 at 06:49:18PM +0100, Robin Murphy wrote: > Since the non-secure copies of banked registers lack architecturally > defined reset values, there is no actual guarantee when entering in Hyp > from secure-only firmware that the Non-Secure PL1 state will look the > way that kernel entry (in particular the decompressor stub) expects. > So far, we've been getting away with it thanks to implementation details > of ARMv7 cores and/or bootloader behaviour, but for the sake of forwards > compatibility let's try to ensure that we have a minimally sane state > before dropping down into it. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> Can we have your acked-by on this patch to merge it via the KVM tree or do you prefer merging this one directly yourself? Thanks, -Christoffer
Hi Russell, On Tue, Aug 16, 2016 at 06:49:18PM +0100, Robin Murphy wrote: > Since the non-secure copies of banked registers lack architecturally > defined reset values, there is no actual guarantee when entering in Hyp > from secure-only firmware that the Non-Secure PL1 state will look the > way that kernel entry (in particular the decompressor stub) expects. > So far, we've been getting away with it thanks to implementation details > of ARMv7 cores and/or bootloader behaviour, but for the sake of forwards > compatibility let's try to ensure that we have a minimally sane state > before dropping down into it. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> Can we have your acked-by on this patch to merge it via the KVM tree or do you prefer merging this one directly yourself? Thanks, -Christoffer
On Wed, Aug 17, 2016 at 01:32:49PM +0200, Christoffer Dall wrote: > Hi Russell, > > On Tue, Aug 16, 2016 at 06:49:18PM +0100, Robin Murphy wrote: > > Since the non-secure copies of banked registers lack architecturally > > defined reset values, there is no actual guarantee when entering in Hyp > > from secure-only firmware that the Non-Secure PL1 state will look the > > way that kernel entry (in particular the decompressor stub) expects. > > So far, we've been getting away with it thanks to implementation details > > of ARMv7 cores and/or bootloader behaviour, but for the sake of forwards > > compatibility let's try to ensure that we have a minimally sane state > > before dropping down into it. > > > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > > Can we have your acked-by on this patch to merge it via the KVM tree or > do you prefer merging this one directly yourself? > I have queued this via the KVM/ARM tree. If you have any objections, please let me know asap. Thanks, -Christoffer
Hi Christoffer, On 06/09/16 17:41, Christoffer Dall wrote: > On Wed, Aug 17, 2016 at 01:32:49PM +0200, Christoffer Dall wrote: >> Hi Russell, >> >> On Tue, Aug 16, 2016 at 06:49:18PM +0100, Robin Murphy wrote: >>> Since the non-secure copies of banked registers lack architecturally >>> defined reset values, there is no actual guarantee when entering in Hyp >>> from secure-only firmware that the Non-Secure PL1 state will look the >>> way that kernel entry (in particular the decompressor stub) expects. >>> So far, we've been getting away with it thanks to implementation details >>> of ARMv7 cores and/or bootloader behaviour, but for the sake of forwards >>> compatibility let's try to ensure that we have a minimally sane state >>> before dropping down into it. >>> >>> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >> >> Can we have your acked-by on this patch to merge it via the KVM tree or >> do you prefer merging this one directly yourself? >> > I have queued this via the KVM/ARM tree. If you have any objections, > please let me know asap. Ah, I should have replied on this thread - I put this one into Russell's patch system the other week, and it's in -next already. Sorry for the lack of communication, Robin. > > Thanks, > -Christoffer >
On Tue, Sep 6, 2016 at 6:44 PM, Robin Murphy <robin.murphy@arm.com> wrote: > Hi Christoffer, > > On 06/09/16 17:41, Christoffer Dall wrote: >> On Wed, Aug 17, 2016 at 01:32:49PM +0200, Christoffer Dall wrote: >>> Hi Russell, >>> >>> On Tue, Aug 16, 2016 at 06:49:18PM +0100, Robin Murphy wrote: >>>> Since the non-secure copies of banked registers lack architecturally >>>> defined reset values, there is no actual guarantee when entering in Hyp >>>> from secure-only firmware that the Non-Secure PL1 state will look the >>>> way that kernel entry (in particular the decompressor stub) expects. >>>> So far, we've been getting away with it thanks to implementation details >>>> of ARMv7 cores and/or bootloader behaviour, but for the sake of forwards >>>> compatibility let's try to ensure that we have a minimally sane state >>>> before dropping down into it. >>>> >>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >>> >>> Can we have your acked-by on this patch to merge it via the KVM tree or >>> do you prefer merging this one directly yourself? >>> >> I have queued this via the KVM/ARM tree. If you have any objections, >> please let me know asap. > > Ah, I should have replied on this thread - I put this one into Russell's > patch system the other week, and it's in -next already. > > Sorry for the lack of communication, No worries, I removed it from the queue again. -Christoffer
diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S index 0b1e4a93d67e..15d073ae5da2 100644 --- a/arch/arm/kernel/hyp-stub.S +++ b/arch/arm/kernel/hyp-stub.S @@ -142,6 +142,19 @@ ARM_BE8(orr r7, r7, #(1 << 25)) @ HSCTLR.EE and r7, #0x1f @ Preserve HPMN mcr p15, 4, r7, c1, c1, 1 @ HDCR + @ Make sure NS-SVC is initialised appropriately + mrc p15, 0, r7, c1, c0, 0 @ SCTLR + orr r7, #(1 << 5) @ CP15 barriers enabled + bic r7, #(3 << 7) @ Clear SED/ITD for v8 (RES0 for v7) + bic r7, #(3 << 19) @ WXN and UWXN disabled + mcr p15, 0, r7, c1, c0, 0 @ SCTLR + + mrc p15, 0, r7, c0, c0, 0 @ MIDR + mcr p15, 4, r7, c0, c0, 0 @ VPIDR + + mrc p15, 0, r7, c0, c0, 5 @ MPIDR + mcr p15, 4, r7, c0, c0, 5 @ VMPIDR + #if !defined(ZIMAGE) && defined(CONFIG_ARM_ARCH_TIMER) @ make CNTP_* and CNTPCT accessible from PL1 mrc p15, 0, r7, c0, c1, 1 @ ID_PFR1
Since the non-secure copies of banked registers lack architecturally defined reset values, there is no actual guarantee when entering in Hyp from secure-only firmware that the Non-Secure PL1 state will look the way that kernel entry (in particular the decompressor stub) expects. So far, we've been getting away with it thanks to implementation details of ARMv7 cores and/or bootloader behaviour, but for the sake of forwards compatibility let's try to ensure that we have a minimally sane state before dropping down into it. Signed-off-by: Robin Murphy <robin.murphy@arm.com> --- v2: Initialise SED/ITD to safe values as well. arch/arm/kernel/hyp-stub.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)