- TF-A - lists.trustedfirmware.org

Re: [TF-A] Erroneous speculative AT workaround

by Soby Mathew

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(a)lists.trustedfirmware.org> On Behalf Of Andrew > Scull via TF-A > Sent: 02 July 2020 09:20 > To: Raghu K <raghu.ncstate(a)icloud.com> > Cc: android-kvm(a)google.com; willdeacon(a)google.com; Marc Zyngier > <mzyngier(a)google.com>; tf-a(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: > > 1) 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(a)lists.trustedfirmware.org > https://lists.trustedfirmware.org/mailman/listinfo/tf-a

6 years, 2 months

4
4
0 0

Re: [TF-A] Erroneous speculative AT workaround

by Raghu K

6 years, 2 months

1
0
0 0

Re: [TF-A] Erroneous speculative AT workaround

by Raghu K

6 years, 2 months

1
0
0 0

Re: [TF-A] Erroneous speculative AT workaround

by Raghu K

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: 1) For EL1, we will need to save SCTLR_EL1, set SCTLR_EL1.M = 1,.EPDx = 0 2) Set whatever bits we need to for EL2 and S2 translations to not succeed(What are these?) 3) DSB, to ensure no speculative AT can be issued until completion of DSB, so any AT that occurs will not fill the TLB with bad translations. On exit(right before ERET), we need to restore the registers saved on entry, and have the ERET followed by a DSB so that there can be no speculative execution of AT instructions. Thanks Raghu On 7/1/20 5:54 AM, Marc Zyngier via TF-A wrote: > Hi Manish, > > On Wed, 1 Jul 2020 at 13:14, Manish Badarkhe <Manish.Badarkhe(a)arm.com> wrote: >> Hi Andrew, >> >> As per current implementation, in “el1_sysregs_context_restore” routine do below things: >> >> 1. TCR_EL1.EPD0 = 1 >> 2. TCR_EL1.EPD1 = 1 >> 3. SCTR_EL1.M = 0 >> 4. Isb >> Code snippet: >> mrs x9, tcr_el1 >> orr x9, x9, #TCR_EPD0_BIT >> orr x9, x9, #TCR_EPD1_BIT >> msr tcr_el1, x9 >> mrs x9, sctlr_el1 >> bic x9, x9, #SCTLR_M_BIT >> msr sctlr_el1, x9 >> isb >> This is to avoid PTW through while updating system registers at step 5 > Unfortunately, this doesn't prevent anything. > > If SCTLR_EL1.M is clear, TCR_EL1.EPDx don't mean much (S1 MMU is > disabled, no S1 page table walk), and you can still have S2 PTWs > (using an idmap for S1) and creating TLB corruption if these entry > alias with any S1 mapping that exists at EL1. > > Which is why KVM does *set* SCTLR_EL1.M, which prevents the use of a > 1:1 mapping at S1, and at which point the TCR_EL1.EPDx bits are > actually useful in preventing a PTW. > >> 5. Restore all system registers for El1 except SCTLR_EL1 and TCR_EL1 >> 6. isb() >> 7. restore SCTLR_EL1 and TCR_EL1 >> Code Snippet: >> ldr x9, [x0, #CTX_SCTLR_EL1] -> saved value from "el2_sysregs_context_save" >> msr sctlr_el1, x9 >> ldr x9, [x0, #CTX_TCR_EL1] >> msr tcr_el1, x9 >> >> As per above steps. SCTLR_EL1 get restored back with actual settings at step 7. >> Similar flow is present for “el2_sysregs_context_restore” to restore SCTLR_EL1 register. >> >> In conclusion, this routine temporarily clear M bit of SCTLR_EL1 to avoid speculation but restored it back >> to its original setting while leaving back to its caller. Please let us know whether this align with KVM >> workaround for speculative AT erratum. > It doesn't, unfortunately. I believe this code actively creates > problems on a system that is affected by speculative AT execution. > > I don't understand your rationale for touching SCTLR_EL2.M either if > you are not context-switching the EL2 S1 state: as far as I understand > no affected cores have S-EL2, so no switch should happen at this > stage. > > Thanks, > > M.

6 years, 2 months

3
2
0 0

Re: [TF-A] Erroneous speculative AT workaround

by Manish Badarkhe

Hi Andrew, As per current implementation, in “el1_sysregs_context_restore” routine do below things: 1. TCR_EL1.EPD0 = 1 2. TCR_EL1.EPD1 = 1 3. SCTR_EL1.M = 0 4. Isb Code snippet: mrs x9, tcr_el1 orr x9, x9, #TCR_EPD0_BIT orr x9, x9, #TCR_EPD1_BIT msr tcr_el1, x9 mrs x9, sctlr_el1 bic x9, x9, #SCTLR_M_BIT msr sctlr_el1, x9 isb This is to avoid PTW through while updating system registers at step 5 5. Restore all system registers for El1 except SCTLR_EL1 and TCR_EL1 6. isb() 7. restore SCTLR_EL1 and TCR_EL1 Code Snippet: ldr x9, [x0, #CTX_SCTLR_EL1] -> saved value from "el2_sysregs_context_save" msr sctlr_el1, x9 ldr x9, [x0, #CTX_TCR_EL1] msr tcr_el1, x9 As per above steps. SCTLR_EL1 get restored back with actual settings at step 7. Similar flow is present for “el2_sysregs_context_restore” to restore SCTLR_EL1 register. In conclusion, this routine temporarily clear M bit of SCTLR_EL1 to avoid speculation but restored it back to its original setting while leaving back to its caller. Please let us know whether this align with KVM workaround for speculative AT erratum. Thanks Manish Badarkhe On 01/07/2020, 17:02, "TF-A on behalf of Joanna Farley via TF-A" <tf-a-bounces(a)lists.trustedfirmware.org on behalf of tf-a(a)lists.trustedfirmware.org> wrote: Thanks Andrew for the notification. We will look into it and get back to this thread. Thanks Joanna On 01/07/2020, 10:16, "TF-A on behalf of Andrew Scull via TF-A" <tf-a-bounces(a)lists.trustedfirmware.org on behalf of tf-a(a)lists.trustedfirmware.org> wrote: Whilst looking at the speculative AT workaround in KVM, I compared it against the workaround in TF-A and noticed an inconsistency whereby TF-A **breaks** KVM's workaround. In `el1_sysregs_context_restore`, the M bit of SCTRL_EL1 is cleared however Linux requires this to be set for its workaround to be correct. If an exception is taken to EL3 partway through a VM context switch, e.g. a secure interrupt, causing a switch to the secure world, TF-A will reintroduce the possibility of TLB corruption. The above explains how it is broken for Linux's chosen workaround however TF-A will also have to be compatible with whatever workaround the EL2 software is using. Starting this thread with the issue identified and we can add more details as needed. Thanks Andrew -- TF-A mailing list TF-A(a)lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a -- TF-A mailing list TF-A(a)lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a

6 years, 2 months

2
1
0 0

Re: [TF-A] Erroneous speculative AT workaround

by Manish Badarkhe

+Updated comment for SCTLR_EL2 register restoration. On 01/07/2020, 17:45, "TF-A on behalf of Manish Badarkhe via TF-A" <tf-a-bounces(a)lists.trustedfirmware.org on behalf of tf-a(a)lists.trustedfirmware.org> wrote: Hi Andrew, As per current implementation, in “el1_sysregs_context_restore” routine do below things: 1. TCR_EL1.EPD0 = 1 2. TCR_EL1.EPD1 = 1 3. SCTR_EL1.M = 0 4. Isb Code snippet: mrs x9, tcr_el1 orr x9, x9, #TCR_EPD0_BIT orr x9, x9, #TCR_EPD1_BIT msr tcr_el1, x9 mrs x9, sctlr_el1 bic x9, x9, #SCTLR_M_BIT msr sctlr_el1, x9 isb This is to avoid PTW through while updating system registers at step 5 5. Restore all system registers for El1 except SCTLR_EL1 and TCR_EL1 6. isb() 7. restore SCTLR_EL1 and TCR_EL1 Code Snippet: ldr x9, [x0, #CTX_SCTLR_EL1] -> saved value from "el2_sysregs_context_save" msr sctlr_el1, x9 ldr x9, [x0, #CTX_TCR_EL1] msr tcr_el1, x9 As per above steps. SCTLR_EL1 get restored back with actual settings at step 7. Similar flow is present for “el2_sysregs_context_restore” to restore SCTLR_EL2 register. In conclusion, this routine temporarily clear M bit of SCTLR_EL1 to avoid speculation but restored it back to its original setting while leaving back to its caller. Please let us know whether this align with KVM workaround for speculative AT erratum. Thanks Manish Badarkhe On 01/07/2020, 17:02, "TF-A on behalf of Joanna Farley via TF-A" <tf-a-bounces(a)lists.trustedfirmware.org on behalf of tf-a(a)lists.trustedfirmware.org> wrote: Thanks Andrew for the notification. We will look into it and get back to this thread. Thanks Joanna On 01/07/2020, 10:16, "TF-A on behalf of Andrew Scull via TF-A" <tf-a-bounces(a)lists.trustedfirmware.org on behalf of tf-a(a)lists.trustedfirmware.org> wrote: Whilst looking at the speculative AT workaround in KVM, I compared it against the workaround in TF-A and noticed an inconsistency whereby TF-A **breaks** KVM's workaround. In `el1_sysregs_context_restore`, the M bit of SCTRL_EL1 is cleared however Linux requires this to be set for its workaround to be correct. If an exception is taken to EL3 partway through a VM context switch, e.g. a secure interrupt, causing a switch to the secure world, TF-A will reintroduce the possibility of TLB corruption. The above explains how it is broken for Linux's chosen workaround however TF-A will also have to be compatible with whatever workaround the EL2 software is using. Starting this thread with the issue identified and we can add more details as needed. Thanks Andrew -- TF-A mailing list TF-A(a)lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a -- TF-A mailing list TF-A(a)lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a -- TF-A mailing list TF-A(a)lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a

6 years, 2 months

1
0
0 0

Re: [TF-A] Erroneous speculative AT workaround

by Joanna Farley

Thanks Andrew for the notification. We will look into it and get back to this thread. Thanks Joanna On 01/07/2020, 10:16, "TF-A on behalf of Andrew Scull via TF-A" <tf-a-bounces(a)lists.trustedfirmware.org on behalf of tf-a(a)lists.trustedfirmware.org> wrote: Whilst looking at the speculative AT workaround in KVM, I compared it against the workaround in TF-A and noticed an inconsistency whereby TF-A **breaks** KVM's workaround. In `el1_sysregs_context_restore`, the M bit of SCTRL_EL1 is cleared however Linux requires this to be set for its workaround to be correct. If an exception is taken to EL3 partway through a VM context switch, e.g. a secure interrupt, causing a switch to the secure world, TF-A will reintroduce the possibility of TLB corruption. The above explains how it is broken for Linux's chosen workaround however TF-A will also have to be compatible with whatever workaround the EL2 software is using. Starting this thread with the issue identified and we can add more details as needed. Thanks Andrew -- TF-A mailing list TF-A(a)lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a

6 years, 2 months

1
0
0 0

Erroneous speculative AT workaround

by Andrew Scull

Whilst looking at the speculative AT workaround in KVM, I compared it against the workaround in TF-A and noticed an inconsistency whereby TF-A **breaks** KVM's workaround. In `el1_sysregs_context_restore`, the M bit of SCTRL_EL1 is cleared however Linux requires this to be set for its workaround to be correct. If an exception is taken to EL3 partway through a VM context switch, e.g. a secure interrupt, causing a switch to the secure world, TF-A will reintroduce the possibility of TLB corruption. The above explains how it is broken for Linux's chosen workaround however TF-A will also have to be compatible with whatever workaround the EL2 software is using. Starting this thread with the issue identified and we can add more details as needed. Thanks Andrew

6 years, 2 months

1
0
0 0

Re: [TF-A] Announcing some changes around the code review label in Gerrit

by Julius Werner

Hi Sandrine, Sounds like good changes in general! I'm curious what the ACLs are for Code-Owner-Review? Is it tied to docs/about/maintainers.rst or just based on the honor system? (I notice that I seem to be able to give a +1 for code I'm not owning, but maybe that's because I am a maintainer?) Also, are code owners allowed to +1 themselves (I think we said we didn't want maintainers to do that, but for code owners I could see how we might want to allow it since there are usually not that many)? What do we do when someone uploads the first patch for a new platform, do they just COR+1 themselves (since there is no code owner yet)? I think it might still be useful to retain the existing Code-Review as a +1/-1 label next to the two new ones, just to allow other interested parties to record their opinion (without it having any enforcing effect on the merge). In other Gerrit instances I have used people often like to give CR+1 as a "I'm not the one who needs to approve this but I have looked at it and think it's fine" or a CR-1 as a "I can't stop you from doing this but I still want to be clear that I don't think it's a good idea". It allows people outside the official approval process a clearer way to participate and can aid the official approvers in their decisions (e.g. when I am reviewing a patch as a maintainer that already has a CR-1 from someone else I know to pay extra attention to their concerns, and it's more visible than just some random comment further up in the list). What do you think? Best regards, Julius

6 years, 2 months

2
1
0 0

Re: [TF-A] Announcing some changes around the code review label in Gerrit

by Javier Almansa Sobrino

Hi Sandrine, If my understanding is right, Code-Owner-Review is the equivalent to the old Code-Review+1 and therefore anyone can add a vote there, owner or not. If that is so, I think the current name might be misleading. Wouldn't it be better just "Code-Review", for instance? As an example, I am not code owner for Measure Boot, but sometimes I am requested to review the patches for it, so in that case it might lead to confusion if I give Code-Owner-Review score. What do you think? Best regards, Javier -----Original Message----- From: Sandrine Bailleux via TF-A <tf-a(a)lists.trustedfirmware.org<mailto:Sandrine%20Bailleux%20via%20TF-A%20%3ctf-a@lists.trustedfirmware.org%3e>> Reply-To: Sandrine Bailleux <sandrine.bailleux(a)arm.com<mailto:Sandrine%20Bailleux%20%3csandrine.bailleux@arm.com%3e>> To: tf-a <tf-a(a)lists.trustedfirmware.org<mailto:tf-a%20%3ctf-a@lists.trustedfirmware.org%3e>> Subject: [TF-A] Announcing some changes around the code review label in Gerrit Date: Tue, 30 Jun 2020 13:29:00 +0000 Hello all, If you recall, the tf.org Project Maintenance Process [1] advocates a code review model where both code owners and maintainers need to review and approve a patch before it gets merged. Until now, the way we would record this in Gerrit was a bit cumbersome and ambiguous. * Code-Review+1 was for code owners to approve a patch. * Code-Review+2 was for maintainers to approve a patch. The submission rules in Gerrit were such that a patch needed a Code-Review+2 to be merged. This meant that if a maintainer was first to take a look at a patch and approved it (Code-Review+2), Gerrit would allow the patch to be merged without any code owners review. To align better with the Project Maintenance Process, I've done some changes in our Gerrit configuration. You will notice that the Code-Review label is gone and has been replaced by 2 new labels: * Code-Owner-Review * Maintainer-Review Stating the obvious but code owners are expected to vote on the Code-Owner-Review label and maintainers on the Maintainer-Review. Note that maintainers might also be code owners for some modules. If they are doing a "code owner" type of review on a patch, they would vote on the Code-Owner-Review label in this case. Any doubt, please ask. I hope I didn't break anything but if you notice any permissions problems (things you used to be able to do and cannot do anymore, or vice versa!) please let me know. One annoying consequence of this change is that if you've got some patches in review right now and got some votes on the former 'Code-Review' label, these have been partially lost. The history of review comments in Gerrit will still show that somebody had voted Code-Review -1/+1 on your patch but this will no longer appear in the labels summary frame and won't count towards the approvals required to get the patch merged. I could not think of a way to avoid this issue... This should only be temporary, while we transition all in-flight patches to the new code review model. Roughly: * Existing "Code-Review -1/+1" votes will have to be replayed as "Code-Owner-Review -1/+1". * Existing "Code-Review -2/+2" votes will have to be replayed as "Maintainer-Review -1/+1". Sorry for the inconvenience. I hope, once we've gone through the transition period, these changes will make the code review process clearer to everybody. I will update the TF-A documentation to explain all of this in the coming future. Best regards, Sandrine [1] https://developer.trustedfirmware.org/w/collaboration/project-maintenance-p…

6 years, 2 months

1
0
0 0

2026

2025

2024

2023

2022

2021

2020

2019

2018

TF-A