LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] ACPI / bus: ignore rather than fail bus driver registrations on non-ACPI boot
@ 2018-04-19 16:58 Ard Biesheuvel
  2018-04-22  9:27 ` Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2018-04-19 16:58 UTC (permalink / raw)
  To: rjw
  Cc: lenb, linux-acpi, linux-kernel, arnd, broonie, lorenzo.pieralisi,
	bill.fletcher, linux-arm-kernel, Ard Biesheuvel

When building ACPI bus drivers such as button.ko into the core kernel,
other drivers that depend on its symbols are loadable even when booting
with ACPI disabled. For instance, nouveau.ko has a link time dependency
on acpi_lid_open() on ACPI capable kernels, and calls it regardless of
whether the system booted via ACPI.

However, when building button.ko as a module, it will refuse to load if
the system did not boot in ACPI mode, which subsequently prevents the
nouveau driver from loading as well, resulting in broken graphics.

Given that returning an error from an initcall() is ignored for drivers
that are built into the kernel, let's align the module case with this,
and not return an error when registering an ACPI bus driver on a system
that did not boot via ACPI.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/acpi/bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 84b4a62018eb..529d3d496fab 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -880,7 +880,7 @@ int acpi_bus_register_driver(struct acpi_driver *driver)
 	int ret;
 
 	if (acpi_disabled)
-		return -ENODEV;
+		return 0;
 	driver->drv.name = driver->name;
 	driver->drv.bus = &acpi_bus_type;
 	driver->drv.owner = driver->owner;
-- 
2.17.0

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

* Re: [PATCH] ACPI / bus: ignore rather than fail bus driver registrations on non-ACPI boot
  2018-04-19 16:58 [PATCH] ACPI / bus: ignore rather than fail bus driver registrations on non-ACPI boot Ard Biesheuvel
@ 2018-04-22  9:27 ` Rafael J. Wysocki
  2018-04-22  9:57   ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2018-04-22  9:27 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List,
	Linux Kernel Mailing List, Arnd Bergmann, Mark Brown,
	Lorenzo Pieralisi, bill.fletcher, linux-arm-kernel

On Thu, Apr 19, 2018 at 6:58 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> When building ACPI bus drivers such as button.ko into the core kernel,
> other drivers that depend on its symbols are loadable even when booting
> with ACPI disabled. For instance, nouveau.ko has a link time dependency
> on acpi_lid_open() on ACPI capable kernels, and calls it regardless of
> whether the system booted via ACPI.
>
> However, when building button.ko as a module, it will refuse to load if
> the system did not boot in ACPI mode, which subsequently prevents the
> nouveau driver from loading as well, resulting in broken graphics.
>
> Given that returning an error from an initcall() is ignored for drivers
> that are built into the kernel,

Which makes sense, because they are present in the kernel anyway.

> let's align the module case with this,
> and not return an error when registering an ACPI bus driver on a system
> that did not boot via ACPI.

But why is loading a module that's never going to be used actually OK?

Isn't this a problem with the assumptions made by the nouveau driver
that need not be met depending on what configuration the kernel is run
in?

Honestly, it doesn't appear quite right to try to change the rest of
the kernel to follow the nouveau's expectations.

Thanks,
Rafael

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

* Re: [PATCH] ACPI / bus: ignore rather than fail bus driver registrations on non-ACPI boot
  2018-04-22  9:27 ` Rafael J. Wysocki
@ 2018-04-22  9:57   ` Ard Biesheuvel
  2018-04-22 12:34     ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2018-04-22  9:57 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List,
	Linux Kernel Mailing List, Arnd Bergmann, Mark Brown,
	Lorenzo Pieralisi, Bill Fletcher, linux-arm-kernel

On 22 April 2018 at 11:27, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Thu, Apr 19, 2018 at 6:58 PM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> When building ACPI bus drivers such as button.ko into the core kernel,
>> other drivers that depend on its symbols are loadable even when booting
>> with ACPI disabled. For instance, nouveau.ko has a link time dependency
>> on acpi_lid_open() on ACPI capable kernels, and calls it regardless of
>> whether the system booted via ACPI.
>>
>> However, when building button.ko as a module, it will refuse to load if
>> the system did not boot in ACPI mode, which subsequently prevents the
>> nouveau driver from loading as well, resulting in broken graphics.
>>
>> Given that returning an error from an initcall() is ignored for drivers
>> that are built into the kernel,
>
> Which makes sense, because they are present in the kernel anyway.
>
>> let's align the module case with this,
>> and not return an error when registering an ACPI bus driver on a system
>> that did not boot via ACPI.
>
> But why is loading a module that's never going to be used actually OK?
>
> Isn't this a problem with the assumptions made by the nouveau driver
> that need not be met depending on what configuration the kernel is run
> in?
>
> Honestly, it doesn't appear quite right to try to change the rest of
> the kernel to follow the nouveau's expectations.
>

I don't disagree here, I am just unsure whether other options are any better.

I think the alternative is to make acpi_lid_open() a non-modular
function of the ACPI core that invokes the button ACPI bus driver if
it was loaded, and always returns false otherwise. Would that work for
you?

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

* Re: [PATCH] ACPI / bus: ignore rather than fail bus driver registrations on non-ACPI boot
  2018-04-22  9:57   ` Ard Biesheuvel
@ 2018-04-22 12:34     ` Ard Biesheuvel
  2018-04-23  7:39       ` Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2018-04-22 12:34 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List,
	Linux Kernel Mailing List, Arnd Bergmann, Mark Brown,
	Lorenzo Pieralisi, Bill Fletcher, linux-arm-kernel

On 22 April 2018 at 11:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 22 April 2018 at 11:27, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Thu, Apr 19, 2018 at 6:58 PM, Ard Biesheuvel
>> <ard.biesheuvel@linaro.org> wrote:
>>> When building ACPI bus drivers such as button.ko into the core kernel,
>>> other drivers that depend on its symbols are loadable even when booting
>>> with ACPI disabled. For instance, nouveau.ko has a link time dependency
>>> on acpi_lid_open() on ACPI capable kernels, and calls it regardless of
>>> whether the system booted via ACPI.
>>>
>>> However, when building button.ko as a module, it will refuse to load if
>>> the system did not boot in ACPI mode, which subsequently prevents the
>>> nouveau driver from loading as well, resulting in broken graphics.
>>>
>>> Given that returning an error from an initcall() is ignored for drivers
>>> that are built into the kernel,
>>
>> Which makes sense, because they are present in the kernel anyway.
>>
>>> let's align the module case with this,
>>> and not return an error when registering an ACPI bus driver on a system
>>> that did not boot via ACPI.
>>
>> But why is loading a module that's never going to be used actually OK?
>>
>> Isn't this a problem with the assumptions made by the nouveau driver
>> that need not be met depending on what configuration the kernel is run
>> in?
>>
>> Honestly, it doesn't appear quite right to try to change the rest of
>> the kernel to follow the nouveau's expectations.
>>
>
> I don't disagree here, I am just unsure whether other options are any better.
>
> I think the alternative is to make acpi_lid_open() a non-modular
> function of the ACPI core that invokes the button ACPI bus driver if
> it was loaded, and always returns false otherwise. Would that work for
> you?

BTW not only nouveau invokes acpi_lid_open(), i915 does it as well.

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

* Re: [PATCH] ACPI / bus: ignore rather than fail bus driver registrations on non-ACPI boot
  2018-04-22 12:34     ` Ard Biesheuvel
@ 2018-04-23  7:39       ` Rafael J. Wysocki
  2018-04-23  7:40         ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2018-04-23  7:39 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List,
	Linux Kernel Mailing List, Arnd Bergmann, Mark Brown,
	Lorenzo Pieralisi, Bill Fletcher, linux-arm-kernel

On Sunday, April 22, 2018 2:34:17 PM CEST Ard Biesheuvel wrote:
> On 22 April 2018 at 11:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 22 April 2018 at 11:27, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >> On Thu, Apr 19, 2018 at 6:58 PM, Ard Biesheuvel
> >> <ard.biesheuvel@linaro.org> wrote:
> >>> When building ACPI bus drivers such as button.ko into the core kernel,
> >>> other drivers that depend on its symbols are loadable even when booting
> >>> with ACPI disabled. For instance, nouveau.ko has a link time dependency
> >>> on acpi_lid_open() on ACPI capable kernels, and calls it regardless of
> >>> whether the system booted via ACPI.
> >>>
> >>> However, when building button.ko as a module, it will refuse to load if
> >>> the system did not boot in ACPI mode, which subsequently prevents the
> >>> nouveau driver from loading as well, resulting in broken graphics.
> >>>
> >>> Given that returning an error from an initcall() is ignored for drivers
> >>> that are built into the kernel,
> >>
> >> Which makes sense, because they are present in the kernel anyway.
> >>
> >>> let's align the module case with this,
> >>> and not return an error when registering an ACPI bus driver on a system
> >>> that did not boot via ACPI.
> >>
> >> But why is loading a module that's never going to be used actually OK?
> >>
> >> Isn't this a problem with the assumptions made by the nouveau driver
> >> that need not be met depending on what configuration the kernel is run
> >> in?
> >>
> >> Honestly, it doesn't appear quite right to try to change the rest of
> >> the kernel to follow the nouveau's expectations.
> >>
> >
> > I don't disagree here, I am just unsure whether other options are any better.
> >
> > I think the alternative is to make acpi_lid_open() a non-modular
> > function of the ACPI core that invokes the button ACPI bus driver if
> > it was loaded, and always returns false otherwise. Would that work for
> > you?
> 
> BTW not only nouveau invokes acpi_lid_open(), i915 does it as well.

Clearly, the design is somewhat ad-hoc here.

It looks like using module_acpi_driver() in button.c is a mistake given the
dependencies.  The module initialization should ignore the
acpi_bus_register_driver() failure in there, but there's no reason for the
other ACPI driver modules to be affected by that.

And if you change that, please add a comment referring to the dependencies
in question.

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

* Re: [PATCH] ACPI / bus: ignore rather than fail bus driver registrations on non-ACPI boot
  2018-04-23  7:39       ` Rafael J. Wysocki
@ 2018-04-23  7:40         ` Ard Biesheuvel
  0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2018-04-23  7:40 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List,
	Linux Kernel Mailing List, Arnd Bergmann, Mark Brown,
	Lorenzo Pieralisi, Bill Fletcher, linux-arm-kernel

On 23 April 2018 at 09:39, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Sunday, April 22, 2018 2:34:17 PM CEST Ard Biesheuvel wrote:
>> On 22 April 2018 at 11:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > On 22 April 2018 at 11:27, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> >> On Thu, Apr 19, 2018 at 6:58 PM, Ard Biesheuvel
>> >> <ard.biesheuvel@linaro.org> wrote:
>> >>> When building ACPI bus drivers such as button.ko into the core kernel,
>> >>> other drivers that depend on its symbols are loadable even when booting
>> >>> with ACPI disabled. For instance, nouveau.ko has a link time dependency
>> >>> on acpi_lid_open() on ACPI capable kernels, and calls it regardless of
>> >>> whether the system booted via ACPI.
>> >>>
>> >>> However, when building button.ko as a module, it will refuse to load if
>> >>> the system did not boot in ACPI mode, which subsequently prevents the
>> >>> nouveau driver from loading as well, resulting in broken graphics.
>> >>>
>> >>> Given that returning an error from an initcall() is ignored for drivers
>> >>> that are built into the kernel,
>> >>
>> >> Which makes sense, because they are present in the kernel anyway.
>> >>
>> >>> let's align the module case with this,
>> >>> and not return an error when registering an ACPI bus driver on a system
>> >>> that did not boot via ACPI.
>> >>
>> >> But why is loading a module that's never going to be used actually OK?
>> >>
>> >> Isn't this a problem with the assumptions made by the nouveau driver
>> >> that need not be met depending on what configuration the kernel is run
>> >> in?
>> >>
>> >> Honestly, it doesn't appear quite right to try to change the rest of
>> >> the kernel to follow the nouveau's expectations.
>> >>
>> >
>> > I don't disagree here, I am just unsure whether other options are any better.
>> >
>> > I think the alternative is to make acpi_lid_open() a non-modular
>> > function of the ACPI core that invokes the button ACPI bus driver if
>> > it was loaded, and always returns false otherwise. Would that work for
>> > you?
>>
>> BTW not only nouveau invokes acpi_lid_open(), i915 does it as well.
>
> Clearly, the design is somewhat ad-hoc here.
>
> It looks like using module_acpi_driver() in button.c is a mistake given the
> dependencies.  The module initialization should ignore the
> acpi_bus_register_driver() failure in there, but there's no reason for the
> other ACPI driver modules to be affected by that.
>
> And if you change that, please add a comment referring to the dependencies
> in question.
>

OK, will do.

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

end of thread, other threads:[~2018-04-23  7:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-19 16:58 [PATCH] ACPI / bus: ignore rather than fail bus driver registrations on non-ACPI boot Ard Biesheuvel
2018-04-22  9:27 ` Rafael J. Wysocki
2018-04-22  9:57   ` Ard Biesheuvel
2018-04-22 12:34     ` Ard Biesheuvel
2018-04-23  7:39       ` Rafael J. Wysocki
2018-04-23  7:40         ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).