From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A5A3EEB64D7 for ; Wed, 21 Jun 2023 18:45:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231332AbjFUSpZ (ORCPT ); Wed, 21 Jun 2023 14:45:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34972 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231256AbjFUSpZ (ORCPT ); Wed, 21 Jun 2023 14:45:25 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5050E1A3 for ; Wed, 21 Jun 2023 11:45:24 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id DAA6B6167F for ; Wed, 21 Jun 2023 18:45:23 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EF7E1C433C0; Wed, 21 Jun 2023 18:45:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1687373123; bh=IgqlO8KTP+GODhAzey1igAT7BLCCxVaUygzSBMIxHXs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=fLNoYFnAKeA8fVVLFypZVx6AC0V3CT+ZhJgz9WxAa9CvRGkCXsd62pI94A1HEFI+b 1rVAhLmRpr8bMm8ClRcE0ZOC29NLrDkKYmV8Ly8nIO1HjL/1/8N8ykaQpOz4ot1GsA Aq8PoaI2tZYxmXagak9hAdUZdVoqxHxfMN3tjCeI= Date: Wed, 21 Jun 2023 20:45:12 +0200 From: Greg KH To: Lino Sanfilippo Cc: Jarkko Sakkinen , Felix Riemann , patches@lists.linux.dev, sashal@kernel.org, stable@vger.kernel.org Subject: Re: [PATCH 5.15 070/371] tpm, tpm_tis: Claim locality before writing interrupt registers Message-ID: <2023062100-retrace-kitten-7241@gregkh> References: <20230508094814.897191675@linuxfoundation.org> <20230515153759.2072-1-svc.sw.rte.linux@sma.de> <382309d7c0ae593d507eb816982f6a66c2cda00a.camel@kernel.org> <2023060906-starter-scrounger-1bca@gregkh> <3bdaa40e-16dc-6c04-b9e4-9e5951267e7e@kunbus.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3bdaa40e-16dc-6c04-b9e4-9e5951267e7e@kunbus.com> Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org On Fri, Jun 09, 2023 at 05:42:20PM +0200, Lino Sanfilippo wrote: > Hi, > > On 09.06.23 11:07, Greg KH wrote: > > > > On Wed, May 24, 2023 at 04:07:41AM +0300, Jarkko Sakkinen wrote: > >> On Mon, 2023-05-15 at 17:37 +0200, Felix Riemann wrote: > >>> Hi! > >>> > >>>> [ Upstream commit 15d7aa4e46eba87242a320f39773aa16faddadee ] > >>>> > >>>> In tpm_tis_probe_single_irq() interrupt registers TPM_INT_VECTOR, > >>>> TPM_INT_STATUS and TPM_INT_ENABLE are modified to setup the interrupts. > >>>> Currently these modifications are done without holding a locality thus they > >>>> have no effect. Fix this by claiming the (default) locality before the > >>>> registers are written. > >>>> > >>>> Since now tpm_tis_gen_interrupt() is called with the locality already > >>>> claimed remove locality request and release from this function. > >>> > >>> On systems with SPI-connected TPM and the interrupt still configured > >>> (despite it not working before) this may introduce a kernel crash. > >>> The issue is that it will now trigger an SPI transfer (which will wait) > >>> from the IRQ handler: > >>> > >>> BUG: scheduling while atomic: systemd-journal/272/0x00010001 > >>> Modules linked in: spi_fsl_lpspi > >>> CPU: 0 PID: 272 Comm: systemd-journal Not tainted 5.15.111-06679-g56b9923f2840 #50 > >>> Call trace: > >>> dump_backtrace+0x0/0x1e0 > >>> show_stack+0x18/0x40 > >>> dump_stack_lvl+0x68/0x84 > >>> dump_stack+0x18/0x34 > >>> __schedule_bug+0x54/0x70 > >>> __schedule+0x664/0x760 > >>> schedule+0x88/0x100 > >>> schedule_timeout+0x80/0xf0 > >>> wait_for_completion_timeout+0x80/0x10c > >>> fsl_lpspi_transfer_one+0x25c/0x4ac [spi_fsl_lpspi] > >>> spi_transfer_one_message+0x22c/0x440 > >>> __spi_pump_messages+0x330/0x5b4 > >>> __spi_sync+0x230/0x264 > >>> spi_sync_locked+0x10/0x20 > >>> tpm_tis_spi_transfer+0x1ec/0x250 > >>> tpm_tis_spi_read_bytes+0x14/0x20 > >>> tpm_tis_spi_read32+0x38/0x70 > >>> tis_int_handler+0x48/0x15c > >>> *snip* > >>> > >>> The immediate error is fixable by also picking 0c7e66e5fd ("tpm, tpm_tis: > >>> Request threaded interrupt handler") from the same patchset[1]. > >>> However, as > >>> the driver's IRQ test logic is still faulty it will fail the check and fall > >>> back to the polling behaviour without actually disabling the IRQ in hard- > >>> and software again. For this at least e644b2f498 ("tpm, tpm_tis: Enable > >>> interrupt test") and 0e069265bc ("tpm, tpm_tis: Claim locality in interrupt > >>> handler") are necessary. > >>> > >>> At this point 9 of the set's 14 patches are applied and I am not sure > >>> whether it's better to pick the remaining five patches as well or just > >>> revert the initial six patches. Especially considering there were initially > >>> no plans to submit these patches to stable[2] and the IRQ feature was (at > >>> least on SPI) not working before. > >> > >> I think the right thing to do would be to revert 6 initial patches. > > > > Ok, I think this isn't needed anymore with the latest 5.15.116 release, > > right? If not, please let me know. > > > > > With 0c7e66e5fd ("tpm, tpm_tis: Request threaded interrupt handler") applied the > above bug is fixed in 5.15.y. There is however still the issue that the interrupts may > not be acknowledged properly in the interrupt handler, since the concerning register is written > without the required locality held (Felix mentions this above). > This can be fixed with 0e069265bce5 ("tpm, tpm_tis: Claim locality in interrupt handler"). > > So instead of reverting the initial patches, I suggest to > > 1. also apply 0e069265bce5 ("tpm, tpm_tis: Claim locality in interrupt handler") I've now done this, thanks. greg k-h