Re: [PATCH] mm, kasan: don't poison boot memory

On 2/28/2021 1:08 PM, Mike Rapoport wrote:

On Fri, Feb 26, 2021 at 11:16:06AM -0500, George Kennedy wrote:

quoted

On 2/26/2021 6:17 AM, Mike Rapoport wrote:

quoted

Hi George,

On Thu, Feb 25, 2021 at 08:19:18PM -0500, George Kennedy wrote:

quoted

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.

quoted

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.

I think something like this should work, but I'm not an ACPI expert to say
if this the best way to reserve the tables.

Adding ACPI maintainers to the CC list.

quoted

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?

It should be ok to memblock_reserve() all the tables very early as long as
we don't run out of static entries in memblock.reserved.

We just need to make sure the tables are reserved before memblock
allocations are possible, so we'd still need to move acpi_table_init() in
x86::setup_arch() before e820__memblock_setup().
Not sure how early ACPI is initialized on arm64.

Thanks Mike. Will try to move the memblock_reserves() before 
e820__memblock_setup().

George

quoted

Ran 10 boots with the above without failure.

George