linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Laurent Dufour <ldufour@linux.vnet.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.cz>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver
Date: Thu, 20 Aug 2015 11:40:11 +1000	[thread overview]
Message-ID: <1440034811.13406.9.camel@ellerman.id.au> (raw)
In-Reply-To: <55D4FAE9.7000504@linux.vnet.ibm.com>

On Wed, 2015-08-19 at 23:53 +0200, Laurent Dufour wrote:
> On 04/08/2015 02:51, Michael Ellerman wrote:
> > On Fri, 2015-07-31 at 11:29 +0200, Laurent Dufour wrote:
> >> 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.
> > 
> > Can you include the sparse output before and after?
> 
> Hi Michael,
> 
> Here is the output message displayed on the console when the bug occurred:
> 
> [    0.000517] irq: (null) didn't like hwirq-0x1000a00 to VIRQ16 mapping (rc=-22)
> [    0.000578] hvsi_console_init: couldn't create irq mapping for 0x1000a00
> 
> With the patch is applied, the hvsi driver is initializing correctly and
> no message is displayed, except the one saying the number of device the
> hvsi driver has configured. For instance:

> [    1.535783] HVSI: registered 1 devices


OK that's also good.

I was talking about the output from sparse endian checking, as the driver seems
to already be endian annotated.

ie, before:

$ make C=2 CF=-D__CHECK_ENDIAN__
...
drivers/tty/hvc/hvsi.c:245:36: warning: restricted __be32 degrades to integer
drivers/tty/hvc/hvsi.c:243:23: warning: restricted __be16 degrades to integer
drivers/tty/hvc/hvsi.c:243:23: warning: restricted __be16 degrades to integer
drivers/tty/hvc/hvsi.c:277:36: warning: restricted __be32 degrades to integer
drivers/tty/hvc/hvsi.c:279:36: warning: restricted __be32 degrades to integer
drivers/tty/hvc/hvsi.c:298:26: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:298:26:    expected restricted __be16 [assigned] [usertype] seqno
drivers/tty/hvc/hvsi.c:298:26:    got int
drivers/tty/hvc/hvsi.c:299:21: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:299:21:    expected restricted __be16 [assigned] [usertype] verb
drivers/tty/hvc/hvsi.c:299:21:    got int
drivers/tty/hvc/hvsi.c:301:28: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:301:28:    expected restricted __be16 [assigned] [usertype] query_seqno
drivers/tty/hvc/hvsi.c:301:28:    got int
drivers/tty/hvc/hvsi.c:322:60: warning: incorrect type in argument 2 (different base types)
drivers/tty/hvc/hvsi.c:322:60:    expected unsigned short [unsigned] [usertype] query_seqno
drivers/tty/hvc/hvsi.c:322:60:    got restricted __be16 [usertype] seqno
drivers/tty/hvc/hvsi.c:558:26: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:558:26:    expected restricted __be16 [assigned] [usertype] seqno
drivers/tty/hvc/hvsi.c:558:26:    got int
drivers/tty/hvc/hvsi.c:559:21: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:559:21:    expected restricted __be16 [assigned] [usertype] verb
drivers/tty/hvc/hvsi.c:559:21:    got unsigned short [unsigned] [usertype] verb
drivers/tty/hvc/hvsi.c:600:26: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:600:26:    expected restricted __be16 [assigned] [usertype] seqno
drivers/tty/hvc/hvsi.c:600:26:    got int
drivers/tty/hvc/hvsi.c:602:21: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:602:21:    expected restricted __be16 [assigned] [usertype] verb
drivers/tty/hvc/hvsi.c:602:21:    got int
drivers/tty/hvc/hvsi.c:603:21: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:603:21:    expected restricted __be32 [assigned] [usertype] mask
drivers/tty/hvc/hvsi.c:603:21:    got int
drivers/tty/hvc/hvsi.c:606:29: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:606:29:    expected restricted __be32 [assigned] [usertype] word
drivers/tty/hvc/hvsi.c:606:29:    got int
drivers/tty/hvc/hvsi.c:683:26: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:683:26:    expected restricted __be16 [assigned] [usertype] seqno
drivers/tty/hvc/hvsi.c:683:26:    got int
drivers/tty/hvc/hvsi.c:700:26: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:700:26:    expected restricted __be16 [assigned] [usertype] seqno
drivers/tty/hvc/hvsi.c:700:26:    got int
drivers/tty/hvc/hvsi.c:702:21: warning: incorrect type in assignment (different base types)
drivers/tty/hvc/hvsi.c:702:21:    expected restricted __be16 [assigned] [usertype] verb
drivers/tty/hvc/hvsi.c:702:21:    got int
...

And with your patch applied there are no warnings from hvsi.c!

So it seems you fixed all the issues, or at least all the issues we can detect
with sparse. So I'll merge this as-is with an updated changelog.

cheers

  reply	other threads:[~2015-08-20  1:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-31  9:29 [PATCH] powerpc/hvsi: Fix endianness issues in the HVSI driver Laurent Dufour
2015-08-03 11:00 ` David Laight
2015-08-19 21:37   ` Laurent Dufour
2015-08-19 21:50     ` Benjamin Herrenschmidt
2015-08-04  0:51 ` Michael Ellerman
2015-08-19 21:53   ` Laurent Dufour
2015-08-20  1:40     ` Michael Ellerman [this message]
2015-08-20  4:20       ` Michael Ellerman
2015-08-21  7:43 ` Michael Ellerman

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=1440034811.13406.9.camel@ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.cz \
    --cc=ldufour@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /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).