* [PATCH v3 1/6] xen/arm: traps: Simplify the switch in do_trap_*_abort_guest
2016-08-04 17:50 [PATCH v3 0/6] xen/arm: Simplify do_trap_*_abort_guest Julien Grall
@ 2016-08-04 17:50 ` Julien Grall
2016-08-04 17:52 ` Julien Grall
2016-08-04 17:50 ` [PATCH v3 2/6] xen/arm: Provide macros to help creating workaround helpers Julien Grall
` (5 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Julien Grall @ 2016-08-04 17:50 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, sstabellini
The fault status we care are in the form BBBBxx where xx is the lookup
level that gave the fault. We can simplify the code by masking the 2 least
significant bits.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
The switch has not been replaced by a simple if because more case
will be added in follow-up patches.
Changes in v2:
- Fix typoes in the commit message
---
xen/arch/arm/traps.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 0f78d21..b11d2e5 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2387,9 +2387,9 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
int rc;
register_t gva = READ_SYSREG(FAR_EL2);
- switch ( hsr.iabt.ifsc & 0x3f )
+ switch ( hsr.iabt.ifsc & ~FSC_LL_MASK )
{
- case FSC_FLT_PERM ... FSC_FLT_PERM + 3:
+ case FSC_FLT_PERM:
{
paddr_t gpa;
const struct npfec npfec = {
@@ -2450,9 +2450,9 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
return; /* Try again */
}
- switch ( dabt.dfsc & 0x3f )
+ switch ( dabt.dfsc & ~FSC_LL_MASK )
{
- case FSC_FLT_PERM ... FSC_FLT_PERM + 3:
+ case FSC_FLT_PERM:
{
const struct npfec npfec = {
.read_access = !dabt.write,
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 1/6] xen/arm: traps: Simplify the switch in do_trap_*_abort_guest
2016-08-04 17:50 ` [PATCH v3 1/6] xen/arm: traps: Simplify the switch in do_trap_*_abort_guest Julien Grall
@ 2016-08-04 17:52 ` Julien Grall
0 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2016-08-04 17:52 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini
Hi,
On 04/08/16 18:50, Julien Grall wrote:
> The fault status we care are in the form BBBBxx where xx is the lookup
> level that gave the fault. We can simplify the code by masking the 2 least
> significant bits.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
I forgot to retain the reviewed-by from Stefano given on the last version:
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>
> ---
> The switch has not been replaced by a simple if because more case
> will be added in follow-up patches.
>
> Changes in v2:
> - Fix typoes in the commit message
> ---
> xen/arch/arm/traps.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 0f78d21..b11d2e5 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2387,9 +2387,9 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
> int rc;
> register_t gva = READ_SYSREG(FAR_EL2);
>
> - switch ( hsr.iabt.ifsc & 0x3f )
> + switch ( hsr.iabt.ifsc & ~FSC_LL_MASK )
> {
> - case FSC_FLT_PERM ... FSC_FLT_PERM + 3:
> + case FSC_FLT_PERM:
> {
> paddr_t gpa;
> const struct npfec npfec = {
> @@ -2450,9 +2450,9 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
> return; /* Try again */
> }
>
> - switch ( dabt.dfsc & 0x3f )
> + switch ( dabt.dfsc & ~FSC_LL_MASK )
> {
> - case FSC_FLT_PERM ... FSC_FLT_PERM + 3:
> + case FSC_FLT_PERM:
> {
> const struct npfec npfec = {
> .read_access = !dabt.write,
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 2/6] xen/arm: Provide macros to help creating workaround helpers
2016-08-04 17:50 [PATCH v3 0/6] xen/arm: Simplify do_trap_*_abort_guest Julien Grall
2016-08-04 17:50 ` [PATCH v3 1/6] xen/arm: traps: Simplify the switch in do_trap_*_abort_guest Julien Grall
@ 2016-08-04 17:50 ` Julien Grall
2016-08-04 17:52 ` Julien Grall
2016-08-04 17:50 ` [PATCH v3 3/6] xen/arm: Use check_workaround to handle the erratum 766422 Julien Grall
` (4 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Julien Grall @ 2016-08-04 17:50 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, sstabellini
Workarounds may require to execute a different path when the platform
is affected by the associated erratum. Furthermore, this may need to
be called in the common code.
To avoid too much intrusion/overhead, the workaround helpers need to
be a nop on architecture which will never have the workaround and have
to be quick to check whether the platform requires it.
The alternative framework is used to transform the check in a single
instruction. When the framework is not available, the helper will have
~6 instructions including 1 instruction load.
The macro will create a handler called check_workaround_xxxxx with
xxxx the erratum number.
For instance, the line bellow will create a workaround helper for
erratum #424242 which is enabled when the capability
ARM64_WORKAROUND_424242 is set and only available for ARM64:
CHECK_WORKAROUND_HELPER(424242, ARM64_WORKAROUND_42424242, CONFIG_ARM64)
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Changes in v2:
- Add Konrad's reviewed-by
---
xen/include/asm-arm/cpuerrata.h | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
index c495ee5..2982a92 100644
--- a/xen/include/asm-arm/cpuerrata.h
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -1,8 +1,47 @@
#ifndef __ARM_CPUERRATA_H__
#define __ARM_CPUERRATA_H__
+#include <xen/config.h>
+#include <asm/cpufeature.h>
+#include <asm/alternative.h>
+
void check_local_cpu_errata(void);
+#ifdef CONFIG_ALTERNATIVE
+
+#define CHECK_WORKAROUND_HELPER(erratum, feature, arch) \
+static inline bool_t check_workaround_##erratum(void) \
+{ \
+ if ( !IS_ENABLED(arch) ) \
+ return 0; \
+ else \
+ { \
+ bool_t ret; \
+ \
+ asm volatile (ALTERNATIVE("mov %0, #0", \
+ "mov %0, #1", \
+ feature) \
+ : "=r" (ret)); \
+ \
+ return unlikely(ret); \
+ } \
+}
+
+#else /* CONFIG_ALTERNATIVE */
+
+#define CHECK_WORKAROUND_HELPER(erratum, feature, arch) \
+static inline bool_t check_workaround_##erratum(void) \
+{ \
+ if ( !IS_ENABLED(arch) ) \
+ return 0; \
+ else \
+ return unlikely(cpus_have_cap(feature)); \
+}
+
+#endif
+
+#undef CHECK_WORKAROUND_HELPER
+
#endif /* __ARM_CPUERRATA_H__ */
/*
* Local variables:
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/6] xen/arm: Provide macros to help creating workaround helpers
2016-08-04 17:50 ` [PATCH v3 2/6] xen/arm: Provide macros to help creating workaround helpers Julien Grall
@ 2016-08-04 17:52 ` Julien Grall
0 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2016-08-04 17:52 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini
Hi,
On 04/08/16 18:50, Julien Grall wrote:
> Workarounds may require to execute a different path when the platform
> is affected by the associated erratum. Furthermore, this may need to
> be called in the common code.
>
> To avoid too much intrusion/overhead, the workaround helpers need to
> be a nop on architecture which will never have the workaround and have
> to be quick to check whether the platform requires it.
>
> The alternative framework is used to transform the check in a single
> instruction. When the framework is not available, the helper will have
> ~6 instructions including 1 instruction load.
>
> The macro will create a handler called check_workaround_xxxxx with
> xxxx the erratum number.
>
> For instance, the line bellow will create a workaround helper for
> erratum #424242 which is enabled when the capability
> ARM64_WORKAROUND_424242 is set and only available for ARM64:
>
> CHECK_WORKAROUND_HELPER(424242, ARM64_WORKAROUND_42424242, CONFIG_ARM64)
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Similar here, I forgot to add the acked-by from Stefano:
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
>
> ---
> Changes in v2:
> - Add Konrad's reviewed-by
> ---
> xen/include/asm-arm/cpuerrata.h | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
> index c495ee5..2982a92 100644
> --- a/xen/include/asm-arm/cpuerrata.h
> +++ b/xen/include/asm-arm/cpuerrata.h
> @@ -1,8 +1,47 @@
> #ifndef __ARM_CPUERRATA_H__
> #define __ARM_CPUERRATA_H__
>
> +#include <xen/config.h>
> +#include <asm/cpufeature.h>
> +#include <asm/alternative.h>
> +
> void check_local_cpu_errata(void);
>
> +#ifdef CONFIG_ALTERNATIVE
> +
> +#define CHECK_WORKAROUND_HELPER(erratum, feature, arch) \
> +static inline bool_t check_workaround_##erratum(void) \
> +{ \
> + if ( !IS_ENABLED(arch) ) \
> + return 0; \
> + else \
> + { \
> + bool_t ret; \
> + \
> + asm volatile (ALTERNATIVE("mov %0, #0", \
> + "mov %0, #1", \
> + feature) \
> + : "=r" (ret)); \
> + \
> + return unlikely(ret); \
> + } \
> +}
> +
> +#else /* CONFIG_ALTERNATIVE */
> +
> +#define CHECK_WORKAROUND_HELPER(erratum, feature, arch) \
> +static inline bool_t check_workaround_##erratum(void) \
> +{ \
> + if ( !IS_ENABLED(arch) ) \
> + return 0; \
> + else \
> + return unlikely(cpus_have_cap(feature)); \
> +}
> +
> +#endif
> +
> +#undef CHECK_WORKAROUND_HELPER
> +
> #endif /* __ARM_CPUERRATA_H__ */
> /*
> * Local variables:
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/6] xen/arm: Use check_workaround to handle the erratum 766422
2016-08-04 17:50 [PATCH v3 0/6] xen/arm: Simplify do_trap_*_abort_guest Julien Grall
2016-08-04 17:50 ` [PATCH v3 1/6] xen/arm: traps: Simplify the switch in do_trap_*_abort_guest Julien Grall
2016-08-04 17:50 ` [PATCH v3 2/6] xen/arm: Provide macros to help creating workaround helpers Julien Grall
@ 2016-08-04 17:50 ` Julien Grall
2016-08-04 17:53 ` Julien Grall
2016-08-04 17:50 ` [PATCH v3 4/6] xen/arm: traps: MMIO should only be emulated for fault translation Julien Grall
` (3 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Julien Grall @ 2016-08-04 17:50 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, sstabellini
Currently, Xen is accessing the stored MIDR everytime it has to check
whether the processor is affected by the erratum 766422.
This could take advantage of the new capability bitfields to detect
whether the processor is affected at boot time.
With this patch, the number of instructions to check the erratum is
going down from ~13 (including 2 loads and a co-processor access) to
~6 instructions (include 1 load).
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
Changes in v2:
- Update the commit message
---
xen/arch/arm/cpuerrata.c | 6 ++++++
xen/arch/arm/traps.c | 3 ++-
xen/include/asm-arm/arm32/processor.h | 4 ----
xen/include/asm-arm/arm64/processor.h | 2 --
xen/include/asm-arm/cpuerrata.h | 2 ++
xen/include/asm-arm/cpufeature.h | 3 ++-
xen/include/asm-arm/processor.h | 2 ++
7 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 3ac97b3..748e02e 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -17,6 +17,12 @@ is_affected_midr_range(const struct arm_cpu_capabilities *entry)
}
static const struct arm_cpu_capabilities arm_errata[] = {
+ {
+ /* Cortex-A15 r0p4 */
+ .desc = "ARM erratum 766422",
+ .capability = ARM32_WORKAROUND_766422,
+ MIDR_RANGE(MIDR_CORTEX_A15, 0x04, 0x04),
+ },
#if defined(CONFIG_ARM64_ERRATUM_827319) || \
defined(CONFIG_ARM64_ERRATUM_824069)
{
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index b11d2e5..7206a7e 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -46,6 +46,7 @@
#include "vtimer.h"
#include <asm/gic.h>
#include <asm/vgic.h>
+#include <asm/cpuerrata.h>
/* The base of the stack must always be double-word aligned, which means
* that both the kernel half of struct cpu_user_regs (which is pushed in
@@ -2481,7 +2482,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
* Erratum 766422: Thumb store translation fault to Hypervisor may
* not have correct HSR Rt value.
*/
- if ( cpu_has_erratum_766422() && (regs->cpsr & PSR_THUMB) && dabt.write )
+ if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) && dabt.write )
{
rc = decode_instruction(regs, &info.dabt);
if ( rc )
diff --git a/xen/include/asm-arm/arm32/processor.h b/xen/include/asm-arm/arm32/processor.h
index f41644d..11366bb 100644
--- a/xen/include/asm-arm/arm32/processor.h
+++ b/xen/include/asm-arm/arm32/processor.h
@@ -115,10 +115,6 @@ struct cpu_user_regs
#define READ_SYSREG(R...) READ_SYSREG32(R)
#define WRITE_SYSREG(V, R...) WRITE_SYSREG32(V, R)
-/* Erratum 766422: only Cortex A15 r0p4 is affected */
-#define cpu_has_erratum_766422() \
- (unlikely(current_cpu_data.midr.bits == 0x410fc0f4))
-
#endif /* __ASSEMBLY__ */
#endif /* __ASM_ARM_ARM32_PROCESSOR_H */
diff --git a/xen/include/asm-arm/arm64/processor.h b/xen/include/asm-arm/arm64/processor.h
index fef35a5..b0726ff 100644
--- a/xen/include/asm-arm/arm64/processor.h
+++ b/xen/include/asm-arm/arm64/processor.h
@@ -111,8 +111,6 @@ struct cpu_user_regs
#define READ_SYSREG(name) READ_SYSREG64(name)
#define WRITE_SYSREG(v, name) WRITE_SYSREG64(v, name)
-#define cpu_has_erratum_766422() 0
-
#endif /* __ASSEMBLY__ */
#endif /* __ASM_ARM_ARM64_PROCESSOR_H */
diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
index 2982a92..5880e77 100644
--- a/xen/include/asm-arm/cpuerrata.h
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -40,6 +40,8 @@ static inline bool_t check_workaround_##erratum(void) \
#endif
+CHECK_WORKAROUND_HELPER(766422, ARM32_WORKAROUND_766422, CONFIG_ARM_32)
+
#undef CHECK_WORKAROUND_HELPER
#endif /* __ARM_CPUERRATA_H__ */
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 78e2263..ac6eaf0 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -37,8 +37,9 @@
#define ARM64_WORKAROUND_CLEAN_CACHE 0
#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE 1
+#define ARM32_WORKAROUND_766422 2
-#define ARM_NCAPS 2
+#define ARM_NCAPS 3
#ifndef __ASSEMBLY__
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 1708253..15bf890 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -46,9 +46,11 @@
#define ARM_CPU_IMP_ARM 0x41
+#define ARM_CPU_PART_CORTEX_A15 0xC0F
#define ARM_CPU_PART_CORTEX_A53 0xD03
#define ARM_CPU_PART_CORTEX_A57 0xD07
+#define MIDR_CORTEX_A15 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A15)
#define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
#define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 3/6] xen/arm: Use check_workaround to handle the erratum 766422
2016-08-04 17:50 ` [PATCH v3 3/6] xen/arm: Use check_workaround to handle the erratum 766422 Julien Grall
@ 2016-08-04 17:53 ` Julien Grall
0 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2016-08-04 17:53 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini
Hi,
On 04/08/16 18:50, Julien Grall wrote:
> Currently, Xen is accessing the stored MIDR everytime it has to check
> whether the processor is affected by the erratum 766422.
>
> This could take advantage of the new capability bitfields to detect
> whether the processor is affected at boot time.
>
> With this patch, the number of instructions to check the erratum is
> going down from ~13 (including 2 loads and a co-processor access) to
> ~6 instructions (include 1 load).
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Likewise, I forgot to add Stefano's reviewed-by:
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>
> ---
> Changes in v2:
> - Update the commit message
> ---
> xen/arch/arm/cpuerrata.c | 6 ++++++
> xen/arch/arm/traps.c | 3 ++-
> xen/include/asm-arm/arm32/processor.h | 4 ----
> xen/include/asm-arm/arm64/processor.h | 2 --
> xen/include/asm-arm/cpuerrata.h | 2 ++
> xen/include/asm-arm/cpufeature.h | 3 ++-
> xen/include/asm-arm/processor.h | 2 ++
> 7 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 3ac97b3..748e02e 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -17,6 +17,12 @@ is_affected_midr_range(const struct arm_cpu_capabilities *entry)
> }
>
> static const struct arm_cpu_capabilities arm_errata[] = {
> + {
> + /* Cortex-A15 r0p4 */
> + .desc = "ARM erratum 766422",
> + .capability = ARM32_WORKAROUND_766422,
> + MIDR_RANGE(MIDR_CORTEX_A15, 0x04, 0x04),
> + },
> #if defined(CONFIG_ARM64_ERRATUM_827319) || \
> defined(CONFIG_ARM64_ERRATUM_824069)
> {
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index b11d2e5..7206a7e 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -46,6 +46,7 @@
> #include "vtimer.h"
> #include <asm/gic.h>
> #include <asm/vgic.h>
> +#include <asm/cpuerrata.h>
>
> /* The base of the stack must always be double-word aligned, which means
> * that both the kernel half of struct cpu_user_regs (which is pushed in
> @@ -2481,7 +2482,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
> * Erratum 766422: Thumb store translation fault to Hypervisor may
> * not have correct HSR Rt value.
> */
> - if ( cpu_has_erratum_766422() && (regs->cpsr & PSR_THUMB) && dabt.write )
> + if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) && dabt.write )
> {
> rc = decode_instruction(regs, &info.dabt);
> if ( rc )
> diff --git a/xen/include/asm-arm/arm32/processor.h b/xen/include/asm-arm/arm32/processor.h
> index f41644d..11366bb 100644
> --- a/xen/include/asm-arm/arm32/processor.h
> +++ b/xen/include/asm-arm/arm32/processor.h
> @@ -115,10 +115,6 @@ struct cpu_user_regs
> #define READ_SYSREG(R...) READ_SYSREG32(R)
> #define WRITE_SYSREG(V, R...) WRITE_SYSREG32(V, R)
>
> -/* Erratum 766422: only Cortex A15 r0p4 is affected */
> -#define cpu_has_erratum_766422() \
> - (unlikely(current_cpu_data.midr.bits == 0x410fc0f4))
> -
> #endif /* __ASSEMBLY__ */
>
> #endif /* __ASM_ARM_ARM32_PROCESSOR_H */
> diff --git a/xen/include/asm-arm/arm64/processor.h b/xen/include/asm-arm/arm64/processor.h
> index fef35a5..b0726ff 100644
> --- a/xen/include/asm-arm/arm64/processor.h
> +++ b/xen/include/asm-arm/arm64/processor.h
> @@ -111,8 +111,6 @@ struct cpu_user_regs
> #define READ_SYSREG(name) READ_SYSREG64(name)
> #define WRITE_SYSREG(v, name) WRITE_SYSREG64(v, name)
>
> -#define cpu_has_erratum_766422() 0
> -
> #endif /* __ASSEMBLY__ */
>
> #endif /* __ASM_ARM_ARM64_PROCESSOR_H */
> diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
> index 2982a92..5880e77 100644
> --- a/xen/include/asm-arm/cpuerrata.h
> +++ b/xen/include/asm-arm/cpuerrata.h
> @@ -40,6 +40,8 @@ static inline bool_t check_workaround_##erratum(void) \
>
> #endif
>
> +CHECK_WORKAROUND_HELPER(766422, ARM32_WORKAROUND_766422, CONFIG_ARM_32)
> +
> #undef CHECK_WORKAROUND_HELPER
>
> #endif /* __ARM_CPUERRATA_H__ */
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index 78e2263..ac6eaf0 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -37,8 +37,9 @@
>
> #define ARM64_WORKAROUND_CLEAN_CACHE 0
> #define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE 1
> +#define ARM32_WORKAROUND_766422 2
>
> -#define ARM_NCAPS 2
> +#define ARM_NCAPS 3
>
> #ifndef __ASSEMBLY__
>
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 1708253..15bf890 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -46,9 +46,11 @@
>
> #define ARM_CPU_IMP_ARM 0x41
>
> +#define ARM_CPU_PART_CORTEX_A15 0xC0F
> #define ARM_CPU_PART_CORTEX_A53 0xD03
> #define ARM_CPU_PART_CORTEX_A57 0xD07
>
> +#define MIDR_CORTEX_A15 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A15)
> #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
> #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
>
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 4/6] xen/arm: traps: MMIO should only be emulated for fault translation
2016-08-04 17:50 [PATCH v3 0/6] xen/arm: Simplify do_trap_*_abort_guest Julien Grall
` (2 preceding siblings ...)
2016-08-04 17:50 ` [PATCH v3 3/6] xen/arm: Use check_workaround to handle the erratum 766422 Julien Grall
@ 2016-08-04 17:50 ` Julien Grall
2016-08-04 17:50 ` [PATCH v3 5/6] xen/arm: traps: Avoid unnecessary VA -> IPA translation in abort handlers Julien Grall
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2016-08-04 17:50 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, sstabellini
The function do_trap_data_abort_guest assumes that a stage-2 data abort
can only be taken for a translation fault or permission fault today.
Whilst this is true today, it might not be in the future. Rather than
emulating the MMIO for any fault other than the permission one, print
a warning message when the fault is not handled by Xen.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
Changes in v2:
- Add Stefano's reviewed-by
---
xen/arch/arm/traps.c | 51 ++++++++++++++++++++++++++++-----------------------
1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 7206a7e..46922eb 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2467,35 +2467,40 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
/* Trap was triggered by mem_access, work here is done */
if ( !rc )
return;
+ break;
}
- break;
- }
-
- if ( dabt.s1ptw )
- goto bad_data_abort;
+ case FSC_FLT_TRANS:
+ if ( dabt.s1ptw )
+ goto bad_data_abort;
- /* XXX: Decode the instruction if ISS is not valid */
- if ( !dabt.valid )
- goto bad_data_abort;
+ /* XXX: Decode the instruction if ISS is not valid */
+ if ( !dabt.valid )
+ goto bad_data_abort;
- /*
- * Erratum 766422: Thumb store translation fault to Hypervisor may
- * not have correct HSR Rt value.
- */
- if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) && dabt.write )
- {
- rc = decode_instruction(regs, &info.dabt);
- if ( rc )
+ /*
+ * Erratum 766422: Thumb store translation fault to Hypervisor may
+ * not have correct HSR Rt value.
+ */
+ if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) &&
+ dabt.write )
{
- gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
- goto bad_data_abort;
+ rc = decode_instruction(regs, &info.dabt);
+ if ( rc )
+ {
+ gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
+ goto bad_data_abort;
+ }
}
- }
- if (handle_mmio(&info))
- {
- advance_pc(regs, hsr);
- return;
+ if ( handle_mmio(&info) )
+ {
+ advance_pc(regs, hsr);
+ return;
+ }
+ break;
+ default:
+ gprintk(XENLOG_WARNING, "Unsupported DFSC: HSR=%#x DFSC=%#x\n",
+ hsr.bits, dabt.dfsc);
}
bad_data_abort:
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 5/6] xen/arm: traps: Avoid unnecessary VA -> IPA translation in abort handlers
2016-08-04 17:50 [PATCH v3 0/6] xen/arm: Simplify do_trap_*_abort_guest Julien Grall
` (3 preceding siblings ...)
2016-08-04 17:50 ` [PATCH v3 4/6] xen/arm: traps: MMIO should only be emulated for fault translation Julien Grall
@ 2016-08-04 17:50 ` Julien Grall
2016-08-04 17:50 ` [PATCH v3 6/6] xen/arm: arm64: Add Cortex-A57 erratum 834220 workaround Julien Grall
2016-08-04 17:59 ` [PATCH v3 0/6] xen/arm: Simplify do_trap_*_abort_guest Stefano Stabellini
6 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2016-08-04 17:50 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, sstabellini
Translating a VA to a IPA is expensive. Currently, Xen is assuming that
HPFAR_EL2 is only valid when the stage-2 data/instruction abort happened
during a translation table walk of a first stage translation (i.e S1PTW
is set).
However, based on the ARM ARM (D7.2.34 in DDI 0487A.j), the register is
also valid when the data/instruction abort occured for a translation
fault.
With this change, the VA -> IPA translation will only happen for
permission faults that are not related to a translation table of a
first stage translation.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
Changes in v3:
- Use the correct union to get the s1ptw bit.
Changes in v2:
- Use fsc in the switch in do_trap_data_abort_guest
---
xen/arch/arm/traps.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 46922eb..0ec0361 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2382,13 +2382,28 @@ static inline paddr_t get_faulting_ipa(vaddr_t gva)
return ipa;
}
+static inline bool hpfar_is_valid(bool s1ptw, uint8_t fsc)
+{
+ /*
+ * HPFAR is valid if one of the following cases are true:
+ * 1. the stage 2 fault happen during a stage 1 page table walk
+ * (the bit ESR_EL2.S1PTW is set)
+ * 2. the fault was due to a translation fault
+ *
+ * Note that technically HPFAR is valid for other cases, but they
+ * are currently not supported by Xen.
+ */
+ return s1ptw || (fsc == FSC_FLT_TRANS);
+}
+
static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
const union hsr hsr)
{
int rc;
register_t gva = READ_SYSREG(FAR_EL2);
+ uint8_t fsc = hsr.iabt.ifsc & ~FSC_LL_MASK;
- switch ( hsr.iabt.ifsc & ~FSC_LL_MASK )
+ switch ( fsc )
{
case FSC_FLT_PERM:
{
@@ -2399,7 +2414,7 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
.kind = hsr.iabt.s1ptw ? npfec_kind_in_gpt : npfec_kind_with_gla
};
- if ( hsr.iabt.s1ptw )
+ if ( hpfar_is_valid(hsr.iabt.s1ptw, fsc) )
gpa = get_faulting_ipa(gva);
else
{
@@ -2434,6 +2449,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
const struct hsr_dabt dabt = hsr.dabt;
int rc;
mmio_info_t info;
+ uint8_t fsc = hsr.dabt.dfsc & ~FSC_LL_MASK;
info.dabt = dabt;
#ifdef CONFIG_ARM_32
@@ -2442,7 +2458,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
info.gva = READ_SYSREG64(FAR_EL2);
#endif
- if ( dabt.s1ptw )
+ if ( hpfar_is_valid(dabt.s1ptw, fsc) )
info.gpa = get_faulting_ipa(info.gva);
else
{
@@ -2451,7 +2467,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
return; /* Try again */
}
- switch ( dabt.dfsc & ~FSC_LL_MASK )
+ switch ( fsc )
{
case FSC_FLT_PERM:
{
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 6/6] xen/arm: arm64: Add Cortex-A57 erratum 834220 workaround
2016-08-04 17:50 [PATCH v3 0/6] xen/arm: Simplify do_trap_*_abort_guest Julien Grall
` (4 preceding siblings ...)
2016-08-04 17:50 ` [PATCH v3 5/6] xen/arm: traps: Avoid unnecessary VA -> IPA translation in abort handlers Julien Grall
@ 2016-08-04 17:50 ` Julien Grall
2016-08-04 17:59 ` [PATCH v3 0/6] xen/arm: Simplify do_trap_*_abort_guest Stefano Stabellini
6 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2016-08-04 17:50 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, sstabellini
The ARM erratum applies to certain revisions of Cortex-A57. The
processor may report a Stage 2 translation fault as the result of
Stage 1 fault for load crossing a page boundary when there is a
permission fault or device memory fault at stage 1 and a translation
fault at Stage 2.
So Xen needs to check that Stage 1 translation does not generate a fault
before handling the Stage 2 fault. If it is a Stage 1 translation fault,
return to the guest to let the processor injecting the correct fault.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
Changes in v2:
- Add Stefano's reviewed-by
---
docs/misc/arm/silicon-errata.txt | 1 +
xen/arch/arm/Kconfig | 21 +++++++++++++++++++++
xen/arch/arm/cpuerrata.c | 9 +++++++++
xen/arch/arm/traps.c | 5 +++--
xen/include/asm-arm/cpuerrata.h | 1 +
xen/include/asm-arm/cpufeature.h | 3 ++-
6 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
index 220f724..c9854c3 100644
--- a/docs/misc/arm/silicon-errata.txt
+++ b/docs/misc/arm/silicon-errata.txt
@@ -47,3 +47,4 @@ stable hypervisors.
| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 |
| ARM | Cortex-A57 | #852523 | N/A |
| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 |
+| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 |
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index e26c4c8..871c243 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -142,6 +142,27 @@ config ARM64_ERRATUM_832075
If unsure, say Y.
+config ARM64_ERRATUM_834220
+ bool "Cortex-A57: 834220: Stage 2 translation fault might be incorrectly reported in presence of a Stage 1 fault"
+ default y
+ depends on ARM_64
+ help
+ This option adds an alternative code sequence to work around ARM
+ erratum 834220 on Cortex-A57 parts up to r1p2.
+
+ Affected Cortex-A57 parts might report a Stage 2 translation
+ fault as the result of a Stage 1 fault for load crossing a
+ page boundary when there is a permission or device memory
+ alignment fault at Stage 1 and a translation fault at Stage 2.
+
+ The workaround is to verify that the Stage 1 translation
+ doesn't generate a fault before handling the Stage 2 fault.
+ 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"
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 748e02e..a3e8dda 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -49,6 +49,15 @@ static const struct arm_cpu_capabilities arm_errata[] = {
(1 << MIDR_VARIANT_SHIFT) | 2),
},
#endif
+#ifdef CONFIG_ARM64_ERRATUM_834220
+ {
+ /* Cortex-A57 r0p0 - r1p2 */
+ .desc = "ARM erratum 834220",
+ .capability = ARM64_WORKAROUND_834220,
+ MIDR_RANGE(MIDR_CORTEX_A57, 0x00,
+ (1 << MIDR_VARIANT_SHIFT) | 2),
+ },
+#endif
{},
};
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 0ec0361..683bcb2 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2388,12 +2388,13 @@ static inline bool hpfar_is_valid(bool s1ptw, uint8_t fsc)
* HPFAR is valid if one of the following cases are true:
* 1. the stage 2 fault happen during a stage 1 page table walk
* (the bit ESR_EL2.S1PTW is set)
- * 2. the fault was due to a translation fault
+ * 2. the fault was due to a translation fault and the processor
+ * does not carry erratum #8342220
*
* Note that technically HPFAR is valid for other cases, but they
* are currently not supported by Xen.
*/
- return s1ptw || (fsc == FSC_FLT_TRANS);
+ return s1ptw || (fsc == FSC_FLT_TRANS && !check_workaround_834220());
}
static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
index 5880e77..5e35b4f 100644
--- a/xen/include/asm-arm/cpuerrata.h
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -41,6 +41,7 @@ static inline bool_t check_workaround_##erratum(void) \
#endif
CHECK_WORKAROUND_HELPER(766422, ARM32_WORKAROUND_766422, CONFIG_ARM_32)
+CHECK_WORKAROUND_HELPER(834220, ARM64_WORKAROUND_834220, CONFIG_ARM_64)
#undef CHECK_WORKAROUND_HELPER
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index ac6eaf0..66e930f 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -38,8 +38,9 @@
#define ARM64_WORKAROUND_CLEAN_CACHE 0
#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE 1
#define ARM32_WORKAROUND_766422 2
+#define ARM64_WORKAROUND_834220 3
-#define ARM_NCAPS 3
+#define ARM_NCAPS 4
#ifndef __ASSEMBLY__
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 0/6] xen/arm: Simplify do_trap_*_abort_guest
2016-08-04 17:50 [PATCH v3 0/6] xen/arm: Simplify do_trap_*_abort_guest Julien Grall
` (5 preceding siblings ...)
2016-08-04 17:50 ` [PATCH v3 6/6] xen/arm: arm64: Add Cortex-A57 erratum 834220 workaround Julien Grall
@ 2016-08-04 17:59 ` Stefano Stabellini
6 siblings, 0 replies; 11+ messages in thread
From: Stefano Stabellini @ 2016-08-04 17:59 UTC (permalink / raw)
To: Julien Grall; +Cc: sstabellini, xen-devel
On Thu, 4 Aug 2016, Julien Grall wrote:
> The current data/instruction abort paths contain unnecessary code and
> translate too often a VA to a IPA. This series aim to simplify this path.
>
> Now that the register HPFAR_EL2 is read in some case that can be affected
> by the erratum 834220 on Cortex-A57, we need to implement a workaround
> for it (see patch #6).
>
> A branch with this series applied can be found on xenbits:
> git://xenbits.xen.org/people/julieng/xen-unstable.git branch abort-handlers-v3
committed, thank you
> Yours sincerely,
>
> Julien Grall (6):
> xen/arm: traps: Simplify the switch in do_trap_*_abort_guest
> xen/arm: Provide macros to help creating workaround helpers
> xen/arm: Use check_workaround to handle the erratum 766422
> xen/arm: traps: MMIO should only be emulated for fault translation
> xen/arm: traps: Avoid unnecessary VA -> IPA translation in abort
> handlers
> xen/arm: arm64: Add Cortex-A57 erratum 834220 workaround
>
> docs/misc/arm/silicon-errata.txt | 1 +
> xen/arch/arm/Kconfig | 21 +++++++++
> xen/arch/arm/cpuerrata.c | 15 +++++++
> xen/arch/arm/traps.c | 81 ++++++++++++++++++++++-------------
> xen/include/asm-arm/arm32/processor.h | 4 --
> xen/include/asm-arm/arm64/processor.h | 2 -
> xen/include/asm-arm/cpuerrata.h | 42 ++++++++++++++++++
> xen/include/asm-arm/cpufeature.h | 4 +-
> xen/include/asm-arm/processor.h | 2 +
> 9 files changed, 136 insertions(+), 36 deletions(-)
>
> --
> 1.9.1
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread