Re: [PATCH] ACPI, APEI: Add apei_exec_run_optional

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 22/03/13 09:14, Jan Beulich wrote:
>>>> On 22.03.13 at 10:07, "Jan Beulich" <JBeulich@suse.com> wrote:
>> All we appear to be missing in this case is Linux commit
>> eecf2f7124834dd1cad21807526a8ea031ba8217. I'll get that
>> ported over...
> ACPI, APEI: Add apei_exec_run_optional
>
> Some actions in APEI ERST and EINJ tables are optional, for example,
> ACPI_EINJ_BEGIN_OPERATION action is used to do some preparation for
> error injection, and firmware may choose to do nothing here.  While
> some other actions are mandatory, for example, firmware must provide
> ACPI_EINJ_GET_ERROR_TYPE implementation.
>
> Original implementation treats all actions as optional (that is, can
> have no instructions), that may cause issue if firmware does not
> provide some mandatory actions.  To fix this, this patch adds
> apei_exec_run_optional, which should be used for optional actions.
> The original apei_exec_run should be used for mandatory actions.
>
> Signed-off-by: Huang Ying <ying.huang@intel.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>

(Via backport to 4.2)

For what it is worth, with the spinlock fix, I disabled the "Intel only"
restriction, and so far I have been unable to find a problematic AMD box.

~Andrew

>
> --- a/xen/drivers/acpi/apei/apei-base.c
> +++ b/xen/drivers/acpi/apei/apei-base.c
> @@ -154,9 +154,10 @@ int apei_exec_noop(struct apei_exec_cont
>   * Interpret the specified action. Go through whole action table,
>   * execute all instructions belong to the action.
>   */
> -int apei_exec_run(struct apei_exec_context *ctx, u8 action)
> +int __apei_exec_run(struct apei_exec_context *ctx, u8 action,
> +		    bool_t optional)
>  {
> -	int rc;
> +	int rc = -ENOENT;
>  	u32 i, ip;
>  	struct acpi_whea_header *entry;
>  	apei_exec_ins_func_t run;
> @@ -195,7 +196,7 @@ rewind:
>  			goto rewind;
>  	}
>  
> -	return 0;
> +	return !optional && rc < 0 ? rc : 0;
>  }
>  
>  typedef int (*apei_exec_entry_func_t)(struct apei_exec_context *ctx,
> --- a/xen/drivers/acpi/apei/apei-internal.h
> +++ b/xen/drivers/acpi/apei/apei-internal.h
> @@ -48,7 +48,18 @@ static inline u64 apei_exec_ctx_get_outp
>  	return ctx->value;
>  }
>  
> -int apei_exec_run(struct apei_exec_context *ctx, u8 action);
> +int __apei_exec_run(struct apei_exec_context *ctx, u8 action, bool_t optional);
> +
> +static inline int apei_exec_run(struct apei_exec_context *ctx, u8 action)
> +{
> +	return __apei_exec_run(ctx, action, 0);
> +}
> +
> +/* It is optional whether the firmware provides the action */
> +static inline int apei_exec_run_optional(struct apei_exec_context *ctx, u8 action)
> +{
> +	return __apei_exec_run(ctx, action, 1);
> +}
>  
>  /* Common instruction implementation */
>  
>
>
>