Messages in this thread | | | Date | Fri, 3 Aug 2018 10:08:45 +0800 | From | Chao Fan <> | Subject | Re: [PATCH v4 2/4] x86/boot: Add acpitb.c to parse acpi tables |
| |
On Fri, Aug 03, 2018 at 10:00:48AM +0800, Dou Liyang wrote: > > >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.
That's a good idea, will add more comments.
> >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.
I will check the logical and fix the mistake.
Thanks, Chao Fan
> >Thanks, > dou
| |