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

["Jarkko Sakkinen" <jarkko@kernel.org>]

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