* [Qemu-devel] [PULL 01/11] s390: avoid potential null dereference in s390_pcihost_unplug()
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
@ 2019-01-18 12:19 ` Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 02/11] pc-bios/s390-ccw: Use proper register names for Clang Cornelia Huck
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-01-18 12:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-s390x, qemu-devel, Li Qiang, Cornelia Huck
From: Li Qiang <liq3ea@163.com>
When getting the 'pbdev', the if...else has no default branch.
>From Coverity, the 'pbdev' maybe null when the 'dev' is not
the TYPE_PCI_BRIDGE/TYPE_PCI_DEVICE/TYPE_S390_PCI_DEVICE.
This patch adds a default branch for device plug and unplug.
Spotted by Coverity: CID 1398593
Signed-off-by: Li Qiang <liq3ea@163.com>
Message-Id: <20190108151114.33140-1-liq3ea@163.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
hw/s390x/s390-pci-bus.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 15759b6514..a94700a78c 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -912,6 +912,8 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
pbdev->fh = pbdev->idx;
QTAILQ_INSERT_TAIL(&s->zpci_devs, pbdev, link);
g_hash_table_insert(s->zpci_table, &pbdev->idx, pbdev);
+ } else {
+ g_assert_not_reached();
}
}
@@ -956,6 +958,8 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
} else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
pbdev = S390_PCI_DEVICE(dev);
pci_dev = pbdev->pdev;
+ } else {
+ g_assert_not_reached();
}
switch (pbdev->state) {
--
2.17.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 02/11] pc-bios/s390-ccw: Use proper register names for Clang
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 01/11] s390: avoid potential null dereference in s390_pcihost_unplug() Cornelia Huck
@ 2019-01-18 12:19 ` Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 03/11] s390x: Return specification exception for unimplemented diag 308 subcodes Cornelia Huck
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-01-18 12:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-s390x, qemu-devel, Thomas Huth, Cornelia Huck
From: Thomas Huth <thuth@redhat.com>
When compiling the s390-ccw firmware with Clang 7.0.1, I get the
following errors:
pc-bios/s390-ccw/start.S:62:19: error: invalid use of length addressing
stctg 0,0,0(15)
^
pc-bios/s390-ccw/start.S:63:12: error: invalid use of length addressing
oi 6(15), 0x2
^
pc-bios/s390-ccw/start.S:64:19: error: invalid use of length addressing
lctlg 0,0,0(15)
^
pc-bios/s390-ccw/start.S:76:19: error: invalid use of length addressing
stctg 0,0,0(15)
^
pc-bios/s390-ccw/start.S:77:12: error: invalid use of length addressing
ni 6(15), 0xfd
^
pc-bios/s390-ccw/start.S:78:19: error: invalid use of length addressing
lctlg 0,0,0(15)
^
pc-bios/s390-ccw/start.S:79:12: error: invalid operand for instruction
br 14
^
Let's use proper register names like in the rest of this file to fix it.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <1547123559-30476-1-git-send-email-thuth@redhat.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
pc-bios/s390-ccw/start.S | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S
index eb8d024dbb..5c22cb0849 100644
--- a/pc-bios/s390-ccw/start.S
+++ b/pc-bios/s390-ccw/start.S
@@ -59,9 +59,9 @@ disabled_wait:
.globl consume_sclp_int
consume_sclp_int:
/* enable service interrupts in cr0 */
- stctg 0,0,0(15)
- oi 6(15), 0x2
- lctlg 0,0,0(15)
+ stctg %c0,%c0,0(%r15)
+ oi 6(%r15),0x2
+ lctlg %c0,%c0,0(%r15)
/* prepare external call handler */
larl %r1, external_new_code
stg %r1, 0x1b8
@@ -73,10 +73,10 @@ consume_sclp_int:
external_new_code:
/* disable service interrupts in cr0 */
- stctg 0,0,0(15)
- ni 6(15), 0xfd
- lctlg 0,0,0(15)
- br 14
+ stctg %c0,%c0,0(%r15)
+ ni 6(%r15),0xfd
+ lctlg %c0,%c0,0(%r15)
+ br %r14
.align 8
disabled_wait_psw:
--
2.17.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 03/11] s390x: Return specification exception for unimplemented diag 308 subcodes
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 01/11] s390: avoid potential null dereference in s390_pcihost_unplug() Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 02/11] pc-bios/s390-ccw: Use proper register names for Clang Cornelia Huck
@ 2019-01-18 12:19 ` Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 04/11] configure: Only build the s390-ccw bios if the compiler supports -march=z900 Cornelia Huck
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-01-18 12:19 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-s390x, qemu-devel, Janosch Frank, qemu-stable, Cornelia Huck
From: Janosch Frank <frankja@linux.ibm.com>
The architecture specifies specification exceptions for all
unavailable subcodes.
The presence of subcodes is indicated by checking some query subcode.
For example 6 will indicate that 3-6 are available. So future systems
might call new subcodes to check for new features. This should not
trigger a hw error, instead we return the architectured specification
exception.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Cc: qemu-stable@nongnu.org
Message-Id: <20190111113657.66195-3-frankja@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
target/s390x/diag.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index acb0f3d4af..aafa740f61 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -130,7 +130,7 @@ out:
}
return;
default:
- hw_error("Unhandled diag308 subcode %" PRIx64, subcode);
+ s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, ra);
break;
}
}
--
2.17.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 04/11] configure: Only build the s390-ccw bios if the compiler supports -march=z900
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
` (2 preceding siblings ...)
2019-01-18 12:19 ` [Qemu-devel] [PULL 03/11] s390x: Return specification exception for unimplemented diag 308 subcodes Cornelia Huck
@ 2019-01-18 12:19 ` Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 05/11] s390x/pci: Send correct event on hotplug Cornelia Huck
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-01-18 12:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-s390x, qemu-devel, Thomas Huth, Cornelia Huck
From: Thomas Huth <thuth@redhat.com>
We want to build our s390-ccw bios with -march=z900 so that it also
works with the oldest s390x CPU that we support with TCG. However,
Clang on s390x does not support -march=z900 anymore, so we can not
use this compiler to build the s390-ccw bios. Thus add a proper test
to the configure script to see whether the compiler is usable.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <1547470346-18416-1-git-send-email-thuth@redhat.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
configure | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 3eee3fcf70..87273d1c12 100755
--- a/configure
+++ b/configure
@@ -5889,8 +5889,12 @@ if test "$cpu" = "ppc64" -a "$targetos" != "Darwin" ; then
roms="$roms spapr-rtas"
fi
+# Only build s390-ccw bios if we're on s390x and the compiler has -march=z900
if test "$cpu" = "s390x" ; then
- roms="$roms s390-ccw"
+ write_c_skeleton
+ if compile_prog "-march=z900" ""; then
+ roms="$roms s390-ccw"
+ fi
fi
# Probe for the need for relocating the user-only binary.
--
2.17.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 05/11] s390x/pci: Send correct event on hotplug
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
` (3 preceding siblings ...)
2019-01-18 12:19 ` [Qemu-devel] [PULL 04/11] configure: Only build the s390-ccw bios if the compiler supports -march=z900 Cornelia Huck
@ 2019-01-18 12:19 ` Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 06/11] s390x/pci: Set the iommu region size mpcifc request Cornelia Huck
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-01-18 12:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-s390x, qemu-devel, David Hildenbrand, Cornelia Huck
From: David Hildenbrand <david@redhat.com>
Comit 2c28c490571f ("s390x/pci: let pci devices start in configured mode")
changed the initial state of zPCI devices from ZPCI_FS_STANDBY to
ZPCI_FS_DISABLED (a.k.a. configured). However we still only send a
HP_EVENT_RESERVED_TO_STANDBY event to the guest, indicating a wrong
state.
Let's send a HP_EVENT_TO_CONFIGURED event instead, to match the actual
state the device is in.
This fixes hotplugged devices having to be enabled explicitly in the
guest e.g. via echo 1 > /sys/bus/pci/slots/00000000/power.
On real HW, a PCI device always pops up in the STANDBY state. In QEMU,
we decided to let it show up directly in the configured state (as
configuring it is otherwise just an extra burden for the admin). We can
safely bypass the STANDBY state when hotplugging PCI devices to a guest.
Fixes: 2c28c490571f ("s390x/pci: let pci devices start in configured mode")
Reported-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190110210358.24035-1-david@redhat.com>
Tested-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
hw/s390x/s390-pci-bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index a94700a78c..1579989213 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -899,7 +899,7 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
}
if (dev->hotplugged) {
- s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
+ s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED ,
pbdev->fh, pbdev->fid);
}
} else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
--
2.17.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 06/11] s390x/pci: Set the iommu region size mpcifc request
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
` (4 preceding siblings ...)
2019-01-18 12:19 ` [Qemu-devel] [PULL 05/11] s390x/pci: Send correct event on hotplug Cornelia Huck
@ 2019-01-18 12:19 ` Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 07/11] s390x/pci: Use hotplug_dev instead of looking up the host bridge Cornelia Huck
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-01-18 12:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-s390x, qemu-devel, Pierre Morel, Cornelia Huck
From: Pierre Morel <pmorel@linux.ibm.com>
The size of the accessible iommu memory region in the guest
is given to the IOMMU by the guest through the mpcifc request
specifying the PCI Base Address and the PCI Address Limit.
Let's set the size of the IOMMU region to:
(PCI Address Limit) - (PCI Base Address) + 1.
Fixes: f7c40aa1e7 ("s390x/pci: fix failures of dma map/unmap")
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Message-Id: <1547125207-16907-2-git-send-email-pmorel@linux.ibm.com>
Acked-by: Collin Walling <walling@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
hw/s390x/s390-pci-bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 1579989213..400d9e4f28 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -660,7 +660,7 @@ void s390_pci_iommu_enable(S390PCIIOMMU *iommu)
char *name = g_strdup_printf("iommu-s390-%04x", iommu->pbdev->uid);
memory_region_init_iommu(&iommu->iommu_mr, sizeof(iommu->iommu_mr),
TYPE_S390_IOMMU_MEMORY_REGION, OBJECT(&iommu->mr),
- name, iommu->pal + 1);
+ name, iommu->pal - iommu->pba + 1);
iommu->enabled = true;
memory_region_add_subregion(&iommu->mr, 0, MEMORY_REGION(&iommu->iommu_mr));
g_free(name);
--
2.17.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 07/11] s390x/pci: Use hotplug_dev instead of looking up the host bridge
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
` (5 preceding siblings ...)
2019-01-18 12:19 ` [Qemu-devel] [PULL 06/11] s390x/pci: Set the iommu region size mpcifc request Cornelia Huck
@ 2019-01-18 12:19 ` Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 08/11] s390x/pci: Move some hotplug checks to the pre_plug handler Cornelia Huck
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-01-18 12:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-s390x, qemu-devel, David Hildenbrand, Cornelia Huck
From: David Hildenbrand <david@redhat.com>
We directly have it in our hands.
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190114103110.10909-2-david@redhat.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
hw/s390x/s390-pci-bus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 400d9e4f28..248e0eb295 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -826,9 +826,9 @@ static bool s390_pci_alloc_idx(S390pciState *s, S390PCIBusDevice *pbdev)
static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
+ S390pciState *s = S390_PCI_HOST_BRIDGE(hotplug_dev);
PCIDevice *pdev = NULL;
S390PCIBusDevice *pbdev = NULL;
- S390pciState *s = s390_get_phb();
if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
BusState *bus;
@@ -937,11 +937,11 @@ static void s390_pcihost_timer_cb(void *opaque)
static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
+ S390pciState *s = S390_PCI_HOST_BRIDGE(hotplug_dev);
PCIDevice *pci_dev = NULL;
PCIBus *bus;
int32_t devfn;
S390PCIBusDevice *pbdev = NULL;
- S390pciState *s = s390_get_phb();
if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
error_setg(errp, "PCI bridge hot unplug currently not supported");
--
2.17.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 08/11] s390x/pci: Move some hotplug checks to the pre_plug handler
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
` (6 preceding siblings ...)
2019-01-18 12:19 ` [Qemu-devel] [PULL 07/11] s390x/pci: Use hotplug_dev instead of looking up the host bridge Cornelia Huck
@ 2019-01-18 12:19 ` Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 09/11] s390x/pci: Always delete and free the release_timer Cornelia Huck
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-01-18 12:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-s390x, qemu-devel, David Hildenbrand, Cornelia Huck
From: David Hildenbrand <david@redhat.com>
Let's move most of the checks to the new pre_plug handler. As a PCI
bridge is just a PCI device, we can simplify the code.
Notes: We cannot yet move the MSIX check or device ID creation +
zPCI device creation to the pre_plug handler as both parts are not
fixed before actual device realization (and therefore after pre_plug and
before plug). Once that part is factored out, we can move these parts to
the pre_plug handler, too and therefore remove all possible errors from
the plug handler.
Reviewed-by: Collin Walling <walling@linux.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190114103110.10909-3-david@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
hw/s390x/s390-pci-bus.c | 41 +++++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 248e0eb295..c467cc526b 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -818,11 +818,31 @@ static bool s390_pci_alloc_idx(S390pciState *s, S390PCIBusDevice *pbdev)
}
pbdev->idx = idx;
- s->next_idx = (idx + 1) & FH_MASK_INDEX;
-
return true;
}
+static void s390_pcihost_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+ S390pciState *s = S390_PCI_HOST_BRIDGE(hotplug_dev);
+
+ if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+ PCIDevice *pdev = PCI_DEVICE(dev);
+
+ if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
+ error_setg(errp, "multifunction not supported in s390");
+ return;
+ }
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
+ S390PCIBusDevice *pbdev = S390_PCI_DEVICE(dev);
+
+ if (!s390_pci_alloc_idx(s, pbdev)) {
+ error_setg(errp, "no slot for plugging zpci device");
+ return;
+ }
+ }
+}
+
static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
@@ -835,11 +855,6 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
PCIBridge *pb = PCI_BRIDGE(dev);
PCIDevice *pdev = PCI_DEVICE(dev);
- if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
- error_setg(errp, "multifunction not supported in s390");
- return;
- }
-
pci_bridge_map_irq(pb, dev->id, s390_pci_map_irq);
pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s);
@@ -859,11 +874,6 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
} else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
pdev = PCI_DEVICE(dev);
- if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
- error_setg(errp, "multifunction not supported in s390");
- return;
- }
-
if (!dev->id) {
/* In the case the PCI device does not define an id */
/* we generate one based on the PCI address */
@@ -905,10 +915,8 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
} else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
pbdev = S390_PCI_DEVICE(dev);
- if (!s390_pci_alloc_idx(s, pbdev)) {
- error_setg(errp, "no slot for plugging zpci device");
- return;
- }
+ /* the allocated idx is actually getting used */
+ s->next_idx = (pbdev->idx + 1) & FH_MASK_INDEX;
pbdev->fh = pbdev->idx;
QTAILQ_INSERT_TAIL(&s->zpci_devs, pbdev, link);
g_hash_table_insert(s->zpci_table, &pbdev->idx, pbdev);
@@ -1045,6 +1053,7 @@ static void s390_pcihost_class_init(ObjectClass *klass, void *data)
dc->reset = s390_pcihost_reset;
dc->realize = s390_pcihost_realize;
+ hc->pre_plug = s390_pcihost_pre_plug;
hc->plug = s390_pcihost_plug;
hc->unplug = s390_pcihost_unplug;
msi_nonbroken = true;
--
2.17.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 09/11] s390x/pci: Always delete and free the release_timer
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
` (7 preceding siblings ...)
2019-01-18 12:19 ` [Qemu-devel] [PULL 08/11] s390x/pci: Move some hotplug checks to the pre_plug handler Cornelia Huck
@ 2019-01-18 12:19 ` Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 10/11] s390x/pci: Ignore the unplug call if we already have a release_timer Cornelia Huck
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-01-18 12:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-s390x, qemu-devel, David Hildenbrand, Cornelia Huck
From: David Hildenbrand <david@redhat.com>
We should always get rid of it. I don't see a reason to keep the timer
alive if the devices are going away. This looks like a memory leak.
(hmp) device_add virtio-mouse-pci,id=test
(hmp) device_del test
-> guest notified, timer pending.
-> guest does not react for some reason (e.g. crash)
-> s390_pcihost_timer_cb(). Timer not pending anymore. qmp_unplug().
-> Device deleted. Timer expired (not pending) but not freed.
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190114103110.10909-4-david@redhat.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
hw/s390x/s390-pci-bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index c467cc526b..e5a4cf03b2 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -986,7 +986,7 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
return;
}
- if (pbdev->release_timer && timer_pending(pbdev->release_timer)) {
+ if (pbdev->release_timer) {
timer_del(pbdev->release_timer);
timer_free(pbdev->release_timer);
pbdev->release_timer = NULL;
--
2.17.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 10/11] s390x/pci: Ignore the unplug call if we already have a release_timer
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
` (8 preceding siblings ...)
2019-01-18 12:19 ` [Qemu-devel] [PULL 09/11] s390x/pci: Always delete and free the release_timer Cornelia Huck
@ 2019-01-18 12:19 ` Cornelia Huck
2019-01-18 12:19 ` [Qemu-devel] [PULL 11/11] s390x/pci: add common function measurement block Cornelia Huck
2019-01-18 18:54 ` [Qemu-devel] [PULL 00/11] s390x updates Peter Maydell
11 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-01-18 12:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-s390x, qemu-devel, David Hildenbrand, Cornelia Huck
From: David Hildenbrand <david@redhat.com>
... otherwise two successive calls to qdev_unplug() (e.g. by an impatient
user) will effectively overwrite pbdev->release_timer, resulting in a
memory leak. We are already processing the unplug.
If there is already a release_timer, the unplug will be performed after
the timeout.
Can be easily triggered by
(hmp) device_add virtio-mouse-pci,id=test
(hmp) stop
(hmp) device_del test
(hmp) device_del test
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190114103110.10909-5-david@redhat.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
hw/s390x/s390-pci-bus.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index e5a4cf03b2..9d23c1b300 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -976,6 +976,9 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
case ZPCI_FS_STANDBY:
break;
default:
+ if (pbdev->release_timer) {
+ return;
+ }
s390_pci_generate_plug_event(HP_EVENT_DECONFIGURE_REQUEST,
pbdev->fh, pbdev->fid);
pbdev->release_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
--
2.17.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 11/11] s390x/pci: add common function measurement block
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
` (9 preceding siblings ...)
2019-01-18 12:19 ` [Qemu-devel] [PULL 10/11] s390x/pci: Ignore the unplug call if we already have a release_timer Cornelia Huck
@ 2019-01-18 12:19 ` Cornelia Huck
2019-01-18 18:54 ` [Qemu-devel] [PULL 00/11] s390x updates Peter Maydell
11 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-01-18 12:19 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-s390x, qemu-devel, Yi Min Zhao, Pierre Morel, Cornelia Huck
From: Yi Min Zhao <zyimin@linux.ibm.com>
Common function measurement block is used to report zPCI internal
counters of successful pcilg/stg/stb and rpcit instructions to
a memory location provided by the program.
This patch introduces a new ZpciFmb structure and schedules a timer
callback to copy the zPCI measures to the FMB in the guest memory
at an interval time set to 4s.
An error while attemping to update the FMB, would generate an error
event to the guest.
The pcilg/stg/stb and rpcit interception handlers increase the
related counter on a successful call.
The guest shall pass a null FMBA (FMB address) in the FIB (Function
Information Block) when it issues a Modify PCI Function Control
instruction to switch off FMB and stop the corresponding timer.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com>
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Message-Id: <1546969050-8884-2-git-send-email-pmorel@linux.ibm.com>
Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
hw/s390x/s390-pci-bus.c | 4 +-
hw/s390x/s390-pci-bus.h | 29 +++++++++
hw/s390x/s390-pci-inst.c | 133 ++++++++++++++++++++++++++++++++++++++-
hw/s390x/s390-pci-inst.h | 1 +
4 files changed, 163 insertions(+), 4 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 9d23c1b300..f017c1ded0 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -1000,6 +1000,7 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
bus = pci_get_bus(pci_dev);
devfn = pci_dev->devfn;
object_unparent(OBJECT(pci_dev));
+ fmb_timer_free(pbdev);
s390_pci_msix_free(pbdev);
s390_pci_iommu_free(s, bus, devfn);
pbdev->pdev = NULL;
@@ -1148,6 +1149,7 @@ static void s390_pci_device_realize(DeviceState *dev, Error **errp)
}
zpci->state = ZPCI_FS_RESERVED;
+ zpci->fmb.format = ZPCI_FMB_FORMAT;
}
static void s390_pci_device_reset(DeviceState *dev)
@@ -1172,7 +1174,7 @@ static void s390_pci_device_reset(DeviceState *dev)
pci_dereg_ioat(pbdev->iommu);
}
- pbdev->fmb_addr = 0;
+ fmb_timer_free(pbdev);
}
static void s390_pci_get_fid(Object *obj, Visitor *v, const char *name,
diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
index f47a0f2da5..dadad1f758 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -285,6 +285,33 @@ typedef struct S390PCIIOMMUTable {
S390PCIIOMMU *iommu[PCI_SLOT_MAX];
} S390PCIIOMMUTable;
+/* Function Measurement Block */
+#define DEFAULT_MUI 4000
+#define UPDATE_U_BIT 0x1ULL
+#define FMBK_MASK 0xfULL
+
+typedef struct ZpciFmbFmt0 {
+ uint64_t dma_rbytes;
+ uint64_t dma_wbytes;
+} ZpciFmbFmt0;
+
+#define ZPCI_FMB_CNT_LD 0
+#define ZPCI_FMB_CNT_ST 1
+#define ZPCI_FMB_CNT_STB 2
+#define ZPCI_FMB_CNT_RPCIT 3
+#define ZPCI_FMB_CNT_MAX 4
+
+#define ZPCI_FMB_FORMAT 0
+
+typedef struct ZpciFmb {
+ uint32_t format;
+ uint32_t sample;
+ uint64_t last_update;
+ uint64_t counter[ZPCI_FMB_CNT_MAX];
+ ZpciFmbFmt0 fmt0;
+} ZpciFmb;
+QEMU_BUILD_BUG_MSG(offsetof(ZpciFmb, fmt0) != 48, "padding in ZpciFmb");
+
struct S390PCIBusDevice {
DeviceState qdev;
PCIDevice *pdev;
@@ -296,6 +323,8 @@ struct S390PCIBusDevice {
uint32_t fid;
bool fid_defined;
uint64_t fmb_addr;
+ ZpciFmb fmb;
+ QEMUTimer *fmb_timer;
uint8_t isc;
uint16_t noi;
uint16_t maxstbl;
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 7b61367ee3..be2896232d 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -19,6 +19,7 @@
#include "exec/memory-internal.h"
#include "qemu/error-report.h"
#include "sysemu/hw_accel.h"
+#include "hw/s390x/tod.h"
#ifndef DEBUG_S390PCI_INST
#define DEBUG_S390PCI_INST 0
@@ -293,7 +294,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
resgrp->fr = 1;
stq_p(&resgrp->dasm, 0);
stq_p(&resgrp->msia, ZPCI_MSI_ADDR);
- stw_p(&resgrp->mui, 0);
+ stw_p(&resgrp->mui, DEFAULT_MUI);
stw_p(&resgrp->i, 128);
stw_p(&resgrp->maxstbl, 128);
resgrp->version = 0;
@@ -456,6 +457,8 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra)
return 0;
}
+ pbdev->fmb.counter[ZPCI_FMB_CNT_LD]++;
+
env->regs[r1] = data;
setcc(cpu, ZPCI_PCI_LS_OK);
return 0;
@@ -561,6 +564,8 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra)
return 0;
}
+ pbdev->fmb.counter[ZPCI_FMB_CNT_ST]++;
+
setcc(cpu, ZPCI_PCI_LS_OK);
return 0;
}
@@ -681,6 +686,7 @@ err:
s390_set_status_code(env, r1, ZPCI_PCI_ST_FUNC_IN_ERR);
s390_pci_generate_error_event(error, pbdev->fh, pbdev->fid, start, 0);
} else {
+ pbdev->fmb.counter[ZPCI_FMB_CNT_RPCIT]++;
setcc(cpu, ZPCI_PCI_LS_OK);
}
return 0;
@@ -783,6 +789,8 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
}
}
+ pbdev->fmb.counter[ZPCI_FMB_CNT_STB]++;
+
setcc(cpu, ZPCI_PCI_LS_OK);
return 0;
@@ -889,6 +897,99 @@ void pci_dereg_ioat(S390PCIIOMMU *iommu)
iommu->g_iota = 0;
}
+void fmb_timer_free(S390PCIBusDevice *pbdev)
+{
+ if (pbdev->fmb_timer) {
+ timer_del(pbdev->fmb_timer);
+ timer_free(pbdev->fmb_timer);
+ pbdev->fmb_timer = NULL;
+ }
+ pbdev->fmb_addr = 0;
+ memset(&pbdev->fmb, 0, sizeof(ZpciFmb));
+}
+
+static int fmb_do_update(S390PCIBusDevice *pbdev, int offset, uint64_t val,
+ int len)
+{
+ MemTxResult ret;
+ uint64_t dst = pbdev->fmb_addr + offset;
+
+ switch (len) {
+ case 8:
+ address_space_stq_be(&address_space_memory, dst, val,
+ MEMTXATTRS_UNSPECIFIED,
+ &ret);
+ break;
+ case 4:
+ address_space_stl_be(&address_space_memory, dst, val,
+ MEMTXATTRS_UNSPECIFIED,
+ &ret);
+ break;
+ case 2:
+ address_space_stw_be(&address_space_memory, dst, val,
+ MEMTXATTRS_UNSPECIFIED,
+ &ret);
+ break;
+ case 1:
+ address_space_stb(&address_space_memory, dst, val,
+ MEMTXATTRS_UNSPECIFIED,
+ &ret);
+ break;
+ default:
+ ret = MEMTX_ERROR;
+ break;
+ }
+ if (ret != MEMTX_OK) {
+ s390_pci_generate_error_event(ERR_EVENT_FMBA, pbdev->fh, pbdev->fid,
+ pbdev->fmb_addr, 0);
+ fmb_timer_free(pbdev);
+ }
+
+ return ret;
+}
+
+static void fmb_update(void *opaque)
+{
+ S390PCIBusDevice *pbdev = opaque;
+ int64_t t = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
+ int i;
+
+ /* Update U bit */
+ pbdev->fmb.last_update *= 2;
+ pbdev->fmb.last_update |= UPDATE_U_BIT;
+ if (fmb_do_update(pbdev, offsetof(ZpciFmb, last_update),
+ pbdev->fmb.last_update,
+ sizeof(pbdev->fmb.last_update))) {
+ return;
+ }
+
+ /* Update FMB sample count */
+ if (fmb_do_update(pbdev, offsetof(ZpciFmb, sample),
+ pbdev->fmb.sample++,
+ sizeof(pbdev->fmb.sample))) {
+ return;
+ }
+
+ /* Update FMB counters */
+ for (i = 0; i < ZPCI_FMB_CNT_MAX; i++) {
+ if (fmb_do_update(pbdev, offsetof(ZpciFmb, counter[i]),
+ pbdev->fmb.counter[i],
+ sizeof(pbdev->fmb.counter[0]))) {
+ return;
+ }
+ }
+
+ /* Clear U bit and update the time */
+ pbdev->fmb.last_update = time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+ pbdev->fmb.last_update *= 2;
+ if (fmb_do_update(pbdev, offsetof(ZpciFmb, last_update),
+ pbdev->fmb.last_update,
+ sizeof(pbdev->fmb.last_update))) {
+ return;
+ }
+ timer_mod(pbdev->fmb_timer, t + DEFAULT_MUI);
+}
+
int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
uintptr_t ra)
{
@@ -1018,9 +1119,35 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
s390_set_status_code(env, r1, ZPCI_MOD_ST_SEQUENCE);
}
break;
- case ZPCI_MOD_FC_SET_MEASURE:
- pbdev->fmb_addr = ldq_p(&fib.fmb_addr);
+ case ZPCI_MOD_FC_SET_MEASURE: {
+ uint64_t fmb_addr = ldq_p(&fib.fmb_addr);
+
+ if (fmb_addr & FMBK_MASK) {
+ cc = ZPCI_PCI_LS_ERR;
+ s390_pci_generate_error_event(ERR_EVENT_FMBPRO, pbdev->fh,
+ pbdev->fid, fmb_addr, 0);
+ fmb_timer_free(pbdev);
+ break;
+ }
+
+ if (!fmb_addr) {
+ /* Stop updating FMB. */
+ fmb_timer_free(pbdev);
+ break;
+ }
+
+ if (!pbdev->fmb_timer) {
+ pbdev->fmb_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
+ fmb_update, pbdev);
+ } else if (timer_pending(pbdev->fmb_timer)) {
+ /* Remove pending timer to update FMB address. */
+ timer_del(pbdev->fmb_timer);
+ }
+ pbdev->fmb_addr = fmb_addr;
+ timer_mod(pbdev->fmb_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + DEFAULT_MUI);
break;
+ }
default:
s390_program_interrupt(&cpu->env, PGM_OPERAND, 6, ra);
cc = ZPCI_PCI_LS_ERR;
diff --git a/hw/s390x/s390-pci-inst.h b/hw/s390x/s390-pci-inst.h
index 91c3d61f2a..fa3bf8b5aa 100644
--- a/hw/s390x/s390-pci-inst.h
+++ b/hw/s390x/s390-pci-inst.h
@@ -303,6 +303,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
uintptr_t ra);
int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
uintptr_t ra);
+void fmb_timer_free(S390PCIBusDevice *pbdev);
#define ZPCI_IO_BAR_MIN 0
#define ZPCI_IO_BAR_MAX 5
--
2.17.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PULL 00/11] s390x updates
2019-01-18 12:19 [Qemu-devel] [PULL 00/11] s390x updates Cornelia Huck
` (10 preceding siblings ...)
2019-01-18 12:19 ` [Qemu-devel] [PULL 11/11] s390x/pci: add common function measurement block Cornelia Huck
@ 2019-01-18 18:54 ` Peter Maydell
11 siblings, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2019-01-18 18:54 UTC (permalink / raw)
To: Cornelia Huck; +Cc: qemu-s390x, QEMU Developers
On Fri, 18 Jan 2019 at 12:19, Cornelia Huck <cohuck@redhat.com> wrote:
>
> The following changes since commit 681d61362d3f766a00806b89d6581869041f73cb:
>
> Merge remote-tracking branch 'remotes/jnsnow/tags/bitmaps-pull-request' into staging (2019-01-17 12:48:42 +0000)
>
> are available in the Git repository at:
>
> https://github.com/cohuck/qemu tags/s390x-20190118
>
> for you to fetch changes up to 6e92c70c37547b6a247a206651dfcc583a57f484:
>
> s390x/pci: add common function measurement block (2019-01-18 11:52:01 +0100)
>
> ----------------------------------------------------------------
> s390x updates:
> - clang compilation fixes
> - fixes in zpci hotplug code
> - handle unimplemented diag 308 subcodes correctly
> - add common fmb in zpci
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread