Messages in this thread Patch in this message | | | Subject | Re: [PATCH] mm, kasan: don't poison boot memory | From | George Kennedy <> | Date | Fri, 26 Feb 2021 11:16:06 -0500 |
| |
Hi Mike,
On 2/26/2021 6:17 AM, Mike Rapoport wrote: > Hi George, > > On Thu, Feb 25, 2021 at 08:19:18PM -0500, George Kennedy wrote: >> Mike, >> >> To get rid of the 0x00000000BE453000 hardcoding, I added the following patch >> to your above patch to get the iBFT table "address" to use with >> memblock_reserve(): >> >> diff --git a/drivers/acpi/acpica/tbfind.c b/drivers/acpi/acpica/tbfind.c >> index 56d81e4..4bc7bf3 100644 >> --- a/drivers/acpi/acpica/tbfind.c >> +++ b/drivers/acpi/acpica/tbfind.c >> @@ -120,3 +120,34 @@ >> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES); >> return_ACPI_STATUS(status); >> } >> + >> +acpi_physical_address >> +acpi_tb_find_table_address(char *signature) >> +{ >> + acpi_physical_address address = 0; >> + struct acpi_table_desc *table_desc; >> + int i; >> + >> + ACPI_FUNCTION_TRACE(tb_find_table_address); >> + >> +printk(KERN_ERR "XXX acpi_tb_find_table_address: signature=%s\n", >> signature); >> + >> + (void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES); >> + for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) { >> + if (memcmp(&(acpi_gbl_root_table_list.tables[i].signature), >> + signature, ACPI_NAMESEG_SIZE)) { >> + >> + /* Not the requested table */ >> + >> + continue; >> + } >> + >> + /* Table with matching signature has been found */ >> + table_desc = &acpi_gbl_root_table_list.tables[i]; >> + address = table_desc->address; >> + } >> + >> + (void)acpi_ut_release_mutex(ACPI_MTX_TABLES); >> +printk(KERN_ERR "XXX acpi_tb_find_table_address(EXIT): address=%llx\n", >> address); >> + return address; >> +} >> diff --git a/drivers/firmware/iscsi_ibft_find.c >> b/drivers/firmware/iscsi_ibft_find.c >> index 95fc1a6..0de70b4 100644 >> --- a/drivers/firmware/iscsi_ibft_find.c >> +++ b/drivers/firmware/iscsi_ibft_find.c >> @@ -28,6 +28,8 @@ >> >> #include <asm/mmzone.h> >> >> +extern acpi_physical_address acpi_tb_find_table_address(char *signature); >> + >> /* >> * Physical location of iSCSI Boot Format Table. >> */ >> @@ -116,24 +118,32 @@ void __init reserve_ibft_region(void) >> { >> struct acpi_table_ibft *table; >> unsigned long size; >> + acpi_physical_address address; >> >> table = find_ibft(); >> if (!table) >> return; >> >> size = PAGE_ALIGN(table->header.length); >> + address = acpi_tb_find_table_address(table->header.signature); >> #if 0 >> printk(KERN_ERR "XXX reserve_ibft_region: table=%llx, >> virt_to_phys(table)=%llx, size=%lx\n", >> (u64)table, virt_to_phys(table), size); >> memblock_reserve(virt_to_phys(table), size); >> #else >> -printk(KERN_ERR "XXX reserve_ibft_region: table=%llx, 0x00000000BE453000, >> size=%lx\n", >> - (u64)table, size); >> - memblock_reserve(0x00000000BE453000, size); >> +printk(KERN_ERR "XXX reserve_ibft_region: table=%llx, address=%llx, >> size=%lx\n", >> + (u64)table, address, size); >> + if (address) >> + memblock_reserve(address, size); >> + else >> + printk(KERN_ERR "%s: Can't find table address\n", __func__); >> #endif >> >> - if (efi_enabled(EFI_BOOT)) >> + if (efi_enabled(EFI_BOOT)) { >> +printk(KERN_ERR "XXX reserve_ibft_region: calling acpi_put_table(%llx)\n", >> (u64)&table->header); >> acpi_put_table(&table->header); >> - else >> + } else { >> ibft_addr = table; >> +printk(KERN_ERR "XXX reserve_ibft_region: ibft_addr=%llx\n", >> (u64)ibft_addr); >> + } >> } >> >> Debug from the above: >> [ 0.050646] ACPI: Early table checksum verification disabled >> [ 0.051778] ACPI: RSDP 0x00000000BFBFA014 000024 (v02 BOCHS ) >> [ 0.052922] ACPI: XSDT 0x00000000BFBF90E8 00004C (v01 BOCHS BXPCFACP >> 00000001 01000013) >> [ 0.054623] ACPI: FACP 0x00000000BFBF5000 000074 (v01 BOCHS BXPCFACP >> 00000001 BXPC 00000001) >> [ 0.056326] ACPI: DSDT 0x00000000BFBF6000 00238D (v01 BOCHS BXPCDSDT >> 00000001 BXPC 00000001) >> [ 0.058016] ACPI: FACS 0x00000000BFBFD000 000040 >> [ 0.058940] ACPI: APIC 0x00000000BFBF4000 000090 (v01 BOCHS BXPCAPIC >> 00000001 BXPC 00000001) >> [ 0.060627] ACPI: HPET 0x00000000BFBF3000 000038 (v01 BOCHS BXPCHPET >> 00000001 BXPC 00000001) >> [ 0.062304] ACPI: BGRT 0x00000000BE49B000 000038 (v01 INTEL EDK2 >> 00000002 01000013) >> [ 0.063987] ACPI: iBFT 0x00000000BE453000 000800 (v01 BOCHS BXPCFACP >> 00000000 00000000) >> [ 0.065683] XXX acpi_tb_find_table_address: signature=iBFT >> [ 0.066754] XXX acpi_tb_find_table_address(EXIT): address=be453000 >> [ 0.067959] XXX reserve_ibft_region: table=ffffffffff240000, >> address=be453000, size=1000 >> [ 0.069534] XXX reserve_ibft_region: calling >> acpi_put_table(ffffffffff240000) >> >> Not sure if it's the right thing to do, but added >> "acpi_tb_find_table_address()" to return the physical address of a table to >> use with memblock_reserve(). >> >> virt_to_phys(table) does not seem to return the physical address for the >> iBFT table (it would be nice if struct acpi_table_header also had a >> "address" element for the physical address of the table). > virt_to_phys() does not work that early because then it is mapped with > early_memremap() which uses different virtual to physical scheme. > > I'd say that acpi_tb_find_table_address() makes sense if we'd like to > reserve ACPI tables outside of drivers/acpi. > > But probably we should simply reserve all the tables during > acpi_table_init() so that any table that firmware put in the normal memory > will be surely reserved. > >> Ran 10 successful boots with the above without failure. > That's good news indeed :)
Wondering if we could do something like this instead (trying to keep changes minimal). Just do the memblock_reserve() for all the standard tables.
diff --git a/drivers/acpi/acpica/tbinstal.c b/drivers/acpi/acpica/tbinstal.c index 0bb15ad..830f82c 100644 --- a/drivers/acpi/acpica/tbinstal.c +++ b/drivers/acpi/acpica/tbinstal.c @@ -7,6 +7,7 @@ * *****************************************************************************/
+#include <linux/memblock.h> #include <acpi/acpi.h> #include "accommon.h" #include "actables.h" @@ -14,6 +15,23 @@ #define _COMPONENT ACPI_TABLES ACPI_MODULE_NAME("tbinstal")
+void +acpi_tb_reserve_standard_table(acpi_physical_address address, + struct acpi_table_header *header) +{ + struct acpi_table_header local_header; + + if ((ACPI_COMPARE_NAMESEG(header->signature, ACPI_SIG_FACS)) || + (ACPI_VALIDATE_RSDP_SIG(header->signature))) { + return; + } + /* Standard ACPI table with full common header */ + + memcpy(&local_header, header, sizeof(struct acpi_table_header)); + + memblock_reserve(address, PAGE_ALIGN(local_header.length)); +} + /******************************************************************************* * * FUNCTION: acpi_tb_install_table_with_override @@ -58,6 +76,9 @@ new_table_desc->flags, new_table_desc->pointer);
+ acpi_tb_reserve_standard_table(new_table_desc->address, + new_table_desc->pointer); + acpi_tb_print_table_header(new_table_desc->address, new_table_desc->pointer);
There should be no harm in doing the memblock_reserve() for all the standard tables, right? Ran 10 boots with the above without failure.
George >> George >>>
| |