On 2018-04-20 11:20, Peng Fan wrote: > Hi Jan, > >> -----Original Message----- >> From: Jan Kiszka [mailto:[email protected]] >> Sent: 2018年4月20日 16:19 >> To: Peng Fan <[email protected]>; [email protected] >> Cc: [email protected] >> Subject: Re: [PATCH] arm64: introduce CONFIG_CPU_PARANGE >> >> On 2018-04-20 08:56, Peng Fan wrote: >>> In i.MX8QM, there are two types of Cortex-A CPUs, Cortex-A53 and >> Cortex-A72. >>> A53 ID_AA64MMFR0_EL1 shows its physical address range supported is >> 40bits. >>> A72 ID_AA64MMFR0_EL1 shows its physical address range supported is >> 44bits. >> >> So, such platforms will naturally be restricted to 40 bits PA for all cores >> and >> never attach resources at higher addresses, correct? Or are there asymmetric >> setups imaginable where only the A72 cores would use such upper resources? > > This is not to set TCR_EL2.PS to 40, but restrict VTCR_EL2.PS to 40. > The issue is jailhouse use the PA to configure VTCR_EL2, it uses PA range on > A72 to configure > A53, this is wrong. > My understanding is the VTCR_EL2 on all the cpus should use same value and > the PS field > should use the minimal value supported by all the cpu types. > > This link includes how xen handles this, but introducing such logic in > jailhouse is not that straight forward, > http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/p2m.c;h=d43c3aa896ab857bd85387f09a3dfea05ca6bac1;hb=HEAD#l1461
Sure, we would have to collect the configuration on each CPU first and only then start laying out the guest page tables. But Jailhouse does all this early on the first CPU that is initialized. Need to think if that could be changed (to the last CPU), but it would be a major overhaul, if possible at all. Claudio, do you think we could have a similar issue on the TX2? It's asymmetric as well IIRC, but maybe with identical parange values. > >> >>> >>> The minimal value 40 needs to be used as cpu parange, if using 44, >>> A53 will be broken. See the error message: >>> >>> " >>> Initializing Jailhouse hypervisor v0.8 (67-gd1b70bbf-dirty) on CPU 4 >>> Code location: 0x0000ffffc0200060 Page pool usage after early setup: >>> mem 43/4067, remap 96/131072 Initializing processors: >>> CPU 4... OK >>> CPU 5... OK >>> CPU 3... OK >>> CPU 1... OK >>> CPU 0... OK >>> CPU 2... OK >>> Page pool usage after late setup: mem 40/4067, remap 368/131072 >>> FATAL: instruction abort at 0x816bbefc >>> >>> FATAL: forbidden access (exception class 0x20) Cell state before >>> exception: >>> pc: ffff000000cd0efc lr: ffff000000cd0efc spsr: 600001c5 EL1 >>> sp: ffff8008fff22fb0 esr: 20 1 0000084 >>> x0: 0000000000000000 x1: 0000000000000000 x2: >> 0000000000000000 >>> x3: 0000000000000000 x4: 0000000000000000 x5: >> 0000000000000000 >>> x6: 0000000000000000 x7: 0000000000000000 x8: >> 0000000000000000 >>> x9: 0000000000000000 x10: 0000000000000000 x11: >> 0000000000000000 >>> x12: 0000000000000000 x13: 0000000000000000 x14: >> 0000000000000000 >>> x15: 0000000000000000 x16: 0000000000000000 x17: >> 0000000000000000 >>> x18: 0000000000000000 x19: ffff000000cd4910 x20: 0000000000000000 >>> x21: 0000000000000000 x22: 0000000000000001 x23: ffff8008f6137dd0 >>> x24: ffff00000945700f x25: ffff8008fff1f0a0 x26: ffff8008fff23090 >>> x27: ffff000009307000 x28: ffff8008f6134000 x29: ffff8008fff22fb0 >>> >>> Parking CPU 2 (Cell: "imx8qm") >>> " >>> >>> Because get_cpu_parange only runs on the master cpu which first runs >>> into hypervisor initialization and A72 runs faster than A53 in >>> i.MX8QM, so A72 will be the master cpu and 44 used as cpu parange. >>> >>> In this patch, introduce a macro to define the cpu parange that could >>> be supported by different cpu types. >>> >>> Signed-off-by: Peng Fan <[email protected]> >>> --- >>> Documentation/hypervisor-configuration.md | 7 +++++++ >>> hypervisor/arch/arm64/include/asm/paging.h | 8 +++++++- >>> 2 files changed, 14 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/hypervisor-configuration.md >>> b/Documentation/hypervisor-configuration.md >>> index 021093e7..4dc4738b 100644 >>> --- a/Documentation/hypervisor-configuration.md >>> +++ b/Documentation/hypervisor-configuration.md >>> @@ -42,6 +42,13 @@ General configuration parameters >>> */ >>> #define CONFIG_BARE_METAL 1 >>> >>> + /* >>> + * To ARM Processors, that has different cpu types integrated, such >>> + * as Big.Little Processors, they have different PA range. >>> + * So define this macro to choose one value works for both cpu types. >>> + */ >>> + #define CONFIG_CPU_PARANGE 40 >>> + >>> ### Example board specific configurations >>> >>> #### ARM >>> diff --git a/hypervisor/arch/arm64/include/asm/paging.h >>> b/hypervisor/arch/arm64/include/asm/paging.h >>> index 0fe1429d..55ee2a44 100644 >>> --- a/hypervisor/arch/arm64/include/asm/paging.h >>> +++ b/hypervisor/arch/arm64/include/asm/paging.h >>> @@ -183,6 +183,10 @@ extern unsigned int cpu_parange; >>> * cpu_parange for later reference */ static inline unsigned int >>> get_cpu_parange(void) { >>> + >>> +#ifdef CONFIG_CPU_PARANGE >>> + return CONFIG_CPU_PARANGE; >>> +#else >>> unsigned long id_aa64mmfr0; >>> >>> arm_read_sysreg(ID_AA64MMFR0_EL1, id_aa64mmfr0); @@ -203,6 >> +207,7 @@ >>> static inline unsigned int get_cpu_parange(void) >>> default: >>> return 0; >>> } >>> +#endif >> >> I dislike compile-time configurations like this. How about making this a >> platform >> parameter? If it's 0, we continue reading it from the first CPU we >> initialize, >> otherwise we use that system config parameter. > > You mean adding an entry in root cell file as the following? > .platform_info = { > .arm = { > Parange = 40, Yes, along this path. Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
