Hi Alex, Ricardo,
Thank you both for the reviews!
On 20/06/2022 09:53, Alexandru Elisei wrote:
Hi,
On Fri, Jun 17, 2022 at 05:38:01PM -0700, Ricardo Koller wrote:
Hi Nikos,
On Fri, May 06, 2022 at 09:55:45PM +0100, Nikos Nikoleris wrote:
XSDT provides pointers to other ACPI tables much like RSDT. However,
contrary to RSDT that provides 32-bit addresses, XSDT provides 64-bit
pointers. ACPI requires that if XSDT is valid then it takes precedence
over RSDT.
Signed-off-by: Nikos Nikoleris <nikos.nikoleris@xxxxxxx>
---
lib/acpi.h | 6 ++++
lib/acpi.c | 103 ++++++++++++++++++++++++++++++++---------------------
2 files changed, 68 insertions(+), 41 deletions(-)
diff --git a/lib/acpi.h b/lib/acpi.h
index 42a2c16..d80b983 100644
--- a/lib/acpi.h
+++ b/lib/acpi.h
@@ -13,6 +13,7 @@
#define RSDP_SIGNATURE ACPI_SIGNATURE('R','S','D','P')
#define RSDT_SIGNATURE ACPI_SIGNATURE('R','S','D','T')
+#define XSDT_SIGNATURE ACPI_SIGNATURE('X','S','D','T')
#define FACP_SIGNATURE ACPI_SIGNATURE('F','A','C','P')
#define FACS_SIGNATURE ACPI_SIGNATURE('F','A','C','S')
@@ -56,6 +57,11 @@ struct rsdt_descriptor_rev1 {
u32 table_offset_entry[0];
} __attribute__ ((packed));
+struct acpi_table_xsdt {
+ ACPI_TABLE_HEADER_DEF
+ u64 table_offset_entry[1];
nit: This should be "[0]" to match the usage above (in rsdt).
I was about to suggest using an unspecified size "[]", but after reading
what the C standard says about it (below), now I'm not sure. was the
"[1]" needed for something that I'm missing?
106) The length is unspecified to allow for the fact that
implementations may give array members different
alignments according to their lengths.
GCC prefers "flexible array members" (array[]) [1]. Linux has deprecated
the use of zero-length arrays [2]. The kernel docs do make a pretty good
case for flexible array members.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://elixir.bootlin.com/linux/v5.18/source/Documentation/process/deprecated.rst#L234
Happy to change this, I don't have a strong preference. To be consistent
with RSDT we would have to declare:
u64 table_offset_entry[0];
But it might be better to change RSDT as well. Linux kernel declares:
u64 table_offset_entry[1];
but it seems, we would rather have:
u64 table_offset_entry[];
For alignment, we shouldn't be relying on the length specifier, all
structs in <acpi.h> should be packed.
Thanks,
Nikos
