On 14/12/2022 14:24, Michal Orzel wrote:
Hi Julien,

Hi Michal,

On 12/12/2022 10:55, Julien Grall wrote:


From: Julien Grall <jgr...@amazon.com>

The sequence for flushing the TLBs is 4 instruction long and often
requires an explanation how it works.

So create an helper and use it in the boot code (switch_ttbr() is left
Here and in title: s/an helper/a helper/

Done.

alone for now).
Could you explain why?

So we need to decide how we expect switch_ttbr(). E.g. if Xen is relocated at a different, should the caller take care of the instruction/branch predictor flush?

I have expanded to "switch_ttbr() is left alone until we decided the semantic of the call".


Note that in secondary_switched, we were also flushing the instruction
cache and branch predictor. Neither of them was necessary because:
     * We are only supporting IVIPT cache on arm32, so the instruction
       cache flush is only necessary when executable code is modified.
       None of the boot code is doing that.
     * The instruction cache is not invalidated and misprediction is not
       a problem at boot.

Signed-off-by: Julien Grall <jgr...@amazon.com>

Apart from that, the patch is good, so:
Reviewed-by: Michal Orzel <michal.or...@amd.com>
Thanks!



---
     Changes in v3:
         * Fix typo
         * Update the documentation
         * Rename the argument from tmp1 to tmp
---
  xen/arch/arm/arm32/head.S | 30 +++++++++++++++++-------------
  1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 40c1d7502007..315abbbaebec 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -66,6 +66,20 @@
          add   \rb, \rb, r10
  .endm

+/*
+ * Flush local TLBs
+ *
+ * @tmp:    Scratch register
As you are respinning a series anyway, could you add just one space after @tmp:?

Ok.

Cheers,

--
Julien Grall

Reply via email to