2022-07-13 18:12:35

by Mario Limonciello

[permalink] [raw]
Subject: [PATCH] pinctrl: Don't allow PINCTRL_AMD to be a module

It was observed that by allowing pinctrl_amd to be loaded
later in the boot process that interrupts sent to the GPIO
controller early in the boot are not serviced. The kernel treats
these as a spurious IRQ and disables the IRQ.

This problem was exacerbated because it happened on a system with
an encrypted partition so the kernel object was not accesssible for
an extended period of time while waiting for a passphrase.

To avoid this situation from occurring, stop allowing pinctrl-amd
from being built as a module and instead require it to be built-in
or disabled.

Reported-by: [email protected]
Suggested-by: [email protected]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216230
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/pinctrl/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index f52960d2dfbe..bff144c97e66 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -32,7 +32,7 @@ config DEBUG_PINCTRL
Say Y here to add some extra checks and diagnostics to PINCTRL calls.

config PINCTRL_AMD
- tristate "AMD GPIO pin control"
+ bool "AMD GPIO pin control"
depends on HAS_IOMEM
depends on ACPI || COMPILE_TEST
select GPIOLIB
--
2.34.1


2022-07-13 21:38:12

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH] pinctrl: Don't allow PINCTRL_AMD to be a module

Hi,

On 7/13/22 19:59, Mario Limonciello wrote:
> It was observed that by allowing pinctrl_amd to be loaded
> later in the boot process that interrupts sent to the GPIO
> controller early in the boot are not serviced. The kernel treats
> these as a spurious IRQ and disables the IRQ.
>
> This problem was exacerbated because it happened on a system with
> an encrypted partition so the kernel object was not accesssible for
> an extended period of time while waiting for a passphrase.
>
> To avoid this situation from occurring, stop allowing pinctrl-amd
> from being built as a module and instead require it to be built-in
> or disabled.
>
> Reported-by: [email protected]
> Suggested-by: [email protected]

Note I generally use: "Hans de Goede <[email protected]>" for all
things kernel related.

This looks good to me. Fedora is already building in amd-pinctrl
which AFAIK is done because of similar issues to the reason given
in the commit message:

Acked-by: Hans de Goede <[email protected]>

Regards,

Hans



> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216230
> Signed-off-by: Mario Limonciello <[email protected]>
> ---
> drivers/pinctrl/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index f52960d2dfbe..bff144c97e66 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -32,7 +32,7 @@ config DEBUG_PINCTRL
> Say Y here to add some extra checks and diagnostics to PINCTRL calls.
>
> config PINCTRL_AMD
> - tristate "AMD GPIO pin control"
> + bool "AMD GPIO pin control"
> depends on HAS_IOMEM
> depends on ACPI || COMPILE_TEST
> select GPIOLIB

2022-07-13 22:06:10

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH] pinctrl: Don't allow PINCTRL_AMD to be a module

[Public]



> -----Original Message-----
> From: Hans de Goede <[email protected]>
> Sent: Wednesday, July 13, 2022 16:25
> To: Limonciello, Mario <[email protected]>; Linus Walleij
> <[email protected]>
> Cc: Natikar, Basavaraj <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH] pinctrl: Don't allow PINCTRL_AMD to be a module
>
> Hi,
>
> On 7/13/22 19:59, Mario Limonciello wrote:
> > It was observed that by allowing pinctrl_amd to be loaded
> > later in the boot process that interrupts sent to the GPIO
> > controller early in the boot are not serviced. The kernel treats
> > these as a spurious IRQ and disables the IRQ.
> >
> > This problem was exacerbated because it happened on a system with
> > an encrypted partition so the kernel object was not accesssible for
> > an extended period of time while waiting for a passphrase.
> >
> > To avoid this situation from occurring, stop allowing pinctrl-amd
> > from being built as a module and instead require it to be built-in
> > or disabled.
> >
> > Reported-by: [email protected]
> > Suggested-by: [email protected]
>
> Note I generally use: "Hans de Goede <[email protected]>" for all
> things kernel related.

Sorry about that, I just took the email from Kernel Bugzilla when I copied/pasted
and didn't realize they were different.
Will try to remember that for the future.

>
> This looks good to me. Fedora is already building in amd-pinctrl
> which AFAIK is done because of similar issues to the reason given
> in the commit message:
>
> Acked-by: Hans de Goede <[email protected]>

Thanks!

FWIW I poked around and at least Ubuntu and RHEL are both enabling it
as built in as well, presumably big distros setting this up by default may be
why this issue hasn't been very widespread.

Linus W,
BTW I do have a suspicion that this might be the same root cause as
https://bugzilla.kernel.org/show_bug.cgi?id=201817
but we'll need someone from that bug report to chime in to confirm.

>
> Regards,
>
> Hans
>
>
>
> > Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> illa.kernel.org%2Fshow_bug.cgi%3Fid%3D216230&amp;data=05%7C01%7Cm
> ario.limonciello%40amd.com%7Cc28ab57936e1427e7a7a08da65162319%7C3d
> d8961fe4884e608e11a82d994e183d%7C0%7C0%7C637933443003195561%7CU
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> 6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=XJB0L03z%2
> B3mVBZxdlX6q6MBanZQwymoQSwlmMCrPjcs%3D&amp;reserved=0
> > Signed-off-by: Mario Limonciello <[email protected]>
> > ---
> > drivers/pinctrl/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> > index f52960d2dfbe..bff144c97e66 100644
> > --- a/drivers/pinctrl/Kconfig
> > +++ b/drivers/pinctrl/Kconfig
> > @@ -32,7 +32,7 @@ config DEBUG_PINCTRL
> > Say Y here to add some extra checks and diagnostics to PINCTRL
> calls.
> >
> > config PINCTRL_AMD
> > - tristate "AMD GPIO pin control"
> > + bool "AMD GPIO pin control"
> > depends on HAS_IOMEM
> > depends on ACPI || COMPILE_TEST
> > select GPIOLIB

2022-07-18 11:07:12

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] pinctrl: Don't allow PINCTRL_AMD to be a module

On Wed, Jul 13, 2022 at 8:00 PM Mario Limonciello
<[email protected]> wrote:

> It was observed that by allowing pinctrl_amd to be loaded
> later in the boot process that interrupts sent to the GPIO
> controller early in the boot are not serviced. The kernel treats
> these as a spurious IRQ and disables the IRQ.
>
> This problem was exacerbated because it happened on a system with
> an encrypted partition so the kernel object was not accesssible for
> an extended period of time while waiting for a passphrase.
>
> To avoid this situation from occurring, stop allowing pinctrl-amd
> from being built as a module and instead require it to be built-in
> or disabled.
>
> Reported-by: [email protected]
> Suggested-by: [email protected]
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216230
> Signed-off-by: Mario Limonciello <[email protected]>

Patch applied for fixes!
Excellent root causing.

Yours,
Linus Walleij