On 11/17/20 6:13 PM, Cornelia Huck wrote:
> zPCI control blocks are big endian, we need to take care that we
> do proper accesses in order not to break tcg guests on little endian
> hosts.
>
> Fixes: 28dc86a07299 ("s390x/pci: use a PCI Group structure")
> Fixes: 9670ee752727 ("s390x/pci: use a PCI Function structure")
> Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
> Signed-off-by: Cornelia Huck <coh...@redhat.com>
> ---
>
> Works for me with virtio-pci devices for tcg on x86 and s390x, and for kvm.
> The vfio changes are not strictly needed; did not test them due to lack of
> hardware -- testing appreciated.
>
> As this fixes a regression, I want this in 5.2.
>
> ---
> hw/s390x/s390-pci-bus.c | 12 ++++++------
> hw/s390x/s390-pci-inst.c | 4 ++--
> hw/s390x/s390-pci-vfio.c | 8 ++++----
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index e0dc20ce4a56..17e64e0b1200 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -787,12 +787,12 @@ static void s390_pci_init_default_group(void)
>
> static void set_pbdev_info(S390PCIBusDevice *pbdev)
> {
> - pbdev->zpci_fn.sdma = ZPCI_SDMA_ADDR;
> - pbdev->zpci_fn.edma = ZPCI_EDMA_ADDR;
> - pbdev->zpci_fn.pchid = 0;
> + stq_p(&pbdev->zpci_fn.sdma, ZPCI_SDMA_ADDR);
"zPCI control blocks are big endian" so don't we
need the _be_ accessors? stq_be_p() etc...
> + stq_p(&pbdev->zpci_fn.edma, ZPCI_EDMA_ADDR);
> + stw_p(&pbdev->zpci_fn.pchid, 0);
> pbdev->zpci_fn.pfgid = ZPCI_DEFAULT_FN_GRP;
> - pbdev->zpci_fn.fid = pbdev->fid;
> - pbdev->zpci_fn.uid = pbdev->uid;
> + stl_p(&pbdev->zpci_fn.fid, pbdev->fid);
> + stl_p(&pbdev->zpci_fn.uid, pbdev->uid);
> pbdev->pci_group = s390_group_find(ZPCI_DEFAULT_FN_GRP);
> }
>
> @@ -871,7 +871,7 @@ static int s390_pci_msix_init(S390PCIBusDevice *pbdev)
> memory_region_init_io(&pbdev->msix_notify_mr, OBJECT(pbdev),
> &s390_msi_ctrl_ops, pbdev, name, PAGE_SIZE);
> memory_region_add_subregion(&pbdev->iommu->mr,
> - pbdev->pci_group->zpci_group.msia,
> + ldq_p(&pbdev->pci_group->zpci_group.msia),
> &pbdev->msix_notify_mr);
> g_free(name);
>
> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> index 58cd041d17fb..7bc6b79f10ce 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));
> if (!group) {
> /* We do not allow access to unknown groups */
> /* The group must have been obtained with a vfio device */
> @@ -788,7 +788,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t
> r3, uint64_t gaddr,
> /* Length must be greater than 8, a multiple of 8 */
> /* and not greater than maxstbl */
> if ((len <= 8) || (len % 8) ||
> - (len > pbdev->pci_group->zpci_group.maxstbl)) {
> + (len > lduw_p(&pbdev->pci_group->zpci_group.maxstbl))) {
> goto specification_error;
> }
> /* Do not cross a 4K-byte boundary */
> diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
> index d5c78063b5bc..f455c6f20a1a 100644
> --- a/hw/s390x/s390-pci-vfio.c
> +++ b/hw/s390x/s390-pci-vfio.c
> @@ -116,10 +116,10 @@ static void s390_pci_read_base(S390PCIBusDevice *pbdev,
> }
> cap = (void *) hdr;
>
> - pbdev->zpci_fn.sdma = cap->start_dma;
> - pbdev->zpci_fn.edma = cap->end_dma;
> - pbdev->zpci_fn.pchid = cap->pchid;
> - pbdev->zpci_fn.vfn = cap->vfn;
> + stq_p(&pbdev->zpci_fn.sdma, cap->start_dma);
> + stq_p(&pbdev->zpci_fn.edma, cap->end_dma);
> + stw_p(&pbdev->zpci_fn.pchid, cap->pchid);
> + stw_p(&pbdev->zpci_fn.vfn, cap->vfn);
> pbdev->zpci_fn.pfgid = cap->gid;
> /* The following values remain 0 until we support other FMB formats */
> pbdev->zpci_fn.fmbl = 0;
>
