From patchwork Tue Mar 13 04:22:42 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Herrenschmidt X-Patchwork-Id: 146344 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 1D9D7B6FB6 for ; Tue, 13 Mar 2012 15:23:10 +1100 (EST) Received: from localhost ([::1]:35554 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7JGO-0004oV-3m for incoming@patchwork.ozlabs.org; Tue, 13 Mar 2012 00:23:08 -0400 Received: from eggs.gnu.org ([208.118.235.92]:47710) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7JGG-0004oE-VG for qemu-devel@nongnu.org; Tue, 13 Mar 2012 00:23:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S7JGF-0004Lg-1x for qemu-devel@nongnu.org; Tue, 13 Mar 2012 00:23:00 -0400 Received: from gate.crashing.org ([63.228.1.57]:43368) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7JGE-0004LR-PY for qemu-devel@nongnu.org; Tue, 13 Mar 2012 00:22:58 -0400 Received: from [IPv6:::1] (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.13.8) with ESMTP id q2D4Mga1031322; Mon, 12 Mar 2012 23:22:44 -0500 Message-ID: <1331612562.3105.96.camel@pasglop> From: Benjamin Herrenschmidt To: Gerd Hoffmann Date: Tue, 13 Mar 2012 15:22:42 +1100 In-Reply-To: <4F59FBA6.9070805@redhat.com> References: <[0/4] RESEND: Outstanding bugfixes and cleanups> <1331167272-9800-1-git-send-email-david@gibson.dropbear.id.au> <1331167272-9800-3-git-send-email-david@gibson.dropbear.id.au> <4F59FBA6.9070805@redhat.com> X-Mailer: Evolution 3.2.2- Mime-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 63.228.1.57 Cc: qemu-devel@nongnu.org, anthony@codemonkey.ws, David Gibson Subject: Re: [Qemu-devel] [PATCH 2/4] Endian fix an assertion in usb-msd X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Fri, 2012-03-09 at 13:46 +0100, Gerd Hoffmann wrote: > On 03/08/12 01:41, David Gibson wrote: > > From: Benjamin Herrenschmidt > > > - assert(s->csw.sig == 0x53425355); > > + assert(s->csw.sig == cpu_to_le32(0x53425355)); > > Correct but incomplete. residue is sent in host endian instead of > little endian too. It is zero most of the time though, so it easily > goes unnoticed. > > Can you try the attached patch instead? I dislike the patch as it does an in-place swap of the structure. I tend to prefer a given structure to have a known endian, never change in place. For example it would make it harder to ever implement endian annotations like the kernel does (to have sparse find the bugs for us). As such I would rather fix the access locations instead. This basically means adding this small patch on top of my previous one (I haven't spotted any other error) : diff --git a/hw/usb-msd.c b/hw/usb-msd.c index c18e76d..d5d7f74 100644 --- a/hw/usb-msd.c +++ b/hw/usb-msd.c @@ -193,7 +193,7 @@ static void usb_msd_send_status(MSDState *s, USBPacket *p) int len; DPRINTF("Command status %d tag 0x%x, len %zd\n", - s->csw.status, s->csw.tag, p->iov.size); + s->csw.status, le32_to_cpu(s->csw.tag), p->iov.size); assert(s->csw.sig == cpu_to_le32(0x53425355)); len = MIN(sizeof(s->csw), p->iov.size); @@ -233,7 +233,7 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status, size_t r s->csw.sig = cpu_to_le32(0x53425355); s->csw.tag = cpu_to_le32(req->tag); - s->csw.residue = s->residue; + s->csw.residue = cpu_to_le32(s->residue); s->csw.status = status != 0; if (s->packet) {