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

[Nicholas Piggin]

On Wed, 11 Apr 2018 11:42:09 +1000
Balbir Singh <bsingharora at 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 at 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