Re: [PATCH] mm, kasan: don't poison boot memory
From: George Kennedy
Date: Fri Feb 26 2021 - 11:18:25 EST
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