Re: [PATCH] xen/arm: Add workaround for Cortex-A53 erratum #845719

[Michal Orzel <michal.orzel@xxxxxxx>]

On 09.12.2020 02:34, Stefano Stabellini wrote:
> On Tue, 8 Dec 2020, Julien Grall wrote:
>> On 08/12/2020 14:38, Bertrand Marquis wrote:
>>> Hi Julien,
>>>
>>>> On 8 Dec 2020, at 09:47, Julien Grall <julien@xxxxxxx> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 08/12/2020 07:23, Michal Orzel wrote:
>>>>> When executing in aarch32 state at EL0, a load at EL0 from a
>>>>> virtual address that matches the bottom 32 bits of the virtual address
>>>>> used by a recent load at (aarch64) EL1 might return incorrect data.
>>>>> The workaround is to insert a write of the contextidr_el1 register
>>>>> on exception return to an aarch32 guest.
>>>>
>>>> I am a bit confused with this comment. In the previous paragraph, you are
>>>> suggesting that the problem is an interaction between EL1 AArch64 and EL0
>>>> AArch32. But here you seem to imply the issue only happen when running a
>>>> AArch32 guest.
>>>>
>>>> Can you clarify it?
>>>
>>> This can happen when switching from an aarch64 guest to an aarch32 guest so
>>> not only when there is interaction.
> 
> Just to confirm: it cannot happen when switching from aarch64 *EL2* to
> aarch32 EL0/1, right?  Because that happens all the time in Xen.
> 
> 
No it cannot. It can only happen when switching from aarch64 EL1 to aarch32 EL0.
>> Right, but the context switch will write to CONTEXTIDR_EL1. So this case
>> should already be handled.
>>
>> Xen will never switch from AArch64 EL1 to AArch32 EL0 without a context 
>> switch
>> (the inverse can happen if we inject an exception to the guest).
>>
>> Reading the Cortex-A53 SDEN, it sounds like this is an OS and not Hypervisor
>> problem. In fact, Linux only seems to workaround it when switching in the OS
>> side rather than the hypervisor.
>>
>> Therefore, I am not sure to understand why we need to workaroud it in Xen.
> 
> It looks like Julien is right in regards to the "aarch64 EL1 to aarch32
> EL0" issue.
> 
Yes I agree. I missed the fact that there is a write to CONTEXTIDR_EL1
in 'ctxt_switch_to'. Let's abandon this.

Thanks,
Michal