Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <4f9e3da1-5cfa-c3cd-a8e5-f97a2dd1cede@redhat.com>
Date:   Wed, 24 May 2023 11:08:27 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH 1/2] tpm, tpm_tis: Handle interrupt storm
Content-Language: en-US, nl
To:     Lukas Wunner <lukas@wunner.de>,
        =?UTF-8?Q?P=c3=a9ter_Ujfalusi?= <peter.ujfalusi@linux.intel.com>
Cc:     Lino Sanfilippo <LinoSanfilippo@gmx.de>, peterhuewe@gmx.de,
        jarkko@kernel.org, jgg@ziepe.ca, jsnitsel@redhat.com,
        oe-lkp@lists.linux.dev, lkp@intel.com, peterz@infradead.org,
        linux@mniewoehner.de, linux-integrity@vger.kernel.org,
        linux-kernel@vger.kernel.org, l.sanfilippo@kunbus.com,
        p.rosenberger@kunbus.com
References: <20230522143105.8617-1-LinoSanfilippo@gmx.de>
 <c772bcdf-8256-2682-857c-9a6d344606d0@linux.intel.com>
 <20230523074443.GA21236@wunner.de>
 <98f7dc1a-6bed-a66f-650e-10caeb7d0bca@linux.intel.com>
 <20230523151642.GA31298@wunner.de>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <20230523151642.GA31298@wunner.de>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Lukas,

On 5/23/23 17:16, Lukas Wunner wrote:
> On Tue, May 23, 2023 at 12:14:28PM +0300, Péter Ujfalusi wrote:
>> On 23/05/2023 10:44, Lukas Wunner wrote:

<snip>

>>> The corresponding message emitted in tpm_tis_core_init() for
>>> an interrupt that's *never* asserted uses dev_err(), so using
>>> dev_err() here as well serves consistency:
>>>
>>> 	dev_err(&chip->dev, FW_BUG
>>> 		"TPM interrupt not working, polling instead\n");
>>>
>>> That way the same severity is used both for the never asserted and
>>> the never deasserted interrupt case.
>>
>> Oh, OK.
>> Is there anything the user can do to have a ERROR less boot?
> 
> You're right that the user can't do anything about it and that
> toning the message down to KERN_WARN or even KERN_NOTICE severity
> may be appropriate.
> 
> However the above-quoted message for the never asserted interrupt
> in tpm_tis_core_init() should then likewise be toned down to the
> same severity.
> 
> I'm wondering why that message uses FW_BUG.  That doesn't make any
> sense to me.  It's typically not a firmware bug, but a hardware issue,
> e.g. an interrupt pin may erroneously not be connected or may be
> connected to ground.  Lino used HW_ERR, which seems more appropriate
> to me.

This issue seems to have mostly been seen on x86/ACPI systems and
the issue seems to mostly be a misconfiguration of the IOAPIC
(e.g. wrong trigger type).

Besides this, the IRQ info always comes from either the ACPI tables
or from devicetree both of which are firmware and if the IRQ does not
work on a specific board then the firmware should simply not provide
any IRQ info to the driver rather the pointing to a broken IRQ.

Hence my suggestion to use dev_warn(dev, FW_BUG "....", ...);

Regards,

Hans