[v2] mambo/mambo_utils.tcl: Inject an MCE at a specified address

Message ID 20180411014209.19178-1-bsingharora@gmail.com
State New
Headers show
Series
  • [v2] mambo/mambo_utils.tcl: Inject an MCE at a specified address
Related show

Commit Message

Balbir Singh April 11, 2018, 1:42 a.m.
Currently we don't support injecting an MCE on a specific address.
This is useful for testing functionality like memcpy_mcsafe()
(see https://patchwork.ozlabs.org/cover/893339/)

This patch refactors exc_mce into setup_mce and exc_mce. setup_mce
is generally useful to setup a MCE context with variables DSISR,
SRR1 and DAR. setup_mce supports setting up dsisr as a new
function argument. It's useful when we want to set up both the
cause and DSIR.

The core of the functionality is a routine called
inject_mce_ue_on_addr, which takes an addr argument and injects
an MCE (load/store with UE) when the specified address is accessed
by code. This functionality can easily be enhanced to cover
instruction UE's as well.

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
Changelog v2
	- Reuse cause, don't add DSISR for d-side
	- Test with nick's pending refactoring

 external/mambo/mambo_utils.tcl | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

Comments

Nicholas Piggin April 11, 2018, 2:11 a.m. | #1
On Wed, 11 Apr 2018 11:42:09 +1000
Balbir Singh <bsingharora@gmail.com> wrote:

> Currently we don't support injecting an MCE on a specific address.
> This is useful for testing functionality like memcpy_mcsafe()
> (see https://patchwork.ozlabs.org/cover/893339/)
> 
> This patch refactors exc_mce into setup_mce and exc_mce. setup_mce
> is generally useful to setup a MCE context with variables DSISR,
> SRR1 and DAR. setup_mce supports setting up dsisr as a new
> function argument. It's useful when we want to set up both the
> cause and DSIR.

Probably needs a changelog tidy.

> 
> The core of the functionality is a routine called
> inject_mce_ue_on_addr, which takes an addr argument and injects
> an MCE (load/store with UE) when the specified address is accessed
> by code. This functionality can easily be enhanced to cover
> instruction UE's as well.
> 
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> ---
> Changelog v2
> 	- Reuse cause, don't add DSISR for d-side
> 	- Test with nick's pending refactoring

I guess it didn't clash then? Okay good I though it may have been a
bigger problem.

> 
>  external/mambo/mambo_utils.tcl | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/external/mambo/mambo_utils.tcl b/external/mambo/mambo_utils.tcl
> index 7a27f0f4..9c6a329c 100644
> --- a/external/mambo/mambo_utils.tcl
> +++ b/external/mambo/mambo_utils.tcl
> @@ -442,11 +442,12 @@ proc mce_trigger { args } {
>  #
>  # Default with no arguments is a recoverable i-side TLB multi-hit
>  # Other options:
> -# d_side=1 cause=0x80 - recoverable d-side SLB multi-hit
> +# d_side=1 dsisr=0x80 - recoverable d-side SLB multi-hit
> +# d_side=1 dsisr=0x8000 - ue error on instruction fetch

DSISR[48] = "Interrupt caused by a UE deferred error, but not for a
             tablewalk (D-side only)." ?

Seems like a LSU error not ifetch.

>  # d_side=0 cause=0xd  - unrecoverable i-side async store timeout (POWER9 only)
>  # d_side=0 cause=0x1  - unrecoverable i-side ifetch
>  #
> -proc exc_mce { { d_side 0 } { cause 0x5 } { recoverable 1 } } {
> +proc setup_mce { { d_side 0 } { cause 0x5 } { recoverable 1 }} {

If you change the above, I might just nitpick the whitespace change
here.

>      variable SRR1
>      variable DSISR
>      variable DAR
> @@ -466,7 +467,6 @@ proc exc_mce { { d_side 0 } { cause 0x5 } { recoverable 1 } } {
>          set msr_ri 0x0
>      }
>  
> -    # recoverable d-side SLB multihit
>      if { $d_side } {
>          set is_dside 1
>          set SRR1_mc_cause 0x0
> @@ -486,7 +486,10 @@ proc exc_mce { { d_side 0 } { cause 0x5 } { recoverable 1 } } {
>      set SRR1 [expr $SRR1 | ((($SRR1_mc_cause >> 2) & 0x1) << (63-43))]
>      set SRR1 [expr $SRR1 | ((($SRR1_mc_cause >> 1) & 0x1) << (63-44))]
>      set SRR1 [expr $SRR1 | ((($SRR1_mc_cause >> 0) & 0x1) << (63-45))]
> +}
>  
> +proc exc_mce { { d_side 0 } { cause 0x5 } { recoverable 1 }} {
> +    setup_mce $d_side $cause $recoverable
>      if { [current_insn] in { "stop" "nap" "sleep" "winkle" } } {
>          # mambo has a quirk that interrupts from idle wake immediately
>          mysim trigger set pc 0x200 "mce_trigger"
> @@ -526,6 +529,22 @@ proc inject_mce { } {
>      mysim trigger clear pc $pc ; list
>  }
>  
> +#
> +# We've stopped at addr and we need to inject the mce and continue
> +#
> +proc trigger_mce_ue_addr {args} {
> +	set addr [lindex [lindex $args 0] 1]
> +	mysim trigger clear memory system rw $addr $addr
> +	setup_mce 0x1 0x8000 0x1
> +	mysim trigger set pc 0x200 "mce_trigger"
> +	mysim cpu 0 interrupt MachineCheck
> +	mce_trigger
> +}

I guess I'm not entirely sure here why you can't just use

 exc_mce 0x1 0x8000 0x1

*That* is the information I'd like in the changelog.

> +
> +proc inject_mce_ue_on_addr {addr} {
> +	mysim trigger set memory system rw $addr $addr 1 "trigger_mce_ue_addr"
> +}

Aside from that, this is nice functionality to have.

Thanks,
Nick

Patch

diff --git a/external/mambo/mambo_utils.tcl b/external/mambo/mambo_utils.tcl
index 7a27f0f4..9c6a329c 100644
--- a/external/mambo/mambo_utils.tcl
+++ b/external/mambo/mambo_utils.tcl
@@ -442,11 +442,12 @@  proc mce_trigger { args } {
 #
 # Default with no arguments is a recoverable i-side TLB multi-hit
 # Other options:
-# d_side=1 cause=0x80 - recoverable d-side SLB multi-hit
+# d_side=1 dsisr=0x80 - recoverable d-side SLB multi-hit
+# d_side=1 dsisr=0x8000 - ue error on instruction fetch
 # d_side=0 cause=0xd  - unrecoverable i-side async store timeout (POWER9 only)
 # d_side=0 cause=0x1  - unrecoverable i-side ifetch
 #
-proc exc_mce { { d_side 0 } { cause 0x5 } { recoverable 1 } } {
+proc setup_mce { { d_side 0 } { cause 0x5 } { recoverable 1 }} {
     variable SRR1
     variable DSISR
     variable DAR
@@ -466,7 +467,6 @@  proc exc_mce { { d_side 0 } { cause 0x5 } { recoverable 1 } } {
         set msr_ri 0x0
     }
 
-    # recoverable d-side SLB multihit
     if { $d_side } {
         set is_dside 1
         set SRR1_mc_cause 0x0
@@ -486,7 +486,10 @@  proc exc_mce { { d_side 0 } { cause 0x5 } { recoverable 1 } } {
     set SRR1 [expr $SRR1 | ((($SRR1_mc_cause >> 2) & 0x1) << (63-43))]
     set SRR1 [expr $SRR1 | ((($SRR1_mc_cause >> 1) & 0x1) << (63-44))]
     set SRR1 [expr $SRR1 | ((($SRR1_mc_cause >> 0) & 0x1) << (63-45))]
+}
 
+proc exc_mce { { d_side 0 } { cause 0x5 } { recoverable 1 }} {
+    setup_mce $d_side $cause $recoverable
     if { [current_insn] in { "stop" "nap" "sleep" "winkle" } } {
         # mambo has a quirk that interrupts from idle wake immediately
         mysim trigger set pc 0x200 "mce_trigger"
@@ -526,6 +529,22 @@  proc inject_mce { } {
     mysim trigger clear pc $pc ; list
 }
 
+#
+# We've stopped at addr and we need to inject the mce and continue
+#
+proc trigger_mce_ue_addr {args} {
+	set addr [lindex [lindex $args 0] 1]
+	mysim trigger clear memory system rw $addr $addr
+	setup_mce 0x1 0x8000 0x1
+	mysim trigger set pc 0x200 "mce_trigger"
+	mysim cpu 0 interrupt MachineCheck
+	mce_trigger
+}
+
+proc inject_mce_ue_on_addr {addr} {
+	mysim trigger set memory system rw $addr $addr 1 "trigger_mce_ue_addr"
+}
+
 # inject and step over one instruction, and repeat.
 proc inject_mce_step { {nr 1} } {
     for { set i 0 } { $i < $nr } { incr i 1 } {