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

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 08/03/18 12:23, Peng Fan wrote:
> > -----Original Message-----
> > From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf Of
> > Julien Grall
> > Sent: 2018年3月8日 19:04
> > To: Peng Fan <van.freenix@xxxxxxxxx>; Stefano Stabellini
> > <sstabellini@xxxxxxxxxx>
> > Cc: xen-devel@xxxxxxxxxxxxx
> > 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel