On 3/1/2021 9:29 AM, George Kennedy wrote:
On 2/28/2021 1:08 PM, Mike Rapoport wrote:
On Fri, Feb 26, 2021 at 11:16:06AM -0500, George Kennedy wrote:
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:
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.
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.
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
Ran 10 boots with the above without failure.
George
