* [PATCH v4 0/7] unsafe big.LITTLE support
@ 2018-03-02 19:05 Stefano Stabellini
2018-03-02 19:06 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Stefano Stabellini
2018-03-08 6:15 ` [PATCH v4 0/7] unsafe big.LITTLE support Peng Fan
0 siblings, 2 replies; 26+ messages in thread
From: Stefano Stabellini @ 2018-03-02 19:05 UTC (permalink / raw)
To: julien.grall; +Cc: sstabellini, xen-devel
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.
Cheers,
Stefano
Julien Grall (1):
xen/arm: Park CPUs with a MIDR different from the boot CPU.
Stefano Stabellini (6):
xen/arm: Read the dcache line size from CTR register
xen/arm: make processor a per cpu variable
xen/arm: read ACTLR on the pcpu where the vcpu will run
xen/arm: set VPIDR based on the MIDR value of the underlying pCPU
xen/arm: update the docs about heterogeneous computing
xen/arm: disable CPUs with different dcache line sizes
docs/misc/arm/big.LITTLE.txt | 46 +++++++++++++++++++++++++++++++++++++
docs/misc/xen-command-line.markdown | 15 ++++++++++++
xen/arch/arm/arm32/head.S | 2 +-
xen/arch/arm/arm64/head.S | 2 +-
xen/arch/arm/domain.c | 15 ++++++------
xen/arch/arm/processor.c | 8 +++----
xen/arch/arm/setup.c | 17 ++------------
xen/arch/arm/smpboot.c | 39 +++++++++++++++++++++++++++++++
xen/arch/arm/vcpreg.c | 4 ++--
xen/include/asm-arm/cpregs.h | 2 ++
xen/include/asm-arm/domain.h | 3 ---
xen/include/asm-arm/page.h | 27 +++++++++++++++-------
12 files changed, 138 insertions(+), 42 deletions(-)
create mode 100644 docs/misc/arm/big.LITTLE.txt
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register 2018-03-02 19:05 [PATCH v4 0/7] unsafe big.LITTLE support Stefano Stabellini @ 2018-03-02 19:06 ` Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 2/7] xen/arm: Park CPUs with a MIDR different from the boot CPU Stefano Stabellini ` (6 more replies) 2018-03-08 6:15 ` [PATCH v4 0/7] unsafe big.LITTLE support Peng Fan 1 sibling, 7 replies; 26+ messages in thread From: Stefano Stabellini @ 2018-03-02 19:06 UTC (permalink / raw) To: julien.grall; +Cc: sstabellini, xen-devel See the corresponding Linux commit as reference: commit f91e2c3bd427239c198351f44814dd39db91afe0 Author: Catalin Marinas <catalin.marinas@arm.com> Date: Tue Dec 7 16:52:04 2010 +0100 ARM: 6527/1: Use CTR instead of CCSIDR for the D-cache line size on ARMv7 The current implementation of the dcache_line_size macro reads the L1 cache size from the CCSIDR register. This, however, is not guaranteed to be the smallest cache line in the cache hierarchy. The patch changes to the macro to use the more architecturally correct CTR register. Reported-by: Kevin Sapp <ksapp@quicinc.com> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> Also rename cacheline_bytes to dcache_line_bytes to clarify that it is the minimum D-Cache line size. Suggested-by: Julien Grall <julien.grall@arm.com> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> --- Changes in v4: - move patch to the beginning of the series - rename cacheline_bytes to dcache_line_bytes - improve commit message --- xen/arch/arm/arm32/head.S | 2 +- xen/arch/arm/arm64/head.S | 2 +- xen/arch/arm/setup.c | 13 ++++++------- xen/include/asm-arm/cpregs.h | 2 ++ xen/include/asm-arm/page.h | 16 ++++++++-------- 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 43374e7..2b12908 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -504,7 +504,7 @@ ENTRY(relocate_xen) dsb /* So the CPU issues all writes to the range */ mov r5, r4 - ldr r6, =cacheline_bytes /* r6 := step */ + ldr r6, =dcache_line_bytes /* r6 := step */ ldr r6, [r6] mov r7, r3 diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index fa0ef70..38899c7 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -631,7 +631,7 @@ ENTRY(relocate_xen) dsb sy /* So the CPU issues all writes to the range */ mov x9, x3 - ldr x10, =cacheline_bytes /* x10 := step */ + ldr x10, =dcache_line_bytes /* x10 := step */ ldr x10, [x10] mov x11, x2 diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 032a6a8..fced75a 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -680,19 +680,18 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size) } #endif -size_t __read_mostly cacheline_bytes; +size_t __read_mostly dcache_line_bytes; /* Very early check of the CPU cache properties */ void __init setup_cache(void) { - uint32_t ccsid; + uint32_t ctr; - /* Read the cache size ID register for the level-0 data cache */ - WRITE_SYSREG32(0, CSSELR_EL1); - ccsid = READ_SYSREG32(CCSIDR_EL1); + /* Read CTR */ + ctr = READ_SYSREG32(CTR_EL0); - /* Low 3 bits are log2(cacheline size in words) - 2. */ - cacheline_bytes = 1U << (4 + (ccsid & 0x7)); + /* Bits 16-19 are the log2 number of words in the cacheline. */ + dcache_line_bytes = (size_t) (4 << ((ctr >> 16) & 0xf)); } /* C entry point for boot CPU */ diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h index 9e13848..8db65d5 100644 --- a/xen/include/asm-arm/cpregs.h +++ b/xen/include/asm-arm/cpregs.h @@ -106,6 +106,7 @@ /* CP15 CR0: CPUID and Cache Type Registers */ #define MIDR p15,0,c0,c0,0 /* Main ID Register */ +#define CTR p15,0,c0,c0,1 /* Cache Type Register */ #define MPIDR p15,0,c0,c0,5 /* Multiprocessor Affinity Register */ #define ID_PFR0 p15,0,c0,c1,0 /* Processor Feature Register 0 */ #define ID_PFR1 p15,0,c0,c1,1 /* Processor Feature Register 1 */ @@ -303,6 +304,7 @@ #define CPACR_EL1 CPACR #define CPTR_EL2 HCPTR #define CSSELR_EL1 CSSELR +#define CTR_EL0 CTR #define DACR32_EL2 DACR #define ESR_EL1 DFSR #define ESR_EL2 HSR diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h index d948250..ce18f0c 100644 --- a/xen/include/asm-arm/page.h +++ b/xen/include/asm-arm/page.h @@ -134,7 +134,7 @@ /* Architectural minimum cacheline size is 4 32-bit words. */ #define MIN_CACHELINE_BYTES 16 /* Actual cacheline size on the boot CPU. */ -extern size_t cacheline_bytes; +extern size_t dcache_line_bytes; #define copy_page(dp, sp) memcpy(dp, sp, PAGE_SIZE) @@ -145,7 +145,7 @@ extern size_t cacheline_bytes; static inline int invalidate_dcache_va_range(const void *p, unsigned long size) { const void *end = p + size; - size_t cacheline_mask = cacheline_bytes - 1; + size_t cacheline_mask = dcache_line_bytes - 1; dsb(sy); /* So the CPU issues all writes to the range */ @@ -153,7 +153,7 @@ static inline int invalidate_dcache_va_range(const void *p, unsigned long size) { p = (void *)((uintptr_t)p & ~cacheline_mask); asm volatile (__clean_and_invalidate_dcache_one(0) : : "r" (p)); - p += cacheline_bytes; + p += dcache_line_bytes; } if ( (uintptr_t)end & cacheline_mask ) { @@ -161,7 +161,7 @@ static inline int invalidate_dcache_va_range(const void *p, unsigned long size) asm volatile (__clean_and_invalidate_dcache_one(0) : : "r" (end)); } - for ( ; p < end; p += cacheline_bytes ) + for ( ; p < end; p += dcache_line_bytes ) asm volatile (__invalidate_dcache_one(0) : : "r" (p)); dsb(sy); /* So we know the flushes happen before continuing */ @@ -173,8 +173,8 @@ static inline int clean_dcache_va_range(const void *p, unsigned long size) { const void *end = p + size; dsb(sy); /* So the CPU issues all writes to the range */ - p = (void *)((uintptr_t)p & ~(cacheline_bytes - 1)); - for ( ; p < end; p += cacheline_bytes ) + p = (void *)((uintptr_t)p & ~(dcache_line_bytes - 1)); + for ( ; p < end; p += dcache_line_bytes ) asm volatile (__clean_dcache_one(0) : : "r" (p)); dsb(sy); /* So we know the flushes happen before continuing */ /* ARM callers assume that dcache_* functions cannot fail. */ @@ -186,8 +186,8 @@ static inline int clean_and_invalidate_dcache_va_range { const void *end = p + size; dsb(sy); /* So the CPU issues all writes to the range */ - p = (void *)((uintptr_t)p & ~(cacheline_bytes - 1)); - for ( ; p < end; p += cacheline_bytes ) + p = (void *)((uintptr_t)p & ~(dcache_line_bytes - 1)); + for ( ; p < end; p += dcache_line_bytes ) asm volatile (__clean_and_invalidate_dcache_one(0) : : "r" (p)); dsb(sy); /* So we know the flushes happen before continuing */ /* ARM callers assume that dcache_* functions cannot fail. */ -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 2/7] xen/arm: Park CPUs with a MIDR different from the boot CPU. 2018-03-02 19:06 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Stefano Stabellini @ 2018-03-02 19:06 ` Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 3/7] xen/arm: make processor a per cpu variable Stefano Stabellini ` (5 subsequent siblings) 6 siblings, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2018-03-02 19:06 UTC (permalink / raw) To: julien.grall; +Cc: sstabellini, xen-devel From: Julien Grall <julien.grall@arm.com> From: Julien Grall <julien.grall@arm.com> Xen does not properly support big.LITTLE platform. All vCPUs of a guest will always have the MIDR of the boot CPU (see arch_domain_create). At best the guest may see unreliable performance (vCPU switching between big and LITTLE), at worst the guest will become unreliable or insecure. This is becoming more apparent with branch predictor hardening in Linux because they target a specific kind of CPUs and may not work on other CPUs. For the time being, park any CPUs with a MDIR different from the boot CPU. This will be revisited in the future once Xen gains understanding of big.LITTLE. [1] https://lists.xenproject.org/archives/html/xen-devel/2016-12/msg00826.html Signed-off-by: Julien Grall <julien.grall@arm.com> Reviewed-by: Oleksandr Tyshchenkko <oleksandr_tyshchenko@epam.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Acked-by: Jan Beulich <jbeulich@suse.com> --- docs/misc/xen-command-line.markdown | 10 ++++++++++ xen/arch/arm/smpboot.c | 26 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index f73990f..7b80119 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -985,6 +985,16 @@ supported only when compiled with XSM on x86. Control Xens use of the APEI Hardware Error Source Table, should one be found. +### hmp-unsafe (arm) +> `= <boolean>` + +> Default : `false` + +Say yes at your own risk if you want to enable heterogenous computing +(such as big.LITTLE). This may result to an unstable and insecure +platform. When the option is disabled (default), CPUs that are not +identical to the boot CPU will be parked and not used by Xen. + ### hpetbroadcast > `= <boolean>` diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index 1255185..7ea4e41 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -27,6 +27,7 @@ #include <xen/smp.h> #include <xen/softirq.h> #include <xen/timer.h> +#include <xen/warning.h> #include <xen/irq.h> #include <xen/console.h> #include <asm/cpuerrata.h> @@ -69,6 +70,13 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_mask); /* representing HT and core siblings of each logical CPU */ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask); +/* + * By default non-boot CPUs not identical to the boot CPU will be + * parked. + */ +static bool __read_mostly opt_hmp_unsafe = false; +boolean_param("hmp-unsafe", opt_hmp_unsafe); + static void setup_cpu_sibling_map(int cpu) { if ( !zalloc_cpumask_var(&per_cpu(cpu_sibling_mask, cpu)) || @@ -255,6 +263,9 @@ void __init smp_init_cpus(void) else acpi_smp_init_cpus(); + if ( opt_hmp_unsafe ) + warning_add("WARNING: HMP COMPUTING HAS BEEN ENABLED.\n" + "It has implications on the security and stability of the system.\n"); } int __init @@ -292,6 +303,21 @@ void start_secondary(unsigned long boot_phys_offset, init_traps(); + /* + * Currently Xen assumes the platform has only one kind of CPUs. + * This assumption does not hold on big.LITTLE platform and may + * result to instability and insecure platform. Better to park them + * for now. + */ + if ( !opt_hmp_unsafe && + current_cpu_data.midr.bits != boot_cpu_data.midr.bits ) + { + printk(XENLOG_ERR "CPU%u MIDR (0x%x) does not match boot CPU MIDR (0x%x).\n", + smp_processor_id(), current_cpu_data.midr.bits, + boot_cpu_data.midr.bits); + stop_cpu(); + } + mmu_init_secondary_cpu(); gic_init_secondary_cpu(); -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 3/7] xen/arm: make processor a per cpu variable 2018-03-02 19:06 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 2/7] xen/arm: Park CPUs with a MIDR different from the boot CPU Stefano Stabellini @ 2018-03-02 19:06 ` Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 4/7] xen/arm: read ACTLR on the pcpu where the vcpu will run Stefano Stabellini ` (4 subsequent siblings) 6 siblings, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2018-03-02 19:06 UTC (permalink / raw) To: julien.grall; +Cc: sstabellini, xen-devel There can be processors of different kinds on a single system. Make processor a per_cpu variable pointing to the right processor type for each core. Suggested-by: Julien Grall <julien.grall@arm.com> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> Reviewed-by: Julien Grall <julien.grall@arm.com> --- Changes in v2: - add patch --- xen/arch/arm/processor.c | 8 ++++---- xen/arch/arm/smpboot.c | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/xen/arch/arm/processor.c b/xen/arch/arm/processor.c index 8c425ce..ce43850 100644 --- a/xen/arch/arm/processor.c +++ b/xen/arch/arm/processor.c @@ -18,7 +18,7 @@ */ #include <asm/procinfo.h> -static const struct processor *processor = NULL; +static DEFINE_PER_CPU(struct processor *, processor); void __init processor_setup(void) { @@ -28,15 +28,15 @@ void __init processor_setup(void) if ( !procinfo ) return; - processor = procinfo->processor; + this_cpu(processor) = procinfo->processor; } void processor_vcpu_initialise(struct vcpu *v) { - if ( !processor || !processor->vcpu_initialise ) + if ( !this_cpu(processor) || !this_cpu(processor)->vcpu_initialise ) return; - processor->vcpu_initialise(v); + this_cpu(processor)->vcpu_initialise(v); } /* diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index 7ea4e41..122c0b5 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -32,6 +32,7 @@ #include <xen/console.h> #include <asm/cpuerrata.h> #include <asm/gic.h> +#include <asm/procinfo.h> #include <asm/psci.h> #include <asm/acpi.h> @@ -300,6 +301,7 @@ void start_secondary(unsigned long boot_phys_offset, set_processor_id(cpuid); identify_cpu(¤t_cpu_data); + processor_setup(); init_traps(); -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 4/7] xen/arm: read ACTLR on the pcpu where the vcpu will run 2018-03-02 19:06 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 2/7] xen/arm: Park CPUs with a MIDR different from the boot CPU Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 3/7] xen/arm: make processor a per cpu variable Stefano Stabellini @ 2018-03-02 19:06 ` Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 5/7] xen/arm: set VPIDR based on the MIDR value of the underlying pCPU Stefano Stabellini ` (3 subsequent siblings) 6 siblings, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2018-03-02 19:06 UTC (permalink / raw) To: julien.grall; +Cc: sstabellini, xen-devel On big.LITTLE systems not all cores have the same ACTLR. Instead of reading ACTLR and setting v->arch.actlr in vcpu_initialise, do it later on the same pcpu where the vcpu will run. This way, assuming that the vcpu has been created with the right pcpu affinity, the guest will be able to read the right ACTLR value, matching the one of the physical cpu. Also move processor_vcpu_initialise(v) to continue_new_vcpu as it can modify v->arch.actlr. Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> Reviewed-by: Julien Grall <julien.grall@arm.com> --- Changes in v2: - move processor_vcpu_initialise to continue_new_vcpu - remove inaccurate sentence from commit message --- xen/arch/arm/domain.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index a74ff1c..5e76809 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -314,6 +314,9 @@ static void schedule_tail(struct vcpu *prev) static void continue_new_vcpu(struct vcpu *prev) { + current->arch.actlr = READ_SYSREG32(ACTLR_EL1); + processor_vcpu_initialise(current); + schedule_tail(prev); if ( is_idle_vcpu(current) ) @@ -540,12 +543,8 @@ int vcpu_initialise(struct vcpu *v) v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id); - v->arch.actlr = READ_SYSREG32(ACTLR_EL1); - v->arch.hcr_el2 = get_default_hcr_flags(); - processor_vcpu_initialise(v); - if ( (rc = vcpu_vgic_init(v)) != 0 ) goto fail; -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 5/7] xen/arm: set VPIDR based on the MIDR value of the underlying pCPU 2018-03-02 19:06 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Stefano Stabellini ` (2 preceding siblings ...) 2018-03-02 19:06 ` [PATCH v4 4/7] xen/arm: read ACTLR on the pcpu where the vcpu will run Stefano Stabellini @ 2018-03-02 19:06 ` Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 6/7] xen/arm: update the docs about heterogeneous computing Stefano Stabellini ` (2 subsequent siblings) 6 siblings, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2018-03-02 19:06 UTC (permalink / raw) To: julien.grall; +Cc: sstabellini, xen-devel On big.LITTLE systems not all cores have the same MIDR. Instead of storing only one VPIDR per domain, initialize it to the value of the MIDR of the pCPU where the vCPU will run. This way, assuming that the vCPU has been created with the right pCPU affinity, the guest will be able to read the right VPIDR value, matching the one of the physical cpu. Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> Reviewed-by: Julien Grall <julien.grall@arm.com> --- Changes in v3: - improve commit message - do not store vpidr in struct vcpu Changes in v2: - remove warning message - make vpidr per vcpu --- xen/arch/arm/domain.c | 8 ++++---- xen/arch/arm/vcpreg.c | 4 ++-- xen/include/asm-arm/domain.h | 3 --- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 5e76809..545bbf6 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -172,6 +172,8 @@ static void ctxt_switch_from(struct vcpu *p) static void ctxt_switch_to(struct vcpu *n) { + uint32_t vpidr; + /* When the idle VCPU is running, Xen will always stay in hypervisor * mode. Therefore we don't need to restore the context of an idle VCPU. */ @@ -180,7 +182,8 @@ static void ctxt_switch_to(struct vcpu *n) p2m_restore_state(n); - WRITE_SYSREG32(n->domain->arch.vpidr, VPIDR_EL2); + vpidr = READ_SYSREG32(MIDR_EL1); + WRITE_SYSREG32(vpidr, VPIDR_EL2); WRITE_SYSREG(n->arch.vmpidr, VMPIDR_EL2); /* VGIC */ @@ -595,9 +598,6 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags, if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL ) goto fail; - /* Default the virtual ID to match the physical */ - d->arch.vpidr = boot_cpu_data.midr.bits; - clear_page(d->shared_info); share_xen_page_with_guest( virt_to_page(d->shared_info), d, XENSHARE_writable); diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c index e363183..b04d996 100644 --- a/xen/arch/arm/vcpreg.c +++ b/xen/arch/arm/vcpreg.c @@ -230,7 +230,6 @@ void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) { const struct hsr_cp32 cp32 = hsr.cp32; int regidx = cp32.reg; - struct domain *d = current->domain; if ( !check_conditional_instr(regs, hsr) ) { @@ -295,7 +294,8 @@ void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) * - Variant and Revision bits match MDIR */ val = (1 << 24) | (5 << 16); - val |= ((d->arch.vpidr >> 20) & 0xf) | (d->arch.vpidr & 0xf); + val |= ((current_cpu_data.midr.bits >> 20) & 0xf) | + (current_cpu_data.midr.bits & 0xf); set_user_reg(regs, regidx, val); break; diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index 4fe189b..0dd8c95 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -63,9 +63,6 @@ struct arch_domain RELMEM_done, } relmem; - /* Virtual CPUID */ - uint32_t vpidr; - struct { uint64_t offset; } phys_timer_base; -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 6/7] xen/arm: update the docs about heterogeneous computing 2018-03-02 19:06 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Stefano Stabellini ` (3 preceding siblings ...) 2018-03-02 19:06 ` [PATCH v4 5/7] xen/arm: set VPIDR based on the MIDR value of the underlying pCPU Stefano Stabellini @ 2018-03-02 19:06 ` Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 7/7] xen/arm: disable CPUs with different dcache line sizes Stefano Stabellini 2018-03-06 10:46 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Julien Grall 6 siblings, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2018-03-02 19:06 UTC (permalink / raw) To: julien.grall Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, tim, xen-devel, jbeulich, ian.jackson Introduce a new document about big.LITTLE and update the documentation of hmp-unsafe. Also update the warning messages to point users to the docs. Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> Acked-by: Julien Grall <julien.grall@arm.com> CC: jbeulich@suse.com CC: konrad.wilk@oracle.com CC: tim@xen.org CC: wei.liu2@citrix.com CC: andrew.cooper3@citrix.com CC: George.Dunlap@eu.citrix.com CC: ian.jackson@eu.citrix.com --- Changes in v3: - split warning messages to be under 72 chars Changes in v2: - add a separate doc for big.LITTLE - improve the warning message --- docs/misc/arm/big.LITTLE.txt | 46 +++++++++++++++++++++++++++++++++++++ docs/misc/xen-command-line.markdown | 7 +++++- xen/arch/arm/smpboot.c | 11 +++++---- 3 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 docs/misc/arm/big.LITTLE.txt diff --git a/docs/misc/arm/big.LITTLE.txt b/docs/misc/arm/big.LITTLE.txt new file mode 100644 index 0000000..b6ea1c9 --- /dev/null +++ b/docs/misc/arm/big.LITTLE.txt @@ -0,0 +1,46 @@ +big.LITTLE is a form of heterogeneous computing that comes with two +types of general purpose cpu cores: big cores, more powerful and with a +higher power consumption rate, and LITTLE cores, less powerful and +cheaper to run. For example, Cortex A53 and Cortex A57 cpus. Typically, +big cores are only recommended for burst activity, especially in +battery powered environments. Please note that Xen doesn't not use any +board specific power management techniques at the moment, it only uses +WFI. It is recommended to check the vendor's big.LITTLE and power +management documentation before using it in a Xen environment. + + +big and LITTLE cores are fully compatible in terms of instruction sets, +but can differ in many subtle ways. For example, their cacheline sizes +might differ. For this reason, vcpu migration between big and LITTLE +cores can lead to data corruptions. + +Today, the Xen scheduler does not have support for big.LITTLE, +therefore, it might unknowingly move any vcpus between big and LITTLE +cores, potentially leading to breakages. To avoid this kind of issues, +at boot time Xen disables all cpus that differ from the boot cpu. + + +Expert users can enable all big.LITTLE cores by passing hmp-unsafe=true +to the Xen command line [1]. Given the lack of big.LITTLE support in the +scheduler, it is only safe if the cpu affinity of all domains is +manually specified, so that the scheduler is not allowed to switch a +vcpu from big to LITTLE or vice versa. + +In the case of dom0, dom0_vcpus_pin needs to be added to the Xen command +line options [1]. For DomUs, the `cpus' option should be added to all VM +config files [2]. + +For example, if the first 4 cpus are big and the last 4 are LITTLE, the +following options run all domain vcpus on either big or LITTLE cores +(not both): + + cpus = "0-3" + cpus = "4-7" + +The following option runs one domain vcpu as big and one as LITTLE: + + cpus = ["0-3", "4-7"] + + +[1] docs/misc/xen-command-line.markdown +[2] docs/man/xl.cfg.pod.5 diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index 7b80119..4391b75 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -992,7 +992,12 @@ Control Xens use of the APEI Hardware Error Source Table, should one be found. Say yes at your own risk if you want to enable heterogenous computing (such as big.LITTLE). This may result to an unstable and insecure -platform. When the option is disabled (default), CPUs that are not +platform, unless you manually specify the cpu affinity of all domains so +that all vcpus are scheduled on the same class of pcpus (big or LITTLE +but not both). vcpu migration between big cores and LITTLE cores is not +supported. See docs/misc/arm/big.LITTLE.txt for more information. + +When the hmp-unsafe option is disabled (default), CPUs that are not identical to the boot CPU will be parked and not used by Xen. ### hpetbroadcast diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index 122c0b5..04efb33 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -266,7 +266,8 @@ void __init smp_init_cpus(void) if ( opt_hmp_unsafe ) warning_add("WARNING: HMP COMPUTING HAS BEEN ENABLED.\n" - "It has implications on the security and stability of the system.\n"); + "It has implications on the security and stability of the system,\n" + "unless the cpu affinity of all domains is specified.\n"); } int __init @@ -308,13 +309,15 @@ void start_secondary(unsigned long boot_phys_offset, /* * Currently Xen assumes the platform has only one kind of CPUs. * This assumption does not hold on big.LITTLE platform and may - * result to instability and insecure platform. Better to park them - * for now. + * result to instability and insecure platform (unless cpu affinity + * is manually specified for all domains). Better to park them for + * now. */ if ( !opt_hmp_unsafe && current_cpu_data.midr.bits != boot_cpu_data.midr.bits ) { - printk(XENLOG_ERR "CPU%u MIDR (0x%x) does not match boot CPU MIDR (0x%x).\n", + printk(XENLOG_ERR "CPU%u MIDR (0x%x) does not match boot CPU MIDR (0x%x),\n" + "disable cpu (see big.LITTLE.txt under docs/).\n", smp_processor_id(), current_cpu_data.midr.bits, boot_cpu_data.midr.bits); stop_cpu(); -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 7/7] xen/arm: disable CPUs with different dcache line sizes 2018-03-02 19:06 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Stefano Stabellini ` (4 preceding siblings ...) 2018-03-02 19:06 ` [PATCH v4 6/7] xen/arm: update the docs about heterogeneous computing Stefano Stabellini @ 2018-03-02 19:06 ` Stefano Stabellini 2018-03-06 10:59 ` Julien Grall 2018-03-06 10:46 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Julien Grall 6 siblings, 1 reply; 26+ messages in thread From: Stefano Stabellini @ 2018-03-02 19:06 UTC (permalink / raw) To: julien.grall; +Cc: sstabellini, xen-devel Even different cpus in big.LITTLE systems are expected to have the same dcache line size. Unless the minimum of all dcache line sizes is used across all cpu cores, cache coherency protocols can go wrong. Instead, for now, just disable any cpu with a different dcache line size. This check is not covered by the hmp-unsafe option, because even with the correct scheduling and vcpu pinning in place, the system breaks if dcache line sizes differ across cores. We don't believe it is a problem for most big.LITTLE systems. This patch moves the implementation of setup_cache to a static inline, still setting dcache_line_bytes at the beginning of start_xen as before. In start_secondary we check that the dcache level 1 line sizes match, otherwise we disable the cpu. Suggested-by: Julien Grall <julien.grall@arm.com> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> --- Changes in v4: - improve commit message - use %zu --- xen/arch/arm/setup.c | 14 +------------- xen/arch/arm/smpboot.c | 8 ++++++++ xen/include/asm-arm/page.h | 11 +++++++++++ 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index fced75a..6ccfdab 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -682,18 +682,6 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size) size_t __read_mostly dcache_line_bytes; -/* Very early check of the CPU cache properties */ -void __init setup_cache(void) -{ - uint32_t ctr; - - /* Read CTR */ - ctr = READ_SYSREG32(CTR_EL0); - - /* Bits 16-19 are the log2 number of words in the cacheline. */ - dcache_line_bytes = (size_t) (4 << ((ctr >> 16) & 0xf)); -} - /* C entry point for boot CPU */ void __init start_xen(unsigned long boot_phys_offset, unsigned long fdt_paddr, @@ -707,7 +695,7 @@ void __init start_xen(unsigned long boot_phys_offset, struct domain *dom0; struct xen_arch_domainconfig config; - setup_cache(); + dcache_line_bytes = read_dcache_line_size(); percpu_init_areas(); set_processor_id(0); /* needed early, for smp_processor_id() */ diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index 04efb33..d15230b 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -323,6 +323,14 @@ void start_secondary(unsigned long boot_phys_offset, stop_cpu(); } + if ( dcache_line_bytes != read_dcache_line_size() ) + { + printk(XENLOG_ERR "CPU%u dcache line size (%zu) does not match the boot CPU (%zu)\n", + smp_processor_id(), read_dcache_line_size(), + dcache_line_bytes); + stop_cpu(); + } + mmu_init_secondary_cpu(); gic_init_secondary_cpu(); diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h index ce18f0c..e539f83 100644 --- a/xen/include/asm-arm/page.h +++ b/xen/include/asm-arm/page.h @@ -138,6 +138,17 @@ extern size_t dcache_line_bytes; #define copy_page(dp, sp) memcpy(dp, sp, PAGE_SIZE) +static inline size_t read_dcache_line_size(void) +{ + uint32_t ctr; + + /* Read CTR */ + ctr = READ_SYSREG32(CTR_EL0); + + /* Bits 16-19 are the log2 number of words in the cacheline. */ + return (size_t) (4 << ((ctr >> 16) & 0xf)); +} + /* Functions for flushing medium-sized areas. * if 'range' is large enough we might want to use model-specific * full-cache flushes. */ -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v4 7/7] xen/arm: disable CPUs with different dcache line sizes 2018-03-02 19:06 ` [PATCH v4 7/7] xen/arm: disable CPUs with different dcache line sizes Stefano Stabellini @ 2018-03-06 10:59 ` Julien Grall 2018-03-06 19:41 ` Stefano Stabellini 0 siblings, 1 reply; 26+ messages in thread From: Julien Grall @ 2018-03-06 10:59 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel Hi Stefano, Something is wrong with your threading again. Patch #2-7 have "In-Reply-To" matching patch #1 and not the cover letter. On 02/03/18 19:06, Stefano Stabellini wrote: > Even different cpus in big.LITTLE systems are expected to have the same > dcache line size. Unless the minimum of all dcache line sizes is used > across all cpu cores, cache coherency protocols can go wrong. Instead, > for now, just disable any cpu with a different dcache line size. > > This check is not covered by the hmp-unsafe option, because even with > the correct scheduling and vcpu pinning in place, the system breaks if > dcache line sizes differ across cores. We don't believe it is a problem > for most big.LITTLE systems. > > This patch moves the implementation of setup_cache to a static inline, > still setting dcache_line_bytes at the beginning of start_xen as > before. > > In start_secondary we check that the dcache level 1 line sizes match, > otherwise we disable the cpu. > > Suggested-by: Julien Grall <julien.grall@arm.com> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> > > --- > Changes in v4: > - improve commit message > - use %zu > --- > xen/arch/arm/setup.c | 14 +------------- > xen/arch/arm/smpboot.c | 8 ++++++++ > xen/include/asm-arm/page.h | 11 +++++++++++ > 3 files changed, 20 insertions(+), 13 deletions(-) > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index fced75a..6ccfdab 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -682,18 +682,6 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size) > > size_t __read_mostly dcache_line_bytes; > > -/* Very early check of the CPU cache properties */ > -void __init setup_cache(void) > -{ > - uint32_t ctr; > - > - /* Read CTR */ > - ctr = READ_SYSREG32(CTR_EL0); > - > - /* Bits 16-19 are the log2 number of words in the cacheline. */ > - dcache_line_bytes = (size_t) (4 << ((ctr >> 16) & 0xf)); > -} > - > /* C entry point for boot CPU */ > void __init start_xen(unsigned long boot_phys_offset, > unsigned long fdt_paddr, > @@ -707,7 +695,7 @@ void __init start_xen(unsigned long boot_phys_offset, > struct domain *dom0; > struct xen_arch_domainconfig config; > > - setup_cache(); > + dcache_line_bytes = read_dcache_line_size(); It feels a bit odd to have one call dcache_line_bytes and the other call read_dcache_line_size. Would it be possible to uniformize the name? With that: Reviewed-by: Julien Grall <julien.grall@arm.com> Cheers, > > percpu_init_areas(); > set_processor_id(0); /* needed early, for smp_processor_id() */ > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > index 04efb33..d15230b 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -323,6 +323,14 @@ void start_secondary(unsigned long boot_phys_offset, > stop_cpu(); > } > > + if ( dcache_line_bytes != read_dcache_line_size() ) > + { > + printk(XENLOG_ERR "CPU%u dcache line size (%zu) does not match the boot CPU (%zu)\n", > + smp_processor_id(), read_dcache_line_size(), > + dcache_line_bytes); > + stop_cpu(); > + } > + > mmu_init_secondary_cpu(); > > gic_init_secondary_cpu(); > diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h > index ce18f0c..e539f83 100644 > --- a/xen/include/asm-arm/page.h > +++ b/xen/include/asm-arm/page.h > @@ -138,6 +138,17 @@ extern size_t dcache_line_bytes; > > #define copy_page(dp, sp) memcpy(dp, sp, PAGE_SIZE) > > +static inline size_t read_dcache_line_size(void) > +{ > + uint32_t ctr; > + > + /* Read CTR */ > + ctr = READ_SYSREG32(CTR_EL0); > + > + /* Bits 16-19 are the log2 number of words in the cacheline. */ > + return (size_t) (4 << ((ctr >> 16) & 0xf)); > +} > + > /* Functions for flushing medium-sized areas. > * if 'range' is large enough we might want to use model-specific > * full-cache flushes. */ > Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 7/7] xen/arm: disable CPUs with different dcache line sizes 2018-03-06 10:59 ` Julien Grall @ 2018-03-06 19:41 ` Stefano Stabellini 0 siblings, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2018-03-06 19:41 UTC (permalink / raw) To: Julien Grall; +Cc: Stefano Stabellini, xen-devel On Tue, 6 Mar 2018, Julien Grall wrote: > Hi Stefano, > > Something is wrong with your threading again. Patch #2-7 have "In-Reply-To" > matching patch #1 and not the cover letter. Thanks, I lost my git configuration. > On 02/03/18 19:06, Stefano Stabellini wrote: > > Even different cpus in big.LITTLE systems are expected to have the same > > dcache line size. Unless the minimum of all dcache line sizes is used > > across all cpu cores, cache coherency protocols can go wrong. Instead, > > for now, just disable any cpu with a different dcache line size. > > > > This check is not covered by the hmp-unsafe option, because even with > > the correct scheduling and vcpu pinning in place, the system breaks if > > dcache line sizes differ across cores. We don't believe it is a problem > > for most big.LITTLE systems. > > > > This patch moves the implementation of setup_cache to a static inline, > > still setting dcache_line_bytes at the beginning of start_xen as > > before. > > > > In start_secondary we check that the dcache level 1 line sizes match, > > otherwise we disable the cpu. > > > > Suggested-by: Julien Grall <julien.grall@arm.com> > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> > > > > --- > > Changes in v4: > > - improve commit message > > - use %zu > > --- > > xen/arch/arm/setup.c | 14 +------------- > > xen/arch/arm/smpboot.c | 8 ++++++++ > > xen/include/asm-arm/page.h | 11 +++++++++++ > > 3 files changed, 20 insertions(+), 13 deletions(-) > > > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > > index fced75a..6ccfdab 100644 > > --- a/xen/arch/arm/setup.c > > +++ b/xen/arch/arm/setup.c > > @@ -682,18 +682,6 @@ static void __init setup_mm(unsigned long dtb_paddr, > > size_t dtb_size) > > size_t __read_mostly dcache_line_bytes; > > -/* Very early check of the CPU cache properties */ > > -void __init setup_cache(void) > > -{ > > - uint32_t ctr; > > - > > - /* Read CTR */ > > - ctr = READ_SYSREG32(CTR_EL0); > > - > > - /* Bits 16-19 are the log2 number of words in the cacheline. */ > > - dcache_line_bytes = (size_t) (4 << ((ctr >> 16) & 0xf)); > > -} > > - > > /* C entry point for boot CPU */ > > void __init start_xen(unsigned long boot_phys_offset, > > unsigned long fdt_paddr, > > @@ -707,7 +695,7 @@ void __init start_xen(unsigned long boot_phys_offset, > > struct domain *dom0; > > struct xen_arch_domainconfig config; > > - setup_cache(); > > + dcache_line_bytes = read_dcache_line_size(); > > It feels a bit odd to have one call dcache_line_bytes and the other call > read_dcache_line_size. Would it be possible to uniformize the name? > > With that: > > Reviewed-by: Julien Grall <julien.grall@arm.com> I fixed that and the other in-code comment, and committed the whole series, thanks! > > percpu_init_areas(); > > set_processor_id(0); /* needed early, for smp_processor_id() */ > > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > > index 04efb33..d15230b 100644 > > --- a/xen/arch/arm/smpboot.c > > +++ b/xen/arch/arm/smpboot.c > > @@ -323,6 +323,14 @@ void start_secondary(unsigned long boot_phys_offset, > > stop_cpu(); > > } > > + if ( dcache_line_bytes != read_dcache_line_size() ) > > + { > > + printk(XENLOG_ERR "CPU%u dcache line size (%zu) does not match the > > boot CPU (%zu)\n", > > + smp_processor_id(), read_dcache_line_size(), > > + dcache_line_bytes); > > + stop_cpu(); > > + } > > + > > mmu_init_secondary_cpu(); > > gic_init_secondary_cpu(); > > diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h > > index ce18f0c..e539f83 100644 > > --- a/xen/include/asm-arm/page.h > > +++ b/xen/include/asm-arm/page.h > > @@ -138,6 +138,17 @@ extern size_t dcache_line_bytes; > > #define copy_page(dp, sp) memcpy(dp, sp, PAGE_SIZE) > > +static inline size_t read_dcache_line_size(void) > > +{ > > + uint32_t ctr; > > + > > + /* Read CTR */ > > + ctr = READ_SYSREG32(CTR_EL0); > > + > > + /* Bits 16-19 are the log2 number of words in the cacheline. */ > > + return (size_t) (4 << ((ctr >> 16) & 0xf)); > > +} > > + > > /* Functions for flushing medium-sized areas. > > * if 'range' is large enough we might want to use model-specific > > * full-cache flushes. */ > > > > Cheers, > > -- > Julien Grall > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register 2018-03-02 19:06 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Stefano Stabellini ` (5 preceding siblings ...) 2018-03-02 19:06 ` [PATCH v4 7/7] xen/arm: disable CPUs with different dcache line sizes Stefano Stabellini @ 2018-03-06 10:46 ` Julien Grall 6 siblings, 0 replies; 26+ messages in thread From: Julien Grall @ 2018-03-06 10:46 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel Hi Stefano, On 02/03/18 19:06, Stefano Stabellini wrote: > diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h > index d948250..ce18f0c 100644 > --- a/xen/include/asm-arm/page.h > +++ b/xen/include/asm-arm/page.h > @@ -134,7 +134,7 @@ > /* Architectural minimum cacheline size is 4 32-bit words. */ > #define MIN_CACHELINE_BYTES 16 > /* Actual cacheline size on the boot CPU. */ You probably want to update that comment either in this patch or #7. With that: Reviewed-by: Julien Grall <julien.grall@arm.com> Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-02 19:05 [PATCH v4 0/7] unsafe big.LITTLE support Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Stefano Stabellini @ 2018-03-08 6:15 ` Peng Fan 2018-03-08 11:03 ` Julien Grall 1 sibling, 1 reply; 26+ messages in thread From: Peng Fan @ 2018-03-08 6:15 UTC (permalink / raw) To: Stefano Stabellini; +Cc: julien.grall, xen-devel 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://source.codeaurora.org/external/imx/linux-imx/commit/?h=imx_4.9.51_imx8_beta2&id=917cc3a8db2f3609ef8e2f59e7bcd31aa2cd4e59 I am not sure whether this issue cause DomU big/Little not work. So wonder has this patchset been tested on Big/Little Hardware? Thanks, Peng. > >Cheers, > >Stefano > > >Julien Grall (1): > xen/arm: Park CPUs with a MIDR different from the boot CPU. > >Stefano Stabellini (6): > xen/arm: Read the dcache line size from CTR register > xen/arm: make processor a per cpu variable > xen/arm: read ACTLR on the pcpu where the vcpu will run > xen/arm: set VPIDR based on the MIDR value of the underlying pCPU > xen/arm: update the docs about heterogeneous computing > xen/arm: disable CPUs with different dcache line sizes > > docs/misc/arm/big.LITTLE.txt | 46 +++++++++++++++++++++++++++++++++++++ > docs/misc/xen-command-line.markdown | 15 ++++++++++++ > xen/arch/arm/arm32/head.S | 2 +- > xen/arch/arm/arm64/head.S | 2 +- > xen/arch/arm/domain.c | 15 ++++++------ > xen/arch/arm/processor.c | 8 +++---- > xen/arch/arm/setup.c | 17 ++------------ > xen/arch/arm/smpboot.c | 39 +++++++++++++++++++++++++++++++ > xen/arch/arm/vcpreg.c | 4 ++-- > xen/include/asm-arm/cpregs.h | 2 ++ > xen/include/asm-arm/domain.h | 3 --- > xen/include/asm-arm/page.h | 27 +++++++++++++++------- > 12 files changed, 138 insertions(+), 42 deletions(-) > create mode 100644 docs/misc/arm/big.LITTLE.txt > >_______________________________________________ >Xen-devel mailing list >Xen-devel@lists.xenproject.org >https://lists.xenproject.org/mailman/listinfo/xen-devel -- _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-08 6:15 ` [PATCH v4 0/7] unsafe big.LITTLE support Peng Fan @ 2018-03-08 11:03 ` Julien Grall 2018-03-08 12:23 ` Peng Fan 0 siblings, 1 reply; 26+ messages in thread From: Julien Grall @ 2018-03-08 11:03 UTC (permalink / raw) To: Peng Fan, Stefano Stabellini; +Cc: xen-devel 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://source.codeaurora.org/external/imx/linux-imx/commit/?h=imx_4.9.51_imx8_beta2&id=917cc3a8db2f3609ef8e2f59e7bcd31aa2cd4e59 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 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. > 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. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-08 11:03 ` Julien Grall @ 2018-03-08 12:23 ` Peng Fan 2018-03-08 12:30 ` Julien Grall 0 siblings, 1 reply; 26+ messages in thread From: Peng Fan @ 2018-03-08 12:23 UTC (permalink / raw) To: Julien Grall, Peng Fan, Stefano Stabellini; +Cc: xen-devel > -----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. 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. 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;" "dsb sy;" "isb;" : : : "memory"); @@ -39,7 +39,7 @@ static inline void flush_tlb_all(void) { asm volatile( "dsb sy;" - "tlbi alle1is;" + "tlbi alle1;" "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(); + //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? Thanks, Peng. > > Cheers, > > -- > Julien Grall > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.xe > nproject.org%2Fmailman%2Flistinfo%2Fxen-devel&data=02%7C01%7Cpeng.fan > %40nxp.com%7Cc7f074c6708647441f2b08d584e45fec%7C686ea1d3bc2b4c6fa > 92cd99c5c301635%7C0%7C0%7C636561038755176475&sdata=hJr8K6RXCjkDT > AMuM84nvzUn3qUtrHdo2e6qFn1%2Fdzg%3D&reserved=0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-08 12:23 ` Peng Fan @ 2018-03-08 12:30 ` Julien Grall 2018-03-08 12:43 ` Peng Fan 0 siblings, 1 reply; 26+ messages in thread From: Julien Grall @ 2018-03-08 12:30 UTC (permalink / raw) To: Peng Fan, Peng Fan, Stefano Stabellini; +Cc: xen-devel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-08 12:30 ` Julien Grall @ 2018-03-08 12:43 ` Peng Fan 2018-03-08 15:13 ` Julien Grall 0 siblings, 1 reply; 26+ messages in thread From: Peng Fan @ 2018-03-08 12:43 UTC (permalink / raw) To: Julien Grall, Peng Fan, Stefano Stabellini; +Cc: xen-devel > -----Original Message----- > From: Julien Grall [mailto:julien.grall@arm.com] > Sent: 2018年3月8日 20:30 > To: Peng Fan <peng.fan@nxp.com>; 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 > > 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%2Fs > >>> ou > >>> > >> > 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? # xl vcpu-list Name ID VCPU CPU State Time(s) Affinity (Hard / Soft) Domain-0 0 0 0 r-- 7.4 0 / all Domain-0 0 1 1 -b- 9.4 1 / all Domain-0 0 2 2 -b- 2.2 2 / all Domain-0 0 3 3 -b- 2.6 3 / all Domain-0 0 4 4 -b- 2.1 4 / all Domain-0 0 5 5 -b- 3.3 5 / all DomU 1 0 2 -b- 9.6 2-3 / all DomU 1 1 3 r-- 7.5 2-3 / all DomU 1 2 5 -b- 5.5 4-5 / all DomU 1 3 4 -b- 6.3 4-5 / all > > > > > 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? Just want to invalid all the tlb, innershareable could be kept. This is not a formal patch, just my trying to narrow the issue. > > > "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. > > > + //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. Thanks, Peng > > Cheers, > > -- > Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-08 12:43 ` Peng Fan @ 2018-03-08 15:13 ` Julien Grall 2018-03-09 9:05 ` Peng Fan 0 siblings, 1 reply; 26+ messages in thread From: Julien Grall @ 2018-03-08 15:13 UTC (permalink / raw) To: Peng Fan, Peng Fan, Stefano Stabellini; +Cc: xen-devel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-08 15:13 ` Julien Grall @ 2018-03-09 9:05 ` Peng Fan 2018-03-09 10:22 ` Julien Grall 0 siblings, 1 reply; 26+ messages in thread From: Peng Fan @ 2018-03-09 9:05 UTC (permalink / raw) To: Julien Grall; +Cc: Peng Fan, Stefano Stabellini, xen-devel On Thu, Mar 08, 2018 at 03:13:50PM +0000, Julien Grall wrote: >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. Just checked this. Because xen toolstack create device tree with compatible "compatible = "xen,xenvm-4.10", "xen,xenvm";", but the linux code use "fsl,imx8qm" to detect soc, then call scu to get revision of chip. After add an entry in linux side "{ .compatible = "xen,xenvm", .data = &imx8qm_soc_data, }," It seems works. Passed a map/unmap stress test which easily fail without the tlb workaround. Wonder is it ok to specific machine compatible in domu.cfg and let xen stack use this machine compatible other than "xen,xenvm"? Is this acceptable by community? Also in domu kernel booting, there is waring. [ 0.201323] Invalid sched_group_energy for CPU3 [ 0.201341] Invalid sched_group_energy for Cluster3 [ 0.201353] Invalid sched_group_energy for CPU2 [ 0.201365] Invalid sched_group_energy for Cluster2 [ 0.201376] Invalid sched_group_energy for CPU1 [ 0.201387] Invalid sched_group_energy for Cluster1 [ 0.201398] Invalid sched_group_energy for CPU0 [ 0.201409] Invalid sched_group_energy for Cluster0 This is because no cpu0/1/2/3 is not under cluster node in dts. As I am using big/little guest, I think need create two cluster nodes ,one for vcpu0-1, the other for vcpu2-3. But this also needs xen toolstack change (: Thanks, Peng. > >Cheers, > >-- >Julien Grall -- _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-09 9:05 ` Peng Fan @ 2018-03-09 10:22 ` Julien Grall 2018-03-09 13:30 ` Peng Fan 0 siblings, 1 reply; 26+ messages in thread From: Julien Grall @ 2018-03-09 10:22 UTC (permalink / raw) To: Peng Fan; +Cc: Peng Fan, Stefano Stabellini, xen-devel Hi Peng, On 09/03/18 09:05, Peng Fan wrote: > On Thu, Mar 08, 2018 at 03:13:50PM +0000, Julien Grall wrote: >> On 08/03/18 12:43, Peng Fan wrote: >> 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. > > Just checked this. Because xen toolstack create device tree > with compatible "compatible = "xen,xenvm-4.10", "xen,xenvm";", > but the linux code use "fsl,imx8qm" to detect soc, then call scu > to get revision of chip. But how does the guest call the scu? > > After add an entry in linux side "{ .compatible = "xen,xenvm", .data = &imx8qm_soc_data, }," > It seems works. Passed a map/unmap stress test which easily fail without > the tlb workaround. > > Wonder is it ok to specific machine compatible in domu.cfg and let xen stack > use this machine compatible other than "xen,xenvm"? Is this acceptable by community? A user should be able to boot a guest safely on any machine without having to hack the configuration file. He should also be able to boot a guest with both ACPI and DT as this is independent from the real machine. So for me the way to find the workaround at the moment is not acceptable for a Xen guest upstream. What could be acceptable for the community is one of the 3 solutions below: 1) Only use one of the 2 clusters 2) Restrict both Xen and Guest to use only 36-bit VA. 3) Trap all TLBs access from the guest and convert them to TLB alle1s/vmalls12e1is I would be happy to consider any other. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-09 10:22 ` Julien Grall @ 2018-03-09 13:30 ` Peng Fan 2018-03-09 14:40 ` Julien Grall 0 siblings, 1 reply; 26+ messages in thread From: Peng Fan @ 2018-03-09 13:30 UTC (permalink / raw) To: Julien Grall; +Cc: Peng Fan, Stefano Stabellini, xen-devel Hi Julien, On Fri, Mar 09, 2018 at 10:22:09AM +0000, Julien Grall wrote: >Hi Peng, > >On 09/03/18 09:05, Peng Fan wrote: >>On Thu, Mar 08, 2018 at 03:13:50PM +0000, Julien Grall wrote: >>>On 08/03/18 12:43, Peng Fan wrote: >>>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. >> >>Just checked this. Because xen toolstack create device tree >>with compatible "compatible = "xen,xenvm-4.10", "xen,xenvm";", >>but the linux code use "fsl,imx8qm" to detect soc, then call scu >>to get revision of chip. > >But how does the guest call the scu? We are doing GPU and display passthrough, also some other IPs passthrough. we could not totally rely on Dom0 to configure the pinmux, gpio, clk, relying on dom0 to do that would bring much hack code to our kernel, also runtime clk set rate in domu could not be done. So we expose an interface to domu to directly communicate with SCU(system control unit). > >> >>After add an entry in linux side "{ .compatible = "xen,xenvm", .data = &imx8qm_soc_data, }," >>It seems works. Passed a map/unmap stress test which easily fail without >>the tlb workaround. >> >>Wonder is it ok to specific machine compatible in domu.cfg and let xen stack >>use this machine compatible other than "xen,xenvm"? Is this acceptable by community? > >A user should be able to boot a guest safely on any machine without >having to hack the configuration file. He should also be able to boot >a guest with both ACPI and DT as this is independent from the real >machine. So for me the way to find the workaround at the moment is >not acceptable for a Xen guest upstream. I have no idea about ACPI (: we are mainly working on embedded case, and mostly we are partitioning our IPs. So our kernel normally only work with the dedicated DTB. I am not asking to replace "xen,xenvm", just would like to add a option that if user specific a machine compatible in cfg or else, xen toolstack could add that in the final device tree. Anyway new silicon will have issue fixed. > >What could be acceptable for the community is one of the 3 solutions below: > 1) Only use one of the 2 clusters The easiest way:) > 2) Restrict both Xen and Guest to use only 36-bit VA. I have no idea, need to check > 3) Trap all TLBs access from the guest and convert them to TLB >alle1s/vmalls12e1is This will introduce performance penalty. I would not do this. Thanks, Peng. > >I would be happy to consider any other. > >Cheers, > >-- >Julien Grall -- _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-09 13:30 ` Peng Fan @ 2018-03-09 14:40 ` Julien Grall 2018-03-10 1:09 ` Stefano Stabellini 2018-03-12 2:32 ` Peng Fan 0 siblings, 2 replies; 26+ messages in thread From: Julien Grall @ 2018-03-09 14:40 UTC (permalink / raw) To: Peng Fan; +Cc: Peng Fan, Stefano Stabellini, xen-devel Hi, On 09/03/18 13:30, Peng Fan wrote: > Hi Julien, > On Fri, Mar 09, 2018 at 10:22:09AM +0000, Julien Grall wrote: >> Hi Peng, >> >> On 09/03/18 09:05, Peng Fan wrote: >>> On Thu, Mar 08, 2018 at 03:13:50PM +0000, Julien Grall wrote: >>>> On 08/03/18 12:43, Peng Fan wrote: >>>> 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. >>> >>> Just checked this. Because xen toolstack create device tree >>> with compatible "compatible = "xen,xenvm-4.10", "xen,xenvm";", >>> but the linux code use "fsl,imx8qm" to detect soc, then call scu >>> to get revision of chip. >> >> But how does the guest call the scu? > > We are doing GPU and display passthrough, also some other IPs passthrough. > we could not totally rely on Dom0 to configure the pinmux, gpio, clk, > relying on dom0 to do that would bring much hack code to our kernel, also > runtime clk set rate in domu could not be done. > > So we expose an interface to domu to directly communicate with SCU(system > control unit). Do you always expect a domain to access the SCU? Even with no passthrough involved? > >> >>> >>> After add an entry in linux side "{ .compatible = "xen,xenvm", .data = &imx8qm_soc_data, }," >>> It seems works. Passed a map/unmap stress test which easily fail without >>> the tlb workaround. >>> >>> Wonder is it ok to specific machine compatible in domu.cfg and let xen stack >>> use this machine compatible other than "xen,xenvm"? Is this acceptable by community? >> >> A user should be able to boot a guest safely on any machine without >> having to hack the configuration file. He should also be able to boot >> a guest with both ACPI and DT as this is independent from the real >> machine. So for me the way to find the workaround at the moment is >> not acceptable for a Xen guest upstream. > > I have no idea about ACPI (: > we are mainly working on embedded case, and mostly we are partitioning > our IPs. So our kernel normally only work with the dedicated DTB. > I am not asking to replace "xen,xenvm", just would like to add a option > that if user specific a machine compatible in cfg or else, xen toolstack > could add that in the final device tree. I know you were suggesting that and my point stands. Xen VM are not compatible with IMX8 platform. And again, a user should not have to tweak his configuration file, have to passthrough some device to an untrusted guest in order to have a guest booting normally on your platform. That is breaking the whole purpose of virtualization. Furthermore, the workaround is not in Linux upstream and I doubt this will be accepted as it is. So I am not convinced that we should modify Xen interface for that. Anyway, given that your silicon is going to be respined, then you probably want to restrict to run on the same cluster. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-09 14:40 ` Julien Grall @ 2018-03-10 1:09 ` Stefano Stabellini 2018-03-12 2:57 ` Peng Fan 2018-03-12 2:32 ` Peng Fan 1 sibling, 1 reply; 26+ messages in thread From: Stefano Stabellini @ 2018-03-10 1:09 UTC (permalink / raw) To: Julien Grall; +Cc: Peng Fan, Peng Fan, Stefano Stabellini, xen-devel On Fri, 9 Mar 2018, Julien Grall wrote: > Furthermore, the workaround is not in Linux upstream and I doubt this will be > accepted as it is. So I am not convinced that we should modify Xen interface > for that. > > Anyway, given that your silicon is going to be respined, then you probably > want to restrict to run on the same cluster. Hi Pen, I think that i.MX8 is a critical platform for the future of embedded virtualization and I really want to support it in Xen out of the box. However, I agree with Julien that if there will be a new version of the silicon with this issue properly fixed in hardware, then it might not make sense to add workarounds in Xen for this. Unless you think the version of the hardware with the errata will be commercialized? Do you plan to upstream your workaround in Linux? If not, then it might be best for you to carry the workaround for Xen in your Xen tree, the same way you'll do for Linux. For workarounds that affect/involve both Linux and Xen, we tend to follow the same policy as the Linux kernel, unless we have good reasons not to. On the other end, if you intend to upstream the Linux workaround, then we can discuss what to do for Xen. Also let me expand on one of Julien's suggestions that actually I think is quite good. Assuming that we have the tlb maintenance workaround in the hypervisor, it would be safe to start guests only in the big or only in the little cluster, right? In other words, you could still use both clusters but only starting guests in one or the other, not both. This is a good compromise because it allows full usage of the hardware, a relatively small patch in Xen, and no guest visible changes (such as toolstack changes to modify the compatible line). Guest visible and user visible changes are particularly troublesome to maintain in the long term and this is reason why we are reticent in introducing them. The tlb maintenance workaround in the hypervisor is something much easier to manage and we could consider taking it in if hardware with the errata will become available to customers. Cheers, Stefano _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-10 1:09 ` Stefano Stabellini @ 2018-03-12 2:57 ` Peng Fan 2018-03-12 11:02 ` Julien Grall 0 siblings, 1 reply; 26+ messages in thread From: Peng Fan @ 2018-03-12 2:57 UTC (permalink / raw) To: Stefano Stabellini; +Cc: Julien Grall, Peng Fan, xen-devel Hi Stefano, On Fri, Mar 09, 2018 at 05:09:20PM -0800, Stefano Stabellini wrote: >On Fri, 9 Mar 2018, Julien Grall wrote: >> Furthermore, the workaround is not in Linux upstream and I doubt this will be >> accepted as it is. So I am not convinced that we should modify Xen interface >> for that. >> >> Anyway, given that your silicon is going to be respined, then you probably >> want to restrict to run on the same cluster. > >Hi Pen, > >I think that i.MX8 is a critical platform for the future of embedded >virtualization and I really want to support it in Xen out of the box. > >However, I agree with Julien that if there will be a new version of the >silicon with this issue properly fixed in hardware, then it might not >make sense to add workarounds in Xen for this. Unless you think the >version of the hardware with the errata will be commercialized? Understand. I just thought some kernel code use machine compatible string to do some check for passthrough case. Some early customers might use the 1.0 chip to do their development, but I think all will switch to use new Silicon in the end. > >Do you plan to upstream your workaround in Linux? If not, then it might No plan. This workaround might not be accepted by Linux community. >be best for you to carry the workaround for Xen in your Xen tree, the >same way you'll do for Linux. For workarounds that affect/involve both >Linux and Xen, we tend to follow the same policy as the Linux kernel, >unless we have good reasons not to. On the other end, if you intend to >upstream the Linux workaround, then we can discuss what to do for Xen. > > >Also let me expand on one of Julien's suggestions that actually I think >is quite good. Assuming that we have the tlb maintenance workaround in >the hypervisor, it would be safe to start guests only in the big or only >in the little cluster, right? In other words, you could still use both I am a bit lost here. Are you refering Julien's suggestion 3? "3) Trap all TLBs access from the guest and convert them to TLB alle1s/vmalls12e1is" Currently, only use one the of the 2 clusters, I do not meet issue. No change to xen and domu not aware of linux workaround. Do you mean it is not safe without tlb maintenance workaround on my current hardware, even if restricting Guest OS only have one kind of cpu? A naive question, what case would require tlb broadcast from A53 to A72 in XEN? page balloon? Thanks Peng. >clusters but only starting guests in one or the other, not both. This is >a good compromise because it allows full usage of the hardware, a >relatively small patch in Xen, and no guest visible changes (such as >toolstack changes to modify the compatible line). Guest visible and user >visible changes are particularly troublesome to maintain in the long >term and this is reason why we are reticent in introducing them. The tlb >maintenance workaround in the hypervisor is something much easier to >manage and we could consider taking it in if hardware with the errata >will become available to customers. > >Cheers, > >Stefano -- _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-12 2:57 ` Peng Fan @ 2018-03-12 11:02 ` Julien Grall 0 siblings, 0 replies; 26+ messages in thread From: Julien Grall @ 2018-03-12 11:02 UTC (permalink / raw) To: Peng Fan, Stefano Stabellini; +Cc: Peng Fan, xen-devel Hi, On 12/03/18 02:57, Peng Fan wrote: > Hi Stefano, > On Fri, Mar 09, 2018 at 05:09:20PM -0800, Stefano Stabellini wrote: >> On Fri, 9 Mar 2018, Julien Grall wrote: >>> Furthermore, the workaround is not in Linux upstream and I doubt this will be >>> accepted as it is. So I am not convinced that we should modify Xen interface >>> for that. >>> >>> Anyway, given that your silicon is going to be respined, then you probably >>> want to restrict to run on the same cluster. >> >> Hi Pen, >> >> I think that i.MX8 is a critical platform for the future of embedded >> virtualization and I really want to support it in Xen out of the box. >> >> However, I agree with Julien that if there will be a new version of the >> silicon with this issue properly fixed in hardware, then it might not >> make sense to add workarounds in Xen for this. Unless you think the >> version of the hardware with the errata will be commercialized? > > Understand. I just thought some kernel code use machine > compatible string to do some check for passthrough case. Usually you give access to a device and describe it in the device-tree. The kernel will then use the appropriate driver for that device. You should never need to know what the machine is. Can you describe why this would be necessary in your use case? > > Some early customers might use the 1.0 chip to do their development, > but I think all will switch to use new Silicon in the end. > >> >> Do you plan to upstream your workaround in Linux? If not, then it might > > No plan. This workaround might not be accepted by Linux community. Based on what you said above, I would not even consider to upstream workaround for Xen. > >> be best for you to carry the workaround for Xen in your Xen tree, the >> same way you'll do for Linux. For workarounds that affect/involve both >> Linux and Xen, we tend to follow the same policy as the Linux kernel, >> unless we have good reasons not to. On the other end, if you intend to >> upstream the Linux workaround, then we can discuss what to do for Xen. >> >> >> Also let me expand on one of Julien's suggestions that actually I think >> is quite good. Assuming that we have the tlb maintenance workaround in >> the hypervisor, it would be safe to start guests only in the big or only >> in the little cluster, right? In other words, you could still use both > > I am a bit lost here. Are you refering Julien's suggestion 3? > "3) Trap all TLBs access from the guest and convert them to TLB alle1s/vmalls12e1is" > > Currently, only use one the of the 2 clusters, I do not meet issue. > No change to xen and domu not aware of linux workaround. > > Do you mean it is not safe without tlb maintenance workaround on my current > hardware, even if restricting Guest OS only have one kind of cpu? I think so. But this is only based on how I understood the description provided in the commit message you pointed early on. I have no insight whether TLB flush in hypervisor are going to be affected by the workaround. I would recommend to speak with your hardware team for that. > > A naive question, what case would require tlb broadcast from A53 to A72 in XEN? page balloon? Xen is sharing page-table between all the pCPU. As the hypervisor will run on the both cluster, you would need to convert all innershareable TLBs flush by VA to a full innershareable. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-09 14:40 ` Julien Grall 2018-03-10 1:09 ` Stefano Stabellini @ 2018-03-12 2:32 ` Peng Fan 2018-03-12 10:34 ` Julien Grall 1 sibling, 1 reply; 26+ messages in thread From: Peng Fan @ 2018-03-12 2:32 UTC (permalink / raw) To: Julien Grall; +Cc: Peng Fan, Stefano Stabellini, xen-devel On Fri, Mar 09, 2018 at 02:40:25PM +0000, Julien Grall wrote: >Hi, > >On 09/03/18 13:30, Peng Fan wrote: >>Hi Julien, >>On Fri, Mar 09, 2018 at 10:22:09AM +0000, Julien Grall wrote: >>>Hi Peng, >>> >>>On 09/03/18 09:05, Peng Fan wrote: >>>>On Thu, Mar 08, 2018 at 03:13:50PM +0000, Julien Grall wrote: >>>>>On 08/03/18 12:43, Peng Fan wrote: >>>>>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. >>>> >>>>Just checked this. Because xen toolstack create device tree >>>>with compatible "compatible = "xen,xenvm-4.10", "xen,xenvm";", >>>>but the linux code use "fsl,imx8qm" to detect soc, then call scu >>>>to get revision of chip. >>> >>>But how does the guest call the scu? >> >>We are doing GPU and display passthrough, also some other IPs passthrough. >>we could not totally rely on Dom0 to configure the pinmux, gpio, clk, >>relying on dom0 to do that would bring much hack code to our kernel, also >>runtime clk set rate in domu could not be done. >> >>So we expose an interface to domu to directly communicate with SCU(system >>control unit). > >Do you always expect a domain to access the SCU? Even with no >passthrough involved? only needed when a domain only needs to directly access hardware. > >> >>> >>>> >>>>After add an entry in linux side "{ .compatible = "xen,xenvm", .data = &imx8qm_soc_data, }," >>>>It seems works. Passed a map/unmap stress test which easily fail without >>>>the tlb workaround. >>>> >>>>Wonder is it ok to specific machine compatible in domu.cfg and let xen stack >>>>use this machine compatible other than "xen,xenvm"? Is this acceptable by community? >>> >>>A user should be able to boot a guest safely on any machine without >>>having to hack the configuration file. He should also be able to boot >>>a guest with both ACPI and DT as this is independent from the real >>>machine. So for me the way to find the workaround at the moment is >>>not acceptable for a Xen guest upstream. >> >>I have no idea about ACPI (: >>we are mainly working on embedded case, and mostly we are partitioning >>our IPs. So our kernel normally only work with the dedicated DTB. >>I am not asking to replace "xen,xenvm", just would like to add a option >>that if user specific a machine compatible in cfg or else, xen toolstack >>could add that in the final device tree. > >I know you were suggesting that and my point stands. Xen VM are not >compatible with IMX8 platform. > >And again, a user should not have to tweak his configuration file, >have to passthrough some device to an untrusted guest in order to >have a guest booting normally on your platform. That is breaking the >whole purpose of virtualization. > >Furthermore, the workaround is not in Linux upstream and I doubt this >will be accepted as it is. So I am not convinced that we should >modify Xen interface for that. > >Anyway, given that your silicon is going to be respined, then you >probably want to restrict to run on the same cluster. Understand. Thanks, Peng. > >Cheers, > >-- >Julien Grall -- _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/7] unsafe big.LITTLE support 2018-03-12 2:32 ` Peng Fan @ 2018-03-12 10:34 ` Julien Grall 0 siblings, 0 replies; 26+ messages in thread From: Julien Grall @ 2018-03-12 10:34 UTC (permalink / raw) To: Peng Fan; +Cc: Peng Fan, Stefano Stabellini, xen-devel On 12/03/18 02:32, Peng Fan wrote: > On Fri, Mar 09, 2018 at 02:40:25PM +0000, Julien Grall wrote: >> Hi, >> >> On 09/03/18 13:30, Peng Fan wrote: >>> Hi Julien, >>> On Fri, Mar 09, 2018 at 10:22:09AM +0000, Julien Grall wrote: >>>> Hi Peng, >>>> >>>> On 09/03/18 09:05, Peng Fan wrote: >>>>> On Thu, Mar 08, 2018 at 03:13:50PM +0000, Julien Grall wrote: >>>>>> On 08/03/18 12:43, Peng Fan wrote: >>>>>> 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. >>>>> >>>>> Just checked this. Because xen toolstack create device tree >>>>> with compatible "compatible = "xen,xenvm-4.10", "xen,xenvm";", >>>>> but the linux code use "fsl,imx8qm" to detect soc, then call scu >>>>> to get revision of chip. >>>> >>>> But how does the guest call the scu? >>> >>> We are doing GPU and display passthrough, also some other IPs passthrough. >>> we could not totally rely on Dom0 to configure the pinmux, gpio, clk, >>> relying on dom0 to do that would bring much hack code to our kernel, also >>> runtime clk set rate in domu could not be done. >>> >>> So we expose an interface to domu to directly communicate with SCU(system >>> control unit). >> >> Do you always expect a domain to access the SCU? Even with no >> passthrough involved? > > only needed when a domain only needs to directly access hardware. Then your suggested workaround can't work for guest with not device passthrough. I would recommend to find a workaround that works for every guests. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2018-03-12 11:02 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-02 19:05 [PATCH v4 0/7] unsafe big.LITTLE support Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 2/7] xen/arm: Park CPUs with a MIDR different from the boot CPU Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 3/7] xen/arm: make processor a per cpu variable Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 4/7] xen/arm: read ACTLR on the pcpu where the vcpu will run Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 5/7] xen/arm: set VPIDR based on the MIDR value of the underlying pCPU Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 6/7] xen/arm: update the docs about heterogeneous computing Stefano Stabellini 2018-03-02 19:06 ` [PATCH v4 7/7] xen/arm: disable CPUs with different dcache line sizes Stefano Stabellini 2018-03-06 10:59 ` Julien Grall 2018-03-06 19:41 ` Stefano Stabellini 2018-03-06 10:46 ` [PATCH v4 1/7] xen/arm: Read the dcache line size from CTR register Julien Grall 2018-03-08 6:15 ` [PATCH v4 0/7] unsafe big.LITTLE support Peng Fan 2018-03-08 11:03 ` Julien Grall 2018-03-08 12:23 ` Peng Fan 2018-03-08 12:30 ` Julien Grall 2018-03-08 12:43 ` Peng Fan 2018-03-08 15:13 ` Julien Grall 2018-03-09 9:05 ` Peng Fan 2018-03-09 10:22 ` Julien Grall 2018-03-09 13:30 ` Peng Fan 2018-03-09 14:40 ` Julien Grall 2018-03-10 1:09 ` Stefano Stabellini 2018-03-12 2:57 ` Peng Fan 2018-03-12 11:02 ` Julien Grall 2018-03-12 2:32 ` Peng Fan 2018-03-12 10:34 ` Julien Grall
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.