-----Original Message----- From: TF-A tf-a-bounces@lists.trustedfirmware.org On Behalf Of Raghu Krishnamurthy via TF-A Sent: 30 April 2020 02:33 To: Manish Badarkhe Manish.Badarkhe@arm.com; tf- a@lists.trustedfirmware.org Cc: nd nd@arm.com Subject: Re: [TF-A] Need input on Errata implementation
Hi Manish,
Really appreciate you for taking time to respond to my concerns/questions.
What about this situation? NS-EL2 makes an SMC call to EL3 to get some basic information like GET_SOC_INFORMATION. This is a simple SMC and there is no call to context save or context restore. During the SMC call, if there is a speculative AT instruction on a lower EL(say NS-EL2), there could be a bad cached translation. Do you not need to apply the errata in this situation ? If not, why?
We can't simply apply this errata on reset and just leave the system.
[RK]Totally agree. See CPU_E_HANDLER_FUNC. It is not necessary that cpu_ops are only called during reset and power down. CPU_E_HANDLER_FUNC is called at runtime due EA's.
We thought of taking different approach for this errata
implementation >>where anybody disable this workaround using macro as this errata is >>applicable for most of the CPUs (by default enabled) and can't be >>placed in cpu_ops.
[RK]This is a poor approach in my view. Most CPU's is not all CPU's. The reason the errata framework exists is to apply CPU specific erratas by checking for them dynamically. Different stepping's of the same CPU's may or may not have the errata and typically you check the MIDR to know if the errata applies or not. Linux does not apply the errata to all CPU's since "most" CPU's have the issue. They check for its existence at runtime and only then apply it. TF-A should not hold itself to a lower standard.
Hi Raghu I guess this depends on what the errata workaround involves. Since this workaround applies bit setting on an out of context register, it was not expected to affect the EL3 execution performance (or the lower level EL because the bits are restored on return). Also it was thought that the act of searching through the list of compiled CPUs and checking if the workaround is applicable might be more detrimental than the unilateral application of the workaround for this case (assuming no extra barriers are added since the code path it is inserted in have them already later in the sequence).
But I agree it is more elegant to have this coupled into CPU_OPS framework. I think Manish has some ideas for this.
Best Regards Soby Mathew
-Raghu
On 4/29/20 1:35 AM, Manish Badarkhe wrote:
Hi Raghu
Just to add/correct one more thing from my previous emails that this errata
workaround proposed is
applied to both normal and secure world switches to EL3.
Thanks Manish Badarkhe
On 29/04/2020, 12:25, "TF-A on behalf of Manish Badarkhe via TF-A" <tf-a-
bounces@lists.trustedfirmware.org on behalf of tf-a@lists.trustedfirmware.org> wrote:
Hi Raghu On 29/04/2020, 02:00, "Raghu K" <raghu.ncstate@icloud.com> wrote: Hi Manish, Thanks. >> we don’t have any AT instances in minimum execution window aftercontext switching from S-EL(1/2)
>> to EL3 and before updating TCR register. 1) What is the minimum execution window? Does that not change basedon micro-architecture?
Not sure about exact minimum execution window. IMO, it really dependupon when "context_save" gets called after
entering into EL3 from S-EL1/2. It may changed upon micro-architecture.Need some experts comment here.
2) Do we know that the "execution window" is exactly the same for allthe CPU's this errata applies to?
It may be but we should not worry on that if we don’t have any ATinstruction execution in that window.
Also, it appears we are only talking about switching from S-EL1/2 to EL3.The same issue can happen when you go from NS-EL1/EL2 to EL3 as well. There also seems to be an assumption in the patch you submitted that this errata happens only during a so called context-switch. From my reading, the cortex- Ax errata notices don't limit the errata to occur only during "context-switches" in the "conditions" section and can occur while executing ANY code, although the work around section does muddy the waters a bit.
In Linux, at NS-EL2 this workaround is already in place. Hence we justthought of considering cases from Secure EL side to put this workaround.
Yes, errata should not limit to particular conditional section but thisparticular errata is not straight-forward like another errata placed in the code currently. We can't simply apply this errata on reset and just leave the system.
Back to problem, AT instruction speculative execution using out-of-context regime that results in page table walk and generate the incorrect
translation which are cached in TLB. To avoid this issue we thought ofdisabling PTW for that particular EL.
for e.g. If AT instruction execution for EL1 present in EL3 then we have tomake sure speculative behaviour of this AT should not result in incorrect translation cached in TLB. If system is always in EL3 (if we loop-in in EL3 always without going back and forth to/from lower EL) then in that case
there is no need of this workaround. Hence we thought to put this workaround over boundary context ofcontext switches. When "context save" (close to EL3 entry) happened we meticulously save all EL system registers (S-EL1/S-EL2) with PTW disabled and continue EL3 execution with PTW disabled ensuring we should not cache any incorrect translation for (S-EL1/S-EL2) and during "context restore" (i.e. close to EL3 exit) again we disabled PTW, restore all system registers for EL (S-EL1/S- EL2) except TCR and then restore TCR.
3) Has there been any work done to actually reproduce this issue andalso to see that this actually fixes the issue?
No this issue is hard to reproduce. 4) Has the CPU errata framework(cpu_ops etc.) been considered topossibly implement the errata? Sprinkling erratas through common framework code does not seem like a good idea.
We thought of taking different approach for this errata implementationwhere anybody disable this workaround using macro as this errata is applicable for most of the CPUs (by default enabled) and can't be placed in cpu_ops.
Thanks Raghu Thanks Manish Badarkhe On 4/28/20 1:44 AM, Manish Badarkhe wrote: > Hi Raghu > > Please see my replies inline. > > Regards > Manish Badarkhe > > On 28/04/2020, 11:29, "Raghu Krishnamurthy"raghu.ncstate@icloud.com wrote:
> > Hi Manish, > > Understood. > > >>Hence before entering in EL3, we ensured that PTW is disabled(at
> context save) > > The context save and restore functions are executed in EL3. So howare
> you disabling PTW before entering EL3 ? > > Yes, I put it wrongly. We thought "context_save/restore" is best placeto disable PTW without much affecting the
> code because we don’t have any AT instances in minimum executionwindow after context switching from S-EL(1/2)
> to EL3 and before updating TCR register. > > -Raghu > > Thanks > Manish Badarkhe > > On 4/27/20 10:53 PM, Manish Badarkhe wrote: > > Hi Raghu > > > > This workaround is specifically need for speculative AT instructionbehaviour in out of context regime.
> > That means executing AT instruction for lower ELs (S-EL1/S-EL2) inhigher EL i.e. EL3.
> > > > Behaviour of AT instruction is unaltered when it get executed insame regime (when AT instruction executed for same EL
> > where it is executing) and there is no possibility to execute ATinstruction for higher EL in lower EL.
> > > > Hence before entering in EL3, we ensured that PTW is disabled (atcontext save) and restore PTW back during
> > exit of EL3. (at context restore). > > > > Thanks > > Manish Badarkhe > > > > On 28/04/2020, 01:23, "Raghu K" <raghu.ncstate@icloud.com>wrote:
> > > > Hi Manish, > > > > >>Hence proposed solution will work as it is > > > > [RK]If you are sure go ahead. I'm not convinced, but that maybe because
> > i don't understand the errata fully/correctly. > > > > >>This workaround is very specific during context switching > > > > [RK] Context switching has many meanings depending on thecontext(OS,
> > hypervisor, TF-A world switch etc). The errata document i sawdoes not
> > elaborate on this. Perhaps clarifying this will help understandwhy the
> > solution you proposed will work. > > > > The solution below in points 2 and 3 have the same problem onentry and
> > exit, mentioned in my first email. Before you call > > el1_sysregs_context_save, an AT instruction could havespeculatively
> > executed through speculation of branches that occur BEFOREyou call this
> > function, when TCR still has the enable bit set. The fact that youdon't
> > have an AT instruction in the context save routine or anyroutine for
> > that matter, does not guarantee that the hardware did notspeculate
> > through some other means to reach an AT instruction. Thesame applies to
> > the context_restore routines. There is no guarantee right afteryou
> > finish the restore routing(and hence TCR has the enable bit set),that
> > the CPU cannot speculate to an AT instruction. > > So i'm not clear how you can say for certain that there was no > > speculative AT instruction with the proposal below. > > > > Thanks > > Raghu > > > > On 4/27/20 10:08 AM, Manish Badarkhe wrote: > > > Hi All, > > > > > > Just update/correct details. > > > > > > Thanks > > > Manish Badarkhe > > > > > > On 27/04/2020, 22:13, "TF-A on behalf of Manish Badarkhevia TF-A" <tf-a-bounces@lists.trustedfirmware.org on behalf of tf- a@lists.trustedfirmware.org> wrote:
> > > > > > Hi Raghu > > > > > > Please ignore my answer on question 2. > > > > > > With internal discussion came to below conclusion: > > > 1. This workaround is very specific during contextswitching.
> > > 2 . If you check in context save routine(el1_sysregs_context_save or el2_sysregs_context_save),
> > > As per proposed solution, First step performed is todisable page table walk and we don’t have
> > > any AT instruction execution in context save routine. > > > This ensures that there will be no possibility ofspeculative AT instruction execution without TCR update.
> > > 3. If you check in context restore routine(el1_sysregs_context_restore or el2_sysregs_context_restore),
> > > As per proposed solution, first step performed is todisable page table walk and we don’t have any
> > > AT instruction execution in context restore routine. > > > This ensures that there will be no possibility ofspeculative AT instruction execution without TCR update.
> > > > > > Hence proposed solution will work as it is ensuring nocaching of translations in TLB while speculative AT instruction execution.
> > > > > > Thanks > > > Manish Badarkhe > > > > > > On 27/04/2020, 13:38, "TF-A on behalf of Manish Badarkhevia TF-A" <tf-a-bounces@lists.trustedfirmware.org on behalf of tf- a@lists.trustedfirmware.org> wrote:
> > > > > > Hi Raghu > > > > > > Please see my answers inline > > > > > > On 25/04/2020, 06:38, "TF-A on behalf of Raghu K via TF-A" <tf-a-bounces@lists.trustedfirmware.org on behalf of tf- a@lists.trustedfirmware.org> wrote:
> > > > > > Hi Manish, > > > > > > Before I agree or disagree with the suggested fix, thefollowing would
> > > be interesting to know/discuss. Please feel free tocorrect me if i've
> > > misunderstood something. > > > > > > 1) Are "speculative" AT instructions subject to TCR_ELxcontrol bits for
> > > all the listed CPU's? I imagine the answer is yes butwould be good to
> > > get confirmation. I could not find any evidence in theinstruction
> > > description or psuedocode in the ARMv8 ARM. It ispossible to play many
> > > tricks on speculative execution of instructions such asskipping checks
> > > and doing them only when the CPU knows theinstruction will be
> > > committed. If this is the case, changing TCR_ELx bitsmay not work. The
> > > errata document is vague about how to fix it. > > > > > > The speculative AT instruction may behave as youmentioned. We need more
> > > opinion on this. > > > Proposed fix I mentioned by referring linux workaroundfor the same errata.
> > > Linux workaround is available in mainline kernel asbelow:
> > >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h... v5.7-rc3&id=bd227553ad5077f21ddb382dcd910ba46181805a
> > > > > > 2) Assuming the answer to question 1 is yes, yourproposal may not work
> > > as is. In the worst case, as soon as you enter EL3, thevery first thing
> > > that may happen, before you ever operate/write toTCR_ELx, is a
> > > speculative AT instruction that caches a bad translationin the TLB's.
> > > The same thing can happen on the exit path. As soon asyou restore the
> > > TCR_ELx register, the first thing that can happen is aspeculative AT
> > > that caches a bad translation. However, the el3_exitpath does have DSB
> > > before ERET, so we will not speculate to an ATinstruction if there are
> > > no branches between the instruction that sets TCR_ELxand the ERET.
> > > Somewhere in between, it looks like we will need aTLBI NSH to be
> > > certain there are no bad translation cached. Thisobviously has a
> > > potential performance cost on the lower EL's. Everyentry into EL3
> > > flushes the TLB for lower EL's. > > > > > > Yes, this seems to be valid case during entry and exit path. > > > I am not quite sure in that case where we need to avoidPTW.
> > > Also "TLBI NSH" works but it may cause performanceissue.
> > > Need some more opinion/thoughts on this. > > > > > > Just thinking, can sequence mentioned for context savedoes not ensure that
> > > PTW is disabled? > > > Something as below as last step in ELx(1/2) context save(elaborated more):
> > > > ·Save TCR register with PTW enable (EPD=0). (Just toenable PTW during
> > > > restore context). Do not operate TCR_EL1x registerhere just save its value to restore.
> > > > This ensures that during entry in EL3 there will be nochance of PTW
> > > >. while executing AT instruction. > > > > > > Thanks > > > > > > Raghu > > > > > > Thanks > > > Manish Badarkhe > > > > > > On 4/24/20 2:56 AM, Manish Badarkhe via TF-A wrote: > > > > > > > > Hi All > > > > > > > > We are trying to implement errata which is applicablefor below CPUs:
> > > > > > > > <CPUs> : <Errata No.> > > > > > > > > Cortex-A53: 1530924 > > > > > > > > Cortex-A76: 1165522 > > > > Cortex-A72: 1319367 > > > > Cortex-A57: 1319537 > > > > Cortex-A55: 1530923 > > > > > > > > *Errata Description:* > > > > > > > > A speculative Address Translation (AT) instructiontranslates using
> > > > registers that are associated with an out-of-contexttranslation
> > > > regime and caches the resulting translation in the TLB.A subsequent
> > > > translation request that is generated when the out-of-context
> > > > translation regime is current uses the previous cachedTLB entry
> > > > producing an incorrect virtual to physical mapping. > > > > > > > > *Probable solution is to implement below fix incontext.S file:*
> > > > > > > > *During ELx (1 or 2) context save:* > > > > > > > > ·Operate TCR_ELx(1/2) to disable page table walk byoperating EPD bits
> > > > > > > > oThis will avoid any page table walk for S-EL1 or S-EL2.This will
> > > > help in avoiding caching of translations in TLB > > > > > > > > for S-EL1/S-EL2 in EL3. > > > > > > > > ·Save all system registers (which is already available)except TCR
> > > > > > > > ·Clear EPD bits of TCR and then save. (Just to enablePTW during
> > > > restore context). > > > > > > > > *During ELx (1 or 2) context restore:* > > > > > > > > * Operate TCR_ELx(1/2) to disable page table walkby operating EPD bits
> > > > * Restore all system registers (which are savedduring context save)
> > > > except TCR register. > > > > * Restore TCR_ELx(1/2) register (which enable backPTW).
> > > > > > > > With above we ensured that there will be no pagetable walk for S-EL1
> > > > and S-EL2 in EL3. > > > > > > > > is this proper other way to fix this problem? Needsome suggestion/use
> > > > cases where and all we need this workaround in TF-Acode.
> > > > > > > > Thanks > > > > > > > > Manish Badarkhe > > > > > > > > IMPORTANT NOTICE: The contents of this email andany attachments are
> > > > confidential and may also be privileged. If you are notthe intended
> > > > recipient, please notify the sender immediately anddo not disclose
> > > > the contents to any other person, use it for anypurpose, or store or
> > > > copy the information in any medium. Thank you. > > > > > > > > > > -- > > > TF-A mailing list > > > TF-A@lists.trustedfirmware.org > > > https://lists.trustedfirmware.org/mailman/listinfo/tf-a > > > > > > IMPORTANT NOTICE: The contents of this email and anyattachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> > > -- > > > TF-A mailing list > > > TF-A@lists.trustedfirmware.org > > > https://lists.trustedfirmware.org/mailman/listinfo/tf-a > > > > > > IMPORTANT NOTICE: The contents of this email and anyattachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> > > -- > > > TF-A mailing list > > > TF-A@lists.trustedfirmware.org > > > https://lists.trustedfirmware.org/mailman/listinfo/tf-a > > > > > > IMPORTANT NOTICE: The contents of this email and anyattachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> > > > > > IMPORTANT NOTICE: The contents of this email and anyattachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> > > > IMPORTANT NOTICE: The contents of this email and any attachmentsare confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
-- TF-A mailing list TF-A@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a-- TF-A mailing list TF-A@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a