linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Vicky <honclo@linux.vnet.ibm.com>, Ashley Lai <ashleydlai@gmail.com>
Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>,
	"Michal Such\?\?nek" <msuchanek@suse.de>,
	Nayna Jain <nayna@linux.vnet.ibm.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Peter Huewe <peterhuewe@gmx.de>,
	Marcel Selhorst <tpmdd@selhorst.net>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	tpmdd-devel@lists.sourceforge.net, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org
Subject: Re: ibmvtpm byteswapping inconsistency
Date: Thu, 02 Feb 2017 21:55:57 +1100	[thread overview]
Message-ID: <87k298c14y.fsf@concordia.ellerman.id.au> (raw)
In-Reply-To: <F14AC3E9-E04F-4EF0-9C7E-999D481EA577@linux.vnet.ibm.com>

Vicky <honclo@linux.vnet.ibm.com> writes:

>> On Jan 26, 2017, at 5:58 PM, Ashley Lai <ashleydlai@gmail.com> wrote:
>> 
>> Adding Vicky from IBM.
>> 
>> 
>> On 01/26/2017 04:05 PM, Jason Gunthorpe wrote:
>>> On Thu, Jan 26, 2017 at 09:22:48PM +0100, Michal Such??nek wrote:
>>> 
>>>> This is repeated a few times in the driver so I added memset to quiet
>>>> gcc and make behavior deterministic in case the unused fields get some
>>>> meaning in the future.
>>> Yep, reserved certainly needs to be zeroed.. Can you send a patch?
>>> memset is overkill...
>>> 
>>>> However, in tpm_ibmvtpm_send the structure is initialized as
>>>> 
>>>> 	struct ibmvtpm_crq crq;
>>>>         __be64 *word = (__be64 *)&crq;
>>>> ...
>>>>         crq.valid = (u8)IBMVTPM_VALID_CMD;
>>>>         crq.msg = (u8)VTPM_TPM_COMMAND;
>>>>         crq.len = cpu_to_be16(count);
>>>>         crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle);
>>>> 
>>>> and submitted with
>>>> 
>>>> 	rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
>>>>                               be64_to_cpu(word[1]));
>>>> meaning it is swapped twice.
>>> No idea, Nayna may know.
>>> 
>>> My guess is that '__be64 *word' should be 'u64 *word'...
>>> 
>>> Jason
>> 
>
> I don’t think we want ‘word' to be changed back to be of type ‘u64’.   Please see commit 62dfd912ab3b5405b6fe72d0135c37e9648071f1

The type of word is basically irrelevant. Unless you're running sparse
and actually checking the errors, which it seems you're not doing:

  drivers/char/tpm/tpm_ibmvtpm.c:90:30: warning: cast removes address space of expression
  drivers/char/tpm/tpm_ibmvtpm.c:91:23: warning: incorrect type in argument 1 (different address spaces)
  drivers/char/tpm/tpm_ibmvtpm.c:91:23:    expected void *<noident>
  drivers/char/tpm/tpm_ibmvtpm.c:91:23:    got void [noderef] <asn:2>*rtce_buf
  drivers/char/tpm/tpm_ibmvtpm.c:136:17: warning: cast removes address space of expression
  drivers/char/tpm/tpm_ibmvtpm.c:188:46: warning: incorrect type in argument 2 (different base types)
  drivers/char/tpm/tpm_ibmvtpm.c:188:46:    expected unsigned long long [unsigned] [usertype] w1
  drivers/char/tpm/tpm_ibmvtpm.c:188:46:    got restricted __be64 [usertype] <noident>
  drivers/char/tpm/tpm_ibmvtpm.c:189:31: warning: incorrect type in argument 3 (different base types)
  drivers/char/tpm/tpm_ibmvtpm.c:189:31:    expected unsigned long long [unsigned] [usertype] w2
  drivers/char/tpm/tpm_ibmvtpm.c:189:31:    got restricted __be64 [usertype] <noident>
  drivers/char/tpm/tpm_ibmvtpm.c:215:46: warning: incorrect type in argument 2 (different base types)
  drivers/char/tpm/tpm_ibmvtpm.c:215:46:    expected unsigned long long [unsigned] [usertype] w1
  drivers/char/tpm/tpm_ibmvtpm.c:215:46:    got restricted __be64 [usertype] <noident>
  drivers/char/tpm/tpm_ibmvtpm.c:216:31: warning: incorrect type in argument 3 (different base types)
  drivers/char/tpm/tpm_ibmvtpm.c:216:31:    expected unsigned long long [unsigned] [usertype] w2
  drivers/char/tpm/tpm_ibmvtpm.c:216:31:    got restricted __be64 [usertype] <noident>
  drivers/char/tpm/tpm_ibmvtpm.c:294:30: warning: incorrect type in argument 1 (different address spaces)
  drivers/char/tpm/tpm_ibmvtpm.c:294:30:    expected void const *<noident>
  drivers/char/tpm/tpm_ibmvtpm.c:294:30:    got void [noderef] <asn:2>*rtce_buf
  drivers/char/tpm/tpm_ibmvtpm.c:342:46: warning: incorrect type in argument 2 (different base types)
  drivers/char/tpm/tpm_ibmvtpm.c:342:46:    expected unsigned long long [unsigned] [usertype] w1
  drivers/char/tpm/tpm_ibmvtpm.c:342:46:    got restricted __be64 [usertype] <noident>
  drivers/char/tpm/tpm_ibmvtpm.c:343:31: warning: incorrect type in argument 3 (different base types)
  drivers/char/tpm/tpm_ibmvtpm.c:343:31:    expected unsigned long long [unsigned] [usertype] w2
  drivers/char/tpm/tpm_ibmvtpm.c:343:31:    got restricted __be64 [usertype] <noident>
  drivers/char/tpm/tpm_ibmvtpm.c:494:43: warning: incorrect type in assignment (different address spaces)
  drivers/char/tpm/tpm_ibmvtpm.c:494:43:    expected void [noderef] <asn:2>*rtce_buf
  drivers/char/tpm/tpm_ibmvtpm.c:494:43:    got void *
  drivers/char/tpm/tpm_ibmvtpm.c:501:52: warning: incorrect type in argument 2 (different address spaces)
  drivers/char/tpm/tpm_ibmvtpm.c:501:52:    expected void *ptr
  drivers/char/tpm/tpm_ibmvtpm.c:501:52:    got void [noderef] <asn:2>*rtce_buf
  drivers/char/tpm/tpm_ibmvtpm.c:507:46: warning: incorrect type in argument 1 (different address spaces)
  drivers/char/tpm/tpm_ibmvtpm.c:507:46:    expected void const *<noident>
  drivers/char/tpm/tpm_ibmvtpm.c:507:46:    got void [noderef] <asn:2>*rtce_buf


What matters is how you actually do the byte swaps.

cheers

  reply	other threads:[~2017-02-02 10:56 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-26 20:22 ibmvtpm byteswapping inconsistency Michal Suchánek
2017-01-26 22:05 ` Jason Gunthorpe
2017-01-26 22:43   ` Michal Suchanek
2017-01-26 22:58   ` Ashley Lai
2017-02-02  4:40     ` Vicky
2017-02-02 10:55       ` Michael Ellerman [this message]
2017-02-02 11:29       ` Michal Suchánek
2017-02-02 15:17         ` David Laight
2017-01-27  1:42 ` Tyrel Datwyler
2017-01-27  1:50   ` Benjamin Herrenschmidt
2017-01-27  9:03     ` Michal Suchanek
2017-01-27 21:19       ` Tyrel Datwyler
2017-01-30  4:32         ` Michael Ellerman
2017-01-30 20:34           ` Tyrel Datwyler
2017-01-31  8:38             ` Michael Ellerman
2017-01-27 18:02     ` Tyrel Datwyler
2017-01-27 19:58       ` Benjamin Herrenschmidt
2017-01-27 20:32         ` Tyrel Datwyler
2017-01-28  0:35           ` msuchanek
2017-01-28  4:28           ` Benjamin Herrenschmidt
2017-01-30 14:42       ` David Laight
2017-01-27 11:18 ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k298c14y.fsf@concordia.ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=ashleydlai@gmail.com \
    --cc=benh@kernel.crashing.org \
    --cc=honclo@linux.vnet.ibm.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jgunthorpe@obsidianresearch.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=msuchanek@suse.de \
    --cc=nayna@linux.vnet.ibm.com \
    --cc=paulus@samba.org \
    --cc=peterhuewe@gmx.de \
    --cc=tpmdd-devel@lists.sourceforge.net \
    --cc=tpmdd@selhorst.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).