[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH-for-5.2] hw/s390x/pci: Fix endianness issue

From: Matthew Rosato
Subject: Re: [RFC PATCH-for-5.2] hw/s390x/pci: Fix endianness issue
Date: Tue, 17 Nov 2020 09:02:37 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1

On 11/17/20 8:31 AM, Cornelia Huck wrote:
On Tue, 17 Nov 2020 14:23:57 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

On 11/17/20 2:00 PM, Peter Maydell wrote:
On Tue, 17 Nov 2020 at 12:03, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

Fix an endianness issue reported by Cornelia:
s390x tcg guest on x86, virtio-pci devices are not detected. The
relevant feature bits are visible to the guest. Same breakage with
different guest kernels.
KVM guests and s390x tcg guests on s390x are fine.

Fixes: 28dc86a0729 ("s390x/pci: use a PCI Group structure")
Reported-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
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 = (ClpReqQueryPciGrp *)reqh;
           S390PCIGroup *group;

-        group = s390_group_find(reqgrp->g);
+        group = s390_group_find(ldl_p(&reqgrp->g));

'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

-- PMM

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;

No, same crash... I fear there are more things broken wrt endianness.

Sorry, just getting online now, looking at the code.... Are the 2 memcpy calls added in 9670ee75 and 28dc86a0 the issue? Won't they just present the Q PCI FN / Q PCI FN GRP results in host endianness?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]