Re: [Xen-devel] [PATCH 07/18] xen/arm: Introduce a command line parameter for SErrors/Aborts

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 13 Mar 2017, Wei Chen wrote:
> In order to distinguish guest-generated SErrors from hypervisor-generated
> SErrors. We have to place SError checking code in every EL1 -> EL2 paths.
         ^ remove .

> That will be an overhead on entries caused by dsb/isb.
> 
> But not all platforms want to categorize the SErrors. For example, a host
> that is running with trusted guests. The administrator can confirm that
> all guests that are running on the host will not trigger such SErrors. In
> this user scene, we should provide some options to administrator to avoid
       ^use-case


> categorizing the SErrors. And then reduce the overhead of dsb/isb.
               ^ remove   ^ remove 


> We provided following 3 options to administrator to determine how to handle
> the SErrors:
> 
> * `diverse`:
>   The hypervisor will distinguish guest SErrors from hypervisor SErrors.
>   The guest generated SErrors will be forwarded to guests, the hypervisor
>   generated SErrors will cause the whole system crash.
>   It requires:
>   1. Place dsb/isb on all EL1 -> EL2 trap entries to categorize SErrors
>      correctly.
>   2. Place dsb/isb on EL2 -> EL1 return paths to prevent slipping hypervisor
>      SErrors to guests.
>   3. Place dsb/isb in context switch to isolate the SErrors between 2 vCPUs.
> 
> * `forward`:
>   The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
>   All SErrors will be forwarded to guests, except the SErrors generated when
>   idle vCPU is running. The idle domain doesn't have the ability to hanle the
>   SErrors, so we have to crash the whole system when we get SErros with idle
>   vCPU. This option will avoid most overhead of the dsb/isb, except the 
> dsb/isb
>   in context switch which is used to isolate the SErrors between 2 vCPUs.
> 
> * `panic`:
>   The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
>   All SErrors will crash the whole system. This option will avoid all overhead
>   of the dsb/isb.
> 
> Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> 
> ---
> About adding dsb/isb to prevent slipping Hypervisor SErrors to Guests if the
> selected option is "diverse". Some Hypervisor SErrors could not be avoid by
> software, for example ECC Error. But I don't know whether it's worth adding
> the overhead by default.
> ---
>  docs/misc/xen-command-line.markdown | 44 
> +++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/traps.c                | 19 ++++++++++++++++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/docs/misc/xen-command-line.markdown 
> b/docs/misc/xen-command-line.markdown
> index 4daf5b5..79554ce 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1481,6 +1481,50 @@ enabling more sockets and cores to go into deeper 
> sleep states.
>  
>  Set the serial transmit buffer size.
>  
> +### serrors (ARM)
> +> `= diverse | forward | panic`
> +
> +> Default: `diverse`
> +
> +This parameter is provided to administrator to determine how to handle the
> +SErrors.

  This parameter is provided to administrators to determine how the
  hypervisors handle SErrors.


> +In order to distinguish guest-generated SErrors from hypervisor-generated
> +SErrors. We have to place SError checking code in every EL1 -> EL2 paths.
          ^remove .

> +That will be an overhead on entries caused by dsb/isb. But not all platforms
   
   That will cause overhead on entries due to dsb/isb. However, not all 
platforms

> +need to categorize the SErrors. For example, a host that is running with
                       ^ remove the

> +trusted guests. The administrator can confirm that all guests that are
> +running on the host will not trigger such SErrors. In this case, the
> +administrator can use this parameter to skip categorizing the SErrors. And
                                                             ^ remove   ^ remove

> +reduce the overhead of dsb/isb.
> +
> +We provided following 3 options to administrator to determine how to handle
              ^ the following         ^ administrators

> +the SErrors:
   ^ remove the


> +
> +* `diverse`:
> +  The hypervisor will distinguish guest SErrors from hypervisor SErrors.
> +  The guest generated SErrors will be forwarded to guests, the hypervisor
> +  generated SErrors will cause the whole system crash.
                                                  ^ to crash

> +  It requires:
> +  1. Place dsb/isb on all EL1 -> EL2 trap entries to categorize SErrors
        ^ remove Place
> +     correctly.
> +  2. Place dsb/isb on EL2 -> EL1 return paths to prevent slipping hypervisor
        ^ remove Place
> +     SErrors to guests.
> +  3. Place dsb/isb in context switch to isolate the SErrors between 2 vCPUs.
        ^ remove Place                             ^ remove the
> +
> +* `forward`:
> +  The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
> +  All SErrors will be forwarded to guests, except the SErrors generated when
> +  idle vCPU is running. The idle domain doesn't have the ability to hanle the
     ^ the idle                                                        ^ handle 
^ remove the 

> +  SErrors, so we have to crash the whole system when we get SErros with idle
                                                                          ^ the 
idle

> +  vCPU. This option will avoid most overhead of the dsb/isb, except the 
> dsb/isb
> +  in context switch which is used to isolate the SErrors between 2 vCPUs.
> +
> +* `panic`:
> +  The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
> +  All SErrors will crash the whole system. This option will avoid all 
> overhead
> +  of the dsb/isb.
    
    of the dsb/isb pairs.

Please make these changes to the commit message too, when applicable.
With these changes:

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


>  ### smap
>  > `= <boolean> | hvm`
>  
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index e425832..5e31699 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -115,6 +115,25 @@ static void __init parse_vwfi(const char *s)
>  }
>  custom_param("vwfi", parse_vwfi);
>  
> +static enum {
> +    SERRORS_DIVERSE,
> +    SERRORS_FORWARD,
> +    SERRORS_PANIC,
> +} serrors_op;
> +
> +static void __init parse_serrors_behavior(const char *str)
> +{
> +    if ( !strcmp(str, "forward") )
> +        serrors_op = SERRORS_FORWARD;
> +    else if ( !strcmp(str, "panic") )
> +        serrors_op = SERRORS_PANIC;
> +    else
> +        serrors_op = SERRORS_DIVERSE;
> +
> +    return;
> +}
> +custom_param("serrors", parse_serrors_behavior);
> +
>  register_t get_default_hcr_flags(void)
>  {
>      return  (HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel