From: "Shaikh, Azhar" <azhar.shaikh@intel.com>
To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
CC: "jarkko.sakkinen@linux.intel.com" <jarkko.sakkinen@linux.intel.com>,
        "tpmdd-devel@lists.sourceforge.net" 
        <tpmdd-devel@lists.sourceforge.net>,
        "linux-security-module@vger.kernel.org" 
        <linux-security-module@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [tpmdd-devel] [PATCH] tpm: Fix the ioremap() call for Braswell
 systems
Thread-Topic: [tpmdd-devel] [PATCH] tpm: Fix the ioremap() call for Braswell
 systems
Thread-Index: AQHS64WQkROWrf8+Ikae2Vao1eB4gqIxyEGA//+MXAA=
Date: Thu, 22 Jun 2017 20:50:49 +0000
Message-ID: <5FFFAD06ADE1CA4381B3F0F7C6AF5828916F50@ORSMSX109.amr.corp.intel.com>
References: <1498156197-180919-1-git-send-email-azhar.shaikh@intel.com>
 <20170622202023.GA3579@obsidianresearch.com>
In-Reply-To: <20170622202023.GA3579@obsidianresearch.com>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 10.0.102.7
dlp-reaction: no-action
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2552
Lines: 78


> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgunthorpe@obsidianresearch.com]
> Sent: Thursday, June 22, 2017 1:20 PM
> To: Shaikh, Azhar <azhar.shaikh@intel.com>
> Cc: jarkko.sakkinen@linux.intel.com; tpmdd-devel@lists.sourceforge.net;
> linux-security-module@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [tpmdd-devel] [PATCH] tpm: Fix the ioremap() call for Braswell
> systems
> 
> On Thu, Jun 22, 2017 at 11:29:57AM -0700, Azhar Shaikh wrote:
> > ioremap() for Intel Braswell processors was done in
> > tpm_tis_pnp_init(). But before this function gets called, platform
> > driver 'tis_drv' gets registered and its probe function
> > tpm_tis_plat_probe() is invoked, which does a TPM access. Now for
> > Braswell processors tpm_platform_begin_xfer() will do an ioread32()
> > without having a mapped address, which will lead to a bad I/O access
> > warning.
> > Hence move the ioremap() call from tpm_tis_pnp_init() to init_tis()
> > before registering the 'tis_drv' or basically before any TPM access.
> > Accordingly also move the iounmap() call from tpm_tis_pnp_remove() to
> > cleanup_tis().
> >
> > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
> > drivers/char/tpm/tpm_tis.c | 21 +++++++++------------
> >  1 file changed, 9 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> > index 506e62ca3576..3224db80816a 100644
> > +++ b/drivers/char/tpm/tpm_tis.c
> > @@ -330,12 +330,6 @@ static int tpm_tis_pnp_init(struct pnp_dev
> *pnp_dev,
> >  	else
> >  		tpm_info.irq = -1;
> >
> > -#ifdef CONFIG_X86
> > -	if (is_bsw())
> > -		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
> > -					ILB_REMAP_SIZE);
> > -#endif
> > -
> >  	return tpm_tis_init(&pnp_dev->dev, &tpm_info);  }
> >
> > @@ -359,12 +353,6 @@ static void tpm_tis_pnp_remove(struct pnp_dev
> > *dev)
> >
> >  	tpm_chip_unregister(chip);
> >  	tpm_tis_remove(chip);
> > -
> > -#ifdef CONFIG_X86
> > -	if (is_bsw())
> > -		iounmap(ilb_base_addr);
> > -#endif
> > -
> >  }
> >
> >  static struct pnp_driver tis_pnp_driver = { @@ -472,6 +460,11 @@
> > static int __init init_tis(void)
> >  	if (rc)
> >  		goto err_force;
> >
> > +#ifdef CONFIG_X86
> > +	if (is_bsw())
> > +		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
> > +					ILB_REMAP_SIZE);
> > +#endif
> 
> The iounmap needs to be in the goto unwind for init_tis as well, surely?
> 

Yes! Should have checked before...

> >  	rc = platform_driver_register(&tis_drv);
> >  	if (rc)
> >  		goto err_platform;
> 
> Jason