linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 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).