lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 3 Aug 2018 10:08:45 +0800 From: Chao Fan <fanc.fnst@...fujitsu.com> To: Dou Liyang <douly.fnst@...fujitsu.com> CC: <linux-kernel@...r.kernel.org>, <x86@...nel.org>, <hpa@...or.com>, <tglx@...utronix.de>, <mingo@...hat.com>, <bhe@...hat.com>, <keescook@...omium.org>, <yasu.isimatu@...il.com>, <indou.takao@...fujitsu.com>, <caoj.fnst@...fujitsu.com>, <fanc.fnst@...fujitsu.com> 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@...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
Powered by blists - more mailing lists