From e2c6cd567422bfa563be026b9741a1854aecdc06 Mon Sep 17 00:00:00 2001 From: Collin Walling Date: Fri, 13 Nov 2020 17:10:22 -0500 Subject: [PATCH 1/3] s390/kvm: fix diag318 propagation and reset functionality The Control Program Name Code (CPNC) portion of the diag318 info must be set within the SIE block of each VCPU in the configuration. The handler will iterate through each VCPU and dirty the diag318_info reg to be synced with KVM on a subsequent sync_regs call. Additionally, the diag318 info resets must be handled via userspace. As such, QEMU will reset this value for each VCPU during a modified clear, load normal, and load clear reset event. Fixes: fabdada9357b ("s390: guest support for diagnose 0x318") Signed-off-by: Collin Walling Message-Id: <20201113221022.257054-1-walling@linux.ibm.com> Reviewed-by: Thomas Huth Reviewed-by: Janosch Frank Signed-off-by: Cornelia Huck --- hw/s390x/s390-virtio-ccw.c | 4 ++++ target/s390x/cpu.c | 7 +++++++ target/s390x/cpu.h | 1 + target/s390x/kvm-stub.c | 4 ++++ target/s390x/kvm.c | 22 +++++++++++++++++----- target/s390x/kvm_s390x.h | 1 + 6 files changed, 34 insertions(+), 5 deletions(-) diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 22222c4fd5..4e140bbead 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -486,6 +486,10 @@ static void s390_machine_reset(MachineState *machine) default: g_assert_not_reached(); } + + CPU_FOREACH(t) { + run_on_cpu(t, s390_do_cpu_set_diag318, RUN_ON_CPU_HOST_ULONG(0)); + } s390_ipl_clear_reset_request(); } diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c index 66f5942b53..7b66718c44 100644 --- a/target/s390x/cpu.c +++ b/target/s390x/cpu.c @@ -447,6 +447,13 @@ void s390_enable_css_support(S390CPU *cpu) kvm_s390_enable_css_support(cpu); } } + +void s390_do_cpu_set_diag318(CPUState *cs, run_on_cpu_data arg) +{ + if (kvm_enabled()) { + kvm_s390_set_diag318(cs, arg.host_ulong); + } +} #endif static gchar *s390_gdb_arch_name(CPUState *cs) diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index f875ebf0f4..60d434d5ed 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -769,6 +769,7 @@ int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit); void s390_set_max_pagesize(uint64_t pagesize, Error **errp); void s390_cmma_reset(void); void s390_enable_css_support(S390CPU *cpu); +void s390_do_cpu_set_diag318(CPUState *cs, run_on_cpu_data arg); int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id, int vq, bool assign); #ifndef CONFIG_USER_ONLY diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c index aa185017a2..9970b5a8c7 100644 --- a/target/s390x/kvm-stub.c +++ b/target/s390x/kvm-stub.c @@ -120,3 +120,7 @@ void kvm_s390_stop_interrupt(S390CPU *cpu) void kvm_s390_restart_interrupt(S390CPU *cpu) { } + +void kvm_s390_set_diag318(CPUState *cs, uint64_t diag318_info) +{ +} diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index baa070fdf7..b8385e6b95 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -1611,10 +1611,23 @@ static int handle_sw_breakpoint(S390CPU *cpu, struct kvm_run *run) return -ENOENT; } +void kvm_s390_set_diag318(CPUState *cs, uint64_t diag318_info) +{ + CPUS390XState *env = &S390_CPU(cs)->env; + + /* Feat bit is set only if KVM supports sync for diag318 */ + if (s390_has_feat(S390_FEAT_DIAG_318)) { + env->diag318_info = diag318_info; + cs->kvm_run->s.regs.diag318 = diag318_info; + cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318; + } +} + static void handle_diag_318(S390CPU *cpu, struct kvm_run *run) { uint64_t reg = (run->s390_sieic.ipa & 0x00f0) >> 4; uint64_t diag318_info = run->s.regs.gprs[reg]; + CPUState *t; /* * DIAG 318 can only be enabled with KVM support. As such, let's @@ -1622,13 +1635,12 @@ static void handle_diag_318(S390CPU *cpu, struct kvm_run *run) */ if (!s390_has_feat(S390_FEAT_DIAG_318)) { kvm_s390_program_interrupt(cpu, PGM_SPECIFICATION); + return; } - cpu->env.diag318_info = diag318_info; - - if (can_sync_regs(CPU(cpu), KVM_SYNC_DIAG318)) { - run->s.regs.diag318 = diag318_info; - run->kvm_dirty_regs |= KVM_SYNC_DIAG318; + CPU_FOREACH(t) { + run_on_cpu(t, s390_do_cpu_set_diag318, + RUN_ON_CPU_HOST_ULONG(diag318_info)); } } diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h index 6ab17c81b7..25bbe98b25 100644 --- a/target/s390x/kvm_s390x.h +++ b/target/s390x/kvm_s390x.h @@ -45,5 +45,6 @@ void kvm_s390_set_max_pagesize(uint64_t pagesize, Error **errp); void kvm_s390_crypto_reset(void); void kvm_s390_restart_interrupt(S390CPU *cpu); void kvm_s390_stop_interrupt(S390CPU *cpu); +void kvm_s390_set_diag318(CPUState *cs, uint64_t diag318_info); #endif /* KVM_S390X_H */ From e67ad058e45aa8b23e9b94e793b9fcf66e70ebb0 Mon Sep 17 00:00:00 2001 From: Matthew Rosato Date: Mon, 16 Nov 2020 16:42:35 -0500 Subject: [PATCH 2/3] s390x/pci: Unregister listeners before destroying IOMMU address space Hot-unplugging a vfio-pci device on s390x causes a QEMU crash: qemu-system-s390x: ../softmmu/memory.c:2772: do_address_space_destroy: Assertion `QTAILQ_EMPTY(&as->listeners)' failed. In s390, the IOMMU address space is freed during device unplug but the associated vfio-pci device may not yet be finalized and therefore may still have a listener registered to the IOMMU address space. Commit a2166410ad74 ("spapr_pci: Unregister listeners before destroying the IOMMU address space") previously resolved this issue for spapr_pci. We are now seeing this in s390x; it would seem the possibility for this issue was already present but based on a bisect commit 2d24a6466154 ("device-core: use RCU for list of children of a bus") has now changed the timing such that it is now readily reproducible. Add logic to ensure listeners are removed before destroying the address space. Reported-by: Niklas Schnelle Signed-off-by: Matthew Rosato Tested-by: Niklas Schnelle Reviewed-by: Pierre Morel Reviewed-by: Thomas Huth Message-Id: <1605562955-21152-1-git-send-email-mjrosato@linux.ibm.com> Signed-off-by: Cornelia Huck --- hw/s390x/s390-pci-bus.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 48a3be802f..e0dc20ce4a 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -732,6 +732,13 @@ static void s390_pci_iommu_free(S390pciState *s, PCIBus *bus, int32_t devfn) table->iommu[PCI_SLOT(devfn)] = NULL; g_hash_table_destroy(iommu->iotlb); + /* + * An attached PCI device may have memory listeners, eg. VFIO PCI. + * The associated subregion will already have been unmapped in + * s390_pci_iommu_disable in response to the guest deconfigure request. + * Remove the listeners now before destroying the address space. + */ + address_space_remove_listeners(&iommu->as); address_space_destroy(&iommu->as); object_unparent(OBJECT(&iommu->mr)); object_unparent(OBJECT(iommu)); From a4e2fff1b104f2b235ea2673968d0b0383f541dc Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Wed, 18 Nov 2020 11:42:02 +0100 Subject: [PATCH 3/3] s390x/pci: fix endianness issues The zPCI group and function structures are big endian. However, we do not consistently store them as big endian locally, and are missing some conversions. Let's just store the structures as host endian instead and convert to big endian when actually handling the instructions retrieving the data. Also fix the layout of ClpReqQueryPciGrp: g is actually only 8 bit. This also fixes accesses on little endian hosts, and makes accesses on big endian hosts consistent. 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 Tested-by: Matthew Rosato Tested-by: Thomas Huth Reviewed-by: Matthew Rosato Reviewed-by: Thomas Huth Message-Id: <20201118104202.1301363-1-cohuck@redhat.com> --- hw/s390x/s390-pci-bus.c | 10 +++++----- hw/s390x/s390-pci-inst.c | 16 ++++++++++++++-- hw/s390x/s390-pci-vfio.c | 12 ++++++------ include/hw/s390x/s390-pci-clp.h | 8 ++++---- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index e0dc20ce4a..05f7460aec 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -777,11 +777,11 @@ static void s390_pci_init_default_group(void) group = s390_group_create(ZPCI_DEFAULT_FN_GRP); resgrp = &group->zpci_group; resgrp->fr = 1; - stq_p(&resgrp->dasm, 0); - stq_p(&resgrp->msia, ZPCI_MSI_ADDR); - stw_p(&resgrp->mui, DEFAULT_MUI); - stw_p(&resgrp->i, 128); - stw_p(&resgrp->maxstbl, 128); + resgrp->dasm = 0; + resgrp->msia = ZPCI_MSI_ADDR; + resgrp->mui = DEFAULT_MUI; + resgrp->i = 128; + resgrp->maxstbl = 128; resgrp->version = 0; } diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c index 58cd041d17..70bfd91bf7 100644 --- a/hw/s390x/s390-pci-inst.c +++ b/hw/s390x/s390-pci-inst.c @@ -281,7 +281,13 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra) goto out; } - memcpy(resquery, &pbdev->zpci_fn, sizeof(*resquery)); + stq_p(&resquery->sdma, pbdev->zpci_fn.sdma); + stq_p(&resquery->edma, pbdev->zpci_fn.edma); + stw_p(&resquery->pchid, pbdev->zpci_fn.pchid); + resquery->flags = pbdev->zpci_fn.flags; + resquery->pfgid = pbdev->zpci_fn.pfgid; + stl_p(&resquery->fid, pbdev->zpci_fn.fid); + stl_p(&resquery->uid, pbdev->zpci_fn.uid); for (i = 0; i < PCI_BAR_COUNT; i++) { uint32_t data = pci_get_long(pbdev->pdev->config + @@ -312,7 +318,13 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra) stw_p(&resgrp->hdr.rsp, CLP_RC_QUERYPCIFG_PFGID); goto out; } - memcpy(resgrp, &group->zpci_group, sizeof(ClpRspQueryPciGrp)); + resgrp->fr = group->zpci_group.fr; + stq_p(&resgrp->dasm, group->zpci_group.dasm); + stq_p(&resgrp->msia, group->zpci_group.msia); + stw_p(&resgrp->mui, group->zpci_group.mui); + stw_p(&resgrp->i, group->zpci_group.i); + stw_p(&resgrp->maxstbl, group->zpci_group.maxstbl); + resgrp->version = group->zpci_group.version; stw_p(&resgrp->hdr.rsp, CLP_RC_OK); break; } diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c index d5c78063b5..9296e1bb6e 100644 --- a/hw/s390x/s390-pci-vfio.c +++ b/hw/s390x/s390-pci-vfio.c @@ -156,12 +156,12 @@ static void s390_pci_read_group(S390PCIBusDevice *pbdev, if (cap->flags & VFIO_DEVICE_INFO_ZPCI_FLAG_REFRESH) { resgrp->fr = 1; } - stq_p(&resgrp->dasm, cap->dasm); - stq_p(&resgrp->msia, cap->msi_addr); - stw_p(&resgrp->mui, cap->mui); - stw_p(&resgrp->i, cap->noi); - stw_p(&resgrp->maxstbl, cap->maxstbl); - stb_p(&resgrp->version, cap->version); + resgrp->dasm = cap->dasm; + resgrp->msia = cap->msi_addr; + resgrp->mui = cap->mui; + resgrp->i = cap->noi; + resgrp->maxstbl = cap->maxstbl; + resgrp->version = cap->version; } } diff --git a/include/hw/s390x/s390-pci-clp.h b/include/hw/s390x/s390-pci-clp.h index ea2b1378cd..96b8e3f133 100644 --- a/include/hw/s390x/s390-pci-clp.h +++ b/include/hw/s390x/s390-pci-clp.h @@ -144,10 +144,10 @@ typedef struct ClpReqQueryPciGrp { ClpReqHdr hdr; uint32_t fmt; uint64_t reserved1; -#define CLP_REQ_QPCIG_MASK_PFGID 0xff - uint32_t g; - uint32_t reserved2; - uint64_t reserved3; + uint8_t reserved2[3]; + uint8_t g; + uint32_t reserved3; + uint64_t reserved4; } QEMU_PACKED ClpReqQueryPciGrp; /* Query PCI function group response */