From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] ACPI, APEI: Add apei_exec_run_optional Date: Fri, 22 Mar 2013 11:27:50 +0000 Message-ID: <514C4036.2090809@citrix.com> References: <514B961E.8010202@citrix.com> <514C2D4E02000078000C7AF2@nat28.tlf.novell.com> <514C2F0502000078000C7AFD@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <514C2F0502000078000C7AFD@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: "Keir (Xen.org)" , Xen-devel List List-Id: xen-devel@lists.xenproject.org On 22/03/13 09:14, Jan Beulich wrote: >>>> On 22.03.13 at 10:07, "Jan Beulich" 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 > Signed-off-by: Jan Beulich Tested-by: Andrew Cooper (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 */ > > > >