Re: [PATCH v4 2/4] x86/boot: Add acpitb.c to parse acpi tables

From: Dou Liyang
Date: Thu Aug 02 2018 - 22:01:04 EST




At 07/23/2018 05:29 PM, Chao Fan wrote:
Imitate the ACPI code to parse ACPI tables. Functions are simplified
cause some operations are not needed here.
And also, this method won't influence the initialization of ACPI.

Signed-off-by: Chao Fan <fanc.fnst@xxxxxxxxxxxxxx>

Hi Fan,

I know you got the code from acpica subsystem and EFI code... and do
many adaptation work for KASLR. It's awesome!

I think you can add some other simple comments.

- what differences between your function and the function you based on
and why did you do that?

... to make this more credible and easy to remember the details as time
goes on.

Also some concerns below.
---
[...]
+ else if (!strncmp(sig, EFI32_LOADER_SIGNATURE, 4))
+ efi_64 = false;
+ else {
+ debug_putstr("Wrong efi loader signature.\n");

s/efi/EFI/, also need fix in the comments below.

+ return false;
+ }
+
[...]
+ /*
+ * Get rsdp from efi tables.
+ * If we find acpi table, go on searching for acpi20 table.
+ * If we didn't get acpi20 table then use acpi table.
+ * If neither acpi table nor acpi20 table found,
+ * return false.
+ */
+ if (!(efi_guidcmp(guid, ACPI_TABLE_GUID)) && !acpi_20) {
+ *rsdp_addr = (acpi_physical_address)table;
+ acpi_20 = false;
+ find_rsdp = true;
+ } else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) {
+ *rsdp_addr = (acpi_physical_address)table;
+ acpi_20 = true;
+ return true;

If we find the ACPI 2.0, we will return immediately, so the variable and
logic of _acpi_20_ is redundant.

Thanks,
dou