| Message ID | 1502107452-24898-1-git-send-email-Sakar.Arora@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
On 2017-08-07 05:04 AM, Sakar Arora wrote: > 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 rsdp acpi. > > Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com> > Signed-off-by: Sakar Arora <Sakar.Arora@arm.com> > --- > src/Makefile.am | 1 + > src/sbbr/rsdp/rsdp.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 136 insertions(+) > create mode 100644 src/sbbr/rsdp/rsdp.c > > diff --git a/src/Makefile.am b/src/Makefile.am > index ca0a449..bd38841 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -97,6 +97,7 @@ fwts_SOURCES = main.c \ > acpi/plddump/plddump.c \ > acpi/pmtt/pmtt.c \ > acpi/rsdp/rsdp.c \ > + sbbr/rsdp/rsdp.c \ > acpi/rsdt/rsdt.c \ > acpi/s3/s3.c \ > acpi/s3power/s3power.c \ > diff --git a/src/sbbr/rsdp/rsdp.c b/src/sbbr/rsdp/rsdp.c > new file mode 100644 > index 0000000..611330d > --- /dev/null > +++ b/src/sbbr/rsdp/rsdp.c > @@ -0,0 +1,135 @@ > +/* > + * Copyright (C) 2015-2017 Canonical > + * Copyright (C) 2017 ARM Ltd > + * > + * Portions of this code original from the Linux-ready Firmware Developer Kit > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > +#include "fwts.h" > + > +#if defined(FWTS_HAS_ACPI) > + > +#include <stdlib.h> > +#include <stdio.h> > +#include <unistd.h> > +#include <inttypes.h> > +#include <string.h> > +#include <ctype.h> > + > +#define CHECKSUM_BYTES 20 > +#define SBBR_RSDP_REVISION 2 > +#define SBBR_RSDP_LENGTH 36 > +#define EXT_CHECKSUM_BYTES 36 > + > +static fwts_acpi_table_info *table; > + > +static int rsdp_sbbr_init(fwts_framework *fw) > +{ > + if (fwts_acpi_find_table(fw, "RSDP", 0, &table) != FWTS_OK) { > + fwts_log_error(fw, "Cannot read ACPI tables."); > + return FWTS_ERROR; > + } > + if (!table) { > + > + fwts_log_error(fw, > + "ACPI RSDP is required for the " > + "%s target architecture.", > + fwts_arch_get_name(fw->target_arch)); > + return FWTS_ERROR; > + } > + > + /* We know there is an RSDP now, so do a quick sanity check */ > + if (table->length == 0) { > + fwts_log_error(fw, > + "ACPI RSDP table has zero length"); > + return FWTS_ERROR; > + } > + return FWTS_OK; > +} > + > +/* > + * RSDP Root System Description Pointer > + */ > +static int rsdp_sbbr_test1(fwts_framework *fw) > +{ > + fwts_acpi_table_rsdp *rsdp = (fwts_acpi_table_rsdp *)table->data; > + uint8_t checksum; > + > + /*This includes only the first 20 bytes of this table, bytes > + 0 to 19, including the checksum field. These bytes must sum to > + zero. */ > + const char RSDP_SIGNATURE[] = {'R', 'S', 'D', ' ', 'P', 'T', 'R', ' '}; > + bool signature_pass = false; > + bool checksum_pass = false; > + bool rsdp_revision_pass = false; > + bool rsdp_length_pass = false; > + bool ext_checksum_pass = false; > + bool xsdt_address_pass = false; > + > + fwts_log_info(fw, "RSDP Signature = %.8s", rsdp->signature); > + signature_pass = strncmp(rsdp->signature, RSDP_SIGNATURE, > + sizeof(rsdp->signature))? false : true; > + > + /* verify first checksum */ > + checksum = fwts_checksum(table->data, CHECKSUM_BYTES); > + fwts_log_info(fw, "RSDP Checksum = 0x%x", checksum); > + checksum_pass = (checksum == 0)? true : false; > + > + fwts_log_info(fw, "RSDP Revision = 0x%x", rsdp->revision); > + rsdp_revision_pass = (rsdp->revision >= SBBR_RSDP_REVISION)? true : false; > + > + fwts_log_info(fw, "RSDP Length = 0x%x", rsdp->length); > + rsdp_length_pass = (rsdp->length == SBBR_RSDP_LENGTH)? true : false; > + > + checksum = fwts_checksum(table->data, EXT_CHECKSUM_BYTES); > + fwts_log_info(fw, "RSDP Extended Checksum = 0x%x", checksum); > + ext_checksum_pass = (checksum == 0)? true : false; > + > + if ((rsdp->xsdt_address != 0) && (rsdp->rsdt_address == 0)) > + { > + xsdt_address_pass = true; > + } > + > + if ((signature_pass == true) && (checksum_pass == true) && > + (rsdp_revision_pass == true) && (rsdp_length_pass == true) && > + (ext_checksum_pass == true) && (xsdt_address_pass == true)) > + { > + fwts_passed(fw, > + "SBBR RSDP: Structure of RSDP Table is consistent with " > + "ACPI 6.0 or later and uses 64 bit xsdt addresses."); > + } > + else > + { > + fwts_failed(fw, LOG_LEVEL_CRITICAL, > + "SBBR RSDP:", > + "Structure of RSDP Table is not consistent with ACPI 6.0 " > + "or later and/or does not use 64 bit xsdt addresses."); > + } > + > + return FWTS_OK; > +} > + > +static fwts_framework_minor_test rsdp_sbbr_tests[] = { > + { rsdp_sbbr_test1, "RSDP Root System Description Pointer test." }, > + { NULL, NULL } > +}; > + > +static fwts_framework_ops rsdp_sbbr_ops = { > + .description = "SBBR RSDP Root System Description Pointer tests.", > + .init = rsdp_sbbr_init, > + .minor_tests = rsdp_sbbr_tests > +}; > + > +FWTS_REGISTER("rsdp_sbbr", &rsdp_sbbr_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_TEST_SBBR) > + > +#endif > Acked-by: Alex Hung <alex.hung@canonical.com>
On 10/08/17 02:18, Alex Hung wrote: > On 2017-08-07 05:04 AM, Sakar Arora wrote: >> 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 rsdp acpi. >> >> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com> >> Signed-off-by: Sakar Arora <Sakar.Arora@arm.com> >> --- >> src/Makefile.am | 1 + >> src/sbbr/rsdp/rsdp.c | 135 >> +++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 136 insertions(+) >> create mode 100644 src/sbbr/rsdp/rsdp.c >> >> diff --git a/src/Makefile.am b/src/Makefile.am >> index ca0a449..bd38841 100644 >> --- a/src/Makefile.am >> +++ b/src/Makefile.am >> @@ -97,6 +97,7 @@ fwts_SOURCES = main.c \ >> acpi/plddump/plddump.c \ >> acpi/pmtt/pmtt.c \ >> acpi/rsdp/rsdp.c \ >> + sbbr/rsdp/rsdp.c \ >> acpi/rsdt/rsdt.c \ >> acpi/s3/s3.c \ >> acpi/s3power/s3power.c \ >> diff --git a/src/sbbr/rsdp/rsdp.c b/src/sbbr/rsdp/rsdp.c >> new file mode 100644 >> index 0000000..611330d >> --- /dev/null >> +++ b/src/sbbr/rsdp/rsdp.c >> @@ -0,0 +1,135 @@ >> +/* >> + * Copyright (C) 2015-2017 Canonical >> + * Copyright (C) 2017 ARM Ltd >> + * >> + * Portions of this code original from the Linux-ready Firmware >> Developer Kit >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License >> + * as published by the Free Software Foundation; either version 2 >> + * of the License, or (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + */ >> +#include "fwts.h" >> + >> +#if defined(FWTS_HAS_ACPI) >> + >> +#include <stdlib.h> >> +#include <stdio.h> >> +#include <unistd.h> >> +#include <inttypes.h> >> +#include <string.h> >> +#include <ctype.h> >> + >> +#define CHECKSUM_BYTES 20 >> +#define SBBR_RSDP_REVISION 2 >> +#define SBBR_RSDP_LENGTH 36 >> +#define EXT_CHECKSUM_BYTES 36 I'd prefer tabbed spacing between macro and the value: #define CHECKSUM_BYTES 20 #define SBBR_RSDP_REVISION 2 #define SBBR_RSDP_LENGTH 36 #define EXT_CHECKSUM_BYTES 36 >> + >> +static fwts_acpi_table_info *table; >> + >> +static int rsdp_sbbr_init(fwts_framework *fw) >> +{ >> + if (fwts_acpi_find_table(fw, "RSDP", 0, &table) != FWTS_OK) { >> + fwts_log_error(fw, "Cannot read ACPI tables."); >> + return FWTS_ERROR; >> + } >> + if (!table) { >> + Can the blank line be removed? >> + fwts_log_error(fw, >> + "ACPI RSDP is required for the " >> + "%s target architecture.", >> + fwts_arch_get_name(fw->target_arch)); >> + return FWTS_ERROR; >> + } >> + >> + /* We know there is an RSDP now, so do a quick sanity check */ >> + if (table->length == 0) { >> + fwts_log_error(fw, >> + "ACPI RSDP table has zero length"); >> + return FWTS_ERROR; >> + } >> + return FWTS_OK; >> +} >> + >> +/* >> + * RSDP Root System Description Pointer >> + */ >> +static int rsdp_sbbr_test1(fwts_framework *fw) >> +{ >> + fwts_acpi_table_rsdp *rsdp = (fwts_acpi_table_rsdp *)table->data; >> + uint8_t checksum; >> + >> + /*This includes only the first 20 bytes of this table, bytes >> + 0 to 19, including the checksum field. These bytes must sum to >> + zero. */ I'd prefer fwts style block comments: /* * This includes only the first 20 bytes of this table, bytes * 0 to 19, including the checksum field. These bytes must sum * to zero. */ >> + const char RSDP_SIGNATURE[] = {'R', 'S', 'D', ' ', 'P', 'T', >> 'R', ' '}; >> + bool signature_pass = false; >> + bool checksum_pass = false; >> + bool rsdp_revision_pass = false; >> + bool rsdp_length_pass = false; >> + bool ext_checksum_pass = false; >> + bool xsdt_address_pass = false; >> + >> + fwts_log_info(fw, "RSDP Signature = %.8s", rsdp->signature); >> + signature_pass = strncmp(rsdp->signature, RSDP_SIGNATURE, >> + sizeof(rsdp->signature))? false : true; Space between ) and ? please sizeof(rsdp->signature)) ? false : true; >> + >> + /* verify first checksum */ >> + checksum = fwts_checksum(table->data, CHECKSUM_BYTES); >> + fwts_log_info(fw, "RSDP Checksum = 0x%x", checksum); >> + checksum_pass = (checksum == 0)? true : false; Space between ) and ? please >> + >> + fwts_log_info(fw, "RSDP Revision = 0x%x", rsdp->revision); >> + rsdp_revision_pass = (rsdp->revision >= SBBR_RSDP_REVISION)? true >> : false; Space between ) and ? please >> + >> + fwts_log_info(fw, "RSDP Length = 0x%x", rsdp->length); >> + rsdp_length_pass = (rsdp->length == SBBR_RSDP_LENGTH)? true : false; >> + Space between ) and ? please >> + checksum = fwts_checksum(table->data, EXT_CHECKSUM_BYTES); >> + fwts_log_info(fw, "RSDP Extended Checksum = 0x%x", checksum); >> + ext_checksum_pass = (checksum == 0)? true : false; >> + >> + if ((rsdp->xsdt_address != 0) && (rsdp->rsdt_address == 0)) >> + { >> + xsdt_address_pass = true; >> + } You may as well make the above: if ((rsdp->xsdt_address != 0) && (rsdp->rsdt_address == 0)) xsdt_address_pass = true; >> + >> + if ((signature_pass == true) && (checksum_pass == true) && >> + (rsdp_revision_pass == true) && (rsdp_length_pass == true) && >> + (ext_checksum_pass == true) && (xsdt_address_pass == true)) >> + { { bracket at the end of the previous line please >> + fwts_passed(fw, >> + "SBBR RSDP: Structure of RSDP Table is consistent with " >> + "ACPI 6.0 or later and uses 64 bit xsdt addresses."); >> + } >> + else >> + { >> + fwts_failed(fw, LOG_LEVEL_CRITICAL, >> + "SBBR RSDP:", >> + "Structure of RSDP Table is not consistent with ACPI 6.0 " >> + "or later and/or does not use 64 bit xsdt addresses."); >> + } if (condition) { do something } else { do something else } K&R style in fwts if/else statements please. >> + >> + return FWTS_OK; >> +} >> + >> +static fwts_framework_minor_test rsdp_sbbr_tests[] = { >> + { rsdp_sbbr_test1, "RSDP Root System Description Pointer test." }, >> + { NULL, NULL } >> +}; >> + >> +static fwts_framework_ops rsdp_sbbr_ops = { >> + .description = "SBBR RSDP Root System Description Pointer tests.", >> + .init = rsdp_sbbr_init, >> + .minor_tests = rsdp_sbbr_tests >> +}; >> + >> +FWTS_REGISTER("rsdp_sbbr", &rsdp_sbbr_ops, FWTS_TEST_ANYTIME, >> FWTS_FLAG_TEST_SBBR) >> + >> +#endif >> > > Acked-by: Alex Hung <alex.hung@canonical.com> >
diff --git a/src/Makefile.am b/src/Makefile.am index ca0a449..bd38841 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -97,6 +97,7 @@ fwts_SOURCES = main.c \ acpi/plddump/plddump.c \ acpi/pmtt/pmtt.c \ acpi/rsdp/rsdp.c \ + sbbr/rsdp/rsdp.c \ acpi/rsdt/rsdt.c \ acpi/s3/s3.c \ acpi/s3power/s3power.c \ diff --git a/src/sbbr/rsdp/rsdp.c b/src/sbbr/rsdp/rsdp.c new file mode 100644 index 0000000..611330d --- /dev/null +++ b/src/sbbr/rsdp/rsdp.c @@ -0,0 +1,135 @@ +/* + * Copyright (C) 2015-2017 Canonical + * Copyright (C) 2017 ARM Ltd + * + * Portions of this code original from the Linux-ready Firmware Developer Kit + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ +#include "fwts.h" + +#if defined(FWTS_HAS_ACPI) + +#include <stdlib.h> +#include <stdio.h> +#include <unistd.h> +#include <inttypes.h> +#include <string.h> +#include <ctype.h> + +#define CHECKSUM_BYTES 20 +#define SBBR_RSDP_REVISION 2 +#define SBBR_RSDP_LENGTH 36 +#define EXT_CHECKSUM_BYTES 36 + +static fwts_acpi_table_info *table; + +static int rsdp_sbbr_init(fwts_framework *fw) +{ + if (fwts_acpi_find_table(fw, "RSDP", 0, &table) != FWTS_OK) { + fwts_log_error(fw, "Cannot read ACPI tables."); + return FWTS_ERROR; + } + if (!table) { + + fwts_log_error(fw, + "ACPI RSDP is required for the " + "%s target architecture.", + fwts_arch_get_name(fw->target_arch)); + return FWTS_ERROR; + } + + /* We know there is an RSDP now, so do a quick sanity check */ + if (table->length == 0) { + fwts_log_error(fw, + "ACPI RSDP table has zero length"); + return FWTS_ERROR; + } + return FWTS_OK; +} + +/* + * RSDP Root System Description Pointer + */ +static int rsdp_sbbr_test1(fwts_framework *fw) +{ + fwts_acpi_table_rsdp *rsdp = (fwts_acpi_table_rsdp *)table->data; + uint8_t checksum; + + /*This includes only the first 20 bytes of this table, bytes + 0 to 19, including the checksum field. These bytes must sum to + zero. */ + const char RSDP_SIGNATURE[] = {'R', 'S', 'D', ' ', 'P', 'T', 'R', ' '}; + bool signature_pass = false; + bool checksum_pass = false; + bool rsdp_revision_pass = false; + bool rsdp_length_pass = false; + bool ext_checksum_pass = false; + bool xsdt_address_pass = false; + + fwts_log_info(fw, "RSDP Signature = %.8s", rsdp->signature); + signature_pass = strncmp(rsdp->signature, RSDP_SIGNATURE, + sizeof(rsdp->signature))? false : true; + + /* verify first checksum */ + checksum = fwts_checksum(table->data, CHECKSUM_BYTES); + fwts_log_info(fw, "RSDP Checksum = 0x%x", checksum); + checksum_pass = (checksum == 0)? true : false; + + fwts_log_info(fw, "RSDP Revision = 0x%x", rsdp->revision); + rsdp_revision_pass = (rsdp->revision >= SBBR_RSDP_REVISION)? true : false; + + fwts_log_info(fw, "RSDP Length = 0x%x", rsdp->length); + rsdp_length_pass = (rsdp->length == SBBR_RSDP_LENGTH)? true : false; + + checksum = fwts_checksum(table->data, EXT_CHECKSUM_BYTES); + fwts_log_info(fw, "RSDP Extended Checksum = 0x%x", checksum); + ext_checksum_pass = (checksum == 0)? true : false; + + if ((rsdp->xsdt_address != 0) && (rsdp->rsdt_address == 0)) + { + xsdt_address_pass = true; + } + + if ((signature_pass == true) && (checksum_pass == true) && + (rsdp_revision_pass == true) && (rsdp_length_pass == true) && + (ext_checksum_pass == true) && (xsdt_address_pass == true)) + { + fwts_passed(fw, + "SBBR RSDP: Structure of RSDP Table is consistent with " + "ACPI 6.0 or later and uses 64 bit xsdt addresses."); + } + else + { + fwts_failed(fw, LOG_LEVEL_CRITICAL, + "SBBR RSDP:", + "Structure of RSDP Table is not consistent with ACPI 6.0 " + "or later and/or does not use 64 bit xsdt addresses."); + } + + return FWTS_OK; +} + +static fwts_framework_minor_test rsdp_sbbr_tests[] = { + { rsdp_sbbr_test1, "RSDP Root System Description Pointer test." }, + { NULL, NULL } +}; + +static fwts_framework_ops rsdp_sbbr_ops = { + .description = "SBBR RSDP Root System Description Pointer tests.", + .init = rsdp_sbbr_init, + .minor_tests = rsdp_sbbr_tests +}; + +FWTS_REGISTER("rsdp_sbbr", &rsdp_sbbr_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_TEST_SBBR) + +#endif