From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751414AbdBBLkH convert rfc822-to-8bit (ORCPT ); Thu, 2 Feb 2017 06:40:07 -0500 Received: from mx2.suse.de ([195.135.220.15]:49182 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751019AbdBBLkF (ORCPT ); Thu, 2 Feb 2017 06:40:05 -0500 Date: Thu, 2 Feb 2017 12:29:42 +0100 From: Michal =?UTF-8?B?U3VjaMOhbmVr?= To: Vicky Cc: Ashley Lai , Nayna Jain , Marcel Selhorst , linux-kernel@vger.kernel.org, Jarkko Sakkinen , Jason Gunthorpe , tpmdd-devel@lists.sourceforge.net, Paul Mackerras , Peter Huewe , linuxppc-dev@lists.ozlabs.org Subject: Re: ibmvtpm byteswapping inconsistency Message-ID: <20170202122942.7fd9988d@kitsune.suse.cz> In-Reply-To: References: <20170126212248.3f3e9103@kitsune.suse.cz> <20170126220536.GB31937@obsidianresearch.com> <024dadba-a75c-ab59-9d12-a9bea81f9bda@gmail.com> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.31; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 1 Feb 2017 23:40:33 -0500 Vicky wrote: > > On Jan 26, 2017, at 5:58 PM, Ashley Lai > > 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 word is marked correctly as __be64 in that patch because count and handle are swapped to BE when saved to it and the whole word is then swapped again when loaded. If you just load ((u64)IBMVTPM_VALID_CMD << 56 | ((u64)VTPM_TPM_COMMAND << 48) | ((u64)count << 32) | ibmvtpm->rtce_dma_handle in a register it works equally well without any __be and swaps involved. Note however that __be64 and u64 are all the same to the compiler. It's just a note for the reader and analysis tools. Thanks Michal From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3vDdMt0fZbzDq8b for ; Thu, 2 Feb 2017 22:40:09 +1100 (AEDT) Date: Thu, 2 Feb 2017 12:29:42 +0100 From: Michal =?UTF-8?B?U3VjaMOhbmVr?= To: Vicky Cc: Ashley Lai , Nayna Jain , Marcel Selhorst , linux-kernel@vger.kernel.org, Jarkko Sakkinen , Jason Gunthorpe , tpmdd-devel@lists.sourceforge.net, Paul Mackerras , Peter Huewe , linuxppc-dev@lists.ozlabs.org Subject: Re: ibmvtpm byteswapping inconsistency Message-ID: <20170202122942.7fd9988d@kitsune.suse.cz> In-Reply-To: References: <20170126212248.3f3e9103@kitsune.suse.cz> <20170126220536.GB31937@obsidianresearch.com> <024dadba-a75c-ab59-9d12-a9bea81f9bda@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Wed, 1 Feb 2017 23:40:33 -0500 Vicky wrote: > > On Jan 26, 2017, at 5:58 PM, Ashley Lai > > wrote: > >=20 > > Adding Vicky from IBM. > >=20 > >=20 > > On 01/26/2017 04:05 PM, Jason Gunthorpe wrote: =20 > >> On Thu, Jan 26, 2017 at 09:22:48PM +0100, Michal Such??nek wrote: > >> =20 > >>> 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. =20 > >> Yep, reserved certainly needs to be zeroed.. Can you send a patch? > >> memset is overkill... > >> =20 > >>> However, in tpm_ibmvtpm_send the structure is initialized as > >>>=20 > >>> struct ibmvtpm_crq crq; > >>> __be64 *word =3D (__be64 *)&crq; > >>> ... > >>> crq.valid =3D (u8)IBMVTPM_VALID_CMD; > >>> crq.msg =3D (u8)VTPM_TPM_COMMAND; > >>> crq.len =3D cpu_to_be16(count); > >>> crq.data =3D cpu_to_be32(ibmvtpm->rtce_dma_handle); > >>>=20 > >>> and submitted with > >>>=20 > >>> rc =3D ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]), > >>> be64_to_cpu(word[1])); > >>> meaning it is swapped twice. =20 > >> No idea, Nayna may know. > >>=20 > >> My guess is that '__be64 *word' should be 'u64 *word'... > >>=20 > >> Jason =20 > > =20 >=20 > I don=E2=80=99t think we want =E2=80=98word' to be changed back to be of = type > =E2=80=98u64=E2=80=99. Please see commit 62dfd912ab3b5405b6fe72d0135c37= e9648071f1 The word is marked correctly as __be64 in that patch because count and handle are swapped to BE when saved to it and the whole word is then swapped again when loaded. If you just load ((u64)IBMVTPM_VALID_CMD << 56 | ((u64)VTPM_TPM_COMMAND << 48) | ((u64)count << 32) | ibmvtpm->rtce_dma_handle in a register it works equally well without any __be and swaps involved. Note however that __be64 and u64 are all the same to the compiler. It's just a note for the reader and analysis tools. Thanks Michal