[10/10] acpi: xsdt: add SBBR compliance tests

Message ID 1500963057-4225-11-git-send-email-Sakar.Arora@arm.com
State Accepted
Headers show

Commit Message

Sakar Arora July 25, 2017, 6:10 a.m.
From: Mahesh Bireddy <mahesh.reddybireddy@arm.com>

Server Base Boot Requirements (SBBR) specification is intended for SBSA-
compliant 64-bit ARMv8 servers. It defines the base firmware
requirements for out-of-box support of any ARM SBSA-compatible
Operating System or hypervisor. The requirements in this specification
are expected to be minimal yet complete for booting a multi-core ARMv8
server platform, while leaving plenty of room for OEM or ODM innovations
and design details. For more information, download the SBBR
specification here:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044b/index.html

This change introduces test cases as per SBBR specification to xsdt acpi table.

Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Signed-off-by: Mahesh Bireddy <mahesh.reddybireddy@arm.com>
---
 src/acpi/xsdt/xsdt.c | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

Comments

Alex Hung Aug. 4, 2017, 1:03 a.m. | #1
On 2017-07-24 11:10 PM, Sakar Arora wrote:
> From: Mahesh Bireddy <mahesh.reddybireddy@arm.com>
> 
> Server Base Boot Requirements (SBBR) specification is intended for SBSA-
> compliant 64-bit ARMv8 servers. It defines the base firmware
> requirements for out-of-box support of any ARM SBSA-compatible
> Operating System or hypervisor. The requirements in this specification
> are expected to be minimal yet complete for booting a multi-core ARMv8
> server platform, while leaving plenty of room for OEM or ODM innovations
> and design details. For more information, download the SBBR
> specification here:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044b/index.html
> 
> This change introduces test cases as per SBBR specification to xsdt acpi table.
> 
> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Signed-off-by: Mahesh Bireddy <mahesh.reddybireddy@arm.com>
> ---
>   src/acpi/xsdt/xsdt.c | 35 +++++++++++++++++++++++++++--------
>   1 file changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/src/acpi/xsdt/xsdt.c b/src/acpi/xsdt/xsdt.c
> index 37a45c8..2c515fd 100644
> --- a/src/acpi/xsdt/xsdt.c
> +++ b/src/acpi/xsdt/xsdt.c
> @@ -37,8 +37,13 @@ static int xsdt_init(fwts_framework *fw)
>   		return FWTS_ERROR;
>   	}
>   	if (table == NULL || (table && table->length == 0)) {
> -		fwts_log_error(fw, "ACPI XSDT table does not exist, skipping test");
> -		return FWTS_SKIP;
> +		if (fw->flags & FWTS_FLAG_TEST_SBBR) {
> +			fwts_log_error(fw, "ACPI XSDT table does not exist");
> +			return FWTS_ERROR;
> +		} else {
> +			fwts_log_error(fw, "ACPI XSDT table does not exist, skipping test");
> +			return FWTS_SKIP;
> +		}
>   	}
>   	return FWTS_OK;
>   }
> @@ -56,9 +61,16 @@ static int xsdt_test1(fwts_framework *fw)
>   	for (i = 0; i < n; i++) {
>   		if (xsdt->entries[i] == 0) {
>   			passed = false;
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				"XSDTEntryNull",
> -				"XSDT Entry %zd is null, should not be non-zero.", i);
> +			if (fw->flags & FWTS_FLAG_TEST_SBBR) {
> +				fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +					"XSDTEntryNull",
> +					"XSDT Entry %zd is null, should not be non-zero.", i);
> +			}
> +			else {
> +				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +					"XSDTEntryNull",
> +					"XSDT Entry %zd is null, should not be non-zero.", i);
> +			}
>   			fwts_advice(fw,
>   				"A XSDT pointer is null and therefore erroneously "
>   				"points to an invalid 64 bit ACPI table header. "
> @@ -67,8 +79,15 @@ static int xsdt_test1(fwts_framework *fw)
>   				"should be fixed where possible.");
>   		}
>   	}
> -	if (passed)
> -		fwts_passed(fw, "No issues found in XSDT table.");
> +	if (passed) {
> +		if (fw->flags & FWTS_FLAG_TEST_SBBR) {
> +			fwts_passed(fw, "XSDT is present, pointed at by XsdrAddress=0x%lx"
> +				" and contain valid pointers to %d other ACPI tables mandated by SBBR",
> +				 xsdt->entries[0], (int)n);
> +		}
> +		else
> +			fwts_passed(fw, "No issues found in XSDT table.");
> +	}
>   
>   	return FWTS_OK;
>   }
> @@ -84,6 +103,6 @@ static fwts_framework_ops xsdt_ops = {
>   	.minor_tests = xsdt_tests
>   };
>   
> -FWTS_REGISTER("xsdt", &xsdt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI)
> +FWTS_REGISTER("xsdt", &xsdt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI | FWTS_FLAG_TEST_SBBR)
>   
>   #endif
> 


Acked-by: Alex Hung <alex.hung@canonical.com>
Colin King Aug. 22, 2017, 4:45 p.m. | #2
On 25/07/17 07:10, Sakar Arora wrote:
> From: Mahesh Bireddy <mahesh.reddybireddy@arm.com>
> 
> Server Base Boot Requirements (SBBR) specification is intended for SBSA-
> compliant 64-bit ARMv8 servers. It defines the base firmware
> requirements for out-of-box support of any ARM SBSA-compatible
> Operating System or hypervisor. The requirements in this specification
> are expected to be minimal yet complete for booting a multi-core ARMv8
> server platform, while leaving plenty of room for OEM or ODM innovations
> and design details. For more information, download the SBBR
> specification here:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044b/index.html
> 
> This change introduces test cases as per SBBR specification to xsdt acpi table.
> 
> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Signed-off-by: Mahesh Bireddy <mahesh.reddybireddy@arm.com>
> ---
>  src/acpi/xsdt/xsdt.c | 35 +++++++++++++++++++++++++++--------
>  1 file changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/src/acpi/xsdt/xsdt.c b/src/acpi/xsdt/xsdt.c
> index 37a45c8..2c515fd 100644
> --- a/src/acpi/xsdt/xsdt.c
> +++ b/src/acpi/xsdt/xsdt.c
> @@ -37,8 +37,13 @@ static int xsdt_init(fwts_framework *fw)
>  		return FWTS_ERROR;
>  	}
>  	if (table == NULL || (table && table->length == 0)) {
> -		fwts_log_error(fw, "ACPI XSDT table does not exist, skipping test");
> -		return FWTS_SKIP;
> +		if (fw->flags & FWTS_FLAG_TEST_SBBR) {
> +			fwts_log_error(fw, "ACPI XSDT table does not exist");
> +			return FWTS_ERROR;
> +		} else {
> +			fwts_log_error(fw, "ACPI XSDT table does not exist, skipping test");
> +			return FWTS_SKIP;
> +		}
>  	}
>  	return FWTS_OK;
>  }
> @@ -56,9 +61,16 @@ static int xsdt_test1(fwts_framework *fw)
>  	for (i = 0; i < n; i++) {
>  		if (xsdt->entries[i] == 0) {
>  			passed = false;
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				"XSDTEntryNull",
> -				"XSDT Entry %zd is null, should not be non-zero.", i);
> +			if (fw->flags & FWTS_FLAG_TEST_SBBR) {
> +				fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +					"XSDTEntryNull",
> +					"XSDT Entry %zd is null, should not be non-zero.", i);
> +			}
> +			else {
> +				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +					"XSDTEntryNull",
> +					"XSDT Entry %zd is null, should not be non-zero.", i);
> +			}
>  			fwts_advice(fw,
>  				"A XSDT pointer is null and therefore erroneously "
>  				"points to an invalid 64 bit ACPI table header. "
> @@ -67,8 +79,15 @@ static int xsdt_test1(fwts_framework *fw)
>  				"should be fixed where possible.");
>  		}
>  	}
> -	if (passed)
> -		fwts_passed(fw, "No issues found in XSDT table.");
> +	if (passed) {
> +		if (fw->flags & FWTS_FLAG_TEST_SBBR) {
> +			fwts_passed(fw, "XSDT is present, pointed at by XsdrAddress=0x%lx"
> +				" and contain valid pointers to %d other ACPI tables mandated by SBBR",
> +				 xsdt->entries[0], (int)n);
> +		}
> +		else
> +			fwts_passed(fw, "No issues found in XSDT table.");
> +	}
>  
>  	return FWTS_OK;
>  }
> @@ -84,6 +103,6 @@ static fwts_framework_ops xsdt_ops = {
>  	.minor_tests = xsdt_tests
>  };
>  
> -FWTS_REGISTER("xsdt", &xsdt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI)
> +FWTS_REGISTER("xsdt", &xsdt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI | FWTS_FLAG_TEST_SBBR)
>  
>  #endif
> 
Acked-by: Colin Ian King <colin.king@canonical.com>

Sorry it took so long to review these patches, I was on vacation for a
few weeks.

Patch

diff --git a/src/acpi/xsdt/xsdt.c b/src/acpi/xsdt/xsdt.c
index 37a45c8..2c515fd 100644
--- a/src/acpi/xsdt/xsdt.c
+++ b/src/acpi/xsdt/xsdt.c
@@ -37,8 +37,13 @@  static int xsdt_init(fwts_framework *fw)
 		return FWTS_ERROR;
 	}
 	if (table == NULL || (table && table->length == 0)) {
-		fwts_log_error(fw, "ACPI XSDT table does not exist, skipping test");
-		return FWTS_SKIP;
+		if (fw->flags & FWTS_FLAG_TEST_SBBR) {
+			fwts_log_error(fw, "ACPI XSDT table does not exist");
+			return FWTS_ERROR;
+		} else {
+			fwts_log_error(fw, "ACPI XSDT table does not exist, skipping test");
+			return FWTS_SKIP;
+		}
 	}
 	return FWTS_OK;
 }
@@ -56,9 +61,16 @@  static int xsdt_test1(fwts_framework *fw)
 	for (i = 0; i < n; i++) {
 		if (xsdt->entries[i] == 0) {
 			passed = false;
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"XSDTEntryNull",
-				"XSDT Entry %zd is null, should not be non-zero.", i);
+			if (fw->flags & FWTS_FLAG_TEST_SBBR) {
+				fwts_failed(fw, LOG_LEVEL_CRITICAL,
+					"XSDTEntryNull",
+					"XSDT Entry %zd is null, should not be non-zero.", i);
+			}
+			else {
+				fwts_failed(fw, LOG_LEVEL_MEDIUM,
+					"XSDTEntryNull",
+					"XSDT Entry %zd is null, should not be non-zero.", i);
+			}
 			fwts_advice(fw,
 				"A XSDT pointer is null and therefore erroneously "
 				"points to an invalid 64 bit ACPI table header. "
@@ -67,8 +79,15 @@  static int xsdt_test1(fwts_framework *fw)
 				"should be fixed where possible.");
 		}
 	}
-	if (passed)
-		fwts_passed(fw, "No issues found in XSDT table.");
+	if (passed) {
+		if (fw->flags & FWTS_FLAG_TEST_SBBR) {
+			fwts_passed(fw, "XSDT is present, pointed at by XsdrAddress=0x%lx"
+				" and contain valid pointers to %d other ACPI tables mandated by SBBR",
+				 xsdt->entries[0], (int)n);
+		}
+		else
+			fwts_passed(fw, "No issues found in XSDT table.");
+	}
 
 	return FWTS_OK;
 }
@@ -84,6 +103,6 @@  static fwts_framework_ops xsdt_ops = {
 	.minor_tests = xsdt_tests
 };
 
-FWTS_REGISTER("xsdt", &xsdt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI)
+FWTS_REGISTER("xsdt", &xsdt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI | FWTS_FLAG_TEST_SBBR)
 
 #endif