Hi Julien, 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/
> alone for now). Could you explain why? > > 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> > > --- > 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:? ~Michal
