| Message ID | 1464013052-32587-13-git-send-email-julien.grall@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 23, 2016 at 7:22 AM Julien Grall <julien.grall@arm.com> wrote: > The ARM errata 819472, 827319 and 824069 define the same workaround for > these hardware issues in certain Cortex-A53 parts. > > The cache instructions "dc cvac" and "dc cvau" need to be upgraded to > "dc civac". > > Use the alternative framework to replace those instructions only on > affected cores. > > Whilst the errata affect cache instructions issued at any exception > level, it is not necessary to trap EL1/EL0 data cache instructions > access in order to upgrade them. Indeed the data cache corruption would > always be at the address used by the data cache instructions. Note that > this address could point to a shared memory between guests and the > hypervisors, however all the information present in it are be validated > before any use. > > Therefore a malicious guest could only hurt itself. Note that all the > guests should implement/enable the workaround for the affected cores. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > docs/misc/arm/silicon-errata.txt | 3 ++ > xen/arch/arm/Kconfig | 71 > ++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/arm64/cache.S | 54 ++++++++++++++++++++++++++++++ > xen/arch/arm/cpuerrata.c | 17 ++++++++++ > xen/include/asm-arm/arm64/page.h | 7 +++- > xen/include/asm-arm/cpufeature.h | 4 ++- > xen/include/asm-arm/processor.h | 6 ++++ > 7 files changed, 160 insertions(+), 2 deletions(-) > > diff --git a/docs/misc/arm/silicon-errata.txt > b/docs/misc/arm/silicon-errata.txt > index 3f0d32b..9a2983b 100644 > --- a/docs/misc/arm/silicon-errata.txt > +++ b/docs/misc/arm/silicon-errata.txt > @@ -42,4 +42,7 @@ stable hypervisors. > | Implementor | Component | Erratum ID | Kconfig > | > > +----------------+-----------------+-----------------+-------------------------+ > | ARM | Cortex-A15 | #766422 | N/A > | > +| ARM | Cortex-A53 | #827319 | > ARM64_ERRATUM_827319 | > +| ARM | Cortex-A53 | #824069 | > ARM64_ERRATUM_824069 | > +| ARM | Cortex-A53 | #819472 | > ARM64_ERRATUM_819472 | > | ARM | Cortex-A57 | #852523 | N/A > | > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig > index 0141bd9..a473d9c 100644 > --- a/xen/arch/arm/Kconfig > +++ b/xen/arch/arm/Kconfig > @@ -53,6 +53,77 @@ config ALTERNATIVE > > endmenu > > +menu "ARM errata workaround via the alternative framework" > + depends on ALTERNATIVE > + > +config ARM64_ERRATUM_827319 > + bool "Cortex-A53: 827319: Data cache clean instructions might > cause overlapping transactions to the interconnect" > + default y > + depends on ARM_64 > + help > + This option adds an alternative code sequence to work around ARM > + erratum 827319 on Cortex-A53 parts up to r0p2 with an AMBA 5 CHI > + master interface and an L2 cache. > + > + Under certain conditions this erratum can cause a clean line > eviction > + to occur at the same time as another transaction to the same > address > + on the AMBA 5 CHI interface, which can cause data corruption if > the > + interconnect reorders the two transactions. > + > + The workaround promotes data cache clean instructions to > + data cache clean-and-invalidate. > + Please note that this does not necessarily enable the workaround, > + as it depends on the alternative framework, which will only patch > + the kernel if an affected CPU is detected. > + > + If unsure, say Y. > + > +config ARM64_ERRATUM_824069 > + bool "Cortex-A53: 824069: Cache line might not be marked as clean > after a CleanShared snoop" > + default y > + depends on ARM_64 > + help > + This option adds an alternative code sequence to work around ARM > + erratum 824069 on Cortex-A53 parts up to r0p2 when it is > connected > + to a coherent interconnect. > + > + If a Cortex-A53 processor is executing a store or prefetch for > + write instruction at the same time as a processor in another > + cluster is executing a cache maintenance operation to the same > + address, then this erratum might cause a clean cache line to be > + incorrectly marked as dirty. > + > + The workaround promotes data cache clean instructions to > + data cache clean-and-invalidate. > + Please note that this option does not necessarily enable the > + workaround, as it depends on the alternative framework, which > will > + only patch the kernel if an affected CPU is detected. > + > + If unsure, say Y. > + > +config ARM64_ERRATUM_819472 > + bool "Cortex-A53: 819472: Store exclusive instructions might cause > data corruption" > + default y > + depends on ARM_64 > + help > + This option adds an alternative code sequence to work around ARM > + erratum 819472 on Cortex-A53 parts up to r0p1 with an L2 cache > + present when it is connected to a coherent interconnect. > + > + If the processor is executing a load and store exclusive > sequence at > + the same time as a processor in another cluster is executing a > cache > + maintenance operation to the same address, then this erratum > might > + cause data corruption. > + > + The workaround promotes data cache clean instructions to > + data cache clean-and-invalidate. > + Please note that this does not necessarily enable the workaround, > + as it depends on the alternative framework, which will only patch > + the kernel if an affected CPU is detected. > + > + If unsure, say Y. > +endmenu > + > source "common/Kconfig" > > source "drivers/Kconfig" > diff --git a/xen/arch/arm/arm64/cache.S b/xen/arch/arm/arm64/cache.S > index eff4e16..6e49e25 100644 > --- a/xen/arch/arm/arm64/cache.S > +++ b/xen/arch/arm/arm64/cache.S > @@ -19,6 +19,8 @@ > * along with this program. If not, see <http://www.gnu.org/licenses/>. > */ > > +#include <asm/alternative.h> > + > /* > * dcache_line_size - get the minimum D-cache line size from the CTR > register. > */ > @@ -30,6 +32,58 @@ > .endm > > /* > +<<<<<<< HEAD > +======= > + * icache_line_size - get the minimum I-cache line size from the CTR > register. > + */ > + .macro icache_line_size, reg, tmp > + mrs \tmp, ctr_el0 // read CTR > + and \tmp, \tmp, #0xf // cache line size encoding > + mov \reg, #4 // bytes per word > + lsl \reg, \reg, \tmp // actual cache line size > + .endm > + > +/* > + * flush_icache_range(start,end) > + * > + * Ensure that the I and D caches are coherent within specified > region. > + * This is typically used when code has been written to a memory > region, > + * and will be executed. > + * > + * - start - virtual start address of region > + * - end - virtual end address of region > + */ > +ENTRY(flush_icache_range) > + dcache_line_size x2, x3 > + sub x3, x2, #1 > + bic x4, x0, x3 > +1: > +alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE > + dc cvau, x4 > +alternative_else > + dc civac, x4 > +alternative_endif > + add x4, x4, x2 > + cmp x4, x1 > + b.lo 1b > + dsb ish > + > + icache_line_size x2, x3 > + sub x3, x2, #1 > + bic x4, x0, x3 > +1: > + ic ivau, x4 // invalidate I line PoU > + add x4, x4, x2 > + cmp x4, x1 > + b.lo 1b > + dsb ish > + isb > + mov x0, #0 > + ret > +ENDPROC(flush_icache_range) > + > +/* > +>>>>>>> 3b39ae7... xen/arm: arm64: Add Cortex-A53 cache errata workaround > Hi Julien, This patch has a conflict, only can patched manually. * __flush_dcache_area(kaddr, size) > * > * Ensure that the data held in the page kaddr is written back to the > diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c > index 52d39f8..211b520 100644 > --- a/xen/arch/arm/cpuerrata.c > +++ b/xen/arch/arm/cpuerrata.c > @@ -17,6 +17,23 @@ is_affected_midr_range(const struct > arm_cpu_capabilities *entry) > } > > static const struct arm_cpu_capabilities arm_errata[] = { > +#if defined(CONFIG_ARM64_ERRATUM_827319) || \ > + defined(CONFIG_ARM64_ERRATUM_824069) > + { > + /* Cortex-A53 r0p[012] */ > + .desc = "ARM errata 827319, 824069", > + .capability = ARM64_WORKAROUND_CLEAN_CACHE, > + MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x02), > + }, > +#endif > +#ifdef CONFIG_ARM64_ERRATUM_819472 > + { > + /* Cortex-A53 r0[01] */ > + .desc = "ARM erratum 819472", > + .capability = ARM64_WORKAROUND_CLEAN_CACHE, > + MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x01), > + }, > +#endif > {}, > }; > > diff --git a/xen/include/asm-arm/arm64/page.h > b/xen/include/asm-arm/arm64/page.h > index fbdc8fb..79ef7bd 100644 > --- a/xen/include/asm-arm/arm64/page.h > +++ b/xen/include/asm-arm/arm64/page.h > @@ -3,6 +3,8 @@ > > #ifndef __ASSEMBLY__ > > +#include <asm/alternative.h> > + > /* Write a pagetable entry */ > static inline void write_pte(lpae_t *p, lpae_t pte) > { > @@ -18,7 +20,10 @@ static inline void write_pte(lpae_t *p, lpae_t pte) > #define __invalidate_dcache_one(R) "dc ivac, %" #R ";" > > /* Inline ASM to flush dcache on register R (may be an inline asm > operand) */ > -#define __clean_dcache_one(R) "dc cvac, %" #R ";" > +#define __clean_dcache_one(R) \ > + ALTERNATIVE("dc cvac, %" #R ";", \ > + "dc civac, %" #R ";", \ > + ARM64_WORKAROUND_CLEAN_CACHE) \ > > /* Inline ASM to clean and invalidate dcache on register R (may be an > * inline asm operand) */ > diff --git a/xen/include/asm-arm/cpufeature.h > b/xen/include/asm-arm/cpufeature.h > index fb57295..474a778 100644 > --- a/xen/include/asm-arm/cpufeature.h > +++ b/xen/include/asm-arm/cpufeature.h > @@ -35,7 +35,9 @@ > #endif > #define cpu_has_security (boot_cpu_feature32(security) > 0) > > -#define ARM_NCAPS 0 > +#define ARM64_WORKAROUND_CLEAN_CACHE 0 > + > +#define ARM_NCAPS 1 > > #ifndef __ASSEMBLY__ > > diff --git a/xen/include/asm-arm/processor.h > b/xen/include/asm-arm/processor.h > index 1b701c5..4123951 100644 > --- a/xen/include/asm-arm/processor.h > +++ b/xen/include/asm-arm/processor.h > @@ -44,6 +44,12 @@ > _model == (model) && _rv >= (rv_min) && _rv <= (rv_max); \ > }) > > +#define ARM_CPU_IMP_ARM 0x41 > + > +#define ARM_CPU_PART_CORTEX_A53 0xD03 > + > +#define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, > ARM_CPU_PART_CORTEX_A53) > + > /* MPIDR Multiprocessor Affinity Register */ > #define _MPIDR_UP (30) > #define MPIDR_UP (_AC(1,U) << _MPIDR_UP) > -- > 1.9.1 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >
Hello, Please configure your e-mail client to quote properly. On 24/05/2016 03:46, Chenxiao Zhao wrote: > On Mon, May 23, 2016 at 7:22 AM Julien Grall <julien.grall@arm.com > <mailto:julien.grall@arm.com>> wrote: > /* > +<<<<<<< HEAD > +======= > + * icache_line_size - get the minimum I-cache line size from the > CTR register. > + */ > + .macro icache_line_size, reg, tmp > + mrs \tmp, ctr_el0 // read CTR > + and \tmp, \tmp, #0xf // cache line size > encoding > + mov \reg, #4 // bytes per word > + lsl \reg, \reg, \tmp // actual cache line > size > + .endm > + > +/* > + * flush_icache_range(start,end) > + * > + * Ensure that the I and D caches are coherent within specified > region. > + * This is typically used when code has been written to a > memory region, > + * and will be executed. > + * > + * - start - virtual start address of region > + * - end - virtual end address of region > + */ > +ENTRY(flush_icache_range) > + dcache_line_size x2, x3 > + sub x3, x2, #1 > + bic x4, x0, x3 > +1: > +alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE > + dc cvau, x4 > +alternative_else > + dc civac, x4 > +alternative_endif > + add x4, x4, x2 > + cmp x4, x1 > + b.lo 1b > + dsb ish > + > + icache_line_size x2, x3 > + sub x3, x2, #1 > + bic x4, x0, x3 > +1: > + ic ivau, x4 // invalidate I line PoU > + add x4, x4, x2 > + cmp x4, x1 > + b.lo 1b > + dsb ish > + isb > + mov x0, #0 > + ret > +ENDPROC(flush_icache_range) > + > +/* > +>>>>>>> 3b39ae7... xen/arm: arm64: Add Cortex-A53 cache errata > > workaround > This patch has a conflict, only can patched manually. Whilst this patch contain a conflict because I have not rebased correctly, it applies without any issue and compile on ARM64. This is because >>>> are part of the comments. I will sent a new version of this patch. Regards,
diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt index 3f0d32b..9a2983b 100644 --- a/docs/misc/arm/silicon-errata.txt +++ b/docs/misc/arm/silicon-errata.txt @@ -42,4 +42,7 @@ stable hypervisors. | Implementor | Component | Erratum ID | Kconfig | +----------------+-----------------+-----------------+-------------------------+ | ARM | Cortex-A15 | #766422 | N/A | +| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 | +| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 | +| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 | | ARM | Cortex-A57 | #852523 | N/A | diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 0141bd9..a473d9c 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -53,6 +53,77 @@ config ALTERNATIVE endmenu +menu "ARM errata workaround via the alternative framework" + depends on ALTERNATIVE + +config ARM64_ERRATUM_827319 + bool "Cortex-A53: 827319: Data cache clean instructions might cause overlapping transactions to the interconnect" + default y + depends on ARM_64 + help + This option adds an alternative code sequence to work around ARM + erratum 827319 on Cortex-A53 parts up to r0p2 with an AMBA 5 CHI + master interface and an L2 cache. + + Under certain conditions this erratum can cause a clean line eviction + to occur at the same time as another transaction to the same address + on the AMBA 5 CHI interface, which can cause data corruption if the + interconnect reorders the two transactions. + + The workaround promotes data cache clean instructions to + data cache clean-and-invalidate. + Please note that this does not necessarily enable the workaround, + as it depends on the alternative framework, which will only patch + the kernel if an affected CPU is detected. + + If unsure, say Y. + +config ARM64_ERRATUM_824069 + bool "Cortex-A53: 824069: Cache line might not be marked as clean after a CleanShared snoop" + default y + depends on ARM_64 + help + This option adds an alternative code sequence to work around ARM + erratum 824069 on Cortex-A53 parts up to r0p2 when it is connected + to a coherent interconnect. + + If a Cortex-A53 processor is executing a store or prefetch for + write instruction at the same time as a processor in another + cluster is executing a cache maintenance operation to the same + address, then this erratum might cause a clean cache line to be + incorrectly marked as dirty. + + The workaround promotes data cache clean instructions to + data cache clean-and-invalidate. + Please note that this option does not necessarily enable the + workaround, as it depends on the alternative framework, which will + only patch the kernel if an affected CPU is detected. + + If unsure, say Y. + +config ARM64_ERRATUM_819472 + bool "Cortex-A53: 819472: Store exclusive instructions might cause data corruption" + default y + depends on ARM_64 + help + This option adds an alternative code sequence to work around ARM + erratum 819472 on Cortex-A53 parts up to r0p1 with an L2 cache + present when it is connected to a coherent interconnect. + + If the processor is executing a load and store exclusive sequence at + the same time as a processor in another cluster is executing a cache + maintenance operation to the same address, then this erratum might + cause data corruption. + + The workaround promotes data cache clean instructions to + data cache clean-and-invalidate. + Please note that this does not necessarily enable the workaround, + as it depends on the alternative framework, which will only patch + the kernel if an affected CPU is detected. + + If unsure, say Y. +endmenu + source "common/Kconfig" source "drivers/Kconfig" diff --git a/xen/arch/arm/arm64/cache.S b/xen/arch/arm/arm64/cache.S index eff4e16..6e49e25 100644 --- a/xen/arch/arm/arm64/cache.S +++ b/xen/arch/arm/arm64/cache.S @@ -19,6 +19,8 @@ * along with this program. If not, see <http://www.gnu.org/licenses/>. */ +#include <asm/alternative.h> + /* * dcache_line_size - get the minimum D-cache line size from the CTR register. */ @@ -30,6 +32,58 @@ .endm /* +<<<<<<< HEAD +======= + * icache_line_size - get the minimum I-cache line size from the CTR register. + */ + .macro icache_line_size, reg, tmp + mrs \tmp, ctr_el0 // read CTR + and \tmp, \tmp, #0xf // cache line size encoding + mov \reg, #4 // bytes per word + lsl \reg, \reg, \tmp // actual cache line size + .endm + +/* + * flush_icache_range(start,end) + * + * Ensure that the I and D caches are coherent within specified region. + * This is typically used when code has been written to a memory region, + * and will be executed. + * + * - start - virtual start address of region + * - end - virtual end address of region + */ +ENTRY(flush_icache_range) + dcache_line_size x2, x3 + sub x3, x2, #1 + bic x4, x0, x3 +1: +alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE + dc cvau, x4 +alternative_else + dc civac, x4 +alternative_endif + add x4, x4, x2 + cmp x4, x1 + b.lo 1b + dsb ish + + icache_line_size x2, x3 + sub x3, x2, #1 + bic x4, x0, x3 +1: + ic ivau, x4 // invalidate I line PoU + add x4, x4, x2 + cmp x4, x1 + b.lo 1b + dsb ish + isb + mov x0, #0 + ret +ENDPROC(flush_icache_range) + +/* +>>>>>>> 3b39ae7... xen/arm: arm64: Add Cortex-A53 cache errata workaround * __flush_dcache_area(kaddr, size) * * Ensure that the data held in the page kaddr is written back to the diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c index 52d39f8..211b520 100644 --- a/xen/arch/arm/cpuerrata.c +++ b/xen/arch/arm/cpuerrata.c @@ -17,6 +17,23 @@ is_affected_midr_range(const struct arm_cpu_capabilities *entry) } static const struct arm_cpu_capabilities arm_errata[] = { +#if defined(CONFIG_ARM64_ERRATUM_827319) || \ + defined(CONFIG_ARM64_ERRATUM_824069) + { + /* Cortex-A53 r0p[012] */ + .desc = "ARM errata 827319, 824069", + .capability = ARM64_WORKAROUND_CLEAN_CACHE, + MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x02), + }, +#endif +#ifdef CONFIG_ARM64_ERRATUM_819472 + { + /* Cortex-A53 r0[01] */ + .desc = "ARM erratum 819472", + .capability = ARM64_WORKAROUND_CLEAN_CACHE, + MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x01), + }, +#endif {}, }; diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h index fbdc8fb..79ef7bd 100644 --- a/xen/include/asm-arm/arm64/page.h +++ b/xen/include/asm-arm/arm64/page.h @@ -3,6 +3,8 @@ #ifndef __ASSEMBLY__ +#include <asm/alternative.h> + /* Write a pagetable entry */ static inline void write_pte(lpae_t *p, lpae_t pte) { @@ -18,7 +20,10 @@ static inline void write_pte(lpae_t *p, lpae_t pte) #define __invalidate_dcache_one(R) "dc ivac, %" #R ";" /* Inline ASM to flush dcache on register R (may be an inline asm operand) */ -#define __clean_dcache_one(R) "dc cvac, %" #R ";" +#define __clean_dcache_one(R) \ + ALTERNATIVE("dc cvac, %" #R ";", \ + "dc civac, %" #R ";", \ + ARM64_WORKAROUND_CLEAN_CACHE) \ /* Inline ASM to clean and invalidate dcache on register R (may be an * inline asm operand) */ diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h index fb57295..474a778 100644 --- a/xen/include/asm-arm/cpufeature.h +++ b/xen/include/asm-arm/cpufeature.h @@ -35,7 +35,9 @@ #endif #define cpu_has_security (boot_cpu_feature32(security) > 0) -#define ARM_NCAPS 0 +#define ARM64_WORKAROUND_CLEAN_CACHE 0 + +#define ARM_NCAPS 1 #ifndef __ASSEMBLY__ diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h index 1b701c5..4123951 100644 --- a/xen/include/asm-arm/processor.h +++ b/xen/include/asm-arm/processor.h @@ -44,6 +44,12 @@ _model == (model) && _rv >= (rv_min) && _rv <= (rv_max); \ }) +#define ARM_CPU_IMP_ARM 0x41 + +#define ARM_CPU_PART_CORTEX_A53 0xD03 + +#define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53) + /* MPIDR Multiprocessor Affinity Register */ #define _MPIDR_UP (30) #define MPIDR_UP (_AC(1,U) << _MPIDR_UP)
The ARM errata 819472, 827319 and 824069 define the same workaround for these hardware issues in certain Cortex-A53 parts. The cache instructions "dc cvac" and "dc cvau" need to be upgraded to "dc civac". Use the alternative framework to replace those instructions only on affected cores. Whilst the errata affect cache instructions issued at any exception level, it is not necessary to trap EL1/EL0 data cache instructions access in order to upgrade them. Indeed the data cache corruption would always be at the address used by the data cache instructions. Note that this address could point to a shared memory between guests and the hypervisors, however all the information present in it are be validated before any use. Therefore a malicious guest could only hurt itself. Note that all the guests should implement/enable the workaround for the affected cores. Signed-off-by: Julien Grall <julien.grall@arm.com> --- docs/misc/arm/silicon-errata.txt | 3 ++ xen/arch/arm/Kconfig | 71 ++++++++++++++++++++++++++++++++++++++++ xen/arch/arm/arm64/cache.S | 54 ++++++++++++++++++++++++++++++ xen/arch/arm/cpuerrata.c | 17 ++++++++++ xen/include/asm-arm/arm64/page.h | 7 +++- xen/include/asm-arm/cpufeature.h | 4 ++- xen/include/asm-arm/processor.h | 6 ++++ 7 files changed, 160 insertions(+), 2 deletions(-)