Re: [PATCH early-RFC 4/5] xen/arm: mm: Rework switch_ttbr()

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Julien,

> On 25 Mar 2022, at 15:42, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Bertrand,
> 
> On 25/03/2022 14:35, Bertrand Marquis wrote:
>>> On 25 Mar 2022, at 15:24, Julien Grall <julien@xxxxxxx> wrote:
>>> On 25/03/2022 13:47, Bertrand Marquis wrote:
>>>> Hi Julien,
>>> 
>>> Hi Bertrand,
>>> 
>>>>> On 9 Mar 2022, at 12:20, Julien Grall <julien@xxxxxxx> wrote:
>>>>> 
>>>>> From: Julien Grall <jgrall@xxxxxxxxxx>
>>>>> 
>>>>> At the moment, switch_ttbr() is switching the TTBR whilst the MMU is
>>>>> still on.
>>>>> 
>>>>> Switching TTBR is like replacing existing mappings with new ones. So
>>>>> we need to follow the break-before-make sequence.
>>>>> 
>>>>> In this case, it means the MMU needs to be switched off while the
>>>>> TTBR is updated. In order to disable the MMU, we need to first
>>>>> jump to an identity mapping.
>>>>> 
>>>>> Rename switch_ttbr() to switch_ttbr_id() and create an helper on
>>>>> top to temporary map the identity mapping and call switch_ttbr()
>>>>> via the identity address.
>>>>> 
>>>>> switch_ttbr_id() is now reworked to temporarily turn off the MMU
>>>>> before updating the TTBR.
>>>>> 
>>>>> We also need to make sure the helper switch_ttbr() is part of the
>>>>> identity mapping. So move _end_boot past it.
>>>>> 
>>>>> Take the opportunity to instruction cache flush as the operation is
>>>>> only necessary when the memory is updated.
>>>> Your code is actually remove the instruction cache invalidation so
>>>> this sentence is a bit misleading.
>>> 
>>> I forgot to add the word "remove" in the sentence.
>> Ok (my sentence was also wrong by the way)
>>> 
>>>> Also an open question: shouldn’t we flush the data cache ?
>>> Do you mean clean/invalidate to PoC/PoU? Something else?
>> Yes, probably to PoU.
>>> 
>>>> As we switch from one TTBR to an other, there might be some data
>>>> in the cache dependent that could be flushed while the MMU is off
>>> 
>>> I am a bit confused. Those flush could also happen with the MMU on. So how 
>>> turning off the MMU would result to a problem? Note that the data cache is 
>>> still enabled during the switch.
>> If the first level of cache is VIPT and we turn off the MMU, I am wondering 
>> if this could not create troubles and could require the cache to be flushed 
>> before turning the MMU off.
> My reading of the Arm Arm (D5.11.1 "Data and unified caches" ARM DDI 0487F.c) 
> suggests the data cache is always PIPT.

You are right, only the instruction cache is VIPT.
So the problem most probably does not exist.

> 
>> I have no idea if this is a problem or not, just raising the question.
>> I can try to dig on that at Arm when I am back in 10 days.
> 
> Enjoy it!

Thanks
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall