On Mon, Dec 07, 2020 at 03:28:03PM -0400, Jason Gunthorpe wrote: > On Sun, Dec 06, 2020 at 08:26:16PM +0100, Thomas Gleixner wrote: > > Just as a side note. I was looking at tpm_tis_probe_irq_single() and > > that function is leaking the interrupt request if any of the checks > > afterwards fails, except for the final interrupt probe check which does > > a cleanup. That means on fail before that the interrupt handler stays > > requested up to the point where the module is removed. If that's a > > shared interrupt and some other device is active on the same line, then > > each interrupt from that device will call into the TPM code. Something > > like the below is needed. > > > > Also the X86 autoprobe mechanism is interesting: > > > > if (IS_ENABLED(CONFIG_X86)) > > for (i = 3; i <= 15; i++) > > if (!tpm_tis_probe_irq_single(chip, intmask, 0, i)) > > return; > > > > The third argument is 'flags' which is handed to request_irq(). So that > > won't ever be able to probe a shared interrupt. But if an interrupt > > number > 0 is handed to tpm_tis_core_init() the interrupt is requested > > with IRQF_SHARED. Same issue when the chip has an interrupt number in > > the register. It's also requested exclusive which is pretty likely > > to fail on ancient x86 machines. > > It is very likely none of this works any more, it has been repeatedly > reworked over the years and just left behind out of fear someone needs > it. I've thought it should be deleted for a while now. > > I suppose the original logic was to try and probe without SHARED > because a probe would need exclusive access to the interrupt to tell > if the TPM was actually the source, not some other device. > > It is all very old and very out of step with current thinking, IMHO. I > skeptical that TPM interrupts were ever valuable enough to deserve > this in the first place. > > Jason +1 for removing it. /Jarkko