Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
* [5.12 regression] DSDT overriding from initrd no longer works
@ 2021-04-12 17:38 Hans de Goede
  2021-04-12 18:01 ` Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2021-04-12 17:38 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: linux-acpi

Hi Rafael,

Sorry about the timing of reporting this regression.

I just noticed that overriding the DSDT (*) from the initrd will not work in 5.12,
this is caused by:

commit 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by ACPI tables")

This makes the initial acpi_locate_initial_tables() call happen earlier
then before, but the acpi_table_upgrade) call in arch/x86/kernel/setup.c is
not moved up, so the tables in the initrd are now only parsed and saved
after the initial ACPI table scanning has already been done.

I guess fixing this might be as easy as moving the acpi_table_upgrade) call
higher in arch/x86/kernel/setup.c but I'm not sure if that is save to do.

I've several devices with ACPI tables which are so broken that they need
an override. I've reverted the commit from my personal tree for now which
avoids the regression.

Regards,

Hans


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [5.12 regression] DSDT overriding from initrd no longer works
  2021-04-12 17:38 [5.12 regression] DSDT overriding from initrd no longer works Hans de Goede
@ 2021-04-12 18:01 ` Rafael J. Wysocki
  2021-04-12 18:11   ` Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2021-04-12 18:01 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Rafael J. Wysocki, linux-acpi

On Mon, Apr 12, 2021 at 7:38 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Rafael,
>
> Sorry about the timing of reporting this regression.

Oh well.

> I just noticed that overriding the DSDT (*) from the initrd will not work in 5.12,
> this is caused by:
>
> commit 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by ACPI tables")
>
> This makes the initial acpi_locate_initial_tables() call happen earlier
> then before, but the acpi_table_upgrade) call in arch/x86/kernel/setup.c is
> not moved up, so the tables in the initrd are now only parsed and saved
> after the initial ACPI table scanning has already been done.
>
> I guess fixing this might be as easy as moving the acpi_table_upgrade) call
> higher in arch/x86/kernel/setup.c but I'm not sure if that is save to do.

Why do you think it may not be safe?

Have you tried to do that?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [5.12 regression] DSDT overriding from initrd no longer works
  2021-04-12 18:01 ` Rafael J. Wysocki
@ 2021-04-12 18:11   ` Rafael J. Wysocki
  2021-04-12 18:53     ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2021-04-12 18:11 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Hans de Goede, Rafael J. Wysocki, linux-acpi

On Mon, Apr 12, 2021 at 8:01 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Mon, Apr 12, 2021 at 7:38 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >
> > Hi Rafael,
> >
> > Sorry about the timing of reporting this regression.
>
> Oh well.
>
> > I just noticed that overriding the DSDT (*) from the initrd will not work in 5.12,
> > this is caused by:
> >
> > commit 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by ACPI tables")
> >
> > This makes the initial acpi_locate_initial_tables() call happen earlier
> > then before, but the acpi_table_upgrade) call in arch/x86/kernel/setup.c is
> > not moved up, so the tables in the initrd are now only parsed and saved
> > after the initial ACPI table scanning has already been done.
> >
> > I guess fixing this might be as easy as moving the acpi_table_upgrade) call
> > higher in arch/x86/kernel/setup.c but I'm not sure if that is save to do.
>
> Why do you think it may not be safe?

OK, so it won't work in some cases, because acpi_table_upgrade() needs
to be called after reserve_initrd(), so I guess the commit above will
need to be reverted.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [5.12 regression] DSDT overriding from initrd no longer works
  2021-04-12 18:11   ` Rafael J. Wysocki
@ 2021-04-12 18:53     ` Hans de Goede
  2021-04-13  9:35       ` Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2021-04-12 18:53 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Rafael J. Wysocki, linux-acpi

Hi,

On 4/12/21 8:11 PM, Rafael J. Wysocki wrote:
> On Mon, Apr 12, 2021 at 8:01 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>
>> On Mon, Apr 12, 2021 at 7:38 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi Rafael,
>>>
>>> Sorry about the timing of reporting this regression.
>>
>> Oh well.
>>
>>> I just noticed that overriding the DSDT (*) from the initrd will not work in 5.12,
>>> this is caused by:
>>>
>>> commit 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by ACPI tables")
>>>
>>> This makes the initial acpi_locate_initial_tables() call happen earlier
>>> then before, but the acpi_table_upgrade) call in arch/x86/kernel/setup.c is
>>> not moved up, so the tables in the initrd are now only parsed and saved
>>> after the initial ACPI table scanning has already been done.
>>>
>>> I guess fixing this might be as easy as moving the acpi_table_upgrade) call
>>> higher in arch/x86/kernel/setup.c but I'm not sure if that is save to do.
>>
>> Why do you think it may not be safe?
> 
> OK, so it won't work in some cases, because acpi_table_upgrade() needs
> to be called after reserve_initrd(),

Right I notice it was sitting right after reserve_initrd() which made me think
that it probably needed to be after that. Sorry I should have mentioned that
in my original email.

> so I guess the commit above will
> need to be reverted.

One possible solution which I was wondering about is to modify
acpi_table_initrd_scan() to have it call acpi_tb_override_table()
instead of acpi_install_table() for existing tables using the matching
logic from acpi_table_initrd_override(). But I'm not sure when the
parsing of the DSDT is done. If acpi_table_initrd_scan() runs before
the first parsing of the DSDT is done then I think that that should work.

This might be more 5.13 material though and for 5.12 a revert is
probably best.

I also just remembered that at least the Intel audio folks rely on
DSDT overrides to get some (prototype) boards in their CI to work.

Regards,

Hans



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [5.12 regression] DSDT overriding from initrd no longer works
  2021-04-12 18:53     ` Hans de Goede
@ 2021-04-13  9:35       ` Rafael J. Wysocki
  2021-04-13 10:52         ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2021-04-13  9:35 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Rafael J. Wysocki, Rafael J. Wysocki, linux-acpi


[-- Attachment #1: Type: text/plain, Size: 2280 bytes --]

On Mon, Apr 12, 2021 at 8:53 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 4/12/21 8:11 PM, Rafael J. Wysocki wrote:
> > On Mon, Apr 12, 2021 at 8:01 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >>
> >> On Mon, Apr 12, 2021 at 7:38 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>>
> >>> Hi Rafael,
> >>>
> >>> Sorry about the timing of reporting this regression.
> >>
> >> Oh well.
> >>
> >>> I just noticed that overriding the DSDT (*) from the initrd will not work in 5.12,
> >>> this is caused by:
> >>>
> >>> commit 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by ACPI tables")
> >>>
> >>> This makes the initial acpi_locate_initial_tables() call happen earlier
> >>> then before, but the acpi_table_upgrade) call in arch/x86/kernel/setup.c is
> >>> not moved up, so the tables in the initrd are now only parsed and saved
> >>> after the initial ACPI table scanning has already been done.
> >>>
> >>> I guess fixing this might be as easy as moving the acpi_table_upgrade) call
> >>> higher in arch/x86/kernel/setup.c but I'm not sure if that is save to do.
> >>
> >> Why do you think it may not be safe?
> >
> > OK, so it won't work in some cases, because acpi_table_upgrade() needs
> > to be called after reserve_initrd(),
>
> Right I notice it was sitting right after reserve_initrd() which made me think
> that it probably needed to be after that. Sorry I should have mentioned that
> in my original email.
>
> > so I guess the commit above will
> > need to be reverted.
>
> One possible solution which I was wondering about is to modify
> acpi_table_initrd_scan() to have it call acpi_tb_override_table()
> instead of acpi_install_table() for existing tables using the matching
> logic from acpi_table_initrd_override(). But I'm not sure when the
> parsing of the DSDT is done. If acpi_table_initrd_scan() runs before
> the first parsing of the DSDT is done then I think that that should work.
>
> This might be more 5.13 material though and for 5.12 a revert is
> probably best.

The attached change should make it work again, though.  Can you please verify?

> I also just remembered that at least the Intel audio folks rely on
> DSDT overrides to get some (prototype) boards in their CI to work.

But they haven't complained so far.

[-- Attachment #2: acpi-x86-ordering.patch --]
[-- Type: text/x-patch, Size: 756 bytes --]

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [5.12 regression] DSDT overriding from initrd no longer works
  2021-04-13  9:35       ` Rafael J. Wysocki
@ 2021-04-13 10:52         ` Hans de Goede
  0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2021-04-13 10:52 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Rafael J. Wysocki, linux-acpi

Hi Rafael,

On 4/13/21 11:35 AM, Rafael J. Wysocki wrote:
> On Mon, Apr 12, 2021 at 8:53 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 4/12/21 8:11 PM, Rafael J. Wysocki wrote:
>>> On Mon, Apr 12, 2021 at 8:01 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>>>
>>>> On Mon, Apr 12, 2021 at 7:38 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>
>>>>> Hi Rafael,
>>>>>
>>>>> Sorry about the timing of reporting this regression.
>>>>
>>>> Oh well.
>>>>
>>>>> I just noticed that overriding the DSDT (*) from the initrd will not work in 5.12,
>>>>> this is caused by:
>>>>>
>>>>> commit 1a1c130ab757 ("ACPI: tables: x86: Reserve memory occupied by ACPI tables")
>>>>>
>>>>> This makes the initial acpi_locate_initial_tables() call happen earlier
>>>>> then before, but the acpi_table_upgrade) call in arch/x86/kernel/setup.c is
>>>>> not moved up, so the tables in the initrd are now only parsed and saved
>>>>> after the initial ACPI table scanning has already been done.
>>>>>
>>>>> I guess fixing this might be as easy as moving the acpi_table_upgrade) call
>>>>> higher in arch/x86/kernel/setup.c but I'm not sure if that is save to do.
>>>>
>>>> Why do you think it may not be safe?
>>>
>>> OK, so it won't work in some cases, because acpi_table_upgrade() needs
>>> to be called after reserve_initrd(),
>>
>> Right I notice it was sitting right after reserve_initrd() which made me think
>> that it probably needed to be after that. Sorry I should have mentioned that
>> in my original email.
>>
>>> so I guess the commit above will
>>> need to be reverted.
>>
>> One possible solution which I was wondering about is to modify
>> acpi_table_initrd_scan() to have it call acpi_tb_override_table()
>> instead of acpi_install_table() for existing tables using the matching
>> logic from acpi_table_initrd_override(). But I'm not sure when the
>> parsing of the DSDT is done. If acpi_table_initrd_scan() runs before
>> the first parsing of the DSDT is done then I think that that should work.
>>
>> This might be more 5.13 material though and for 5.12 a revert is
>> probably best.
> 
> The attached change should make it work again, though.  Can you please verify?

I can confirm that the attached change fixes things, thank you.

>> I also just remembered that at least the Intel audio folks rely on
>> DSDT overrides to get some (prototype) boards in their CI to work.
> 
> But they haven't complained so far.

Weird, I'll drop them an email with you in the Cc.

Regards,

Hans


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12 17:38 [5.12 regression] DSDT overriding from initrd no longer works Hans de Goede
2021-04-12 18:01 ` Rafael J. Wysocki
2021-04-12 18:11   ` Rafael J. Wysocki
2021-04-12 18:53     ` Hans de Goede
2021-04-13  9:35       ` Rafael J. Wysocki
2021-04-13 10:52         ` Hans de Goede

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git