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: <da435e0d-5f22-fac7-bc10-96a0fd4c6d54@kunbus.com>
Date:   Tue, 23 May 2023 22:46:44 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH 1/2] tpm, tpm_tis: Handle interrupt storm
Content-Language: en-US
To:     Jarkko Sakkinen <jarkko@kernel.org>,
        Lino Sanfilippo <LinoSanfilippo@gmx.de>, peterhuewe@gmx.de,
        jgg@ziepe.ca
Cc:     jsnitsel@redhat.com, hdegoede@redhat.com, oe-lkp@lists.linux.dev,
        lkp@intel.com, peter.ujfalusi@linux.intel.com,
        peterz@infradead.org, linux@mniewoehner.de,
        linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
        lukas@wunner.de, p.rosenberger@kunbus.com
References: <20230522143105.8617-1-LinoSanfilippo@gmx.de>
 <CSTVVFNKUVJW.P69FKI6IF3ZN@suppilovahvero>
From:   Lino Sanfilippo <l.sanfilippo@kunbus.com>
In-Reply-To: <CSTVVFNKUVJW.P69FKI6IF3ZN@suppilovahvero>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZEIyWEVkZUx1M1NQQ1NWYzdQOCsxYzUwUXlTZjZRRStyVkNtVk1WdnM0TjBL?=
 =?utf-8?B?V2dhT0tpWHhiTmJhRDZvc3BWdG5SMjNWblhpQlBFVUhqL0dOZ1VTRlN1Q0R5?=
 =?utf-8?B?K3hYbG5odVJYaTUvM3ZjQTFwOU9zVEpBOVBxbHd4VUsrZ2dRTUdTTllNRWJs?=
 =?utf-8?B?SWd5TCtBSlRCK2doVVdKS1cyT0YxRXQ4TEtZTE1XNnZZRGlzcXoyMnNjWUhB?=
 =?utf-8?B?MUVZM25tZGp5Q0lvdkowb2VDMTJiRmM2WTBXZlp0YUZPd2ZmWmI0Uk0zMnVL?=
 =?utf-8?B?VHg5VjZlYVozM21kVFVsT1RWNWRTMkpvRVBIRUk3YmNMNDVXcWpFb3pjcUI2?=
 =?utf-8?B?MUlYNytFa1c1Q0w0dk9laGxjNW5MTUxIUm8zN3JGMzVFUDlXL2RCQllsUG50?=
 =?utf-8?B?TXdxbVpxOEhZKzBRQWNVMkV5eW0zRDlpa3QyWUxqMUNSYlNxM1o5S3VUN2p1?=
 =?utf-8?B?czlnYjVPTnpzMHlVd3A0R0dmMjlMaG4wdVdqQUdNSVZhUzN1clkrZk5keVFF?=
 =?utf-8?B?eGY1N2E2Wnk1V2xCYWNNK1l4QWRvaVlENTFqbjMyTGdBajlhOVBkVk5UTlFy?=
 =?utf-8?B?dXUyN0lLRGRuODFUODBuYXlPM0pwWmR0Z2kxL0RhUFJMRmxRbzlYZjVJdStP?=
 =?utf-8?B?eExaWXZPWm1BaDE2RDVDZHh0VHNac1kvUG1VelZkanZQZTllb0NDM0RYSmxD?=
 =?utf-8?B?cGZldVo3eEp4UDZad291My91eXQ4YlR6MkxiRDRmR0FaeXVpby9manNGM01O?=
 =?utf-8?B?bFhJcUxKNmRiNlgzTWNBMGJTVWNZTXlmOVdDSnRlTnRWV255dE1OOTlDYUdz?=
 =?utf-8?B?ZndsWTVNQTBQZk1qTzV4MXNDZWhGVThFOXpEOE1lTTc1Wk1MZno0cVQ0a090?=
 =?utf-8?B?THlvaHYxM0VDeFpVTUphMVpWVkhyRUhRaGx1YzFwMkZtYkVBRWcwVnIzSjZ1?=
 =?utf-8?B?QzlQNEVMTTQwMlFVeFlhUDE1MzJoYzduR3ZjWWoyNmlhUW1EVDVRTFI2VmtV?=
 =?utf-8?B?ZjEycnI2cWowNm1PV1BpRDh4dVR3K2ZNbE5TeWo1MnFka1lFTEUwQVlBNmNP?=
 =?utf-8?B?RXlYU3JPQzFnd2VsYTI2dk5HakZoN3UxRCtDdG5qR0FhbUhOcFAzK1c3VU1N?=
 =?utf-8?B?TW9MWENsWjNMZ2Q0VThhNzNCWVppYUtSRlFBSTZHZ3dJTU1zYndvZnN4dE9t?=
 =?utf-8?B?MkFBL1BrdmhjOFd3N1duOWNxazU4dFMxQmpwZ3BoWmpKY3ptaEw1RXdoaHJB?=
 =?utf-8?B?d1FmV2x5dXdvSTdXMWZKa01xWmxHZm54QkVjU3Nadk5tWkJJNUE1UU5XNHZQ?=
 =?utf-8?B?K2VnaTJUcUFtS25iQ2tYa1dOdXVVS08xL0IrSnEzcW1wZlhEN0M1Vm9JVElW?=
 =?utf-8?B?M0NycmJ6VUhhMDhacFBENHRydG1lMTBEZVVYeVlURmF1YXNHeWNHR3pnLzNM?=
 =?utf-8?B?Zm9DL1Q5TkJleGVZZEJvWmYrcnYzSGpSRC9XWGhDcng1YUJKOC9la0ZrWTli?=
 =?utf-8?B?RUR4NGpMTmF3aWNqV0JsVUlQMnFuK1Y5Y0tkWmNLb012LzRsSVNYYm4zenk3?=
 =?utf-8?B?RlNXVmt2QUpjVk51cTJaTWpwMklMUHF0UkdPbGQ5REV2bXJJSXNwZDdUdElL?=
 =?utf-8?B?Q2VpZndaa281V1g4UkZXdjhBM25NMkdRY05PK3RYMmJmQWVGQmYxQUdERWZo?=
 =?utf-8?B?M3l3WTJjRTd1dXhIVGpBWGxFS01wUEZmdFJiT2w4blhjRFZsMzNTM2U0Z1Rp?=
 =?utf-8?B?ODBIa3UveFNXcTM5aUpnTnlSVEJueXBrcCtCc2lwZG9ieUYwS2ZVYm5TbWxY?=
 =?utf-8?B?YW5wcGJxU21hVVJMZ1d0aGs1S2g5REV3VlZGNWgrcElFQ1FmNlZtSlh4aFNG?=
 =?utf-8?B?SlVhaTJzR1VKTnQvOHNQTldYSm1WZmwxOFliTE9WWEVoc1FlRkp1U05ZcEJx?=
 =?utf-8?B?UzBxYmtkUWlJNFBJSlVqdEFOQkR5dEtNeXUxOThQbitZRmtPdFBaRlg3WU9i?=
 =?utf-8?B?aDlwdmJzMmQ4SGZiMG9kWWNZYmc4eXU2N1A2djVQL2tIbCtCdWl1RURpd2NK?=
 =?utf-8?B?S0JKRkNRVmp5cDVKOStZOFVZYXRaZ0dsaTBlNTc5MEh2MjB3VEYrZWI1VjZN?=
 =?utf-8?B?YzEyeW9xUGFWVURodXNwb1crTWtLTFBjcnp5Mi9aYkVMZE0xRnZMQnFQYmU4?=
 =?utf-8?Q?N9asAk4J/TOOsJUEL/Npf2XgoaEBFdZFEK6g3RyG/p+z?=
X-OriginatorOrg: kunbus.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 57f5fe86-08e2-4b5c-51d5-08db5bced343
X-MS-Exchange-CrossTenant-AuthSource: VI1P193MB0413.EURP193.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2023 20:46:47.1288
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: aaa4d814-e659-4b0a-9698-1c671f11520b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Vd7iqpmxQ1TDVNVanAtiBefyUaVPsNllMXpS+5qAUridRKUapRVsBYImDBYDjuH2tvySByhICuOZrFWH/IlWqQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8P193MB2192
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi,

On 23.05.23 20:53, Jarkko Sakkinen wrote:
> ATTENTION: This e-mail is from an external sender. Please check attachments and links before opening e.g. with mouseover.
> 
> 
> On Mon May 22, 2023 at 5:31 PM EEST, Lino Sanfilippo wrote:
>> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
>>
>> Commit e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") enabled
>> interrupts instead of polling on all capable TPMs. Unfortunately, on some
>> products the interrupt line is either never asserted or never deasserted.
>>
>> The former causes interrupt timeouts and is detected by
>> tpm_tis_core_init(). The latter results in interrupt storms.
>>
>> Recent reports concern the Lenovo ThinkStation P360 Tiny, Lenovo ThinkPad
>> L490 and Inspur NF5180M6:
>>
>> https://lore.kernel.org/linux-integrity/20230511005403.24689-1-jsnitsel@redhat.com/
>> https://lore.kernel.org/linux-integrity/d80b180a569a9f068d3a2614f062cfa3a78af5a6.camel@kernel.org/
>>
>> The current approach to avoid those storms is to disable interrupts by
>> adding a DMI quirk for the concerned device.
>>
>> However this is a maintenance burden in the long run, so use a generic
>> approach:
> 
> I'm trying to comprehend how you evaluate, how big maintenance burden
> this would be. Adding even a few dozen table entries is not a
> maintenance burden.
> 
> On the other hand any new functionality is objectively a maintanance
> burden of some measure (applies to any functionality). So how do we know
> that taking this change is less of a maintenance burden than just add
> new table entries, as they come up?
> 

Initially this set was created as a response to this 0-day bug report which you asked me
to have a look at:

https://lore.kernel.org/linux-integrity/d80b180a569a9f068d3a2614f062cfa3a78af5a6.camel@kernel.org/

My hope was that it could also avoid some of (existing or future) DMI entries. But even if it does not
(e.g. the problem Péter Ujfalusi reported with the UPX-i11 cannot be fixed by this patch set and thus
needs the DMI quirk) we may at least avoid more bug reports due to interrupt storms once
6.4 is released. 


>> Detect an interrupt storm by counting the number of unhandled interrupts
>> within a 10 ms time interval. In case that more than 1000 were unhandled
>> deactivate interrupts, deregister the handler and fall back to polling.
> 
> I know it can be sometimes hard to evaluate but can you try to explain
> how you came up to the 10 ms sampling period and 1000 interrupt
> threshold? I just don't like abritrary numbers.

At least the 100 ms is not plucked out of thin air but its the same time period
that the generic code in note_interrupt() uses - I assume for a good reason.
Not only this number but the whole irq storm detection logic is taken from 
there: 

> 
>> This equals the implementation that handles interrupt storms in
>> note_interrupt() by means of timestamps and counters in struct irq_desc.

The number of 1000 unhandled interrupts is still far below the 99900 used in
note_interrupt() but IMHO enough to indicate that there is something seriously
wrong with interrupt processing and it is probably saver to fall back to polling.


Regards,
Lino