[RFC][PATCH]ACPI,APEI,ERST, back end driver for NVRAM

[RFC][PATCH]ACPI,APEI,ERST, back end driver for NVRAM

[Seiji Aguchi]

Hi,

I developed write/read/clear functions of ERST driver for NVRAM. 
According to ACPI 4.0 specification, we can use the same procedure as storage.

This patch is tested on DELL PowerEdge T310.

Any comments are welcome.

 Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com>

---
 drivers/acpi/apei/erst.c |  145 +++++++++++++++++++++++++++++++++++++---------
 1 files changed, 117 insertions(+), 28 deletions(-)

diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c
index 1274080..4806283 100644
--- a/drivers/acpi/apei/erst.c
+++ b/drivers/acpi/apei/erst.c
@@ -755,30 +755,125 @@ static int __erst_clear_from_storage(u64 record_id)
 	return erst_errno(val);
 }
 
-/* NVRAM ERST Error Log Address Range is not supported yet */
-static void pr_unimpl_nvram(void)
+static int __erst_write_to_nvram(u64 offset)
 {
-	if (printk_ratelimit())
-		pr_warning(ERST_PFX
-		"NVRAM ERST Log Address Range is not implemented yet\n");
-}
+	struct apei_exec_context ctx;
+	u64 timeout = FIRMWARE_TIMEOUT;
+	u64 val;
+	int rc;
 
-static int __erst_write_to_nvram(const struct cper_record_header *record)
-{
-	/* do not print message, because printk is not safe for NMI */
-	return -ENOSYS;
+	erst_exec_ctx_init(&ctx);
+	rc = apei_exec_run_optional(&ctx, ACPI_ERST_BEGIN_WRITE);
+	if (rc)
+		return rc;
+	apei_exec_ctx_set_input(&ctx, offset);
+	rc = apei_exec_run(&ctx, ACPI_ERST_SET_RECORD_OFFSET);
+	if (rc)
+		return rc;
+	rc = apei_exec_run(&ctx, ACPI_ERST_EXECUTE_OPERATION);
+	if (rc)
+		return rc;
+	for (;;) {
+		rc = apei_exec_run(&ctx, ACPI_ERST_CHECK_BUSY_STATUS);
+		if (rc)
+			return rc;
+		val = apei_exec_ctx_get_output(&ctx);
+		if (!val)
+			break;
+		if (erst_timedout(&timeout, SPIN_UNIT))
+			return -EIO;
+	}
+	rc = apei_exec_run(&ctx, ACPI_ERST_GET_COMMAND_STATUS);
+	if (rc)
+		return rc;
+	val = apei_exec_ctx_get_output(&ctx);
+	rc = apei_exec_run_optional(&ctx, ACPI_ERST_END);
+	if (rc)
+		return rc;
+
+	return erst_errno(val);
 }
 
-static int __erst_read_to_erange_from_nvram(u64 record_id, u64 *offset)
+static int __erst_read_to_erange_from_nvram(u64 record_id, u64 offset)
 {
-	pr_unimpl_nvram();
-	return -ENOSYS;
+	struct apei_exec_context ctx;
+	u64 timeout = FIRMWARE_TIMEOUT;
+	u64 val;
+	int rc;
+
+	erst_exec_ctx_init(&ctx);
+	rc = apei_exec_run_optional(&ctx, ACPI_ERST_BEGIN_READ);
+	if (rc)
+		return rc;
+	apei_exec_ctx_set_input(&ctx, offset);
+	rc = apei_exec_run(&ctx, ACPI_ERST_SET_RECORD_OFFSET);
+	if (rc)
+		return rc;
+	apei_exec_ctx_set_input(&ctx, record_id);
+	rc = apei_exec_run(&ctx, ACPI_ERST_SET_RECORD_ID);
+	if (rc)
+		return rc;
+	rc = apei_exec_run(&ctx, ACPI_ERST_EXECUTE_OPERATION);
+	if (rc)
+		return rc;
+	for (;;) {
+		rc = apei_exec_run(&ctx, ACPI_ERST_CHECK_BUSY_STATUS);
+		if (rc)
+			return rc;
+		val = apei_exec_ctx_get_output(&ctx);
+		if (!val)
+			break;
+		if (erst_timedout(&timeout, SPIN_UNIT))
+			return -EIO;
+	}
+	rc = apei_exec_run(&ctx, ACPI_ERST_GET_COMMAND_STATUS);
+	if (rc)
+		return rc;
+	val = apei_exec_ctx_get_output(&ctx);
+	rc = apei_exec_run_optional(&ctx, ACPI_ERST_END);
+	if (rc)
+		return rc;
+
+	return erst_errno(val);
 }
 
 static int __erst_clear_from_nvram(u64 record_id)
 {
-	pr_unimpl_nvram();
-	return -ENOSYS;
+	struct apei_exec_context ctx;
+	u64 timeout = FIRMWARE_TIMEOUT;
+	u64 val;
+	int rc;
+
+	erst_exec_ctx_init(&ctx);
+	rc = apei_exec_run_optional(&ctx, ACPI_ERST_BEGIN_CLEAR);
+	if (rc)
+		return rc;
+	apei_exec_ctx_set_input(&ctx, record_id);
+	rc = apei_exec_run(&ctx, ACPI_ERST_SET_RECORD_ID);
+	if (rc)
+		return rc;
+	rc = apei_exec_run(&ctx, ACPI_ERST_EXECUTE_OPERATION);
+	if (rc)
+		return rc;
+	for (;;) {
+		rc = apei_exec_run(&ctx, ACPI_ERST_CHECK_BUSY_STATUS);
+		if (rc)
+			return rc;
+		val = apei_exec_ctx_get_output(&ctx);
+		if (!val)
+			break;
+		if (erst_timedout(&timeout, SPIN_UNIT))
+			return -EIO;
+	}
+	rc = apei_exec_run(&ctx, ACPI_ERST_GET_COMMAND_STATUS);
+	if (rc)
+		return rc;
+	val = apei_exec_ctx_get_output(&ctx);
+	rc = apei_exec_run_optional(&ctx, ACPI_ERST_END);
+	if (rc)
+		return rc;
+
+	return erst_errno(val);
 }
 
 int erst_write(const struct cper_record_header *record)
@@ -793,14 +888,6 @@ int erst_write(const struct cper_record_header *record)
 	if (memcmp(record->signature, CPER_SIG_RECORD, CPER_SIG_SIZE))
 		return -EINVAL;
 
-	if (erst_erange.attr & ERST_RANGE_NVRAM) {
-		if (!raw_spin_trylock_irqsave(&erst_lock, flags))
-			return -EBUSY;
-		rc = __erst_write_to_nvram(record);
-		raw_spin_unlock_irqrestore(&erst_lock, flags);
-		return rc;
-	}
-
 	if (record->record_length > erst_erange.size)
 		return -EINVAL;
 
@@ -811,7 +898,10 @@ int erst_write(const struct cper_record_header *record)
 	/* signature for serialization system */
 	memcpy(&rcd_erange->persistence_information, "ER", 2);
 
-	rc = __erst_write_to_storage(0);
+	if (erst_erange.attr & ERST_RANGE_NVRAM)
+		rc = __erst_write_to_nvram(0);
+	else
+		rc = __erst_write_to_storage(0);
 	raw_spin_unlock_irqrestore(&erst_lock, flags);
 
 	return rc;
@@ -823,10 +913,9 @@ static int __erst_read_to_erange(u64 record_id, u64 *offset)
 	int rc;
 
 	if (erst_erange.attr & ERST_RANGE_NVRAM)
-		return __erst_read_to_erange_from_nvram(
-			record_id, offset);
-
-	rc = __erst_read_from_storage(record_id, 0);
+		rc = __erst_read_to_erange_from_nvram(record_id, 0);
+	else
+		rc = __erst_read_from_storage(record_id, 0);
 	if (rc)
 		return rc;
 	*offset = 0;
-- 
1.7.1

Re: [RFC][PATCH]ACPI,APEI,ERST, back end driver for NVRAM

[Huang Ying]

On Fri, 2011-11-18 at 04:47 +0800, Seiji Aguchi wrote:
> Hi,
> 
> I developed write/read/clear functions of ERST driver for NVRAM. 
> According to ACPI 4.0 specification, we can use the same procedure as storage.
> 
> This patch is tested on DELL PowerEdge T310.

This machine has error log address range in NVRAM?

According to ACPI 4.0a spec:

"""
17.5.2.4.1 Error Log Address Range Resides in NVRAM

If the Error Log Address Range resides in NVRAM, then when OSPM writes a
record into the logging
range, the record is automatically persistent and the busy bit can be
cleared immediately. On a subsequent
boot, OSPM can read any persisted error records directly from the
persistent store range. The size of the
persistent store, in this case, is expected to be enough for several
error records.
"""

So it should not work with the same procedure.

Best Regards,
Huang Ying

RE: [RFC][PATCH]ACPI,APEI,ERST, back end driver for NVRAM

[Seiji Aguchi]

Hi,

>According to ACPI 4.0a spec:
>
>"""
>17.5.2.4.1 Error Log Address Range Resides in NVRAM
>

Thank you for giving me the information.
Let me clarify one thing.

If the busy bit can be cleared immediately, we don't need to
check busy bit with CHECK_BUSY_STATUS.
So, we should simply execute OSPM operations as follows.

 - Writing
   1. BEGIN_WRITE
   2. SET_RECORD_OFFSET
   3. EXECUTE_OPERATION
   4. END

 - Reading
   1. BEGIN_READ
   2. SET_RECORD_OFFSET
   3. SET_RECORD_ID
   4. EXECUTE_OPERATION
   (END operation is not needed because OSPM requires no platform 
    support to read.)

 - Clearing
   1. BEGIN_CLEAR
   2. SET_RECORD_ID
   3. EXECUTE_OPERATION
   (END operation is not needed because OSPM requires no platform 
    support to clear.)

Is this what you expected?

Seiji

RE: [RFC][PATCH]ACPI,APEI,ERST, back end driver for NVRAM

[Huang Ying]

On Sat, 2011-11-19 at 07:04 +0800, Seiji Aguchi wrote:
> Hi,
> 
> >According to ACPI 4.0a spec:
> >
> >"""
> >17.5.2.4.1 Error Log Address Range Resides in NVRAM
> >
> 
> Thank you for giving me the information.
> Let me clarify one thing.
> 
> If the busy bit can be cleared immediately, we don't need to
> check busy bit with CHECK_BUSY_STATUS.
> So, we should simply execute OSPM operations as follows.
> 
>  - Writing
>    1. BEGIN_WRITE
>    2. SET_RECORD_OFFSET
>    3. EXECUTE_OPERATION
>    4. END
> 
>  - Reading
>    1. BEGIN_READ
>    2. SET_RECORD_OFFSET
>    3. SET_RECORD_ID
>    4. EXECUTE_OPERATION
>    (END operation is not needed because OSPM requires no platform 
>     support to read.)
> 
>  - Clearing
>    1. BEGIN_CLEAR
>    2. SET_RECORD_ID
>    3. EXECUTE_OPERATION
>    (END operation is not needed because OSPM requires no platform 
>     support to clear.)
> 
> Is this what you expected?

No.  I have different understanding.

Because error log address range resides in NVRAM, the contents will be
reserved even after reboot.  So we do not need read/clear operations at
all, and should place all records in error log address range.

Best Regards,
Huang Ying

RE: [RFC][PATCH]ACPI,APEI,ERST, back end driver for NVRAM

[Seiji Aguchi]

Hi,

>
>No.  I have different understanding.
>
>Because error log address range resides in NVRAM, the contents will be
>reserved even after reboot.  So we do not need read/clear operations at
>all, and should place all records in error log address range.

I have a quick question about your comment above.
Do you know whether this optimization of read/clear operations works 
on machines which APEI is enabled by WHEA _OSC call?

Seiji

RE: [RFC][PATCH]ACPI,APEI,ERST, back end driver for NVRAM

[Huang Ying]

On Fri, 2011-12-16 at 00:03 +0800, Seiji Aguchi wrote:
> Hi,
> 
> >
> >No.  I have different understanding.
> >
> >Because error log address range resides in NVRAM, the contents will be
> >reserved even after reboot.  So we do not need read/clear operations at
> >all, and should place all records in error log address range.
> 
> I have a quick question about your comment above.
> Do you know whether this optimization of read/clear operations works 
> on machines which APEI is enabled by WHEA _OSC call?

I have no machine with

!!(erst_erange.attr & ERST_RANGE_NVRAM) == 1

Do you have?

Best Regards,
Huang Ying