Re: [RFC v2 03/12] xen/arm32: head: Introduce an helper to flush the TLBs

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Julien,

On 22/10/2022 17:04, Julien Grall wrote:
> 
> 
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> The sequence for flushing the TLBs is 4 instruction long and often
> require an explanation how it works.
s/require/requires/

> 
> So create an helper and use it in the boot code (switch_ttbr() is left
> alone for now).
> 
> 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 <jgrall@xxxxxxxxxx>
> ---
>  xen/arch/arm/arm32/head.S | 31 ++++++++++++++++++-------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 163bd6596dec..aeaa8d105aeb 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -66,6 +66,21 @@
>          add   \rb, \rb, r10
>  .endm
> 
> +/*
> + * Flush local TLBs
> + *
> + * tmp1:    Scratch register
I would love to adhere to the way of describing macro params like you did in 
mov_w. This would mean:
@tmp: scratch register

Apart from that, the change looks ok.

Question on the side:
Why do we use nshst in assembly and ishst in TLB helper macro?
Is it because the latter is also used to flush the inner TLBs whereas the 
former only local ones?

~Michal