Re: [PATCH v4 0/7] unsafe big.LITTLE support

[Julien Grall <julien.grall@arm.com>]

Hi,

On 08/03/18 12:43, Peng Fan wrote:
>>>>> I am not sure whether this issue cause DomU big/Little not work.
>>>>
>>>> Well, I would recommend to speak with NXP whether this errata affects
>>>> TLB flush for Hypervisor Page-Table or Stage-2 Page-Table.
>>>
>>> I tried the following, but no help. Not sure my patch is correct. I
>>> think it affects stage2 TLB.
>>>
>>> --- a/xen/include/asm-arm/arm64/flushtlb.h
>>> +++ b/xen/include/asm-arm/arm64/flushtlb.h
>>> @@ -6,7 +6,7 @@ static inline void flush_tlb_local(void)
>>>    {
>>>        asm volatile(
>>>            "dsb sy;"
>>> -        "tlbi vmalls12e1;"
>>> +        "tlbi alle1;"
>>>            "dsb sy;"
>>>            "isb;"
>>>            : : : "memory");
>>> @@ -17,7 +17,7 @@ static inline void flush_tlb(void)
>>>    {
>>>        asm volatile(
>>>            "dsb sy;"
>>> -        "tlbi vmalls12e1is;"
>>> +        "tlbi alle1;"
>>
>> I am not sure why you drop the innershareable here?
> Just want to invalid all the tlb, innershareable could be kept.
> This is not a formal patch, just my trying to narrow the issue.

alle1 will only flush the TLBs of the local processor. The flush will 
not get propagated to the other CPUs of the system. So you definitely 
want this to be innershareable to avoid the other processors containing 
stale TLBs.

>>
>>>            "dsb sy;"
>>>            "isb;"
>>>            : : : "memory");
>>> @@ -39,7 +39,7 @@ static inline void flush_tlb_all(void)
>>>    {
>>>        asm volatile(
>>>            "dsb sy;"
>>> -        "tlbi alle1is;"
>>> +        "tlbi alle1;"
>>
>> Ditto.
>>
>>>            "dsb sy;"
>>>            "isb;"
>>>            : : : "memory");
>>> --- a/xen/include/asm-arm/arm64/page.h
>>> +++ b/xen/include/asm-arm/arm64/page.h
>>> @@ -74,14 +74,16 @@ static inline void flush_xen_data_tlb_local(void)
>>>    /* Flush TLB of local processor for address va. */
>>>    static inline void  __flush_xen_data_tlb_one_local(vaddr_t va)
>>>    {
>>> -    asm volatile("tlbi vae2, %0;" : : "r" (va>>PAGE_SHIFT) : "memory");
>>> +       flush_xen_data_tlb_local();
>>> +    //asm volatile("tlbi vae2, %0;" : : "r" (va>>PAGE_SHIFT) :
>>> + "memory");
>>>    }
>>>
>>>    /* Flush TLB of all processors in the inner-shareable domain for
>>>     * address va. */
>>>    static inline void __flush_xen_data_tlb_one(vaddr_t va)
>>>    {
>>> -    asm volatile("tlbi vae2is, %0;" : : "r" (va>>PAGE_SHIFT) : "memory");
>>> +       flush_xen_data_tlb_local();
>>
>> Why do you replace an innershareable call to a local call? Is it part of the errata?
> 
> No. Just my trying to narrow down.

Then you should keep the innershareable. See above.

>>
>>> +    //asm volatile("tlbi vae2is, %0;" : : "r" (va>>PAGE_SHIFT) :
>>> + "memory");
>>>    }
>>>
>>>>
>>>>> So wonder has this patchset been tested on Big/Little Hardware?
>>>>
>>>> This series only adds facility to report the correct MIDR to the guest.
>>>> If your platform requires more, then it would be necessary send a patch for
>> Xen.
>>>
>>> Do you have any suggestions? Besides MIDR/ACTLR/Cacheline, are there more
>> needed?
>>
>> Having a bit more details from your side would be helpful. At the moment, I have
>> no clue what's going on.
> 
> As from the linux kernel commit:
>      on i.MX8QM TO1.0, there is an issue: the bus width between A53-CCI-A72
>      is limited to 36bits.TLB maintenance through DVM messages over AR channel,
>      some bits will be forced(truncated) to zero as the followings:
> 
>      ASID[15:12] is forced to 0
>      VA[48:45] is forced to 0
>      VA[44:41] is forced to 0
>      VA[39:36] is forced to 0
> 
>      This issue will result in the TLB aintenance across the clusters not working
>      as expected due to some VA and ASID bits get truncated and forced to be zero.
> 
>      The SW workaround is: use the vmalle1is if VA larger than 36bits or
>      ASID[15:12] is not zero, otherwise, we use original TLB maintenance path.
> 
> When doing tlb maintenance through DVM from A53 to A72, some bits are forced
> to 0, this means TLB may not be really invalidated from A72 perspective.
> 
> Currently I am trying a domu with big/little capability, but not allowing big/little vcpu
> migration.
> 
> I am not sure whether this hardware issue impacts DomU or not. Or it is software issue.
> As you could see dom0 has 6 vcpus, I did a stress test and not found issue on dom0.

There are a major difference between Dom0 and DomU in your setup. Dom0 
vCPUs are pinned to a specific pCPU, so they can't move around. For 
DomU, each vCPU are pinned to a set of pCPUs, so they can move around.

But, did you check the DomU has the workaround enabled? I am asking that 
because it looks like to me the way to detect the workaround is based on 
a device (scu) and not processor. So I am not convinced that DomU is 
actually using your workaround.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel