Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751802AbbHSVvM (ORCPT ); Wed, 19 Aug 2015 17:51:12 -0400 Received: from gate.crashing.org ([63.228.1.57]:34503 "EHLO gate.crashing.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752209AbbHSVvK (ORCPT ); Wed, 19 Aug 2015 17:51:10 -0400 Message-ID: <1440021013.2737.14.camel@kernel.crashing.org> Subject: Re: [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver From: Benjamin Herrenschmidt To: Laurent Dufour , David Laight , "linuxppc-dev@lists.ozlabs.org" Cc: Greg Kroah-Hartman , Jiri Slaby , "linux-kernel@vger.kernel.org" Date: Thu, 20 Aug 2015 07:50:13 +1000 In-Reply-To: <55D4F702.4020503@linux.vnet.ibm.com> References: <1438334990-11765-1-git-send-email-ldufour@linux.vnet.ibm.com> <063D6719AE5E284EB5DD2968C1650D6D1CB71D3E@AcuExch.aculab.com> <55D4F702.4020503@linux.vnet.ibm.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.5 (3.16.5-1.fc22) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2114 Lines: 59 On Wed, 2015-08-19 at 23:37 +0200, Laurent Dufour wrote: > On 03/08/2015 13:00, David Laight wrote: From: Laurent Dufour > > > Sent: 31 July 2015 10:30 > > > This patch fixes several endianness issues detected when running > > > the HVSI > > > driver in little endian mode. > > > > > > These issues are raised in little endian mode because the data > > > exchanged in > > > memory between the kernel and the hypervisor has to be in big > > > endian > > > format. > > ... > > > diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c > > > index 41901997c0d6..a75146f600cb 100644 > > > --- a/drivers/tty/hvc/hvsi.c > > > +++ b/drivers/tty/hvc/hvsi.c > > > @@ -240,9 +240,9 @@ static void hvsi_recv_control(struct > > > hvsi_struct *hp, uint8_t *packet, > > > { > > > struct hvsi_control *header = (struct hvsi_control > > > *)packet; > > > > > > - switch (header->verb) { > > > + switch (be16_to_cpu(header->verb)) { > > > case VSV_MODEM_CTL_UPDATE: > > > - if ((header->word & HVSI_TSCD) == 0) { > > > + if ((be32_to_cpu(header->word) & > > > HVSI_TSCD) == 0) { > > > > It is generally best to byteswap constants. > > > > David > > Thanks David for your review. > Regarding the byte swapping of the constants, I'm wondering if this > the > best way here. > For instance, Benjamin wrote a similar patch to fix another > endianness > issue (99fc1d91b8fc) and he doesn't convert the constant neither. > It think that byte swapping the constant value will impact more code, > and may not ease code reading. Right, I disagree with byteswapping the constants at their definition point, however maybe he meant using cpu_to_be16(CONSTANT) ? In any case, it's pretty moot as we have the lhbrx instruction which will do the load and byteswap and for HVSI, even if it was a tad slower than a normal load, it would not make a noticeable difference. Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/