Thanks Andrew for raising the issue and Marc and Raghu for your inputs. The effect of stage 2 when Stage 1 is disabled is something that was overlooked in the workaround implementation. After some discussions this is what we currently understand regarding the problem.
1. EL3 does not modify the EL2 stage 1 translations and hence any speculative execution of AT execution in EL3 and resulting PTW and TLB caching should not be a problem as we always return to the same context in EL2. TF-A in EL3 need not worry about this. Also, we expect that future CPUs having v8.4 extensions for S-EL2 will have already corrected this errata.
2. Whenever the EL1/0 Stage 1 translations are switched in EL3 as part of switching the worlds, it should take Stage 2 into account. Hence disabling the Stage 1 MMU is not effective, as Marc pointed out, because speculative PTW can still take place using Stage 2 identity mapping. After discussion with James, we also realized that flipping the SCR_EL3.NS bit could take Stage 2 out of context (Stage 2 is not valid in secure world) leaving Stage 1 pointing to invalid translations.
As discussed in previous emails, on Entry to EL3, if TCR_EL1.EPDx = 1 and SCTLR_EL3.M = 1, any speculative AT PTWs for EL1 context can be prevented (including Stage 2).
So the fix as we currently understand would involve the following sequence :
a. On Entry to EL3, save the incoming SCTLR_EL1.M and TCR_EL1.EPDx bits and set them (ensure TCR_EL1.EPDx =1 prior to SCTLR_EL1.M =1 using isb()) b. Prior to Exit from EL3, after the target context is restored, restore the SCTLR_EL1.M and TCR_EL1.EPDx bits.
The above sequence now means that any use of AT instruction targeted at lower EL from EL3 that require PTW will fault. So prior to use of AT, ensure the PTW are re-enabled and disabled back again after the AT instructions.
If the above sequence is agreed upon to resolve the errata, then we can work on a patch for the same. I suspect current el1 register save and restore sequence in TF-A is a bit unwieldy and we may need to analyze all the entry points to EL3 to ensure we cover everything.
Best Regards Soby Mathew
-----Original Message----- From: TF-A tf-a-bounces@lists.trustedfirmware.org On Behalf Of Andrew Scull via TF-A Sent: 02 July 2020 09:20 To: Raghu K raghu.ncstate@icloud.com Cc: android-kvm@google.com; willdeacon@google.com; Marc Zyngier mzyngier@google.com; tf-a@lists.trustedfirmware.org Subject: Re: [TF-A] Erroneous speculative AT workaround
On Wed, Jul 01, 2020 at 05:47:00PM -0700, Raghu K wrote:
This is interesting. It appears that there is no way on entry to EL3 to guarantee that the out-of-context(el2 and el1) translation regimes are in a consistent state and on every entry into EL3, we have to conservatively assume that it is in an inconsistent state. This is because of the situation Andrew mentioned(interrupts to EL3 can occur at any
time).
If this is the case, on EL3 entry:
- For EL1, we will need to save SCTLR_EL1, set SCTLR_EL1.M = 1,.EPDx
= 0
This would still be racing against any potential speculative execution of an AT instruction upon the switch to EL3, IIUC. The window would be much smaller but not entirely eliminated.
For KVM, this would be enough as KVM will have already applied this workaround (with Marc's corrections) whenever it is going to enter an inconsistent state. However, other EL2 software may choose to handle the errata differently, possibly going to the lengths of ensuring that no AT instruction is ever mapped executable. -- TF-A mailing list TF-A@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a
On Thu, Jul 02, 2020 at 02:15:47PM +0000, Soby Mathew wrote:
So the fix as we currently understand would involve the following sequence :
a. On Entry to EL3, save the incoming SCTLR_EL1.M and TCR_EL1.EPDx bits and set them (ensure TCR_EL1.EPDx =1 prior to SCTLR_EL1.M =1 using isb()) b. Prior to Exit from EL3, after the target context is restored, restore the SCTLR_EL1.M and TCR_EL1.EPDx bits.The above sequence now means that any use of AT instruction targeted at lower EL from EL3 that require PTW will fault. So prior to use of AT, ensure the PTW are re-enabled and disabled back again after the AT instructions.
If the above sequence is agreed upon to resolve the errata, then we can work on a patch for the same. I suspect current el1 register save and restore sequence in TF-A is a bit unwieldy and we may need to analyze all the entry points to EL3 to ensure we cover everything.
Looks good to me, but there's still one niggle that I don't know how to solve. If EL2 has been audited not to have any executable AT instructions, it may not have a software workaround. However, if a secure interrupt is taken from EL2 to EL3 while EL2 is the middle of a world switch, then there is a small window where an AT instruction present at EL3 cold be speculatively executed before you've had a chance to mess with SCTLR_EL1.
Fun! Maybe it's worth documenting this somewhere?
Will
-----Original Message----- From: Will Deacon willdeacon@google.com Sent: 02 July 2020 15:47 To: Soby Mathew Soby.Mathew@arm.com Cc: Andrew Scull ascull@google.com; Raghu K raghu.ncstate@icloud.com; android-kvm@google.com; Marc Zyngier mzyngier@google.com; James Morse James.Morse@arm.com; tf- a@lists.trustedfirmware.org Subject: Re: [TF-A] Erroneous speculative AT workaround
On Thu, Jul 02, 2020 at 02:15:47PM +0000, Soby Mathew wrote:
So the fix as we currently understand would involve the following sequence :
a. On Entry to EL3, save the incoming SCTLR_EL1.M and TCR_EL1.EPDxbits and set them (ensure TCR_EL1.EPDx =1 prior to SCTLR_EL1.M =1 using isb())
b. Prior to Exit from EL3, after the target context is restored, restorethe SCTLR_EL1.M and TCR_EL1.EPDx bits.
The above sequence now means that any use of AT instruction targeted at lower EL from EL3 that require PTW will fault. So prior to use of AT, ensure the PTW are re-enabled and disabled back again after the AT instructions.
If the above sequence is agreed upon to resolve the errata, then we can work on a patch for the same. I suspect current el1 register save and restore sequence in TF-A is a bit unwieldy and we may need to analyze all the entry points to EL3 to ensure we cover everything.
Looks good to me, but there's still one niggle that I don't know how to solve. If EL2 has been audited not to have any executable AT instructions, it may not have a software workaround. However, if a secure interrupt is taken from EL2 to EL3 while EL2 is the middle of a world switch, then there is a small window where an AT instruction present at EL3 cold be speculatively executed before you've had a chance to mess with SCTLR_EL1.
Fun! Maybe it's worth documenting this somewhere?
Hi Will, Good point, this effectively means every EL2 software must implement the fix similar to KVM for this workaround to be effective (or else EL3 should also be audited to not to have any executable AT instruction). This needs to be communicated.
Since this is crossing TF-A boundary and need wider communication, I can initiate some internal discussion on how to communicate this properly.
Best Regards Soby Mathew
Will
Hi Raghu,
On 02/07/2020 15:52, Raghu K wrote:
- However, is an ISB sufficient? ISB does not prevent speculative execution of any
instruction, before the ISB is complete, it only causes a refetch after the completion of ISB, IIUC.
So in an implementation, an AT instruction could speculatively execute before the ISB(for whatever micro-architectural reason, and this may not apply to the cores under consideration), fill the TLB with garbage
For this to happen, the target translation regime must have already had its registers in a state that allowed the wrong translation.
Any workaround needs to ensure that a speculated AT instruction either gets the correct result, or fails.
(the damage is done), the ISB is executed and the speculative instructions are flushed and refetch occurs.
I think you are referring to: | ensure TCR_EL1.EPDx =1 prior to SCTLR_EL1.M =1 using isb()
(the hazards of top posting)
In this case, EL3 needs to be aware that SCTLR_EL1.M could have been 0. Its possible the TTBRx_EL1 registers are junk. EL3 needs to ensure the TCR_EL1.EPDx bits have been set, and the CPU is honouring the side effects, before it maybe-enables the EL1 stage1 MMU.
Without the barrier, the side effects from these two system register writes could complete in either order.
The only barrier that guarantees this wont happen is a DSB, which is why i was suggesting it.
If the registers were already in a state that allowed the wrong translation, I don't think there is any barrier that would help.
- For (b) in the fix, would you not require a barrier after the restore? If you don't
have it, you could have a speculative AT between the window of time where the SCTLR and TCR registers are restored and ERET.
Provided the TCR_EL1 and SCTLR_EL1 writes complete in the expected order, this would get the correct result.
There certainly needs to be something to ensure the TCR_EL1 and SCTLR_EL1 writes happen in the order that means a speculated AT either fails, or gets the correct result.
- "So prior to use of AT, ensure the PTW are re-enabled and disabled back again after the AT instructions" -
I assume this cannot be done on any entry to EL3 except through SMC's. Otherwise we would have the same issue of not being sure that the EL1 registers are in a state that would not cause this issue.
This would be needed wherever EL3 uses AT instructions deliberately. Because step 'a' nobbled stage1 translations, it can't expect AT instructions to work.
Thanks,
James
tf-a@lists.trustedfirmware.org
-
James Morse -
Raghu K -
Soby Mathew -
Will Deacon