On Wed, Apr 14, 2021 at 09:42:01AM +0200, David Hildenbrand wrote: > On 13.04.21 19:53, Rafael J. Wysocki wrote: > > On Tue, Apr 13, 2021 at 7:43 PM David Hildenbrand <da...@redhat.com> wrote: > > > > > > On 13.04.21 16:01, Rafael J. Wysocki wrote: > > > > From: Rafael J. Wysocki <rafael.j.wyso...@intel.com> > > > > > > > > Commit 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by > > > > ACPI tables") attempted to address an issue with reserving the memory > > > > occupied by ACPI tables, but it broke the initrd-based table override > > > > mechanism relied on by multiple users. > > > > > > > > To restore the initrd-based ACPI table override functionality, move > > > > the acpi_boot_table_init() invocation in setup_arch() on x86 after > > > > the acpi_table_upgrade() one. > > > > > > > > Fixes: 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by > > > > ACPI tables") > > > > Reported-by: Hans de Goede <hdego...@redhat.com> > > > > Tested-by: Hans de Goede <hdego...@redhat.com> > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wyso...@intel.com> > > > > --- > > > > > > > > George, can you please check if this reintroduces the issue addressed by > > > > the above commit for you? > > > > > > > > --- > > > > arch/x86/kernel/setup.c | 5 ++--- > > > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > > > > > Index: linux-pm/arch/x86/kernel/setup.c > > > > =================================================================== > > > > --- linux-pm.orig/arch/x86/kernel/setup.c > > > > +++ linux-pm/arch/x86/kernel/setup.c > > > > @@ -1045,9 +1045,6 @@ void __init setup_arch(char **cmdline_p) > > > > > > > > cleanup_highmap(); > > > > > > > > - /* Look for ACPI tables and reserve memory occupied by them. */ > > > > - acpi_boot_table_init(); > > > > - > > > > memblock_set_current_limit(ISA_END_ADDRESS); > > > > e820__memblock_setup(); > > > > > > > > @@ -1132,6 +1129,8 @@ void __init setup_arch(char **cmdline_p) > > > > reserve_initrd(); > > > > > > > > acpi_table_upgrade(); > > > > + /* Look for ACPI tables and reserve memory occupied by them. */ > > > > + acpi_boot_table_init(); > > > > > > > > vsmp_init(); > > > > > > This is fairly late; especially, it's after actual allocations -- see > > > e820__memblock_alloc_reserved_mpc_new(). > > > > > > Can't the table upgrade mechanism fix up when adjusting something? > > > > Not at this point of the cycle I'm afraid. > > > > > Some details on what actually breaks would be helpful. > > > > Generally speaking, the table overrides that come from the initrd are > > not taken into account if acpi_boot_table_init() runs before > > acpi_table_upgrade() and the latter cannot run before > > reserve_initrd(). > > I see. (looking at Documentation/acpi/initrd_table_override.txt I understand > what acpi table overrides are for :) ) > > > > > Honestly, I'm not sure how much effort it would take to untangle this ATM. > > Also true; ideally, we wouldn't have any allocations (find+reserve) before > ordinary reservations are done. > > However, I have no idea if we can move > e820__memblock_alloc_reserved_mpc_new() and reserve_real_mode() around > easily. Also, reserve_initrd()->relocate_initrd() does allocations.
Even if we can move e820__memblock_alloc_reserved_mpc_new() and reserve_real_mode(), the allocation in reserve_initrd() has to be before the tables override, we would only reduce the probability of allocating an ACPI page. I think what we can do is to override the ACPI tables separately from their initial parsing. Rafael, what do you say? > This is a mess. True :( -- Sincerely yours, Mike.