[PATCH 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[PATCH 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Christian Hesse]

This device suffer an irq storm, so add it in tpm_tis_dmi_table to
force polling.

https://bugs.archlinux.org/user/38129
https://bugzilla.kernel.org/show_bug.cgi?id=217631

Fixes: e644b2f498d297a928efcb7ff6f900c27f8b788e
Cc: stable@vger.kernel.org
Reported-by:  <roubro1991@gmail.com>
Signed-off-by: Christian Hesse <mail@eworm.de>
---
 drivers/char/tpm/tpm_tis.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 7db3593941ea..2979f8b9aaa0 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -114,6 +114,14 @@ static int tpm_tis_disable_irq(const struct dmi_system_id *d)
 }
 
 static const struct dmi_system_id tpm_tis_dmi_table[] = {
+	{
+		.callback = tpm_tis_disable_irq,
+		.ident = "Framework Laptop Intel 12th gen",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "A4"),
+		},
+	},
 	{
 		.callback = tpm_tis_disable_irq,
 		.ident = "ThinkPad T490s",
-- 
2.41.0

[PATCH 2/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 13th gen

[Christian Hesse]

This device suffer an irq storm, so add it in tpm_tis_dmi_table to
force polling.

https://bugs.archlinux.org/user/38129
https://bugzilla.kernel.org/show_bug.cgi?id=217631

Fixes: e644b2f498d297a928efcb7ff6f900c27f8b788e
Cc: stable@vger.kernel.org
Reported-by:  <roubro1991@gmail.com>
Signed-off-by: Christian Hesse <mail@eworm.de>
---
 drivers/char/tpm/tpm_tis.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 2979f8b9aaa0..a8bcd2134e6b 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -122,6 +122,14 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "A4"),
 		},
 	},
+	{
+		.callback = tpm_tis_disable_irq,
+		.ident = "Framework Laptop Intel 13th gen",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "A6"),
+		},
+	},
 	{
 		.callback = tpm_tis_disable_irq,
 		.ident = "ThinkPad T490s",
-- 
2.41.0

Re: [PATCH 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Linux regression tracking (Thorsten Leemhuis)]

Hi, Thorsten here, the Linux kernel's regression tracker. I'm not a
proper reviewer for patches like this, but nevertheless two small things:

On 10.07.23 15:38, Christian Hesse wrote:
> This device suffer an irq storm, so add it in tpm_tis_dmi_table to
> force polling.

Thx for working on this!

> https://bugs.archlinux.org/user/38129

Not sure why this link might be helpful. Did you maybe mean to include
another one? Like this one?

https://community.frame.work/t/boot-and-shutdown-hangs-with-arch-linux-kernel-6-4-1-mainline-and-arch/33118

> https://bugzilla.kernel.org/show_bug.cgi?id=217631
>
> Fixes: e644b2f498d297a928efcb7ff6f900c27f8b788e

This should be:

Fixes: e644b2f498d ("tpm, tpm_tis: Enable interrupt test")

> Cc: stable@vger.kernel.org
> Reported-by:  <roubro1991@gmail.com>

The bugzilla link from above above should be here like this instead:

Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217631

BTW, checkpatch.pl should have mentioned the two latter things.

HTH, Ciao, Thorsten

> Signed-off-by: Christian Hesse <mail@eworm.de>
> ---
>  drivers/char/tpm/tpm_tis.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 7db3593941ea..2979f8b9aaa0 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -114,6 +114,14 @@ static int tpm_tis_disable_irq(const struct dmi_system_id *d)
>  }
>  
>  static const struct dmi_system_id tpm_tis_dmi_table[] = {
> +	{
> +		.callback = tpm_tis_disable_irq,
> +		.ident = "Framework Laptop Intel 12th gen",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
> +			DMI_MATCH(DMI_PRODUCT_VERSION, "A4"),
> +		},
> +	},
>  	{
>  		.callback = tpm_tis_disable_irq,
>  		.ident = "ThinkPad T490s",

Re: [PATCH 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Greg KH]

On Mon, Jul 10, 2023 at 03:38:35PM +0200, Christian Hesse wrote:
> This device suffer an irq storm, so add it in tpm_tis_dmi_table to
> force polling.
> 
> https://bugs.archlinux.org/user/38129
> https://bugzilla.kernel.org/show_bug.cgi?id=217631

Add "Link:" tags?

> 
> Fixes: e644b2f498d297a928efcb7ff6f900c27f8b788e

As per the kernel documentation, this should be:

Fixes: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")

otherwise you're going to get some emails from the linux-next bot.

thanks,

greg k-h

[PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Christian Hesse]

This device suffer an irq storm, so add it in tpm_tis_dmi_table to
force polling.

Link: https://community.frame.work/t/boot-and-shutdown-hangs-with-arch-linux-kernel-6-4-1-mainline-and-arch/33118
Fixes: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Cc: stable@vger.kernel.org
Reported-by: <roubro1991@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217631
Signed-off-by: Christian Hesse <mail@eworm.de>
---
 drivers/char/tpm/tpm_tis.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 7db3593941ea..2979f8b9aaa0 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -114,6 +114,14 @@ static int tpm_tis_disable_irq(const struct dmi_system_id *d)
 }
 
 static const struct dmi_system_id tpm_tis_dmi_table[] = {
+	{
+		.callback = tpm_tis_disable_irq,
+		.ident = "Framework Laptop Intel 12th gen",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "A4"),
+		},
+	},
 	{
 		.callback = tpm_tis_disable_irq,
 		.ident = "ThinkPad T490s",
-- 
2.41.0

[PATCH v2 2/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 13th gen

[Christian Hesse]

This device suffer an irq storm, so add it in tpm_tis_dmi_table to
force polling.

Link: https://community.frame.work/t/boot-and-shutdown-hangs-with-arch-linux-kernel-6-4-1-mainline-and-arch/33118
Fixes: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Cc: stable@vger.kernel.org
Reported-by: <roubro1991@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217631
Signed-off-by: Christian Hesse <mail@eworm.de>
---
 drivers/char/tpm/tpm_tis.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 2979f8b9aaa0..a8bcd2134e6b 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -122,6 +122,14 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "A4"),
 		},
 	},
+	{
+		.callback = tpm_tis_disable_irq,
+		.ident = "Framework Laptop Intel 13th gen",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "A6"),
+		},
+	},
 	{
 		.callback = tpm_tis_disable_irq,
 		.ident = "ThinkPad T490s",
-- 
2.41.0

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Linux regression tracking (Thorsten Leemhuis)]

On 10.07.23 16:28, Christian Hesse wrote:
> This device suffer an irq storm, so add it in tpm_tis_dmi_table to
> force polling.
> 
> Link: https://community.frame.work/t/boot-and-shutdown-hangs-with-arch-linux-kernel-6-4-1-mainline-and-arch/33118
> Fixes: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
> Cc: stable@vger.kernel.org
> Reported-by: <roubro1991@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217631

Thx again for working on this. FWIW, a quote from a recent comment there
FYI:

```
 mapleleaf 2023-07-10 16:37:03 UTC

I have the same problem and I own a Framework 12th-gen, but for whatever
reason my DMI_PRODUCT_VERSION is A8 instead of A6...

$ sudo dmidecode -s baseboard-version
A8

[tag] [reply] [−]
Private
Comment 13 mapleleaf 2023-07-10 16:41:29 UTC

And also:

$ sudo dmidecode -s system-version
A8

```

> ---
>  drivers/char/tpm/tpm_tis.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 7db3593941ea..2979f8b9aaa0 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -114,6 +114,14 @@ static int tpm_tis_disable_irq(const struct dmi_system_id *d)
>  }
>  
>  static const struct dmi_system_id tpm_tis_dmi_table[] = {
> +	{
> +		.callback = tpm_tis_disable_irq,
> +		.ident = "Framework Laptop Intel 12th gen",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
> +			DMI_MATCH(DMI_PRODUCT_VERSION, "A4"),
> +		},
> +	},
>  	{
>  		.callback = tpm_tis_disable_irq,
>  		.ident = "ThinkPad T490s",

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jason Gunthorpe]

On Mon, Jul 10, 2023 at 04:28:43PM +0200, Christian Hesse wrote:
> This device suffer an irq storm, so add it in tpm_tis_dmi_table to
> force polling.

I can't help but feel like we are doing something wrong in the Linux
driver that we keep having IRQ problems.

Surely Windows uses the IRQ on these devices? How does it work
reliably there?

Jason

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Mon, 2023-07-10 at 15:01 -0300, Jason Gunthorpe wrote:
> On Mon, Jul 10, 2023 at 04:28:43PM +0200, Christian Hesse wrote:
> > This device suffer an irq storm, so add it in tpm_tis_dmi_table to
> > force polling.
> 
> I can't help but feel like we are doing something wrong in the Linux
> driver that we keep having IRQ problems.
> 
> Surely Windows uses the IRQ on these devices? How does it work
> reliably there?
> 
> Jason

I'm about to send a PR to Linus with a pile of IRQ fixes for v6.4
feature.

BR, Jarkko

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[James Bottomley]

On Mon, 2023-07-10 at 15:01 -0300, Jason Gunthorpe wrote:
> On Mon, Jul 10, 2023 at 04:28:43PM +0200, Christian Hesse wrote:
> > This device suffer an irq storm, so add it in tpm_tis_dmi_table to
> > force polling.
> 
> I can't help but feel like we are doing something wrong in the Linux
> driver that we keep having IRQ problems.
> 
> Surely Windows uses the IRQ on these devices? How does it work
> reliably there?

I've had some friends in the MS Linux team ask around to find out if
anyone in the windows group is willing to divulge a little on this.  My
suspicion is that if interrupts are enabled at all on Windows TIS, it's
via a whitelist.

James

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jerry Snitselaar]

On Mon, Jul 10, 2023 at 03:01:33PM -0300, Jason Gunthorpe wrote:
> On Mon, Jul 10, 2023 at 04:28:43PM +0200, Christian Hesse wrote:
> > This device suffer an irq storm, so add it in tpm_tis_dmi_table to
> > force polling.
> 
> I can't help but feel like we are doing something wrong in the Linux
> driver that we keep having IRQ problems.
> 
> Surely Windows uses the IRQ on these devices? How does it work
> reliably there?
> 
> Jason

I worry about it as well, especially as more vendors get added to the
list. On the other hand it seems like every 6-12 months I am
interacting with vendors that repeatedly forget to have reserved
memory regions added to IVRS and DMAR tables for devices that need
them. So I guess it is possible for the problem to be on their end as
well. For at least one case that someone looked at back in May, it
looked like he could see the issue looking at a schematic for the
system.

Regards,
Jerry

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Christian Hesse]

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

Christian Hesse <mail@eworm.de> on Mon, 2023/07/10 16:28:
> This device suffer an irq storm, so add it in tpm_tis_dmi_table to
> force polling.

Uh, looks like this is not quite right. The product version specifies the CPU
"level" across generations (for example "i5-1240P" vs. "i7-1260P" vs.
"i7-1280P"). So I guess we should match on the product name instead...

I will send an updated set of patches. Would be nice if anybody could verify
this...
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v3 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Christian Hesse]

This device suffer an irq storm, so add it in tpm_tis_dmi_table to
force polling.

Link: https://community.frame.work/t/boot-and-shutdown-hangs-with-arch-linux-kernel-6-4-1-mainline-and-arch/33118
Fixes: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Cc: stable@vger.kernel.org
Reported-by: <roubro1991@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217631
Signed-off-by: Christian Hesse <mail@eworm.de>
---
 drivers/char/tpm/tpm_tis.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 7db3593941ea..52bb8b642207 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -114,6 +114,14 @@ static int tpm_tis_disable_irq(const struct dmi_system_id *d)
 }
 
 static const struct dmi_system_id tpm_tis_dmi_table[] = {
+	{
+		.callback = tpm_tis_disable_irq,
+		.ident = "Framework Laptop (12th Gen Intel Core)",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Laptop (12th Gen Intel Core)"),
+		},
+	},
 	{
 		.callback = tpm_tis_disable_irq,
 		.ident = "ThinkPad T490s",
-- 
2.41.0

[PATCH v3 2/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 13th gen

[Christian Hesse]

This device suffer an irq storm, so add it in tpm_tis_dmi_table to
force polling.

Link: https://community.frame.work/t/boot-and-shutdown-hangs-with-arch-linux-kernel-6-4-1-mainline-and-arch/33118
Fixes: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Cc: stable@vger.kernel.org
Reported-by: <roubro1991@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217631
Signed-off-by: Christian Hesse <mail@eworm.de>
---
 drivers/char/tpm/tpm_tis.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 52bb8b642207..f8a8587c66f3 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -122,6 +122,14 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Laptop (12th Gen Intel Core)"),
 		},
 	},
+	{
+		.callback = tpm_tis_disable_irq,
+		.ident = "Framework Laptop (13th Gen Intel Core)",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Laptop (13th Gen Intel Core)"),
+		},
+	},
 	{
 		.callback = tpm_tis_disable_irq,
 		.ident = "ThinkPad T490s",
-- 
2.41.0

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Mon, 2023-07-10 at 23:13 +0200, Christian Hesse wrote:
> Christian Hesse <mail@eworm.de> on Mon, 2023/07/10 16:28:
> > This device suffer an irq storm, so add it in tpm_tis_dmi_table to
> > force polling.
> 
> Uh, looks like this is not quite right. The product version specifies the CPU
> "level" across generations (for example "i5-1240P" vs. "i7-1260P" vs.
> "i7-1280P"). So I guess we should match on the product name instead...
> 
> I will send an updated set of patches. Would be nice if anybody could verify
> this...

OK, this good to hear! I've been late with my pull request (past rc1)
because of kind of conflicting timing with Finnish holiday season and
relocating my home office.

I'll replace v2 patches with v3 and send the PR for rc2 after that.
So unluck turned into luck this time :-)

Thank you for spotting this!

BR, Jarkko

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Tue, 2023-07-11 at 00:29 +0300, Jarkko Sakkinen wrote:
> On Mon, 2023-07-10 at 23:13 +0200, Christian Hesse wrote:
> > Christian Hesse <mail@eworm.de> on Mon, 2023/07/10 16:28:
> > > This device suffer an irq storm, so add it in tpm_tis_dmi_table to
> > > force polling.
> > 
> > Uh, looks like this is not quite right. The product version specifies the CPU
> > "level" across generations (for example "i5-1240P" vs. "i7-1260P" vs.
> > "i7-1280P"). So I guess we should match on the product name instead...
> > 
> > I will send an updated set of patches. Would be nice if anybody could verify
> > this...
> 
> OK, this good to hear! I've been late with my pull request (past rc1)
> because of kind of conflicting timing with Finnish holiday season and
> relocating my home office.
> 
> I'll replace v2 patches with v3 and send the PR for rc2 after that.
> So unluck turned into luck this time :-)
> 
> Thank you for spotting this!

Please, sanity check before I send the PR for rc2:

https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/

BR, Jarkko

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Christian Hesse]

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

Jarkko Sakkinen <jarkko@kernel.org> on Tue, 2023/07/11 00:51:
> On Tue, 2023-07-11 at 00:29 +0300, Jarkko Sakkinen wrote:
> > OK, this good to hear! I've been late with my pull request (past rc1)
> > because of kind of conflicting timing with Finnish holiday season and
> > relocating my home office.
> > 
> > I'll replace v2 patches with v3 and send the PR for rc2 after that.
> > So unluck turned into luck this time :-)
> > 
> > Thank you for spotting this!  
> 
> Please, sanity check before I send the PR for rc2:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/

Looks good and works as expected on Framework Laptop 12th Gen, verified by me
and someone in the linked bugzilla ticket. I do not have Framework Laptop
13th Gen available for testing.

Looks like there are general workarounds by disabling interrupts after a
number of unhandled IRQs. Will this still go in?
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Wed Jul 12, 2023 at 6:48 AM UTC, Christian Hesse wrote:
> Jarkko Sakkinen <jarkko@kernel.org> on Tue, 2023/07/11 00:51:
> > On Tue, 2023-07-11 at 00:29 +0300, Jarkko Sakkinen wrote:
> > > OK, this good to hear! I've been late with my pull request (past rc1)
> > > because of kind of conflicting timing with Finnish holiday season and
> > > relocating my home office.
> > > 
> > > I'll replace v2 patches with v3 and send the PR for rc2 after that.
> > > So unluck turned into luck this time :-)
> > > 
> > > Thank you for spotting this!  
> > 
> > Please, sanity check before I send the PR for rc2:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/
>
> Looks good and works as expected on Framework Laptop 12th Gen, verified by me
> and someone in the linked bugzilla ticket. I do not have Framework Laptop
> 13th Gen available for testing.
>
> Looks like there are general workarounds by disabling interrupts after a
> number of unhandled IRQs. Will this still go in?
> -- 
> main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
> "CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
> putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

So I think it should, what is deny listed now will stay like that even
if Lino's patch is included.

BR, Jarkko

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Grundik]

On Tue, 2023-07-11 at 00:29 +0300, Jarkko Sakkinen wrote:
> On Mon, 2023-07-10 at 23:13 +0200, Christian Hesse wrote:
> 
> 
> OK, this good to hear! I've been late with my pull request (past rc1)
> because of kind of conflicting timing with Finnish holiday season and
> relocating my home office.
> 
> I'll replace v2 patches with v3 and send the PR for rc2 after that.
> So unluck turned into luck this time :-)
> 
> Thank you for spotting this!

I want to say: this issue is NOT limited to Framework laptops.

For example this MSI gen12 i5-1240P laptop also suffers from same
problem:
        Manufacturer: Micro-Star International Co., Ltd.
        Product Name: Summit E13FlipEvo A12MT
        Version: REV:1.0
        SKU Number: 13P3.1
        Family: Summit

So, probably just blacklisting affected models is not the best
solution...

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Linux regression tracking (Thorsten Leemhuis)]

On 11.07.23 14:41, Grundik wrote:
> On Tue, 2023-07-11 at 00:29 +0300, Jarkko Sakkinen wrote:
>> On Mon, 2023-07-10 at 23:13 +0200, Christian Hesse wrote:
>>
>>
>> OK, this good to hear! I've been late with my pull request (past rc1)
>> because of kind of conflicting timing with Finnish holiday season and
>> relocating my home office.
>>
>> I'll replace v2 patches with v3 and send the PR for rc2 after that.
>> So unluck turned into luck this time :-)
>>
>> Thank you for spotting this!
> 
> I want to say: this issue is NOT limited to Framework laptops.
> 
> For example this MSI gen12 i5-1240P laptop also suffers from same
> problem:
>         Manufacturer: Micro-Star International Co., Ltd.
>         Product Name: Summit E13FlipEvo A12MT
>         Version: REV:1.0
>         SKU Number: 13P3.1
>         Family: Summit
> 
> So, probably just blacklisting affected models is not the best
> solution...

Hmmm. Jarkko, you earlier in the thread said: "I'm about to send a PR to
Linus with a pile of IRQ fixes for v6.4 feature.". Could any of those
potentially help Grundik or those with the Framework laptop? Is that
worth testing?

Ciao, Thorsten

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Tue, 2023-07-11 at 15:41 +0300, Grundik wrote:
> On Tue, 2023-07-11 at 00:29 +0300, Jarkko Sakkinen wrote:
> > On Mon, 2023-07-10 at 23:13 +0200, Christian Hesse wrote:
> > 
> > 
> > OK, this good to hear! I've been late with my pull request (past rc1)
> > because of kind of conflicting timing with Finnish holiday season and
> > relocating my home office.
> > 
> > I'll replace v2 patches with v3 and send the PR for rc2 after that.
> > So unluck turned into luck this time :-)
> > 
> > Thank you for spotting this!
> 
> I want to say: this issue is NOT limited to Framework laptops.
> 
> For example this MSI gen12 i5-1240P laptop also suffers from same
> problem:
>         Manufacturer: Micro-Star International Co., Ltd.
>         Product Name: Summit E13FlipEvo A12MT
>         Version: REV:1.0
>         SKU Number: 13P3.1
>         Family: Summit
> 
> So, probably just blacklisting affected models is not the best
> solution...

It will be supplemented with

https://lore.kernel.org/linux-integrity/CTYXI8TL7C36.2SCWH82FAZWBO@suppilovahvero/T/#me895f1920ca6983f791b58a6fa0c157161a33849

Together they should fairly sustainable framework.

Lino, can you add the same fixes tag as for this. It would probably
ignore inline comments to keep the patch minimal since it is a
critical fix. Just do the renames, remove inline comments and
send v3.

For tpm_tis_check_for_interrupt_storm(), you can could rename it
simply as tpm_tis_update_unhandle_irqs() as that it what it does
(my review did not include a suggestion for this).

This way I think it should be fairly trivial to get a version that
can be landed.

To put short:
1. Do the renames as suggested, they are good enough for me.
2. Drop inline comments, their usefulness is somewhat questionable
   and they increase the diff.
3. Generally aim for minimal diff but I think this should be good
   enough if you do steps 1 and 2.

If you don't have the time at hand, I can carefully do these cleanups
and apply the patch. If you have the time and motivation, go ahead
and send v3.

BR, Jarkko

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Lino Sanfilippo]

Hi,

On 11.07.23 23:50, Jarkko Sakkinen wrote:
> On Tue, 2023-07-11 at 15:41 +0300, Grundik wrote:
>> On Tue, 2023-07-11 at 00:29 +0300, Jarkko Sakkinen wrote:
>>> On Mon, 2023-07-10 at 23:13 +0200, Christian Hesse wrote:
>>>
>>>
>>> OK, this good to hear! I've been late with my pull request (past rc1)
>>> because of kind of conflicting timing with Finnish holiday season and
>>> relocating my home office.
>>>
>>> I'll replace v2 patches with v3 and send the PR for rc2 after that.
>>> So unluck turned into luck this time :-)
>>>
>>> Thank you for spotting this!
>>
>> I want to say: this issue is NOT limited to Framework laptops.
>>
>> For example this MSI gen12 i5-1240P laptop also suffers from same
>> problem:
>>         Manufacturer: Micro-Star International Co., Ltd.
>>         Product Name: Summit E13FlipEvo A12MT
>>         Version: REV:1.0
>>         SKU Number: 13P3.1
>>         Family: Summit
>>
>> So, probably just blacklisting affected models is not the best
>> solution...
>
> It will be supplemented with
>
> https://lore.kernel.org/linux-integrity/CTYXI8TL7C36.2SCWH82FAZWBO@suppilovahvero/T/#me895f1920ca6983f791b58a6fa0c157161a33849
>
> Together they should fairly sustainable framework.
>
> Lino, can you add the same fixes tag as for this. It would probably
> ignore inline comments to keep the patch minimal since it is a
> critical fix. Just do the renames, remove inline comments and
> send v3.
>
> For tpm_tis_check_for_interrupt_storm(), you can could rename it
> simply as tpm_tis_update_unhandle_irqs() as that it what it does
> (my review did not include a suggestion for this).
>
> This way I think it should be fairly trivial to get a version that
> can be landed.
>
> To put short:
> 1. Do the renames as suggested, they are good enough for me.
> 2. Drop inline comments, their usefulness is somewhat questionable
>    and they increase the diff.
> 3. Generally aim for minimal diff but I think this should be good
>    enough if you do steps 1 and 2.
>
> If you don't have the time at hand, I can carefully do these cleanups
> and apply the patch. If you have the time and motivation, go ahead
> and send v3.
>> BR, Jarkko


No problem, I can do this. But note that the next version will be v4 not
v3 (as v3 is the version you recently reviewed).


Regards,
Lino

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Grundik]

On Wed, 2023-07-12 at 00:50 +0300, Jarkko Sakkinen wrote:
> > I want to say: this issue is NOT limited to Framework laptops.
> > 
> > For example this MSI gen12 i5-1240P laptop also suffers from same
> > problem:
> >         Manufacturer: Micro-Star International Co., Ltd.
> >         Product Name: Summit E13FlipEvo A12MT
> >         Version: REV:1.0
> >         SKU Number: 13P3.1
> >         Family: Summit
> > 
> > So, probably just blacklisting affected models is not the best
> > solution...
> 
> It will be supplemented with
> 
> https://lore.kernel.org/linux-integrity/CTYXI8TL7C36.2SCWH82FAZWBO@suppilovahvero/T/#me895f1920ca6983f791b58a6fa0c157161a33849
> 
> Together they should fairly sustainable framework.
> 

Unfortunately, they dont. Problem still occurs in debian 6.5-rc4
kernel, with forementioned laptop. According to sources, these patches
are applied in that kernel version.

What can I do to assist fixing it?..

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Thorsten Leemhuis]

On 06.08.23 18:30, Grundik wrote:
> On Wed, 2023-07-12 at 00:50 +0300, Jarkko Sakkinen wrote:
>>> I want to say: this issue is NOT limited to Framework laptops.
>>>
>>> For example this MSI gen12 i5-1240P laptop also suffers from same
>>> problem:
>>>         Manufacturer: Micro-Star International Co., Ltd.
>>>         Product Name: Summit E13FlipEvo A12MT
> [...]
>>
>> It will be supplemented with
>> https://lore.kernel.org/linux-integrity/CTYXI8TL7C36.2SCWH82FAZWBO@suppilovahvero/T/#me895f1920ca6983f791b58a6fa0c157161a33849
>>
>> Together they should fairly sustainable framework.
> 
> Unfortunately, they dont. Problem still occurs in debian 6.5-rc4
> kernel, with forementioned laptop. According to sources, these patches
> are applied in that kernel version.

Jarkko & Lino, did you see this msg Grundik posted that about a week
ago? It looks like there is still something wrong there that need
attention. Or am I missing something?

FWIW, two more users reported that they still see similar problems with
recent 6.4.y kernels that contain the "tpm,tpm_tis: Disable interrupts
after 1000 unhandled IRQs" patch. Both also with MSI laptops:

https://bugzilla.kernel.org/show_bug.cgi?id=217631#c18
https://bugzilla.kernel.org/show_bug.cgi?id=217631#c20

No reply either afaics.

Ciao, Thorsten

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Grundik]

On Fri, 2023-08-11 at 10:18 +0200, Thorsten Leemhuis wrote:
> Jarkko & Lino, did you see this msg Grundik posted that about a week
> ago? It looks like there is still something wrong there that need
> attention. Or am I missing something?
> 
> FWIW, two more users reported that they still see similar problems
> with
> recent 6.4.y kernels that contain the "tpm,tpm_tis: Disable
> interrupts
> after 1000 unhandled IRQs" patch. Both also with MSI laptops:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=217631#c18
> https://bugzilla.kernel.org/show_bug.cgi?id=217631#c20
> 
> No reply either afaics.

As I said before: it does not looks like blacklisting is a good
solution at all.

If there are general fix, then blacklisting only makes testing of that
fix more difficult. If general fix works, why blacklist? If it does not
work and its impossible to figure out why — maybe there should be
kernel boot option to select between polling/irq instead of/in addition
to hard-coded blacklist.

Unfortunately, its very hard to test this fixes on my side: since TPM
is not a module, but compiled into kernel itself, it requires
recompiling a whole kernel, which is quite a task for a laptop. But I
will try my best, if needed.

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Fri Aug 11, 2023 at 1:44 PM EEST, Grundik wrote:
> On Fri, 2023-08-11 at 10:18 +0200, Thorsten Leemhuis wrote:
> > Jarkko & Lino, did you see this msg Grundik posted that about a week
> > ago? It looks like there is still something wrong there that need
> > attention. Or am I missing something?
> > 
> > FWIW, two more users reported that they still see similar problems
> > with
> > recent 6.4.y kernels that contain the "tpm,tpm_tis: Disable
> > interrupts
> > after 1000 unhandled IRQs" patch. Both also with MSI laptops:
> > 
> > https://bugzilla.kernel.org/show_bug.cgi?id=217631#c18
> > https://bugzilla.kernel.org/show_bug.cgi?id=217631#c20
> > 
> > No reply either afaics.
>
> As I said before: it does not looks like blacklisting is a good
> solution at all.
>
> If there are general fix, then blacklisting only makes testing of that
> fix more difficult. If general fix works, why blacklist? If it does not
> work and its impossible to figure out why — maybe there should be
> kernel boot option to select between polling/irq instead of/in addition
> to hard-coded blacklist.
>
> Unfortunately, its very hard to test this fixes on my side: since TPM
> is not a module, but compiled into kernel itself, it requires
> recompiling a whole kernel, which is quite a task for a laptop. But I
> will try my best, if needed.

I agree with this.

Can you check my response to Thorsten and share your opinions?

BR, Jarkko

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Fri Aug 11, 2023 at 11:18 AM EEST, Thorsten Leemhuis wrote:
> On 06.08.23 18:30, Grundik wrote:
> > On Wed, 2023-07-12 at 00:50 +0300, Jarkko Sakkinen wrote:
> >>> I want to say: this issue is NOT limited to Framework laptops.
> >>>
> >>> For example this MSI gen12 i5-1240P laptop also suffers from same
> >>> problem:
> >>>         Manufacturer: Micro-Star International Co., Ltd.
> >>>         Product Name: Summit E13FlipEvo A12MT
> > [...]
> >>
> >> It will be supplemented with
> >> https://lore.kernel.org/linux-integrity/CTYXI8TL7C36.2SCWH82FAZWBO@suppilovahvero/T/#me895f1920ca6983f791b58a6fa0c157161a33849
> >>
> >> Together they should fairly sustainable framework.
> > 
> > Unfortunately, they dont. Problem still occurs in debian 6.5-rc4
> > kernel, with forementioned laptop. According to sources, these patches
> > are applied in that kernel version.
>
> Jarkko & Lino, did you see this msg Grundik posted that about a week
> ago? It looks like there is still something wrong there that need
> attention. Or am I missing something?
>
> FWIW, two more users reported that they still see similar problems with
> recent 6.4.y kernels that contain the "tpm,tpm_tis: Disable interrupts
> after 1000 unhandled IRQs" patch. Both also with MSI laptops:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=217631#c18
> https://bugzilla.kernel.org/show_bug.cgi?id=217631#c20
>
> No reply either afaics.
>
> Ciao, Thorsten

I was planning to send a PR to Linus with a quirk for MSI GS66 Stealth
11UG, and apparently this bug report would add two additional MSI
entries. This is becoming quickly a maintenance hell.

For Lenovo, I also added patch that will categorically disable irqs for
all with vendor "LENOVO" because there was already six entries, and I
got patch for 7th.

Now we at least know that IRQs TPMs are somewhat broken so it is time to
make decisions to make this converge to something.

I see two long-standing options:

A. Move from deny list to allow list when considering using IRQs. This
   can be supplemented with a kernel command-line parameter to enforce
   IRQs and ignore the allow list (and IRQ storm detection provides
   additional measure in case you try to enforce)
B. Change deny list to match only vendors for the time being. This can
   be supplemented with a allow list that is processed after the deny
   list for models where IRQs are known to work.

This would the implementation of (B):

static const struct dmi_system_id tpm_tis_dmi_table[] = {
	{
		.callback = tpm_tis_disable_irq,
		.matches = {
			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
		},
	},
	{
		.callback = tpm_tis_disable_irq,
		.ident = "MSI GS66 Stealth 11UG",
		.matches = {
			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
		},
	},
	{
		.callback = tpm_tis_disable_irq,
		.matches = {
			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
		},
	},
	{
		.callback = tpm_tis_disable_irq,
		.matches = {
			DMI_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
		},
	},
	{
		.callback = tpm_tis_disable_irq,
		.matches = {
			DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),
		},
	},
	{}
};

BR, Jarkko

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Fri Aug 11, 2023 at 8:22 PM EEST, Jarkko Sakkinen wrote:
> On Fri Aug 11, 2023 at 11:18 AM EEST, Thorsten Leemhuis wrote:
> > On 06.08.23 18:30, Grundik wrote:
> > > On Wed, 2023-07-12 at 00:50 +0300, Jarkko Sakkinen wrote:
> > >>> I want to say: this issue is NOT limited to Framework laptops.
> > >>>
> > >>> For example this MSI gen12 i5-1240P laptop also suffers from same
> > >>> problem:
> > >>>         Manufacturer: Micro-Star International Co., Ltd.
> > >>>         Product Name: Summit E13FlipEvo A12MT
> > > [...]
> > >>
> > >> It will be supplemented with
> > >> https://lore.kernel.org/linux-integrity/CTYXI8TL7C36.2SCWH82FAZWBO@suppilovahvero/T/#me895f1920ca6983f791b58a6fa0c157161a33849
> > >>
> > >> Together they should fairly sustainable framework.
> > > 
> > > Unfortunately, they dont. Problem still occurs in debian 6.5-rc4
> > > kernel, with forementioned laptop. According to sources, these patches
> > > are applied in that kernel version.
> >
> > Jarkko & Lino, did you see this msg Grundik posted that about a week
> > ago? It looks like there is still something wrong there that need
> > attention. Or am I missing something?
> >
> > FWIW, two more users reported that they still see similar problems with
> > recent 6.4.y kernels that contain the "tpm,tpm_tis: Disable interrupts
> > after 1000 unhandled IRQs" patch. Both also with MSI laptops:
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=217631#c18
> > https://bugzilla.kernel.org/show_bug.cgi?id=217631#c20
> >
> > No reply either afaics.
> >
> > Ciao, Thorsten
>
> I was planning to send a PR to Linus with a quirk for MSI GS66 Stealth
> 11UG, and apparently this bug report would add two additional MSI
> entries. This is becoming quickly a maintenance hell.
>
> For Lenovo, I also added patch that will categorically disable irqs for
> all with vendor "LENOVO" because there was already six entries, and I
> got patch for 7th.
>
> Now we at least know that IRQs TPMs are somewhat broken so it is time to
> make decisions to make this converge to something.
>
> I see two long-standing options:
>
> A. Move from deny list to allow list when considering using IRQs. This
>    can be supplemented with a kernel command-line parameter to enforce
>    IRQs and ignore the allow list (and IRQ storm detection provides
>    additional measure in case you try to enforce)
> B. Change deny list to match only vendors for the time being. This can
>    be supplemented with a allow list that is processed after the deny
>    list for models where IRQs are known to work.
>
> This would the implementation of (B):
>
> static const struct dmi_system_id tpm_tis_dmi_table[] = {
> 	{
> 		.callback = tpm_tis_disable_irq,
> 		.matches = {
> 			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
> 		},
> 	},
> 	{
> 		.callback = tpm_tis_disable_irq,
> 		.ident = "MSI GS66 Stealth 11UG",
> 		.matches = {
> 			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
> 		},
> 	},
> 	{
> 		.callback = tpm_tis_disable_irq,
> 		.matches = {
> 			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> 		},
> 	},
> 	{
> 		.callback = tpm_tis_disable_irq,
> 		.matches = {
> 			DMI_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
> 		},
> 	},
> 	{
> 		.callback = tpm_tis_disable_irq,
> 		.matches = {
> 			DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),
> 		},
> 	},
> 	{}
> };
>
> BR, Jarkko

I.e. this is a non-sustainable solution:

	if (vendor && product) {
		dev_info(&chip->dev,
			"Consider adding the following entry to tpm_tis_dmi_table:\n");
		dev_info(&chip->dev, "\tDMI_SYS_VENDOR: %s\n", vendor);
		dev_info(&chip->dev, "\tDMI_PRODUCT_VERSION: %s\n", product);
	}

This is also super time consuming and takes the focus away from more
important matters (like most likely the AMD rng fix would have gone
smoother without these getting in the way all the time).

BR, Jarkko

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Grundik]

On Fri, 2023-08-11 at 20:40 +0300, Jarkko Sakkinen wrote:
> On Fri Aug 11, 2023 at 8:22 PM EEST, Jarkko Sakkinen wrote:
> > On Fri Aug 11, 2023 at 11:18 AM EEST, Thorsten Leemhuis wrote:
> > 
> > 
> > I see two long-standing options:
> > 
> > A. Move from deny list to allow list when considering using IRQs.
> > This
> >    can be supplemented with a kernel command-line parameter to
> > enforce
> >    IRQs and ignore the allow list (and IRQ storm detection provides
> >    additional measure in case you try to enforce)
> > B. Change deny list to match only vendors for the time being. This
> > can
> >    be supplemented with a allow list that is processed after the
> > deny
> >    list for models where IRQs are known to work.
[...]
> 
> This is also super time consuming and takes the focus away from more
> important matters (like most likely the AMD rng fix would have gone
> smoother without these getting in the way all the time).

Main problem of any list is maintaining of them. So, I think there
should not be any black or white lists at all. Module should work with
reasonable default (polling is the one, which lived without problems
for years and years due to bug, as I understand), and probably a boot
option to force IRQ. Maybe module should warn user to try that option.

I don't know: is it even worth it to use IRQ, if it so problematic? Are
there any significant advantages of that? I understand, polling is a
resource consumer, but its just TPM, which is used mainly at the boot
time, is it worth it?

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Fri Aug 11, 2023 at 9:47 PM EEST, Grundik wrote:
> On Fri, 2023-08-11 at 20:40 +0300, Jarkko Sakkinen wrote:
> > On Fri Aug 11, 2023 at 8:22 PM EEST, Jarkko Sakkinen wrote:
> > > On Fri Aug 11, 2023 at 11:18 AM EEST, Thorsten Leemhuis wrote:
> > > 
> > > 
> > > I see two long-standing options:
> > > 
> > > A. Move from deny list to allow list when considering using IRQs.
> > > This
> > >    can be supplemented with a kernel command-line parameter to
> > > enforce
> > >    IRQs and ignore the allow list (and IRQ storm detection provides
> > >    additional measure in case you try to enforce)
> > > B. Change deny list to match only vendors for the time being. This
> > > can
> > >    be supplemented with a allow list that is processed after the
> > > deny
> > >    list for models where IRQs are known to work.
> [...]
> > 
> > This is also super time consuming and takes the focus away from more
> > important matters (like most likely the AMD rng fix would have gone
> > smoother without these getting in the way all the time).
>
> Main problem of any list is maintaining of them. So, I think there
> should not be any black or white lists at all. Module should work with
> reasonable default (polling is the one, which lived without problems
> for years and years due to bug, as I understand), and probably a boot
> option to force IRQ. Maybe module should warn user to try that option.
>
> I don't know: is it even worth it to use IRQ, if it so problematic? Are
> there any significant advantages of that? I understand, polling is a
> resource consumer, but its just TPM, which is used mainly at the boot
> time, is it worth it?

+1

Thanks for sharing your opinion. I'll take the necessary steps.

BR, Jarkko

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Grundik]

On Fri, 2023-08-11 at 23:01 +0300, Jarkko Sakkinen wrote:
> 
> 
> Thanks for sharing your opinion. I'll take the necessary steps.
> 

I was thinking... Maybe I'm wrong, maybe I mistaking, but isnt this TPM
located inside of the CPU chip? So that issue is not specific to laptop
vendor or model, but its CPU-specific.

My MSI A12MT laptop has i5-1240P, Framework laptops mentioned in this
thread, also has i5-1240P CPU. Unfortunately there are no such
information about other affected models, but could it be just that CPU
line?

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Sat Aug 12, 2023 at 2:28 PM EEST, Grundik wrote:
> On Fri, 2023-08-11 at 23:01 +0300, Jarkko Sakkinen wrote:
> > 
> > 
> > Thanks for sharing your opinion. I'll take the necessary steps.
> > 
>
> I was thinking... Maybe I'm wrong, maybe I mistaking, but isnt this TPM
> located inside of the CPU chip? So that issue is not specific to laptop
> vendor or model, but its CPU-specific.

Nope, tpm_tis is MMIO interface for so called FIFO register interface,
it could be backed e.g. by SPI [*]. It is a physical layer for MMIO.
There's other backends too such as tpm_tis_i2c where kernel is directly
exposed to the physical layer.

> My MSI A12MT laptop has i5-1240P, Framework laptops mentioned in this
> thread, also has i5-1240P CPU. Unfortunately there are no such
> information about other affected models, but could it be just that CPU
> line?

[*] https://lkml.org/lkml/2023/8/14/1065

BR, Jarkko

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Linus Torvalds]

On Fri, 11 Aug 2023 at 10:22, Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> I was planning to send a PR to Linus with a quirk for MSI GS66 Stealth
> 11UG, and apparently this bug report would add two additional MSI
> entries. This is becoming quickly a maintenance hell.

Honestly, what would be the immediate effects of just not enabling the
TPM irq by default at all, and making it an explicit opt-in?

When a common solution is to just disable the TPM in the BIOS
entirely, and the end result is a working system, I really get the
teeling that this is all pain for very very little gain.

Would anybody even notice if we just disabled it by default and added
a "if you really want it, use 'tpm=irq' kernel command line"?

Hmm?

             Linus

Re: [PATCH v2 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Fri Aug 11, 2023 at 9:55 PM EEST, Linus Torvalds wrote:
> On Fri, 11 Aug 2023 at 10:22, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > I was planning to send a PR to Linus with a quirk for MSI GS66 Stealth
> > 11UG, and apparently this bug report would add two additional MSI
> > entries. This is becoming quickly a maintenance hell.
>
> Honestly, what would be the immediate effects of just not enabling the
> TPM irq by default at all, and making it an explicit opt-in?
>
> When a common solution is to just disable the TPM in the BIOS
> entirely, and the end result is a working system, I really get the
> teeling that this is all pain for very very little gain.
>
> Would anybody even notice if we just disabled it by default and added
> a "if you really want it, use 'tpm=irq' kernel command line"?
>
> Hmm?
>
>              Linus

It would be in line with what I proposed in [1] (option A), and also
what Grundik said in this thread.

Right now IRQs should be only enabled by a command-line option, and what
"allow list" would mean in practice should really be something that
would be advertised e.g. Device Tree or ACPI, not really part of the
solution implemented right now.

I'll send a patch ASAP, which makes this feature opt-in.

[1] https://lore.kernel.org/linux-integrity/CUPW0XP1RFXI.162GZ78E46TBJ@suppilovahvero/

BR, Jarkko

Re: [PATCH 1/2] tpm/tpm_tis: Disable interrupts for Framework Laptop Intel 12th gen

[Jarkko Sakkinen]

On Mon, 2023-07-10 at 15:38 +0200, Christian Hesse wrote:
> This device suffer an irq storm, so add it in tpm_tis_dmi_table to
> force polling.
> 
> https://bugs.archlinux.org/user/38129
> https://bugzilla.kernel.org/show_bug.cgi?id=217631
> 
> Fixes: e644b2f498d297a928efcb7ff6f900c27f8b788e
> Cc: stable@vger.kernel.org
> Reported-by:  <roubro1991@gmail.com>
> Signed-off-by: Christian Hesse <mail@eworm.de>
> ---
>  drivers/char/tpm/tpm_tis.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 7db3593941ea..2979f8b9aaa0 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -114,6 +114,14 @@ static int tpm_tis_disable_irq(const struct dmi_system_id *d)
>  }
>  
>  static const struct dmi_system_id tpm_tis_dmi_table[] = {
> +	{
> +		.callback = tpm_tis_disable_irq,
> +		.ident = "Framework Laptop Intel 12th gen",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
> +			DMI_MATCH(DMI_PRODUCT_VERSION, "A4"),
> +		},
> +	},
>  	{
>  		.callback = tpm_tis_disable_irq,
>  		.ident = "ThinkPad T490s",

Hi, no other issues except fixes tag should be in both:

Fixes: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")

I.e.

# $1 = commit ID
function git-fixes {
  git --no-pager log --format='Fixes: %h ("%s")' --abbrev=12 -1 $1;
}

I'll update them, thank you.

BR, Jarkko