All of lore.kernel.org
 help / color / mirror / Atom feed
From: Huang Ying <ying.huang@intel.com>
To: Gong Chen <clumsycg@gmail.com>
Cc: Len Brown <lenb@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Andi Kleen <andi@firstfloor.org>,
	"Luck, Tony" <tony.luck@intel.com>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	Thomas Renninger <trenn@novell.com>
Subject: Re: [RFC 1/2] ACPI, APEI, Add apei_exec_run_optional
Date: Wed, 06 Jul 2011 08:26:49 +0800	[thread overview]
Message-ID: <4E13ABC9.1090209@intel.com> (raw)
In-Reply-To: <4E13174B.30403@gmail.com>

Hi, Gong,

Thanks for review.

On 07/05/2011 09:53 PM, Gong Chen wrote:
> 于 2011/7/5 14:07, Huang Ying 写道:
>> 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.
>>
>> Cc: Thomas Renninger<trenn@novell.com>
>> Signed-off-by: Huang Ying<ying.huang@intel.com>
>> ---
>>   drivers/acpi/apei/apei-base.c     |    9 +++++----
>>   drivers/acpi/apei/apei-internal.h |   13 ++++++++++++-
>>   2 files changed, 17 insertions(+), 5 deletions(-)
>>
>> --- a/drivers/acpi/apei/apei-base.c
>> +++ b/drivers/acpi/apei/apei-base.c
>> @@ -157,9 +157,10 @@ EXPORT_SYMBOL_GPL(apei_exec_noop);
>>    * 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 optional)
>>   {
>> -	int rc;
>> +	int rc = -ENOENT;
>>   	u32 i, ip;
>>   	struct acpi_whea_header *entry;
>>   	apei_exec_ins_func_t run;
>> @@ -198,9 +199,9 @@ rewind:
>>   			goto rewind;
>>   	}
>>
>> -	return 0;
>> +	return !optional&&  rc<  0 ? rc : 0;
> 
> if one operation is optional but running into errors when executing this 
> kind of command,
> here just ignoring it. Is it reasonable ?

If we running into errors except there is no instructions for the
action, we will return the error code before this.  Please take a look
at the whole function.

Best Regards,
Huang Ying
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Huang Ying <ying.huang@intel.com>
To: Gong Chen <clumsycg@gmail.com>
Cc: Len Brown <lenb@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Andi Kleen <andi@firstfloor.org>,
	"Luck, Tony" <tony.luck@intel.com>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	Thomas Renninger <trenn@novell.com>
Subject: Re: [RFC 1/2] ACPI, APEI, Add apei_exec_run_optional
Date: Wed, 06 Jul 2011 08:26:49 +0800	[thread overview]
Message-ID: <4E13ABC9.1090209@intel.com> (raw)
In-Reply-To: <4E13174B.30403@gmail.com>

Hi, Gong,

Thanks for review.

On 07/05/2011 09:53 PM, Gong Chen wrote:
> 于 2011/7/5 14:07, Huang Ying 写道:
>> 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.
>>
>> Cc: Thomas Renninger<trenn@novell.com>
>> Signed-off-by: Huang Ying<ying.huang@intel.com>
>> ---
>>   drivers/acpi/apei/apei-base.c     |    9 +++++----
>>   drivers/acpi/apei/apei-internal.h |   13 ++++++++++++-
>>   2 files changed, 17 insertions(+), 5 deletions(-)
>>
>> --- a/drivers/acpi/apei/apei-base.c
>> +++ b/drivers/acpi/apei/apei-base.c
>> @@ -157,9 +157,10 @@ EXPORT_SYMBOL_GPL(apei_exec_noop);
>>    * 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 optional)
>>   {
>> -	int rc;
>> +	int rc = -ENOENT;
>>   	u32 i, ip;
>>   	struct acpi_whea_header *entry;
>>   	apei_exec_ins_func_t run;
>> @@ -198,9 +199,9 @@ rewind:
>>   			goto rewind;
>>   	}
>>
>> -	return 0;
>> +	return !optional&&  rc<  0 ? rc : 0;
> 
> if one operation is optional but running into errors when executing this 
> kind of command,
> here just ignoring it. Is it reasonable ?

If we running into errors except there is no instructions for the
action, we will return the error code before this.  Please take a look
at the whole function.

Best Regards,
Huang Ying

  reply	other threads:[~2011-07-06  0:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-05  6:07 [RFC 1/2] ACPI, APEI, Add apei_exec_run_optional Huang Ying
2011-07-05  6:07 ` [RFC 2/2] ACPI, APEI, Use apei_exec_run_optional in APEI EINJ and ERST Huang Ying
2011-07-05 13:53 ` [RFC 1/2] ACPI, APEI, Add apei_exec_run_optional Gong Chen
2011-07-05 13:53   ` Gong Chen
2011-07-06  0:26   ` Huang Ying [this message]
2011-07-06  0:26     ` Huang Ying

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E13ABC9.1090209@intel.com \
    --to=ying.huang@intel.com \
    --cc=andi@firstfloor.org \
    --cc=clumsycg@gmail.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tony.luck@intel.com \
    --cc=trenn@novell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.