Re: [linus:master] [tpm, tpm_tis] e644b2f498: RIP:acpi_safe_halt

From: Jerry Snitselaar <jsnitsel@redhat.com>
To: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Cc: "Lukas Wunner" <lukas@wunner.de>,
	"Jarkko Sakkinen" <jarkko@kernel.org>,
	oe-lkp@lists.linux.dev, lkp@intel.com,
	linux-kernel@vger.kernel.org,
	"Michael Niewöhner" <linux@mniewoehner.de>,
	linux-integrity@vger.kernel.org,
	"Philipp Rosenberger" <p.rosenberger@kunbus.com>,
	peterz@infradead.org
Subject: Re: [linus:master] [tpm, tpm_tis] e644b2f498: RIP:acpi_safe_halt
Date: Thu, 11 May 2023 07:59:21 -0700	[thread overview]
Message-ID: <6nf5n6fdnkhx6taa2mecnsmyw7sdgaz6fbvkqy7jqkyd7jq2u7@ogsi6ije32in> (raw)
In-Reply-To: <1a8ecf90-80a4-9aac-95e1-9ce0c4e09ba5@kunbus.com>

On Thu, May 11, 2023 at 04:22:02PM +0200, Lino Sanfilippo wrote:
> 
> Hi Lukas,
> 
> On 11.05.23 16:16, Lukas Wunner wrote:
> > ATTENTION: This e-mail is from an external sender. Please check attachments and links before opening e.g. with mouseover.
> > 
> > 
> > On Thu, May 11, 2023 at 01:22:19PM +0200, Lino Sanfilippo wrote:
> >> Since beside the one reported by Peter Zijlstra
> >> (https://lore.kernel.org/linux-integrity/CSJ0AD1CFYQP.T6T68M6ZVK49@suppilovahvero/T/#t)
> >> we have another interrupt storm here, it is probably the best to handle those in general
> >> and to disable interrupts in this case to fall back to polling (this is also what Jerry
> >> suggested in the thread above).
> >>
> >> I will try to provide a patch for this.
> > 
> > In tpm_tis_probe_irq_single(), after you've requested the irq,
> > you could convert it to a struct irq_desc (via irq_to_desc()
> > from <linux/irqnr.h>) and cache that pointer in priv.
> > 
> > Then in tis_int_handler(), you could access the irqs_unhandled
> > member of struct irq_desc (from <linux/irqdesc.h>) and check
> > if it exceeds, say, 5000.
> 
> This is the solution I am currently working on, but thanks for confirming that 
> I am on the right track with this :)
> 
> > 
> > If it does, schedule a work_struct which calls disable_interrupts().
> > You can't call that from the IRQ handler because devm_free_irq()
> > waits for the IRQ handler to finish, so you'd deadlock.  You *can*
> > of course clear the TPM_GLOBAL_INT_ENABLE bit from the IRQ handler,
> > though it's unclear to me if that's sufficient to quiesce the
> > interrupt line.
> > 
> 
> Will try this, thx.
> 
> > By reusing the genirq subsystem's irqs_unhandled infrastructure,
> > you avoid having to reimplement all of that.
> > 
> 
> Agreed.
> 
> Regards,
> Lino
> 

Thanks for finishing this off Lino.

IIRC trying to catch the irq storm didn't work in the L490 case for
some reason, so we might still need the dmi entry for that one.

The info that the T490s had a pin wired up wrong came from Lenovo, but
this one even looks to be a different vendor so I wonder how often
this happens or if there is something else going on. Is it possible to
get info about the tpm used in the Inspur system? The datasheet online
doesn't mention it.

Regards,
Jerry

> > Thanks,
> > 
> > Lukas