From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752456AbbIOQq0 (ORCPT ); Tue, 15 Sep 2015 12:46:26 -0400 Received: from mga02.intel.com ([134.134.136.20]:60987 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751573AbbIOQqZ (ORCPT ); Tue, 15 Sep 2015 12:46:25 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,536,1437462000"; d="scan'208";a="645329063" Date: Tue, 15 Sep 2015 19:46:19 +0300 From: Jarkko Sakkinen To: Jason Gunthorpe Cc: Jarkko Sakkinen , tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, Peter Huewe , Marcel Selhorst Subject: Re: [PATCH] tpm, tpm_crb: fix unaligned read of the command buffer address Message-ID: <20150915164619.GA7164@intel.com> References: <1442250923-19804-1-git-send-email-jarkko.sakkinen@linux.intel.com> <20150914173523.GC21652@obsidianresearch.com> <20150915100956.GA3264@intel.com> <20150915163039.GA17975@obsidianresearch.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150915163039.GA17975@obsidianresearch.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 15, 2015 at 10:30:39AM -0600, Jason Gunthorpe wrote: > On Tue, Sep 15, 2015 at 01:09:56PM +0300, Jarkko Sakkinen wrote: > > However, for MMIO address the hardware might abort the entire request > > when trying to do a 64-bit read, which causes the CPU to fill the result > > with 1's. > > Okay, yes, for iomem you can't rely on packed. > > packed actually can mess up iomem loads on some arches as it also > tells the compiler things are unaligned. I'd drop the __packed since > the new structure is naturally packed in this case. (for other cases > be careful to add __aligned(2) to avoid unaligned accesses) > > However, I'm still confused, the original code did: > memcpy_fromio(&pa, &priv->cca->cmd_pa, 8); > > Which might do byte reads from the iomem cmd_pa, but there should be > no problem with an unaligned access. > > Is the real issue that you can't do memcpy_fromio to tpm control > memory? That would not suprise me one bit. In which case the commit > message should be revised. Good question and point. Emprically it seems to be so. I guess you have to do exactly 32-bit read for the field. I'll revise the commit message. > > This is not hypothetical bug. We are experiencing this on some platforms > > and the proposed fix resolves the issue. > > I am confused because of the memcpy_fromio: > > memcpy_fromio(&pa, &priv->cca->cmd_pa, 8); > - pa = le64_to_cpu(pa); > + > + pa = ((u64) le32_to_cpu(ioread32(&priv->cca->cmd_pa_high)) << 32) + > + (u64) le32_to_cpu(ioread32(&priv->cca->cmd_pa_low)); > priv->cmd = devm_ioremap_nocache(dev, pa, > ioread32(&priv->cca->cmd_size)); > > The code wasn't doing a direct load from cmd_pa, so the type doesn't > matter. > > BTW. Does the above even compile with that memcpy_fromio left in? Nope :) See my own reply to the original message. > Jason /Jarkko