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

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

Hi,

On 08/03/18 12:23, Peng Fan wrote:
> 
> 
>> -----Original Message-----
>> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf Of
>> Julien Grall
>> Sent: 2018年3月8日 19:04
>> To: Peng Fan <van.freenix@gmail.com>; Stefano Stabellini
>> <sstabellini@kernel.org>
>> Cc: xen-devel@lists.xen.org
>> Subject: Re: [Xen-devel] [PATCH v4 0/7] unsafe big.LITTLE support
>>
>> Hello,
>>
>> On 08/03/18 06:15, Peng Fan wrote:
>>> Hi Stefano,
>>> On Fri, Mar 02, 2018 at 11:05:54AM -0800, Stefano Stabellini wrote:
>>>> Hi all,
>>>>
>>>> This series changes the initialization of two virtual registers to
>>>> make sure they match the value of the underlying physical cpu.
>>>>
>>>> It also disables cpus different from the boot cpu, unless a newly
>>>> introduced command line option is specified. In that case, it
>>>> explains how to setup the system to avoid corruptions, which involves
>>>> manually specifying the cpu affinity of all domains, because the
>>>> scheduler still lacks big.LITTLE support.
>>>>
>>>> In the uncommon case of a system where the cacheline sizes are
>>>> different  across cores, it disables all cores that have a different
>>>> dcache line size from the boot cpu. In fact, it is not sufficient to
>>>> use the dcache line  size of the current cpu, it would be necessary to
>>>> use the minimum across  all dcache line sizes of all cores.  Given
>>>> that it is actually uncommon  even in big.LITTLE systems, just disable cpus
>> for now.
>>>>
>>>> The first patch in the series is a fix for the way we read the dcache
>>>> line size.
>>>
>>> I am trying the patchset, but I meet issue that Guest Big/Little with
>>> vcpu not working properly. As my current hardware has an issue which
>>> has fix in Kernel,
>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsou
>>>
>> rce.codeaurora.org%2Fexternal%2Fimx%2Flinux-imx%2Fcommit%2F%3Fh%3Di
>> mx_
>>>
>> 4.9.51_imx8_beta2%26id%3D917cc3a8db2f3609ef8e2f59e7bcd31aa2cd4e59&
>> data
>>>
>> =02%7C01%7Cpeng.fan%40nxp.com%7Cc7f074c6708647441f2b08d584e45fec
>> %7C686
>>>
>> ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636561038755176475&sdata
>> =S%2BI
>>> 7g1BwUDgAnXGP8%2FFc1bVZZTIimd3J7%2FkTIdeWL4o%3D&reserved=0
>>
>> Can you describe what you mean by not working properly? Also what is your
>> setup? Did you pin the different vCPUs as requested by the documentation.
>>
> 
> I may not describe clearly. It is domu with big/little guest could not bootup correctly.

What do you mean by "could not bootup correctly"? Can you please provide 
logs or a bit more feedback. Without them, it is nearly impossible to me 
to help to debugging the problem.

> For dom0, the args are
> dom0_mem=2048M dom0_max_vcpus=6 dom0_vcpus_pin=true hmp-unsafe=true
> 
> For domu
> vcpus = 4
> 
> #vcpu pin
> cpus = ['2-3', '2-3', '4-5', '4-5']
> 
> The hardware is cpu0-3 is A53, cpu4-5 is A72.

What does "xl vcpu-list" give you?

> 
> I do not met issue for dom0.
> 
>>>
>>> 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?

>           "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?

> +    //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.

Cheers,

-- 
Julien Grall

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