From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 80B44C2D0E4 for ; Tue, 17 Nov 2020 15:21:19 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E537824198 for ; Tue, 17 Nov 2020 15:21:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="hScRW12y" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E537824198 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:34736 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kf2nF-00068Y-E2 for qemu-devel@archiver.kernel.org; Tue, 17 Nov 2020 10:21:17 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:48528) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kf2jj-0002RD-GG for qemu-devel@nongnu.org; Tue, 17 Nov 2020 10:17:40 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:43399) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1kf2jf-0007kx-8s for qemu-devel@nongnu.org; Tue, 17 Nov 2020 10:17:39 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1605626253; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5yW2vxOUKDaggfU5vkMEnwiUtU9gdEHNkQzXqnGxScE=; b=hScRW12yIpwH91nPdqZaxjF+xAy4mzrrP1BmA70RFdpuIaDTReA3Ugq++ypn3c5QYaN0Xa mR7xi5bQM5PPakCq4rMy7Ntu1RwA4N2lxhE+eDoZudY/NWV1Impj8aJ2FHpKFsbKH3hj7l uDXNklkzToY/dtFiglYTumBroZdSAgg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-568-25nJ6-RzOr2wIheV0gu_YA-1; Tue, 17 Nov 2020 10:17:29 -0500 X-MC-Unique: 25nJ6-RzOr2wIheV0gu_YA-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 04723879524; Tue, 17 Nov 2020 15:17:28 +0000 (UTC) Received: from gondolin (ovpn-113-115.ams2.redhat.com [10.36.113.115]) by smtp.corp.redhat.com (Postfix) with ESMTP id A2CB375121; Tue, 17 Nov 2020 15:17:19 +0000 (UTC) Date: Tue, 17 Nov 2020 16:17:16 +0100 From: Cornelia Huck To: Matthew Rosato Subject: Re: [RFC PATCH-for-5.2] hw/s390x/pci: Fix endianness issue Message-ID: <20201117161716.7e3e67ca.cohuck@redhat.com> In-Reply-To: References: <20201117120115.1234994-1-philmd@redhat.com> <20201117143117.4b05db78.cohuck@redhat.com> <20201117151340.539d55b2.cohuck@redhat.com> Organization: Red Hat GmbH MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=cohuck@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Received-SPF: pass client-ip=63.128.21.124; envelope-from=cohuck@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/11/17 00:41:22 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] [fuzzy] X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , Thomas Huth , Pierre Morel , David Hildenbrand , qemu-s390x , Richard Henderson , QEMU Developers , Halil Pasic , Christian Borntraeger , Alex Williamson , Philippe =?UTF-8?B?TWF0aGlldS1EYXVkw6k=?= Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On Tue, 17 Nov 2020 09:34:41 -0500 Matthew Rosato wrote: > On 11/17/20 9:13 AM, Cornelia Huck wrote: > > On Tue, 17 Nov 2020 09:02:37 -0500 > > Matthew Rosato wrote: > > =20 > >> On 11/17/20 8:31 AM, Cornelia Huck wrote: =20 > >>> On Tue, 17 Nov 2020 14:23:57 +0100 > >>> Pierre Morel wrote: > >>> =20 > >>>> On 11/17/20 2:00 PM, Peter Maydell wrote: =20 > >>>>> On Tue, 17 Nov 2020 at 12:03, Philippe Mathieu-Daud=C3=A9 wrote: =20 > >>>>>> > >>>>>> Fix an endianness issue reported by Cornelia: > >>>>>> =20 > >>>>>>> s390x tcg guest on x86, virtio-pci devices are not detected. The > >>>>>>> relevant feature bits are visible to the guest. Same breakage wit= h > >>>>>>> different guest kernels. > >>>>>>> KVM guests and s390x tcg guests on s390x are fine. =20 > >>>>>> > >>>>>> Fixes: 28dc86a0729 ("s390x/pci: use a PCI Group structure") > >>>>>> Reported-by: Cornelia Huck > >>>>>> Signed-off-by: Philippe Mathieu-Daud=C3=A9 > >>>>>> --- > >>>>>> RFC because review-only patch, untested > >>>>>> --- > >>>>>> hw/s390x/s390-pci-inst.c | 2 +- > >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>>> > >>>>>> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c > >>>>>> index 58cd041d17f..cfb54b4d8ec 100644 > >>>>>> --- a/hw/s390x/s390-pci-inst.c > >>>>>> +++ b/hw/s390x/s390-pci-inst.c > >>>>>> @@ -305,7 +305,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2,= uintptr_t ra) > >>>>>> ClpReqQueryPciGrp *reqgrp =3D (ClpReqQueryPciGrp *)req= h; > >>>>>> S390PCIGroup *group; > >>>>>> > >>>>>> - group =3D s390_group_find(reqgrp->g); > >>>>>> + group =3D s390_group_find(ldl_p(&reqgrp->g)); =20 > >>>>> > >>>>> 'g' in the ClpReqQueryPciGrp struct is a uint32_t, so > >>>>> adding the ldl_p() will have no effect unless (a) the > >>>>> structure is not 4-aligned and (b) the host will fault on > >>>>> unaligned accesses, which isn't the case for x86 hosts. > >>>>> > >>>>> Q: is this struct really in host order, or should we > >>>>> be using ldl_le_p() or ldl_be_p() and friends here and > >>>>> elsewhere? > >>>>> > >>>>> thanks > >>>>> -- PMM > >>>>> =20 > >>>> > >>>> Hi, I think we better modify the structure here, g should be a byte. > >>>> > >>>> Connie, can you please try this if it resolves the issue? > >>>> > >>>> diff --git a/hw/s390x/s390-pci-inst.h b/hw/s390x/s390-pci-inst.h > >>>> index fa3bf8b5aa..641d19c815 100644 > >>>> --- a/hw/s390x/s390-pci-inst.h > >>>> +++ b/hw/s390x/s390-pci-inst.h > >>>> @@ -146,7 +146,8 @@ typedef struct ClpReqQueryPciGrp { > >>>> uint32_t fmt; > >>>> uint64_t reserved1; > >>>> #define CLP_REQ_QPCIG_MASK_PFGID 0xff > >>>> - uint32_t g; > >>>> + uint32_t g0 :24; > >>>> + uint32_t g :8; > >>>> uint32_t reserved2; > >>>> uint64_t reserved3; > >>>> } QEMU_PACKED ClpReqQueryPciGrp; > >>>> =20 > >>> > >>> No, same crash... I fear there are more things broken wrt endianness. > >>> =20 > >> > >> Sorry, just getting online now, looking at the code.... Are the 2 > >> memcpy calls added in 9670ee75 and 28dc86a0 the issue? Won't they jus= t > >> present the Q PCI FN / Q PCI FN GRP results in host endianness? > >> =20 > >=20 > > I just re-added some st?_p operations in set_pbdev_info and that fixes > > at least the crash I was seeing with Phil's patch applied. Still, no > > pci functions get detected, so that's not enough. Those memcpy calls > > look like a possible culprit. > > =20 >=20 > OK, so if everything in set_pbdev_info and s390_pci_init_default_group()= =20 > is handled with st?_p operations, then the memcpy should be OK... >=20 > Pierre was on to something with his recommendation, as the group id is=20 > only 1B of the 'g' field (see CLP_REQ_QPCIG_MASK_PFGID) - the other bits= =20 > just happen to be unused. >=20 > Did you include his change with your st?_p changes to set_pbdev_info=20 > (sorry, I don't have this environment set up but clearly need to do so=20 > for future testing) I tried in conjunction with Phil's patch (otherwise, I don't even get to the part where it crashes.) Do we need to apply that mask somewhere? It is hard to guess if you don't know what the structure is supposed to look like :)