Messages in this thread | | | From | Dou Liyang <> | Subject | Re: [PATCH v4 2/4] x86/boot: Add acpitb.c to parse acpi tables | Date | Fri, 3 Aug 2018 10:00:48 +0800 |
| |
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@cn.fujitsu.com>
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
| |