[08/10] acpi: madt: add SBBR compliance tests

Message ID 1500963057-4225-9-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 madt acpi table.

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

Comments

Alex Hung Aug. 4, 2017, 12:44 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 madt acpi table.
> 
> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Signed-off-by: Mahesh Bireddy <mahesh.reddybireddy@arm.com>
> ---
>   src/acpi/madt/madt.c | 79 +++++++++++++++++++++++++++++++---------------------
>   1 file changed, 47 insertions(+), 32 deletions(-)
> 
> diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c
> index a24fa00..655f298 100644
> --- a/src/acpi/madt/madt.c
> +++ b/src/acpi/madt/madt.c
> @@ -125,6 +125,8 @@
>   #define NUM_SUBTABLE_TYPES	16
>   #define MAX_IO_APIC_ID		256 /* IO APIC ID field is 1 byte */
>   
> +#define SBBR_ACPI_MAJOR_VERSION 6
> +
>   struct acpi_madt_subtable_lengths {
>   	unsigned short major_version;	/* from revision in FADT header */
>   	unsigned short minor_version;	/* FADT field starting with 5.1 */
> @@ -351,6 +353,15 @@ static int madt_init(fwts_framework *fw)
>   
>   	fadt_major = fadt->header.revision;
>   	fadt_minor = 0;
> +	if (fw->flags & FWTS_FLAG_TEST_SBBR) {
> +		if (fadt_major < SBBR_ACPI_MAJOR_VERSION) {
> +			fwts_log_error(fw, "SBBR support starts with ACPI v6.0,"
> +					" Current revision is outdated: %d.%d",
> +					fadt_major, fadt_minor);
> +			return FWTS_ERROR;
> +		}
> +	}
> +
>   	if (fadt_major >= 5 && fadt->header.length >= 268)
>   		fadt_minor = fadt->minor_version;   /* field added ACPI 5.1 */
>   
> @@ -1430,37 +1441,39 @@ static int madt_subtables(fwts_framework *fw)
>   				    hdr->type, madt_sub_names[type]);
>   		}
>   
> -		/* verify that the length is what we expect */
> -		if (len == SUBTABLE_VARIABLE) {
> -			if (hdr->type == FWTS_ACPI_MADT_LOCAL_SAPIC) {
> -				lsapic = (fwts_acpi_madt_local_sapic *)hdr;
> -				proper_len =
> -				     sizeof(fwts_acpi_madt_local_sapic) +
> -				     strlen(lsapic->uid_string) + 1;
> -
> -				if (proper_len != hdr->length)
> +		if (!(fw->flags & FWTS_FLAG_TEST_SBBR)) {
> +			/* verify that the length is what we expect */
> +			if (len == SUBTABLE_VARIABLE) {
> +				if (hdr->type == FWTS_ACPI_MADT_LOCAL_SAPIC) {
> +					lsapic = (fwts_acpi_madt_local_sapic *)hdr;
> +					proper_len =
> +					     sizeof(fwts_acpi_madt_local_sapic) +
> +					     strlen(lsapic->uid_string) + 1;
> +
> +					if (proper_len != hdr->length)
> +						passed = false;
> +				}
> +			} else {
> +				if (hdr->length != len)
>   					passed = false;
>   			}
> -		} else {
> -			if (hdr->length != len)
> -				passed = false;
> -		}
> -		if (passed) {
> -			fwts_passed(fw,
> -				    "Subtable %d (offset 0x%x) of "
> -				    "type %d (%s) is the correct length: %d",
> -				    ii, offset, hdr->type,
> -				    madt_sub_names[type],
> -				    hdr->length);
> -		} else {
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				    "SPECMADTSubLen",
> -				    "Subtable %d (offset 0x%x) of "
> -				    "type %d (%s) is %d bytes "
> -				    "long but should be %d bytes",
> -				    ii, offset, hdr->type,
> -				    madt_sub_names[type],
> -				    hdr->length, len);
> +			if (passed) {
> +				fwts_passed(fw,
> +					    "Subtable %d (offset 0x%x) of "
> +					    "type %d (%s) is the correct length: %d",
> +					    ii, offset, hdr->type,
> +					    madt_sub_names[type],
> +					    hdr->length);
> +			} else {
> +				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +					    "SPECMADTSubLen",
> +					    "Subtable %d (offset 0x%x) of "
> +					    "type %d (%s) is %d bytes "
> +					    "long but should be %d bytes",
> +					    ii, offset, hdr->type,
> +					    madt_sub_names[type],
> +					    hdr->length, len);
> +			}
>   		}
>   
>   		/* perform checks specific to subtable types */
> @@ -1573,8 +1586,10 @@ static int madt_subtables(fwts_framework *fw)
>   		length -= skip;
>   	}
>   
> -	/* run comparison tests */
> -	madt_ioapic_sapic_compare(fw, num_ioapics, num_iosapics);
> +	if (!(fw->flags & FWTS_FLAG_TEST_SBBR)) {
> +		/* run comparison tests */
> +		madt_ioapic_sapic_compare(fw, num_ioapics, num_iosapics);
> +	}
>   
>   	return FWTS_OK;
>   }
> @@ -1607,6 +1622,6 @@ static fwts_framework_ops madt_ops = {
>   	.minor_tests = madt_tests
>   };
>   
> -FWTS_REGISTER("madt", &madt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI | FWTS_FLAG_TEST_COMPLIANCE_ACPI)
> +FWTS_REGISTER("madt", &madt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI | FWTS_FLAG_TEST_COMPLIANCE_ACPI | FWTS_FLAG_TEST_SBBR)
>   
>   #endif
> 


Acked-by: Alex Hung <alex.hung@canonical.com>
Colin King Aug. 22, 2017, 4:44 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 madt acpi table.
> 
> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Signed-off-by: Mahesh Bireddy <mahesh.reddybireddy@arm.com>
> ---
>  src/acpi/madt/madt.c | 79 +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 47 insertions(+), 32 deletions(-)
> 
> diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c
> index a24fa00..655f298 100644
> --- a/src/acpi/madt/madt.c
> +++ b/src/acpi/madt/madt.c
> @@ -125,6 +125,8 @@
>  #define NUM_SUBTABLE_TYPES	16
>  #define MAX_IO_APIC_ID		256 /* IO APIC ID field is 1 byte */
>  
> +#define SBBR_ACPI_MAJOR_VERSION 6
> +
>  struct acpi_madt_subtable_lengths {
>  	unsigned short major_version;	/* from revision in FADT header */
>  	unsigned short minor_version;	/* FADT field starting with 5.1 */
> @@ -351,6 +353,15 @@ static int madt_init(fwts_framework *fw)
>  
>  	fadt_major = fadt->header.revision;
>  	fadt_minor = 0;
> +	if (fw->flags & FWTS_FLAG_TEST_SBBR) {
> +		if (fadt_major < SBBR_ACPI_MAJOR_VERSION) {
> +			fwts_log_error(fw, "SBBR support starts with ACPI v6.0,"
> +					" Current revision is outdated: %d.%d",
> +					fadt_major, fadt_minor);
> +			return FWTS_ERROR;
> +		}
> +	}
> +
>  	if (fadt_major >= 5 && fadt->header.length >= 268)
>  		fadt_minor = fadt->minor_version;   /* field added ACPI 5.1 */
>  
> @@ -1430,37 +1441,39 @@ static int madt_subtables(fwts_framework *fw)
>  				    hdr->type, madt_sub_names[type]);
>  		}
>  
> -		/* verify that the length is what we expect */
> -		if (len == SUBTABLE_VARIABLE) {
> -			if (hdr->type == FWTS_ACPI_MADT_LOCAL_SAPIC) {
> -				lsapic = (fwts_acpi_madt_local_sapic *)hdr;
> -				proper_len =
> -				     sizeof(fwts_acpi_madt_local_sapic) +
> -				     strlen(lsapic->uid_string) + 1;
> -
> -				if (proper_len != hdr->length)
> +		if (!(fw->flags & FWTS_FLAG_TEST_SBBR)) {
> +			/* verify that the length is what we expect */
> +			if (len == SUBTABLE_VARIABLE) {
> +				if (hdr->type == FWTS_ACPI_MADT_LOCAL_SAPIC) {
> +					lsapic = (fwts_acpi_madt_local_sapic *)hdr;
> +					proper_len =
> +					     sizeof(fwts_acpi_madt_local_sapic) +
> +					     strlen(lsapic->uid_string) + 1;
> +
> +					if (proper_len != hdr->length)
> +						passed = false;
> +				}
> +			} else {
> +				if (hdr->length != len)
>  					passed = false;
>  			}
> -		} else {
> -			if (hdr->length != len)
> -				passed = false;
> -		}
> -		if (passed) {
> -			fwts_passed(fw,
> -				    "Subtable %d (offset 0x%x) of "
> -				    "type %d (%s) is the correct length: %d",
> -				    ii, offset, hdr->type,
> -				    madt_sub_names[type],
> -				    hdr->length);
> -		} else {
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				    "SPECMADTSubLen",
> -				    "Subtable %d (offset 0x%x) of "
> -				    "type %d (%s) is %d bytes "
> -				    "long but should be %d bytes",
> -				    ii, offset, hdr->type,
> -				    madt_sub_names[type],
> -				    hdr->length, len);
> +			if (passed) {
> +				fwts_passed(fw,
> +					    "Subtable %d (offset 0x%x) of "
> +					    "type %d (%s) is the correct length: %d",
> +					    ii, offset, hdr->type,
> +					    madt_sub_names[type],
> +					    hdr->length);
> +			} else {
> +				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +					    "SPECMADTSubLen",
> +					    "Subtable %d (offset 0x%x) of "
> +					    "type %d (%s) is %d bytes "
> +					    "long but should be %d bytes",
> +					    ii, offset, hdr->type,
> +					    madt_sub_names[type],
> +					    hdr->length, len);
> +			}
>  		}
>  
>  		/* perform checks specific to subtable types */
> @@ -1573,8 +1586,10 @@ static int madt_subtables(fwts_framework *fw)
>  		length -= skip;
>  	}
>  
> -	/* run comparison tests */
> -	madt_ioapic_sapic_compare(fw, num_ioapics, num_iosapics);
> +	if (!(fw->flags & FWTS_FLAG_TEST_SBBR)) {
> +		/* run comparison tests */
> +		madt_ioapic_sapic_compare(fw, num_ioapics, num_iosapics);
> +	}
>  
>  	return FWTS_OK;
>  }
> @@ -1607,6 +1622,6 @@ static fwts_framework_ops madt_ops = {
>  	.minor_tests = madt_tests
>  };
>  
> -FWTS_REGISTER("madt", &madt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI | FWTS_FLAG_TEST_COMPLIANCE_ACPI)
> +FWTS_REGISTER("madt", &madt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI | FWTS_FLAG_TEST_COMPLIANCE_ACPI | FWTS_FLAG_TEST_SBBR)
>  
>  #endif
> 
Acked-by: Colin Ian King <colin.king@canonical.com>

Patch

diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c
index a24fa00..655f298 100644
--- a/src/acpi/madt/madt.c
+++ b/src/acpi/madt/madt.c
@@ -125,6 +125,8 @@ 
 #define NUM_SUBTABLE_TYPES	16
 #define MAX_IO_APIC_ID		256 /* IO APIC ID field is 1 byte */
 
+#define SBBR_ACPI_MAJOR_VERSION 6
+
 struct acpi_madt_subtable_lengths {
 	unsigned short major_version;	/* from revision in FADT header */
 	unsigned short minor_version;	/* FADT field starting with 5.1 */
@@ -351,6 +353,15 @@  static int madt_init(fwts_framework *fw)
 
 	fadt_major = fadt->header.revision;
 	fadt_minor = 0;
+	if (fw->flags & FWTS_FLAG_TEST_SBBR) {
+		if (fadt_major < SBBR_ACPI_MAJOR_VERSION) {
+			fwts_log_error(fw, "SBBR support starts with ACPI v6.0,"
+					" Current revision is outdated: %d.%d",
+					fadt_major, fadt_minor);
+			return FWTS_ERROR;
+		}
+	}
+
 	if (fadt_major >= 5 && fadt->header.length >= 268)
 		fadt_minor = fadt->minor_version;   /* field added ACPI 5.1 */
 
@@ -1430,37 +1441,39 @@  static int madt_subtables(fwts_framework *fw)
 				    hdr->type, madt_sub_names[type]);
 		}
 
-		/* verify that the length is what we expect */
-		if (len == SUBTABLE_VARIABLE) {
-			if (hdr->type == FWTS_ACPI_MADT_LOCAL_SAPIC) {
-				lsapic = (fwts_acpi_madt_local_sapic *)hdr;
-				proper_len =
-				     sizeof(fwts_acpi_madt_local_sapic) +
-				     strlen(lsapic->uid_string) + 1;
-
-				if (proper_len != hdr->length)
+		if (!(fw->flags & FWTS_FLAG_TEST_SBBR)) {
+			/* verify that the length is what we expect */
+			if (len == SUBTABLE_VARIABLE) {
+				if (hdr->type == FWTS_ACPI_MADT_LOCAL_SAPIC) {
+					lsapic = (fwts_acpi_madt_local_sapic *)hdr;
+					proper_len =
+					     sizeof(fwts_acpi_madt_local_sapic) +
+					     strlen(lsapic->uid_string) + 1;
+
+					if (proper_len != hdr->length)
+						passed = false;
+				}
+			} else {
+				if (hdr->length != len)
 					passed = false;
 			}
-		} else {
-			if (hdr->length != len)
-				passed = false;
-		}
-		if (passed) {
-			fwts_passed(fw,
-				    "Subtable %d (offset 0x%x) of "
-				    "type %d (%s) is the correct length: %d",
-				    ii, offset, hdr->type,
-				    madt_sub_names[type],
-				    hdr->length);
-		} else {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				    "SPECMADTSubLen",
-				    "Subtable %d (offset 0x%x) of "
-				    "type %d (%s) is %d bytes "
-				    "long but should be %d bytes",
-				    ii, offset, hdr->type,
-				    madt_sub_names[type],
-				    hdr->length, len);
+			if (passed) {
+				fwts_passed(fw,
+					    "Subtable %d (offset 0x%x) of "
+					    "type %d (%s) is the correct length: %d",
+					    ii, offset, hdr->type,
+					    madt_sub_names[type],
+					    hdr->length);
+			} else {
+				fwts_failed(fw, LOG_LEVEL_MEDIUM,
+					    "SPECMADTSubLen",
+					    "Subtable %d (offset 0x%x) of "
+					    "type %d (%s) is %d bytes "
+					    "long but should be %d bytes",
+					    ii, offset, hdr->type,
+					    madt_sub_names[type],
+					    hdr->length, len);
+			}
 		}
 
 		/* perform checks specific to subtable types */
@@ -1573,8 +1586,10 @@  static int madt_subtables(fwts_framework *fw)
 		length -= skip;
 	}
 
-	/* run comparison tests */
-	madt_ioapic_sapic_compare(fw, num_ioapics, num_iosapics);
+	if (!(fw->flags & FWTS_FLAG_TEST_SBBR)) {
+		/* run comparison tests */
+		madt_ioapic_sapic_compare(fw, num_ioapics, num_iosapics);
+	}
 
 	return FWTS_OK;
 }
@@ -1607,6 +1622,6 @@  static fwts_framework_ops madt_ops = {
 	.minor_tests = madt_tests
 };
 
-FWTS_REGISTER("madt", &madt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI | FWTS_FLAG_TEST_COMPLIANCE_ACPI)
+FWTS_REGISTER("madt", &madt_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_TEST_ACPI | FWTS_FLAG_TEST_COMPLIANCE_ACPI | FWTS_FLAG_TEST_SBBR)
 
 #endif