* Re: [PATCH v1 0/3] manually add Error Synchronization Barrier at exception handler entry and exit
2017-11-01 19:14 [PATCH v1 0/3] manually add Error Synchronization Barrier at exception handler entry and exit Dongjiu Geng
@ 2017-11-01 11:21 ` Robin Murphy
2017-11-01 11:32 ` James Morse
` (3 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Robin Murphy @ 2017-11-01 11:21 UTC (permalink / raw)
To: Dongjiu Geng, catalin.marinas, will.deacon, marc.zyngier,
christoffer.dall, james.morse, mark.rutland, ard.biesheuvel, cov,
Dave.Martin, suzuki.poulose, linux-arm-kernel, linux-kernel,
kvmarm
On 01/11/17 19:14, Dongjiu Geng wrote:
> Some hardware platform can support RAS Extension, but not support IESB,
> such as Huawei's platform, so software need to insert Synchronization Barrier
> operations at exception handler entry.
>
> This series patches are based on James's series patches "SError rework +
> RAS&IESB for firmware first support". In Huawei's platform, we do not
> support IESB, so software needs to insert that.
>
>
> Dongjiu Geng (3):
> arm64: add a macro for SError synchronization
> arm64: add error synchronization barrier in kernel_entry/kernel_exit
> KVM: arm64: add ESB in exception handler entry and exit.
>
> James Morse (18):
> arm64: explicitly mask all exceptions
> arm64: introduce an order for exceptions
> arm64: Move the async/fiq helpers to explicitly set process context
> flags
> arm64: Mask all exceptions during kernel_exit
> arm64: entry.S: Remove disable_dbg
> arm64: entry.S: convert el1_sync
> arm64: entry.S convert el0_sync
> arm64: entry.S: convert elX_irq
> KVM: arm/arm64: mask/unmask daif around VHE guests
> arm64: kernel: Survive corrected RAS errors notified by SError
> arm64: cpufeature: Enable IESB on exception entry/return for
> firmware-first
> arm64: kernel: Prepare for a DISR user
> KVM: arm64: Set an impdef ESR for Virtual-SError using VSESR_EL2.
> KVM: arm64: Save/Restore guest DISR_EL1
> KVM: arm64: Save ESR_EL2 on guest SError
> KVM: arm64: Handle RAS SErrors from EL1 on guest exit
> KVM: arm64: Handle RAS SErrors from EL2 on guest exit
> KVM: arm64: Take any host SError before entering the guest
>
> Xie XiuQi (2):
> arm64: entry.S: move SError handling into a C function for future
> expansion
> arm64: cpufeature: Detect CPU RAS Extentions
>
> arch/arm64/Kconfig | 33 +++++++++++++-
> arch/arm64/include/asm/assembler.h | 59 +++++++++++++++++-------
> arch/arm64/include/asm/barrier.h | 1 +
> arch/arm64/include/asm/cpucaps.h | 4 +-
> arch/arm64/include/asm/daifflags.h | 61 +++++++++++++++++++++++++
> arch/arm64/include/asm/esr.h | 17 +++++++
> arch/arm64/include/asm/exception.h | 14 ++++++
> arch/arm64/include/asm/irqflags.h | 40 ++++++----------
> arch/arm64/include/asm/kvm_emulate.h | 10 ++++
> arch/arm64/include/asm/kvm_host.h | 16 +++++++
> arch/arm64/include/asm/processor.h | 2 +
> arch/arm64/include/asm/sysreg.h | 6 +++
> arch/arm64/include/asm/traps.h | 36 +++++++++++++++
> arch/arm64/kernel/asm-offsets.c | 1 +
> arch/arm64/kernel/cpufeature.c | 43 ++++++++++++++++++
> arch/arm64/kernel/debug-monitors.c | 5 +-
> arch/arm64/kernel/entry.S | 88 +++++++++++++++++++++---------------
> arch/arm64/kernel/hibernate.c | 5 +-
> arch/arm64/kernel/machine_kexec.c | 4 +-
> arch/arm64/kernel/process.c | 3 ++
> arch/arm64/kernel/setup.c | 8 ++--
> arch/arm64/kernel/signal.c | 8 +++-
> arch/arm64/kernel/smp.c | 12 ++---
> arch/arm64/kernel/suspend.c | 7 +--
> arch/arm64/kernel/traps.c | 64 +++++++++++++++++++++++++-
> arch/arm64/kvm/handle_exit.c | 19 +++++++-
> arch/arm64/kvm/hyp-init.S | 3 ++
> arch/arm64/kvm/hyp/entry.S | 15 ++++++
> arch/arm64/kvm/hyp/hyp-entry.S | 1 +
> arch/arm64/kvm/hyp/switch.c | 19 ++++++--
> arch/arm64/kvm/hyp/sysreg-sr.c | 6 +++
> arch/arm64/kvm/inject_fault.c | 13 +++++-
> arch/arm64/kvm/sys_regs.c | 1 +
> arch/arm64/mm/proc.S | 14 ++++--
> virt/kvm/arm/arm.c | 4 ++
> 35 files changed, 527 insertions(+), 115 deletions(-)
> create mode 100644 arch/arm64/include/asm/daifflags.h
If you're sending a patch series, please have the cover letter describe
*those patches*, rather than dozens of other patches that aren't part of
it. This diffstat and summary is very confusing.
Robin.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/3] arm64: add a macro for SError synchronization
2017-11-01 19:14 ` [PATCH v1 1/3] arm64: add a macro for SError synchronization Dongjiu Geng
@ 2017-11-01 11:24 ` Robin Murphy
2017-11-01 12:54 ` gengdongjiu
0 siblings, 1 reply; 15+ messages in thread
From: Robin Murphy @ 2017-11-01 11:24 UTC (permalink / raw)
To: Dongjiu Geng, catalin.marinas, will.deacon, marc.zyngier,
christoffer.dall, james.morse, mark.rutland, ard.biesheuvel, cov,
Dave.Martin, suzuki.poulose, linux-arm-kernel, linux-kernel,
kvmarm
On 01/11/17 19:14, Dongjiu Geng wrote:
> ARMv8.2 adds a control bit to each SCTLR_ELx to insert implicit
> Error Synchronization Barrier(IESB) operations at exception handler entry
> and exit. But not all hardware platform which support RAS Extension
> can support IESB. So for this case, software needs to manually insert
> Error Synchronization Barrier(ESB) operations.
>
> In this macros, if system supports RAS Extensdddon instead of IESB,
> it will insert an ESB instruction.
>
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> ---
> arch/arm64/include/asm/assembler.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index d4c0adf..e6c79c4 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -517,4 +517,13 @@
> #endif
> .endm
>
> + .macro error_synchronize
> +alternative_if ARM64_HAS_IESB
> + b 1f
> +alternative_else_nop_endif
> +alternative_if ARM64_HAS_RAS_EXTN
> + esb
> +alternative_else_nop_endif
> +1:
> + .endm
Having a branch in here is pretty horrible, and furthermore using label
number 1 has a pretty high chance of subtly breaking code where this
macro is inserted.
Can we not somehow nest or combine the alternative conditions here?
Robin.
> #endif /* __ASM_ASSEMBLER_H */
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 0/3] manually add Error Synchronization Barrier at exception handler entry and exit
2017-11-01 19:14 [PATCH v1 0/3] manually add Error Synchronization Barrier at exception handler entry and exit Dongjiu Geng
2017-11-01 11:21 ` Robin Murphy
@ 2017-11-01 11:32 ` James Morse
2017-11-01 12:44 ` gengdongjiu
2017-11-01 19:14 ` [PATCH v1 1/3] arm64: add a macro for SError synchronization Dongjiu Geng
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: James Morse @ 2017-11-01 11:32 UTC (permalink / raw)
To: Dongjiu Geng
Cc: catalin.marinas, will.deacon, marc.zyngier, christoffer.dall,
mark.rutland, ard.biesheuvel, robin.murphy, cov, Dave.Martin,
suzuki.poulose, linux-arm-kernel, linux-kernel, kvmarm
Hi Dongjiu Geng,
On 01/11/17 19:14, Dongjiu Geng wrote:
> Some hardware platform can support RAS Extension, but not support IESB,
> such as Huawei's platform, so software need to insert Synchronization Barrier
> operations at exception handler entry.
>
> This series patches are based on James's series patches "SError rework +
> RAS&IESB for firmware first support". In Huawei's platform, we do not
> support IESB, so software needs to insert that.
Surely you don't implement it because your CPU doesn't need it. Can
unrecoverable errors really cross an exception without becoming an SError?
The ESB instruction does the barrier, but it also consumes any pending SError.
As it is this series will silently consume-and-discard uncontainable errors.
Thanks,
James
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 0/3] manually add Error Synchronization Barrier at exception handler entry and exit
2017-11-01 11:32 ` James Morse
@ 2017-11-01 12:44 ` gengdongjiu
0 siblings, 0 replies; 15+ messages in thread
From: gengdongjiu @ 2017-11-01 12:44 UTC (permalink / raw)
To: James Morse
Cc: catalin.marinas, will.deacon, marc.zyngier, christoffer.dall,
mark.rutland, ard.biesheuvel, robin.murphy, cov, Dave.Martin,
suzuki.poulose, linux-arm-kernel, linux-kernel, kvmarm, lishuo1,
liujun88
On 2017/11/1 19:32, James Morse wrote:
>> RAS&IESB for firmware first support". In Huawei's platform, we do not
>> support IESB, so software needs to insert that.
> Surely you don't implement it because your CPU doesn't need it. Can
> unrecoverable errors really cross an exception without becoming an SError?
CC lishuo and liujun.
Hi James,
we will needs that if we can ensure that the Error is not propagated.
The unrecoverable errors can cross an different exception, so hardware use IESB to isolate it.
Afterwards we may also not support IESB(about the definition as shown in [3] ), the reason that
we are not support IESB is that hardware colleague think software insert it in exception entry/exit can be flexibly.
And ARM spec allow that:
ARM spec allow software inset ESB instead of hardware insert as shown in [1].
About how to use the ESB in the software, the SPEC even give an example[2]:
[1]
AArch_v8A_v8.2_Extension_ARM-ECM-0326763-20-0:
"ARMv8.2 adds an extension to RAS to allow system software to insert implicit Error Synchronization Barrier operations at exception handler entry and exit".
[2]
RAS_Extension_PRD03-PRDC-010953-39-0:
Example exception entry sequence
Example 1 shows a typical sequence for exception entry.
Vector: ESB ;; Error Synchronization Barrier
SUB SP,SP,#34*8
STP X0,X1,[SP] ;; Save all general purpose registers
STP X2,X3,[SP,#2*8]
...
STP X28,X29,[SP,#28*8]
MRS X21,SP_EL0
MRS X22,ELR_EL1 ;; Save exception syndrome registers
MRS X23,SPSR_EL1
STP X30,X21,[SP,#30*8]
STP X22,X23,[SP,#32*8]
...
MRS X21,DISR_EL1 ;; Check for deferred error
TBNZ X21,#DISR_A_bit,Error_Handler
MSR DAIFClr,#0xF ;; Clear flags
Example 1: Exception entry sequence
[3]
Below is the SCTLR_ELx.IESB definition.
SCTLR_ELx[21]: IESB – Implicit Error Synchronization Barrier enable.
0: Disabled.
1: An implicit ErrorSynchronizationBarrier() call is added:
After each exception taken to ELx.
Before the operational pseudocode of each ERET instruction executed at ELx.
Hi lishuo/liujun,
could you explain more to James about the reason that why we need software inserts ESB instead of hardware support IESB?
>
> The ESB instruction does the barrier, but it also consumes any pending SError.
> As it is this series will silently consume-and-discard uncontainable errors.
In the firmware-first solution, if ESB consumed a SError, it will trap to firmware.
firmware will handle it and record the Error. the DISR_EL1 will not record, so no need to Check for deferred error
But if it is non-firmware solution, it will not trap, it needs to check the defered error through DISR_EL1 and
call the Error_Handler, as shown in [2].
In the kernel patchset, I do not are check the defered error. If you think it should be added, I can add it.
welcome comments.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/3] arm64: add a macro for SError synchronization
2017-11-01 11:24 ` Robin Murphy
@ 2017-11-01 12:54 ` gengdongjiu
2017-11-01 13:31 ` Robin Murphy
2017-11-01 14:16 ` Mark Rutland
0 siblings, 2 replies; 15+ messages in thread
From: gengdongjiu @ 2017-11-01 12:54 UTC (permalink / raw)
To: Robin Murphy, catalin.marinas, will.deacon, marc.zyngier,
christoffer.dall, james.morse, mark.rutland, ard.biesheuvel, cov,
Dave.Martin, suzuki.poulose, linux-arm-kernel, linux-kernel,
kvmarm
Hi Robin,
On 2017/11/1 19:24, Robin Murphy wrote:
>> + esb
>> +alternative_else_nop_endif
>> +1:
>> + .endm
> Having a branch in here is pretty horrible, and furthermore using label
> number 1 has a pretty high chance of subtly breaking code where this
> macro is inserted.
>
> Can we not somehow nest or combine the alternative conditions here?
I found it will report error if combine the alternative conditions here.
For example:
+ .macro error_synchronize
+alternative_if ARM64_HAS_IESB
+alternative_if ARM64_HAS_RAS_EXTN
+ esb
+alternative_else_nop_endif
+alternative_else_nop_endif
+ .endm
And even using b.eq/cbz instruction in the alternative instruction in arch/arm64/kernel/entry.S,
it will report Error.
For example below
alternative_if ARM64_HAS_PAN
xxxxxxxxxxxxxxxxxxxx
b.eq xxxxx
alternative_else_nop_endif
I do not dig it deeply, do you know the reason about it or good suggestion about that?
Thanks a lot in advance.
>
> Robin.
>
>> #endif /* __ASM_ASSEMBLER_H */
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/3] arm64: add a macro for SError synchronization
2017-11-01 12:54 ` gengdongjiu
@ 2017-11-01 13:31 ` Robin Murphy
2017-11-01 14:16 ` Mark Rutland
1 sibling, 0 replies; 15+ messages in thread
From: Robin Murphy @ 2017-11-01 13:31 UTC (permalink / raw)
To: gengdongjiu, catalin.marinas, will.deacon, marc.zyngier,
christoffer.dall, james.morse, mark.rutland, ard.biesheuvel, cov,
Dave.Martin, suzuki.poulose, linux-arm-kernel, linux-kernel,
kvmarm
On 01/11/17 12:54, gengdongjiu wrote:
> Hi Robin,
>
> On 2017/11/1 19:24, Robin Murphy wrote:
>>> + esb
>>> +alternative_else_nop_endif
>>> +1:
>>> + .endm
>> Having a branch in here is pretty horrible, and furthermore using label
>> number 1 has a pretty high chance of subtly breaking code where this
>> macro is inserted.
>>
>> Can we not somehow nest or combine the alternative conditions here?
>
> I found it will report error if combine the alternative conditions here.
>
> For example:
>
> + .macro error_synchronize
> +alternative_if ARM64_HAS_IESB
> +alternative_if ARM64_HAS_RAS_EXTN
> + esb
> +alternative_else_nop_endif
> +alternative_else_nop_endif
> + .endm
>
> And even using b.eq/cbz instruction in the alternative instruction in arch/arm64/kernel/entry.S,
> it will report Error.
>
> For example below
>
> alternative_if ARM64_HAS_PAN
> xxxxxxxxxxxxxxxxxxxx
> b.eq xxxxx
> alternative_else_nop_endif
>
> I do not dig it deeply, do you know the reason about it or good suggestion about that?
> Thanks a lot in advance.
Actually, on second look ARM64_HAS_RAS_EXTN doesn't even matter - ESB is
a hint, so if the CPU doesn't have RAS it should behave as a NOP anyway.
On which note, since I don't see one here - are any of those other
patches defining an "esb" assembly macro similar to the inline asm case?
If not then this isn't going to build with older toolchains - perhaps we
should just use the raw hint syntax directly.
Robin.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/3] arm64: add a macro for SError synchronization
2017-11-01 12:54 ` gengdongjiu
2017-11-01 13:31 ` Robin Murphy
@ 2017-11-01 14:16 ` Mark Rutland
2017-11-02 8:52 ` gengdongjiu
1 sibling, 1 reply; 15+ messages in thread
From: Mark Rutland @ 2017-11-01 14:16 UTC (permalink / raw)
To: gengdongjiu
Cc: Robin Murphy, catalin.marinas, will.deacon, marc.zyngier,
christoffer.dall, james.morse, ard.biesheuvel, cov, Dave.Martin,
suzuki.poulose, linux-arm-kernel, linux-kernel, kvmarm
On Wed, Nov 01, 2017 at 08:54:44PM +0800, gengdongjiu wrote:
> On 2017/11/1 19:24, Robin Murphy wrote:
> >> + esb
> >> +alternative_else_nop_endif
> >> +1:
> >> + .endm
> > Having a branch in here is pretty horrible, and furthermore using label
> > number 1 has a pretty high chance of subtly breaking code where this
> > macro is inserted.
> >
> > Can we not somehow nest or combine the alternative conditions here?
>
> I found it will report error if combine the alternative conditions here.
>
> For example:
>
> + .macro error_synchronize
> +alternative_if ARM64_HAS_IESB
> +alternative_if ARM64_HAS_RAS_EXTN
> + esb
> +alternative_else_nop_endif
> +alternative_else_nop_endif
> + .endm
>
> And even using b.eq/cbz instruction in the alternative instruction in arch/arm64/kernel/entry.S,
> it will report Error.
Alternatives cannot be nested. You need to define a cap like:
ARM64_HAS_RAS_NOT_IESB
... which is set when ARM64_HAS_RAS_EXTN && !ARM64_HAS_IESB.
Then you can do:
alternative_if ARM64_HAS_RAS_NOT_IESB
esb
alternative_else_nop_endif
See ARM64_ALT_PAN_NOT_UAO for an example.
That said, as Robin points out we may not even need the alternative.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v1 0/3] manually add Error Synchronization Barrier at exception handler entry and exit
@ 2017-11-01 19:14 Dongjiu Geng
2017-11-01 11:21 ` Robin Murphy
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Dongjiu Geng @ 2017-11-01 19:14 UTC (permalink / raw)
To: catalin.marinas, will.deacon, marc.zyngier, christoffer.dall,
james.morse, mark.rutland, ard.biesheuvel, robin.murphy, cov,
Dave.Martin, gengdongjiu, suzuki.poulose, linux-arm-kernel,
linux-kernel, kvmarm
Some hardware platform can support RAS Extension, but not support IESB,
such as Huawei's platform, so software need to insert Synchronization Barrier
operations at exception handler entry.
This series patches are based on James's series patches "SError rework +
RAS&IESB for firmware first support". In Huawei's platform, we do not
support IESB, so software needs to insert that.
Dongjiu Geng (3):
arm64: add a macro for SError synchronization
arm64: add error synchronization barrier in kernel_entry/kernel_exit
KVM: arm64: add ESB in exception handler entry and exit.
James Morse (18):
arm64: explicitly mask all exceptions
arm64: introduce an order for exceptions
arm64: Move the async/fiq helpers to explicitly set process context
flags
arm64: Mask all exceptions during kernel_exit
arm64: entry.S: Remove disable_dbg
arm64: entry.S: convert el1_sync
arm64: entry.S convert el0_sync
arm64: entry.S: convert elX_irq
KVM: arm/arm64: mask/unmask daif around VHE guests
arm64: kernel: Survive corrected RAS errors notified by SError
arm64: cpufeature: Enable IESB on exception entry/return for
firmware-first
arm64: kernel: Prepare for a DISR user
KVM: arm64: Set an impdef ESR for Virtual-SError using VSESR_EL2.
KVM: arm64: Save/Restore guest DISR_EL1
KVM: arm64: Save ESR_EL2 on guest SError
KVM: arm64: Handle RAS SErrors from EL1 on guest exit
KVM: arm64: Handle RAS SErrors from EL2 on guest exit
KVM: arm64: Take any host SError before entering the guest
Xie XiuQi (2):
arm64: entry.S: move SError handling into a C function for future
expansion
arm64: cpufeature: Detect CPU RAS Extentions
arch/arm64/Kconfig | 33 +++++++++++++-
arch/arm64/include/asm/assembler.h | 59 +++++++++++++++++-------
arch/arm64/include/asm/barrier.h | 1 +
arch/arm64/include/asm/cpucaps.h | 4 +-
arch/arm64/include/asm/daifflags.h | 61 +++++++++++++++++++++++++
arch/arm64/include/asm/esr.h | 17 +++++++
arch/arm64/include/asm/exception.h | 14 ++++++
arch/arm64/include/asm/irqflags.h | 40 ++++++----------
arch/arm64/include/asm/kvm_emulate.h | 10 ++++
arch/arm64/include/asm/kvm_host.h | 16 +++++++
arch/arm64/include/asm/processor.h | 2 +
arch/arm64/include/asm/sysreg.h | 6 +++
arch/arm64/include/asm/traps.h | 36 +++++++++++++++
arch/arm64/kernel/asm-offsets.c | 1 +
arch/arm64/kernel/cpufeature.c | 43 ++++++++++++++++++
arch/arm64/kernel/debug-monitors.c | 5 +-
arch/arm64/kernel/entry.S | 88 +++++++++++++++++++++---------------
arch/arm64/kernel/hibernate.c | 5 +-
arch/arm64/kernel/machine_kexec.c | 4 +-
arch/arm64/kernel/process.c | 3 ++
arch/arm64/kernel/setup.c | 8 ++--
arch/arm64/kernel/signal.c | 8 +++-
arch/arm64/kernel/smp.c | 12 ++---
arch/arm64/kernel/suspend.c | 7 +--
arch/arm64/kernel/traps.c | 64 +++++++++++++++++++++++++-
arch/arm64/kvm/handle_exit.c | 19 +++++++-
arch/arm64/kvm/hyp-init.S | 3 ++
arch/arm64/kvm/hyp/entry.S | 15 ++++++
arch/arm64/kvm/hyp/hyp-entry.S | 1 +
arch/arm64/kvm/hyp/switch.c | 19 ++++++--
arch/arm64/kvm/hyp/sysreg-sr.c | 6 +++
arch/arm64/kvm/inject_fault.c | 13 +++++-
arch/arm64/kvm/sys_regs.c | 1 +
arch/arm64/mm/proc.S | 14 ++++--
virt/kvm/arm/arm.c | 4 ++
35 files changed, 527 insertions(+), 115 deletions(-)
create mode 100644 arch/arm64/include/asm/daifflags.h
--
1.9.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v1 1/3] arm64: add a macro for SError synchronization
2017-11-01 19:14 [PATCH v1 0/3] manually add Error Synchronization Barrier at exception handler entry and exit Dongjiu Geng
2017-11-01 11:21 ` Robin Murphy
2017-11-01 11:32 ` James Morse
@ 2017-11-01 19:14 ` Dongjiu Geng
2017-11-01 11:24 ` Robin Murphy
2017-11-01 19:14 ` [PATCH v1 2/3] arm64: add error synchronization barrier in kernel_entry/kernel_exit Dongjiu Geng
2017-11-01 19:14 ` [PATCH v1 3/3] KVM: arm64: add ESB in exception handler entry and exit Dongjiu Geng
4 siblings, 1 reply; 15+ messages in thread
From: Dongjiu Geng @ 2017-11-01 19:14 UTC (permalink / raw)
To: catalin.marinas, will.deacon, marc.zyngier, christoffer.dall,
james.morse, mark.rutland, ard.biesheuvel, robin.murphy, cov,
Dave.Martin, gengdongjiu, suzuki.poulose, linux-arm-kernel,
linux-kernel, kvmarm
ARMv8.2 adds a control bit to each SCTLR_ELx to insert implicit
Error Synchronization Barrier(IESB) operations at exception handler entry
and exit. But not all hardware platform which support RAS Extension
can support IESB. So for this case, software needs to manually insert
Error Synchronization Barrier(ESB) operations.
In this macros, if system supports RAS Extensdddon instead of IESB,
it will insert an ESB instruction.
Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
---
arch/arm64/include/asm/assembler.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index d4c0adf..e6c79c4 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -517,4 +517,13 @@
#endif
.endm
+ .macro error_synchronize
+alternative_if ARM64_HAS_IESB
+ b 1f
+alternative_else_nop_endif
+alternative_if ARM64_HAS_RAS_EXTN
+ esb
+alternative_else_nop_endif
+1:
+ .endm
#endif /* __ASM_ASSEMBLER_H */
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 2/3] arm64: add error synchronization barrier in kernel_entry/kernel_exit
2017-11-01 19:14 [PATCH v1 0/3] manually add Error Synchronization Barrier at exception handler entry and exit Dongjiu Geng
` (2 preceding siblings ...)
2017-11-01 19:14 ` [PATCH v1 1/3] arm64: add a macro for SError synchronization Dongjiu Geng
@ 2017-11-01 19:14 ` Dongjiu Geng
2017-11-04 17:34 ` kbuild test robot
2017-11-01 19:14 ` [PATCH v1 3/3] KVM: arm64: add ESB in exception handler entry and exit Dongjiu Geng
4 siblings, 1 reply; 15+ messages in thread
From: Dongjiu Geng @ 2017-11-01 19:14 UTC (permalink / raw)
To: catalin.marinas, will.deacon, marc.zyngier, christoffer.dall,
james.morse, mark.rutland, ard.biesheuvel, robin.murphy, cov,
Dave.Martin, gengdongjiu, suzuki.poulose, linux-arm-kernel,
linux-kernel, kvmarm
If taking an exception from or return to user space, insert
a Error Synchronization Barrier(ESB) to isolate the error.
If a user space process is pending a SError, when enter to
kernel, the SError will be immediately synchronized in the
handler entry. Otherwise if kernel space is pending a SError,
before return to user space, the SError will be synchronized
in the handler exit.
In order to reduce impact on performance, not check the DISR_EL1
to see whether an SError is consumed by an ESB instruction.
This is because DISR_EL1 is RAZ/WI in firmware-first RAS
solution, if happen SError, it will immediately trap to EL3
firmware.
Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
---
arch/arm64/kernel/entry.S | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index e147c1d..6dde644 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -138,6 +138,7 @@
stp x28, x29, [sp, #16 * 14]
.if \el == 0
+ error_synchronize
mrs x21, sp_el0
ldr_this_cpu tsk, __entry_task, x20 // Ensure MDSCR_EL1.SS is clear,
ldr x19, [tsk, #TSK_TI_FLAGS] // since we can unmask debug
@@ -281,6 +282,7 @@ alternative_if ARM64_WORKAROUND_845719
1:
alternative_else_nop_endif
#endif
+ error_synchronize
.endif
msr elr_el1, x21 // set up the return data
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 3/3] KVM: arm64: add ESB in exception handler entry and exit.
2017-11-01 19:14 [PATCH v1 0/3] manually add Error Synchronization Barrier at exception handler entry and exit Dongjiu Geng
` (3 preceding siblings ...)
2017-11-01 19:14 ` [PATCH v1 2/3] arm64: add error synchronization barrier in kernel_entry/kernel_exit Dongjiu Geng
@ 2017-11-01 19:14 ` Dongjiu Geng
2017-11-04 18:49 ` kbuild test robot
4 siblings, 1 reply; 15+ messages in thread
From: Dongjiu Geng @ 2017-11-01 19:14 UTC (permalink / raw)
To: catalin.marinas, will.deacon, marc.zyngier, christoffer.dall,
james.morse, mark.rutland, ard.biesheuvel, robin.murphy, cov,
Dave.Martin, gengdongjiu, suzuki.poulose, linux-arm-kernel,
linux-kernel, kvmarm
Some hardware platform can support RAS Extension instead
of support IESB, so software need to insert Synchronization
Barrier operations at exception handler entry and exit.
In the __guest_exit(), it added a ESB instruction, but can
not cover the path which is not guest exit. For example, if
EL1 host call HVC instruction enter to hypervisor, it will
not call __guest_exit().
In the kvm_arm_vhe_guest_enter(), it synchronised any host
RAS errors for VHE mode, but it can not handle the non-VHE
mode. For example, if EL1 host is pending a SError, the error
can be propagated to guest without error synchronization
operation.
Only add the ESB in the important exception handler path to
reduce the impact on performance.
Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
---
arch/arm64/kvm/hyp/entry.S | 2 ++
arch/arm64/kvm/hyp/hyp-entry.S | 1 +
2 files changed, 3 insertions(+)
diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index 96caa53..fce6806 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -84,6 +84,8 @@ ENTRY(__guest_enter)
// Restore guest reg x18
ldr x18, [x18, #CPU_XREG_OFFSET(18)]
+ // synchronize host pending asynchronous error
+ error_synchronize
// Do not touch any register after this!
eret
ENDPROC(__guest_enter)
diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
index 5170ce1..ac85029 100644
--- a/arch/arm64/kvm/hyp/hyp-entry.S
+++ b/arch/arm64/kvm/hyp/hyp-entry.S
@@ -54,6 +54,7 @@ ENTRY(__vhe_hyp_call)
ENDPROC(__vhe_hyp_call)
el1_sync: // Guest trapped into EL2
+ error_synchronize
stp x0, x1, [sp, #-16]!
alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/3] arm64: add a macro for SError synchronization
2017-11-01 14:16 ` Mark Rutland
@ 2017-11-02 8:52 ` gengdongjiu
0 siblings, 0 replies; 15+ messages in thread
From: gengdongjiu @ 2017-11-02 8:52 UTC (permalink / raw)
To: Mark Rutland
Cc: Robin Murphy, catalin.marinas, will.deacon, marc.zyngier,
christoffer.dall, james.morse, ard.biesheuvel, cov, Dave.Martin,
suzuki.poulose, linux-arm-kernel, linux-kernel, kvmarm
On 2017/11/1 22:16, Mark Rutland wrote:
>> it will report Error.
> Alternatives cannot be nested. You need to define a cap like:
>
> ARM64_HAS_RAS_NOT_IESB
>
> ... which is set when ARM64_HAS_RAS_EXTN && !ARM64_HAS_IESB.
>
> Then you can do:
>
> alternative_if ARM64_HAS_RAS_NOT_IESB
> esb
> alternative_else_nop_endif
>
> See ARM64_ALT_PAN_NOT_UAO for an example.
>
> That said, as Robin points out we may not even need the alternative.
Ok, got it. thank you very much for your point and opinion
>
> Thanks,
> Mark.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] arm64: add error synchronization barrier in kernel_entry/kernel_exit
2017-11-01 19:14 ` [PATCH v1 2/3] arm64: add error synchronization barrier in kernel_entry/kernel_exit Dongjiu Geng
@ 2017-11-04 17:34 ` kbuild test robot
0 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2017-11-04 17:34 UTC (permalink / raw)
To: Dongjiu Geng
Cc: kbuild-all, catalin.marinas, will.deacon, marc.zyngier,
christoffer.dall, james.morse, mark.rutland, ard.biesheuvel,
robin.murphy, cov, Dave.Martin, gengdongjiu, suzuki.poulose,
linux-arm-kernel, linux-kernel, kvmarm
[-- Attachment #1: Type: text/plain, Size: 4016 bytes --]
Hi Dongjiu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.14-rc7 next-20171103]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Dongjiu-Geng/arm64-add-a-macro-for-SError-synchronization/20171104-224216
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
arch/arm64/kernel/entry.S: Assembler messages:
>> arch/arm64/kernel/entry.S:439: Error: selected processor does not support `esb'
arch/arm64/kernel/entry.S:443: Error: selected processor does not support `esb'
arch/arm64/kernel/entry.S:447: Error: selected processor does not support `esb'
arch/arm64/kernel/entry.S:451: Error: selected processor does not support `esb'
arch/arm64/kernel/entry.S:456: Error: selected processor does not support `esb'
arch/arm64/kernel/entry.S:460: Error: selected processor does not support `esb'
arch/arm64/kernel/entry.S:601: Error: selected processor does not support `esb'
arch/arm64/kernel/entry.S:629: Error: selected processor does not support `esb'
arch/arm64/kernel/entry.S:670: Error: selected processor does not support `esb'
arch/arm64/kernel/entry.S:777: Error: selected processor does not support `esb'
arch/arm64/kernel/entry.S:806: Error: selected processor does not support `esb'
arch/arm64/kernel/entry.S:832: Error: selected processor does not support `esb'
vim +439 arch/arm64/kernel/entry.S
872d8327 Mark Rutland 2017-07-14 425
60ffc30d Catalin Marinas 2012-03-05 426 /*
60ffc30d Catalin Marinas 2012-03-05 427 * Invalid mode handlers
60ffc30d Catalin Marinas 2012-03-05 428 */
60ffc30d Catalin Marinas 2012-03-05 429 .macro inv_entry, el, reason, regsize = 64
b660950c Ard Biesheuvel 2016-03-18 430 kernel_entry \el, \regsize
60ffc30d Catalin Marinas 2012-03-05 431 mov x0, sp
60ffc30d Catalin Marinas 2012-03-05 432 mov x1, #\reason
60ffc30d Catalin Marinas 2012-03-05 433 mrs x2, esr_el1
2d0e751a Mark Rutland 2017-07-26 434 bl bad_mode
2d0e751a Mark Rutland 2017-07-26 435 ASM_BUG()
60ffc30d Catalin Marinas 2012-03-05 436 .endm
60ffc30d Catalin Marinas 2012-03-05 437
60ffc30d Catalin Marinas 2012-03-05 438 el0_sync_invalid:
60ffc30d Catalin Marinas 2012-03-05 @439 inv_entry 0, BAD_SYNC
60ffc30d Catalin Marinas 2012-03-05 440 ENDPROC(el0_sync_invalid)
60ffc30d Catalin Marinas 2012-03-05 441
60ffc30d Catalin Marinas 2012-03-05 442 el0_irq_invalid:
60ffc30d Catalin Marinas 2012-03-05 443 inv_entry 0, BAD_IRQ
60ffc30d Catalin Marinas 2012-03-05 444 ENDPROC(el0_irq_invalid)
60ffc30d Catalin Marinas 2012-03-05 445
60ffc30d Catalin Marinas 2012-03-05 446 el0_fiq_invalid:
60ffc30d Catalin Marinas 2012-03-05 447 inv_entry 0, BAD_FIQ
60ffc30d Catalin Marinas 2012-03-05 448 ENDPROC(el0_fiq_invalid)
60ffc30d Catalin Marinas 2012-03-05 449
60ffc30d Catalin Marinas 2012-03-05 450 el0_error_invalid:
60ffc30d Catalin Marinas 2012-03-05 451 inv_entry 0, BAD_ERROR
60ffc30d Catalin Marinas 2012-03-05 452 ENDPROC(el0_error_invalid)
60ffc30d Catalin Marinas 2012-03-05 453
:::::: The code at line 439 was first introduced by commit
:::::: 60ffc30d5652810dd34ea2eec41504222f5d5791 arm64: Exception handling
:::::: TO: Catalin Marinas <catalin.marinas@arm.com>
:::::: CC: Catalin Marinas <catalin.marinas@arm.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36749 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 3/3] KVM: arm64: add ESB in exception handler entry and exit.
2017-11-01 19:14 ` [PATCH v1 3/3] KVM: arm64: add ESB in exception handler entry and exit Dongjiu Geng
@ 2017-11-04 18:49 ` kbuild test robot
0 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2017-11-04 18:49 UTC (permalink / raw)
To: Dongjiu Geng
Cc: kbuild-all, catalin.marinas, will.deacon, marc.zyngier,
christoffer.dall, james.morse, mark.rutland, ard.biesheuvel,
robin.murphy, cov, Dave.Martin, gengdongjiu, suzuki.poulose,
linux-arm-kernel, linux-kernel, kvmarm
[-- Attachment #1: Type: text/plain, Size: 2679 bytes --]
Hi Dongjiu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.14-rc7 next-20171103]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Dongjiu-Geng/arm64-add-a-macro-for-SError-synchronization/20171104-224216
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
arch/arm64/kvm/hyp/entry.S: Assembler messages:
>> arch/arm64/kvm/hyp/entry.S:88: Error: selected processor does not support `esb'
--
arch/arm64/kvm/hyp/hyp-entry.S: Assembler messages:
>> arch/arm64/kvm/hyp/hyp-entry.S:57: Error: selected processor does not support `esb'
vim +88 arch/arm64/kvm/hyp/entry.S
51
52 /*
53 * u64 __guest_enter(struct kvm_vcpu *vcpu,
54 * struct kvm_cpu_context *host_ctxt);
55 */
56 ENTRY(__guest_enter)
57 // x0: vcpu
58 // x1: host context
59 // x2-x17: clobbered by macros
60 // x18: guest context
61
62 // Store the host regs
63 save_callee_saved_regs x1
64
65 // Store the host_ctxt for use at exit time
66 str x1, [sp, #-16]!
67
68 add x18, x0, #VCPU_CONTEXT
69
70 // Restore guest regs x0-x17
71 ldp x0, x1, [x18, #CPU_XREG_OFFSET(0)]
72 ldp x2, x3, [x18, #CPU_XREG_OFFSET(2)]
73 ldp x4, x5, [x18, #CPU_XREG_OFFSET(4)]
74 ldp x6, x7, [x18, #CPU_XREG_OFFSET(6)]
75 ldp x8, x9, [x18, #CPU_XREG_OFFSET(8)]
76 ldp x10, x11, [x18, #CPU_XREG_OFFSET(10)]
77 ldp x12, x13, [x18, #CPU_XREG_OFFSET(12)]
78 ldp x14, x15, [x18, #CPU_XREG_OFFSET(14)]
79 ldp x16, x17, [x18, #CPU_XREG_OFFSET(16)]
80
81 // Restore guest regs x19-x29, lr
82 restore_callee_saved_regs x18
83
84 // Restore guest reg x18
85 ldr x18, [x18, #CPU_XREG_OFFSET(18)]
86
87 // synchronize host pending asynchronous error
> 88 error_synchronize
89 // Do not touch any register after this!
90 eret
91 ENDPROC(__guest_enter)
92
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 57567 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/3] arm64: add a macro for SError synchronization
@ 2017-11-01 14:13 gengdongjiu
0 siblings, 0 replies; 15+ messages in thread
From: gengdongjiu @ 2017-11-01 14:13 UTC (permalink / raw)
To: Robin Murphy, catalin.marinas, will.deacon, marc.zyngier,
christoffer.dall, james.morse, mark.rutland, ard.biesheuvel, cov,
Dave.Martin, suzuki.poulose, linux-arm-kernel, linux-kernel,
kvmarm
>
> On 01/11/17 12:54, gengdongjiu wrote:
> > Hi Robin,
> >
> > On 2017/11/1 19:24, Robin Murphy wrote:
> >>> + esb
> >>> +alternative_else_nop_endif
> >>> +1:
> >>> + .endm
> >> Having a branch in here is pretty horrible, and furthermore using
> >> label number 1 has a pretty high chance of subtly breaking code where
> >> this macro is inserted.
> >>
> >> Can we not somehow nest or combine the alternative conditions here?
> >
> > I found it will report error if combine the alternative conditions here.
> >
> > For example:
> >
> > + .macro error_synchronize
> > +alternative_if ARM64_HAS_IESB
> > +alternative_if ARM64_HAS_RAS_EXTN
> > + esb
> > +alternative_else_nop_endif
> > +alternative_else_nop_endif
> > + .endm
> >
> > And even using b.eq/cbz instruction in the alternative instruction in
> > arch/arm64/kernel/entry.S, it will report Error.
> >
> > For example below
> >
> > alternative_if ARM64_HAS_PAN
> > xxxxxxxxxxxxxxxxxxxx
> > b.eq xxxxx
> > alternative_else_nop_endif
> >
> > I do not dig it deeply, do you know the reason about it or good suggestion about that?
> > Thanks a lot in advance.
>
> Actually, on second look ARM64_HAS_RAS_EXTN doesn't even matter - ESB is a hint, so if the CPU doesn't have RAS it should behave as a
> NOP anyway.
Yes, you are right. It is "HINT #16"
So in fact it can be written below:
+ .macro error_synchronize
+alternative_if_not ARM64_HAS_IESB
+ esb
+alternative_else_nop_endif
+ .endm
If written to that, whether it will be strange? although ESB should behave as a
NOP anyway if the CPU doesn't have RAS.
>
> On which note, since I don't see one here - are any of those other patches defining an "esb" assembly macro similar to the inline asm case?
> If not then this isn't going to build with older toolchains - perhaps we should just use the raw hint syntax directly.
Sorry for that I do not push the dependent patch[1].
The "ESB" is defined as a macro
/*
+ * RAS Error Synchronization barrier
+ */
+ .macro esb
+ hint #16
+ .endm
+
+/*
[1]
https://www.spinics.net/lists/arm-kernel/msg612884.html
>
> Robin.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-11-04 18:49 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-01 19:14 [PATCH v1 0/3] manually add Error Synchronization Barrier at exception handler entry and exit Dongjiu Geng
2017-11-01 11:21 ` Robin Murphy
2017-11-01 11:32 ` James Morse
2017-11-01 12:44 ` gengdongjiu
2017-11-01 19:14 ` [PATCH v1 1/3] arm64: add a macro for SError synchronization Dongjiu Geng
2017-11-01 11:24 ` Robin Murphy
2017-11-01 12:54 ` gengdongjiu
2017-11-01 13:31 ` Robin Murphy
2017-11-01 14:16 ` Mark Rutland
2017-11-02 8:52 ` gengdongjiu
2017-11-01 19:14 ` [PATCH v1 2/3] arm64: add error synchronization barrier in kernel_entry/kernel_exit Dongjiu Geng
2017-11-04 17:34 ` kbuild test robot
2017-11-01 19:14 ` [PATCH v1 3/3] KVM: arm64: add ESB in exception handler entry and exit Dongjiu Geng
2017-11-04 18:49 ` kbuild test robot
-- strict thread matches above, loose matches on Subject: below --
2017-11-01 14:13 [PATCH v1 1/3] arm64: add a macro for SError synchronization gengdongjiu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).