Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: 13WpvrGfTejv2N6p1adizMtoR7SBYme3eZHlhX+uKiZI3KXdyQ9GMKSm9rkz9JBOJA6yKPHLqU
 af1C7nyhs8BA==
IronPort-SDR: jFS7KGHVaRAaMznduEL5fQXp/b6JrXmLh7bBjVzc4f+0S77nN/JJ84Mu6X2OqlMKgmg2yQmoLZ
 kIHjP3M6+oLw==
Date:   Mon, 6 Jul 2020 17:47:44 +0300
From:   Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To:     Peter.Huewe@infineon.com
Cc:     linux-integrity@vger.kernel.org, kjhall@us.ibm.com,
        ferry.toth@elsinga.info, peterhuewe@gmx.de, jgg@ziepe.ca,
        arnd@arndb.de, gregkh@linuxfoundation.org, akpm@osdl.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] tpm_tis: Remove the HID IFX0102
Message-ID: <20200706144744.GB6956@linux.intel.com>
References: <20200625023111.270458-1-jarkko.sakkinen@linux.intel.com>
 <e9caad58aba44bb3abeac8569a6bd8ed@infineon.com>
 <20200706140051.GB3816@linux.intel.com>
 <20200706144328.GA6956@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200706144328.GA6956@linux.intel.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jul 06, 2020 at 05:43:35PM +0300, Jarkko Sakkinen wrote:
> On Mon, Jul 06, 2020 at 05:00:51PM +0300, Jarkko Sakkinen wrote:
> > On Mon, Jul 06, 2020 at 11:46:46AM +0000, Peter.Huewe@infineon.com wrote:
> > > Hi,
> > > NACK
> > > 
> > > > % git --no-pager grep IFX0102 drivers/char/tpm
> > > > drivers/char/tpm/tpm_infineon.c:	{"IFX0102", 0},
> > > > drivers/char/tpm/tpm_tis.c:	{"IFX0102", 0},		/* Infineon */
> > > > Obviously IFX0102 was added to the HID table for the TCG TIS driver by mistake.
> > > 
> > > The HID IFX0102 was NOT added by mistake.
> > > Let me explain the history a bit:
> > > 
> > > Old SLB 9635 / 9630 TPMs had two ways to interface them
> > > - proprietary 'io' mapped protocol (tpm_infineon) - tis protocol  (tpm_tis)
> > > 
> > > Both match the same HID.
> > > However with the emerging of the tis protocol, the io protocol eventually went away for newer products.
> > > So all TPM1.2 by IFX match the HID0102 and the TCG generic ones PNP0C31
> > > 
> > > So basically you break TPM1.2 support for all (newer) Infineon chips if the platform vendor used the IFX0102 HID as they would speak via tpm_infineon driver.
> > > The bug must be something different, especially as it only seems to happen after suspend resume.
> > 
> > Peter,
> > 
> > Looking at dmesg:
> > 
> > 1. tmp_infineon initializes cleanly
> > 2. tpm_tis fails misserably with bunch error messages
> > 
> > I'm cool with reverting the patch though. Please send a revert patch and
> > explain this in the commit message because right now what you are saying
> > is completely undocumented.
> > 
> > Also, this tpm_infineon issue needs to be fixed properly after the
> > revert.
> > 
> > The bugzilla bug is unrelated to this issue but it causes extra harm
> > fixing any bugs and confusion among the users as the bug discussions
> > proves.
> > 
> > How do we get the quirks for tpm_tis and tpm_infineon so that they can
> > separate each other?
> 
> Also in the revert commit, please add a comment to tpm_tis.c
> about the existing conflict, e.g.
> 
> /*
>  * Legacy Infineon devices can emit illegit warnings as tpm_tis and
>  * tpm_infineon have a conflicting device ID IFX0102.
>  */
> 
> I'm cool reverting it as long as I get a patch with the required
> premises to do so and proper documentation, because the issue is
> still real.

We do have this kind of thing for MSFT0101 in tpm_tis.c: check_acpi_tpm2().

This is not the root cause for the bugzilla bug, but is a bug itself,
and this kind of behaviour should not exist. It makes fixing real bugs
factors harder when you have drivers putting arbirtrary warnings to
klog.


/Jarkko