On 3/1/2021 9:29 AM, George Kennedy wrote:

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

quoted

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

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().

Hi Mike,

Moved acpi_table_init() in x86::setup_arch() before 
e820__memblock_setup() as you suggested.

Ran 10 boots with the following without error.

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 740f3bdb..3b1dd24 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c

@@ -1047,6 +1047,7 @@ void __init setup_arch(char **cmdline_p)
      cleanup_highmap();

      memblock_set_current_limit(ISA_END_ADDRESS);
+    acpi_boot_table_init();
      e820__memblock_setup();

      /*

@@ -1140,8 +1141,6 @@ void __init setup_arch(char **cmdline_p)
      /*
       * Parse the ACPI tables for possible boot-time SMP configuration.
       */
-    acpi_boot_table_init();
-
      early_acpi_boot_init();

      initmem_init();

diff --git a/drivers/acpi/acpica/tbinstal.c b/drivers/acpi/acpica/tbinstal.c
index 0bb15ad..7830109 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"

@@ -16,6 +17,33 @@

  /*******************************************************************************
   *
+ * FUNCTION:    acpi_tb_reserve_standard_table
+ *
+ * PARAMETERS:  address             - Table physical address
+ *              header              - Table header
+ *
+ * RETURN:      None
+ *
+ * DESCRIPTION: To avoid an acpi table page from being "stolen" by the 

buddy
+ *              allocator run memblock_reserve() on all the standard 
acpi tables.
+ *
+ 
******************************************************************************/
+void
+acpi_tb_reserve_standard_table(acpi_physical_address address,
+               struct acpi_table_header *header)
+{
+    if ((ACPI_COMPARE_NAMESEG(header->signature, ACPI_SIG_FACS)) ||
+        (ACPI_VALIDATE_RSDP_SIG(header->signature)))
+        return;
+
+    if (header->length > PAGE_SIZE) /* same check as in acpi_map() */
+        return;
+
+    memblock_reserve(address, PAGE_ALIGN(header->length));
+}
+
+/*******************************************************************************
+ *
   * FUNCTION:    acpi_tb_install_table_with_override
   *
   * PARAMETERS:  new_table_desc          - New table descriptor to install

@@ -58,6 +86,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);

George

George

quoted

quoted

Ran 10 boots with the above without failure.

George

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel