Re: [PATCH] target/s390x: improve cpu compatibility check error message
Am 14.03.24 um 20:00 schrieb Claudio Fontana:
some users were confused by this message showing under TCG:
Selected CPU generation is too new. Maximum supported model
in the configuration: 'xyz'
Try to clarify that the maximum can depend on the accel by
adding also the current accelerator to the message as such:
Selected CPU generation is too new. Maximum supported model
in the accelerator 'tcg' configuration: 'xyz'
Signed-off-by: Claudio Fontana
Independent on which message we end up with (see comments
in this mail thread), I agree that improving the
error message is helpful.
Acked-by: Christian Borntraeger
---
target/s390x/cpu_models.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 1a1c096122..0d6d8fc727 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -508,14 +508,14 @@ static void check_compatibility(const S390CPUModel
*max_model,
if (model->def->gen > max_model->def->gen) {
error_setg(errp, "Selected CPU generation is too new. Maximum "
- "supported model in the configuration: \'%s\'",
- max_model->def->name);
+ "supported model in the accelerator \'%s\' configuration:
\'%s\'",
+ current_accel_name(), max_model->def->name);
return;
} else if (model->def->gen == max_model->def->gen &&
model->def->ec_ga > max_model->def->ec_ga) {
error_setg(errp, "Selected CPU GA level is too new. Maximum "
- "supported model in the configuration: \'%s\'",
- max_model->def->name);
+ "supported model in the accelerator \'%s\' configuration:
\'%s\'",
+ current_accel_name(), max_model->def->name);
return;
}
@@ -537,7 +537,8 @@ static void check_compatibility(const S390CPUModel *max_model,
error_setg(errp, " ");
s390_feat_bitmap_to_ascii(missing, errp, error_prepend_missing_feat);
error_prepend(errp, "Some features requested in the CPU model are not "
- "available in the configuration: ");
+ "available in the accelerator \'%s\' configuration: ",
+ current_accel_name());
}
S390CPUModel *get_max_cpu_model(Error **errp)
Re: [PULL v2 00/39] tcg patch queue
Am 06.02.24 um 22:24 schrieb Peter Maydell: [..] Christian: this is on the s390x machine we have. Does the VM setup for that share IO or CPU with other VMs somehow? Yes it does, this is a shared system. I will talk to the team if there is anything that we can do about this.
Re: [PULL 2/7] s390x: do a subsystem reset before the unprotect on reboot
Am 11.01.24 um 10:43 schrieb Cédric Le Goater: [...] On a side note, I am also seeing : Michael? [ 73.989688] [ cut here ] [ 73.989696] unexpected non zero alert.mask 0x20 [ 73.989748] WARNING: CPU: 9 PID: 4503 at arch/s390/kvm/interrupt.c:3214 kvm_s390_gisa_destroy+0xd4/0xe8 [kvm] [ 73.989791] Modules linked in: vfio_pci vfio_pci_core irqbypass vhost_net vhost vhost_iotlb tap tun xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_compat nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables nfnetlink 8021q garp mrp rfkill sunrpc ext4 mbcache jbd2 vfio_ap zcrypt_cex4 vfio_ccw mdev vfio_iommu_type1 vfio drm fuse i2c_core drm_panel_orientation_quirks xfs libcrc32c dm_service_time mlx5_core sd_mod t10_pi ghash_s390 sg prng des_s390 libdes sha3_512_s390 sha3_256_s390 mlxfw tls scm_block psample eadm_sch qeth_l2 bridge stp llc dasd_eckd_mod zfcp qeth dasd_mod scsi_transport_fc ccwgroup qdio dm_multipath dm_mirror dm_region_hash dm_log dm_mod pkey zcrypt kvm aes_s390 [ 73.989825] CPU: 9 PID: 4503 Comm: worker Kdump: loaded Not tainted 6.7.0-clg-dirty #52 [ 73.989827] Hardware name: IBM 3931 LA1 400 (LPAR) [ 73.989829] Krnl PSW : 0704c0018000 03ff7fcd2198 (kvm_s390_gisa_destroy+0xd8/0xe8 [kvm]) [ 73.989845] R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:0 PM:0 RI:0 EA:3 [ 73.989847] Krnl GPRS: c000fffe 00070027 0023 0007df4249c8 [ 73.989849] 03800649b858 03800649b850 0007fcb9db00 [ 73.989851] 8ebae8c8 83a8c4f0 00b69900 8ebac000 [ 73.989853] 03ff903aef68 03800649bd98 03ff7fcd2194 03800649b9f8 [ 73.989859] Krnl Code: 03ff7fcd2188: c0224f88 larl %r2,03ff7fd1c098 03ff7fcd218e: c0e5fffea360 brasl %r14,03ff7fca684e #03ff7fcd2194: af00 mc 0,0 >03ff7fcd2198: e310b7680204 lg %r1,10088(%r11) 03ff7fcd219e: a7f4ffae brc 15,03ff7fcd20fa 03ff7fcd21a2: 0707 bcr 0,%r7 03ff7fcd21a4: 0707 bcr 0,%r7 03ff7fcd21a6: 0707 bcr 0,%r7 [ 73.989929] Call Trace: [ 73.989931] [<03ff7fcd2198>] kvm_s390_gisa_destroy+0xd8/0xe8 [kvm] [ 73.989946] ([<03ff7fcd2194>] kvm_s390_gisa_destroy+0xd4/0xe8 [kvm]) [ 73.989960] [<03ff7fcc1578>] kvm_arch_destroy_vm+0x50/0x118 [kvm] [ 73.989974] [<03ff7fcb00a2>] kvm_destroy_vm+0x15a/0x260 [kvm] [ 73.989985] [<03ff7fcb021e>] kvm_vm_release+0x36/0x48 [kvm] [ 73.989996] [<0007de4f830c>] __fput+0x94/0x2d0 [ 73.990009] [<0007de20d838>] task_work_run+0x88/0xe8 [ 73.990013] [<0007de1e75e0>] do_exit+0x2e0/0x4e0 [ 73.990016] [<0007de1e79c0>] do_group_exit+0x40/0xb8 [ 73.990017] [<0007de1f96e8>] send_sig_info+0x0/0xa8 [ 73.990021] [<0007de194b26>] arch_do_signal_or_restart+0x56/0x318 [ 73.990025] [<0007de28bf12>] exit_to_user_mode_prepare+0x10a/0x1a0 [ 73.990028] [<0007deb607d2>] __do_syscall+0x152/0x1f8 [ 73.990032] [<0007deb70ac8>] system_call+0x70/0x98 [ 73.990036] Last Breaking-Event-Address: [ 73.990037] [<0007de1e0c58>] __warn_printk+0x78/0xe8
Re: [PATCH 3/4] hw/s390x/ipl: Remove unused 'exec/exec-all.h' included header
Am 12.12.23 um 16:28 schrieb Eric Farman: So why do you think exec-all.h is unused? I think because that got moved out of exec-all.h a few months ago, via commit 3549118b498873c84b442bc280a5edafbb61e0a4 Author: Philippe Mathieu-Daudé Date: Thu Sep 14 20:57:08 2023 +0200 exec: Move cpu_loop_foo() target agnostic functions to 'cpu- common.h' While these functions are not TCG specific, they are not target specific. Move them to "exec/cpu-common.h" so their callers don't have to be tainted as target specific. Ah right, I was looking at an old QEMU version
Re: [PATCH 3/4] hw/s390x/ipl: Remove unused 'exec/exec-all.h' included header
Am 12.12.23 um 12:36 schrieb Philippe Mathieu-Daudé:
Signed-off-by: Philippe Mathieu-Daudé
---
hw/s390x/ipl.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 515dcf51b5..62182d81a0 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -35,7 +35,6 @@
#include "qemu/cutils.h"
#include "qemu/option.h"
#include "standard-headers/linux/virtio_ids.h"
-#include "exec/exec-all.h"
Philippe,
This include came with
commit a30fb811cbe940020a498d2cdac9326cac38b4d9
Author: David Hildenbrand
AuthorDate: Tue Apr 24 12:18:59 2018 +0200
Commit: Cornelia Huck
CommitDate: Mon May 14 17:10:02 2018 +0200
s390x: refactor reset/reipl handling
And I think one reason was
cpu_loop_exit
This is still part of ipl.c
a30fb811cbe (David Hildenbrand 2018-04-24 12:18:59 +0200 664) /* as
this is triggered by a CPU, make sure to exit the loop */
a30fb811cbe (David Hildenbrand 2018-04-24 12:18:59 +0200 665) if
(tcg_enabled()) {
a30fb811cbe (David Hildenbrand 2018-04-24 12:18:59 +0200 666)
cpu_loop_exit(cs);
a30fb811cbe (David Hildenbrand 2018-04-24 12:18:59 +0200 667) }
So why do you think exec-all.h is unused?
Re: [PATCH v2 06/13] migration: Use VMSTATE_INSTANCE_ID_ANY for s390 devices
Am 20.10.23 um 11:07 schrieb Juan Quintela:
Just with make check I can see that we can have more than one of this
devices, so use ANY.
ok 5 /s390x/device/introspect/abstract-interfaces
...
Broken pipe
../../../../../mnt/code/qemu/full/tests/qtest/libqtest.c:195: kill_qemu() tried
to terminate QEMU process but encountered exit status 1 (expected 0)
Aborted (core dumped)
Reviewed-by: Stefan Berger
Signed-off-by: Juan Quintela
---
hw/s390x/s390-skeys.c| 3 ++-
hw/s390x/s390-stattrib.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
Actually both devices should be theŕe only once - I think.
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 5024faf411..ef089e1967 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -22,6 +22,7 @@
#include "sysemu/kvm.h"
#include "migration/qemu-file-types.h"
#include "migration/register.h"
+#include "migration/vmstate.h"
#define S390_SKEYS_BUFFER_SIZE (128 * KiB) /* Room for 128k storage keys */
#define S390_SKEYS_SAVE_FLAG_EOS 0x01
@@ -457,7 +458,7 @@ static inline void s390_skeys_set_migration_enabled(Object
*obj, bool value,
ss->migration_enabled = value;
if (ss->migration_enabled) {
-register_savevm_live(TYPE_S390_SKEYS, 0, 1,
+register_savevm_live(TYPE_S390_SKEYS, VMSTATE_INSTANCE_ID_ANY, 1,
_s390_storage_keys, ss);
} else {
unregister_savevm(VMSTATE_IF(ss), TYPE_S390_SKEYS, ss);
diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
index 220e845d12..055d382c3c 100644
--- a/hw/s390x/s390-stattrib.c
+++ b/hw/s390x/s390-stattrib.c
@@ -13,6 +13,7 @@
#include "qemu/units.h"
#include "migration/qemu-file.h"
#include "migration/register.h"
+#include "migration/vmstate.h"
#include "hw/s390x/storage-attributes.h"
#include "qemu/error-report.h"
#include "exec/ram_addr.h"
@@ -380,7 +381,7 @@ static void s390_stattrib_instance_init(Object *obj)
{
S390StAttribState *sas = S390_STATTRIB(obj);
-register_savevm_live(TYPE_S390_STATTRIB, 0, 0,
+register_savevm_live(TYPE_S390_STATTRIB, VMSTATE_INSTANCE_ID_ANY, 0,
_s390_stattrib_handlers, sas);
object_property_add_bool(obj, "migration-enabled",
Re: [PATCH v2 2/2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code
Am 09.10.23 um 19:07 schrieb Thomas Huth:
[...]
@@ -483,20 +482,14 @@ int kvm_arch_put_registers(CPUState *cs, int level)
cs->kvm_run->psw_addr = env->psw.addr;
cs->kvm_run->psw_mask = env->psw.mask;
-if (can_sync_regs(cs, KVM_SYNC_GPRS)) {
-for (i = 0; i < 16; i++) {
-cs->kvm_run->s.regs.gprs[i] = env->regs[i];
-cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GPRS;
-}
-} else {
-for (i = 0; i < 16; i++) {
-regs.gprs[i] = env->regs[i];
-}
-r = kvm_vcpu_ioctl(cs, KVM_SET_REGS, );
-if (r < 0) {
-return r;
-}
-}
+g_assert((cs->kvm_run->kvm_valid_regs & KVM_SYNC_REQUIRED_BITS) ==
+ KVM_SYNC_REQUIRED_BITS);
+cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_REQUIRED_BITS;
+memcpy(cs->kvm_run->s.regs.gprs, env->regs,
sizeof(cs->kvm_run->s.regs.gprs));
+memcpy(cs->kvm_run->s.regs.acrs, env->aregs,
sizeof(cs->kvm_run->s.regs.acrs));
+memcpy(cs->kvm_run->s.regs.crs, env->cregs,
sizeof(cs->kvm_run->s.regs.crs));
+
+cs->kvm_run->s.regs.prefix = env->psa;
These 3 have only been saved for level> KVM_PUT_RUNTIME_STATE. By moving the
memcpy
into this position you would always write them. Probably ok but a change in
behaviour. Was this intentional?
Re: [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement
Am 10.10.23 um 13:12 schrieb Thomas Huth:
On 10/10/2023 13.02, Christian Borntraeger wrote:
Am 09.10.23 um 19:07 schrieb Thomas Huth:
Since we already require at least kernel 3.15 in the s390x KVM code,
we can assume that the KVM_CAP_SYNC_REGS capability is always there.
Thus turn this into a hard requirement now.
Signed-off-by: Thomas Huth
---
target/s390x/kvm/kvm.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index bc5c56a305..b3e2eaa2eb 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -337,21 +337,29 @@ int kvm_arch_get_default_type(MachineState *ms)
int kvm_arch_init(MachineState *ms, KVMState *s)
{
+ int required_caps[] = {
+ KVM_CAP_DEVICE_CTRL,
+ KVM_CAP_SYNC_REGS,
+ };
+
+ for (int i = 0; i < ARRAY_SIZE(required_caps); i++) {
+ if (!kvm_check_extension(s, required_caps[i])) {
+ error_report("KVM is missing capability #%d - "
+ "please use kernel 3.15 or newer", required_caps[i]);
+ return -1;
+ }
+ }
+
object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,
false, NULL);
- if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
- error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "
- "please use kernel 3.15 or newer");
- return -1;
- }
if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
error_report("KVM is missing capability KVM_CAP_S390_COW - "
"unsupported environment");
return -1;
}
Not sure if we also want to move KVM_CAP_S390_COW somehow. The message would be
different.
IIRC that error could happen when you ran KVM within an older version of z/VM, so the
"please use kernel 3.15 or newer" message would be completely misleading there.
Yes, thats what I was trying to say, we would need a different message.
Lets go with this patch.
Aparch from that:
Reviewed-by: Christian Borntraeger
Thanks,
Thomas
Re: [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement
Am 09.10.23 um 19:07 schrieb Thomas Huth:
Since we already require at least kernel 3.15 in the s390x KVM code,
we can assume that the KVM_CAP_SYNC_REGS capability is always there.
Thus turn this into a hard requirement now.
Signed-off-by: Thomas Huth
---
target/s390x/kvm/kvm.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index bc5c56a305..b3e2eaa2eb 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -337,21 +337,29 @@ int kvm_arch_get_default_type(MachineState *ms)
int kvm_arch_init(MachineState *ms, KVMState *s)
{
+int required_caps[] = {
+KVM_CAP_DEVICE_CTRL,
+KVM_CAP_SYNC_REGS,
+};
+
+for (int i = 0; i < ARRAY_SIZE(required_caps); i++) {
+if (!kvm_check_extension(s, required_caps[i])) {
+error_report("KVM is missing capability #%d - "
+ "please use kernel 3.15 or newer", required_caps[i]);
+return -1;
+}
+}
+
object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,
false, NULL);
-if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
-error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "
- "please use kernel 3.15 or newer");
-return -1;
-}
if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
error_report("KVM is missing capability KVM_CAP_S390_COW - "
"unsupported environment");
return -1;
}
Not sure if we also want to move KVM_CAP_S390_COW somehow. The message would be
different.
Aparch from that:
Reviewed-by: Christian Borntraeger
-cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
+cap_sync_regs = true;
cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
cap_mem_op_extension = kvm_check_extension(s,
KVM_CAP_S390_MEM_OP_EXTENSION);
Re: [PATCH] s390x: do a subsystem reset before the unprotect on reboot
Am 01.09.23 um 13:48 schrieb Janosch Frank:
Bound APQNs have to be reset before tearing down the secure config via
s390_machine_unprotect(). Otherwise the Ultravisor will return a error
code.
So let's do a subsystem_reset() which includes a AP reset before the
unprotect call. We'll do a full device_reset() afterwards which will
reset some devices twice. That's ok since we can't move the
device_reset() before the unprotect as it includes a CPU clear reset
which the Ultravisor does not expect at that point in time.
Signed-off-by: Janosch Frank
Makes sense.
Acked-by: Christian Borntraeger
---
I'm not able to test this for the PV AP case right new, that has to
wait to early next week. However Marc told me that the non-AP PV test
works fine now.
---
hw/s390x/s390-virtio-ccw.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3dd0b2372d..2d75f2131f 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -438,10 +438,20 @@ static void s390_machine_reset(MachineState *machine,
ShutdownCause reason)
switch (reset_type) {
case S390_RESET_EXTERNAL:
case S390_RESET_REIPL:
+/*
+ * Reset the subsystem which includes a AP reset. If a PV
+ * guest had APQNs attached the AP reset is a prerequisite to
+ * unprotecting since the UV checks if all APQNs are reset.
+ */
+subsystem_reset();
if (s390_is_pv()) {
s390_machine_unprotect(ms);
}
+/*
+ * Device reset includes CPU clear resets so this has to be
+ * done AFTER the unprotect call above.
+ */
qemu_devices_reset(reason);
s390_crypto_reset();
Re: s390 intermittent test failure in qemu:block / io-qcow2-copy-before-write
Am 25.07.23 um 18:45 schrieb Peter Maydell: There seems to be an intermittent failure on the s390 host in the qemu:block / io-qcow2-copy-before-write test: https://gitlab.com/qemu-project/qemu/-/jobs/4737819873 The log says the test was expecting to do some reading and writing but got an unexpected 'permission denied' error on the read. Any idea why this might happen ? 768/835 qemu:block / io-qcow2-copy-before-write ERROR 12.05s exit status 1 PYTHON=/home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/build/pyvenv/bin/python3 MALLOC_PERTURB_=101 /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/build/pyvenv/bin/python3 /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/build/../tests/qemu-iotests/check -tap -qcow2 copy-before-write --source-dir /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/tests/qemu-iotests --build-dir /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/build/tests/qemu-iotests ― ✀ ― stderr: --- /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/tests/qemu-iotests/tests/copy-before-write.out +++ /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/build/scratch/qcow2-file-copy-before-write/copy-before-write.out.bad @@ -1,5 +1,21 @@ - +...F +== +FAIL: test_timeout_break_snapshot (__main__.TestCbwError) +-- +Traceback (most recent call last): + File "/home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/tests/qemu-iotests/tests/copy-before-write", line 210, in test_timeout_break_snapshot + self.assertEqual(log, """\ +AssertionError: 'wrot[195 chars]read 1048576/1048576 bytes at offset 0\n1 MiB,[46 chars]c)\n' != 'wrot[195 chars]read failed: Permission denied\n' + wrote 524288/524288 bytes at offset 0 + 512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + wrote 524288/524288 bytes at offset 524288 + 512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) ++ read failed: Permission denied +- read 1048576/1048576 bytes at offset 0 +- 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + + -- Ran 4 tests -OK +FAILED (failures=1) (test program exited with status code 1) ―― Same failure, previous job: https://gitlab.com/qemu-project/qemu/-/jobs/4736463062 This one's a "Failed to get write lock" in io-qcow2-161: https://gitlab.com/qemu-project/qemu/-/jobs/4734846533 See https://lore.kernel.org/qemu-devel/028059df-eaf4-9e65-a195-4733b708a...@linux.ibm.com/ I was not yet able to understand that. And I can only reproduce with Ubuntu (RHEL,SLES,Fedora are fine). Upstream kernel on Ubuntu does not help, so I assume it must be something in the libraries or config.
Re: [PATCH] pc-bios/s390-ccw: Get rid of the the __u* types
Am 28.06.23 um 10:32 schrieb Thomas Huth: On 27/06/2023 14.19, Christian Borntraeger wrote: Am 27.06.23 um 13:41 schrieb Thomas Huth: Using types starting with double underscores should be avoided since these names are marked as reserved by the C standard. The corresponding Linux In general I think this change is fine, but this is kind of interesting, as /usr/include/linux/types.h does have __u64 and friends. In fact there is __u64 but not u64 in /usr/include. And yes a google search for double underscore has The use of two underscores (` __ ') in identifiers is reserved for the compiler's internal use according to the ANSI-C standard. Underscores (` _ ') are often used in names of library functions (such as " _main " and " _exit "). In order to avoid collisions, do not begin an identifier with an underscore. kernel header file has also been changed accordingly a long time ago: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/diff/drivers/s390/cio/cio.h?id=cd6b4f27b9bb2a but IIRC from a kernel perspective u64 is for kernel internal uint64_t and __u64 is for uapi, e.g. see https://lkml.indiana.edu/hypermail/linux/kernel/1401.2/02851.html So in essence we (QEMU/s390-ccw) have to decide what to use for our internal purposes. And yes, u64 and this patch is certainly ok. But we might need to change the patch description Ok, agreed, the patch description could be better. Maybe just something like this: " The types starting with double underscores have likely been introduced into the s390-ccw bios to be able to re-use structs from the Linux kernel in the past, but the corresponding structs in cio.h have been changed there a long time ago already to not use the variants with the double underscores anymore: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/diff/drivers/s390/cio/cio.h?id=cd6b4f27b9bb2a So it would be good to replace these in the s390-ccw bios now, too. Yes, looks good.
Re: [PATCH] pc-bios/s390-ccw: Get rid of the the __u* types
Am 27.06.23 um 13:41 schrieb Thomas Huth:
Using types starting with double underscores should be avoided since these
names are marked as reserved by the C standard. The corresponding Linux
In general I think this change is fine, but this is kind of interesting, as
/usr/include/linux/types.h does have __u64 and friends. In fact there is
__u64 but not u64 in /usr/include.
And yes a google search for double underscore has
The use of two underscores (` __ ') in identifiers is reserved for the
compiler's internal use according to the ANSI-C standard. Underscores
(` _ ') are often used in names of library functions (such as " _main
" and " _exit "). In order to avoid collisions, do not begin an
identifier with an underscore.
kernel header file has also been changed accordingly a long time ago:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/diff/drivers/s390/cio/cio.h?id=cd6b4f27b9bb2a
but IIRC from a kernel perspective u64 is for kernel internal uint64_t
and __u64 is for uapi, e.g. see
https://lkml.indiana.edu/hypermail/linux/kernel/1401.2/02851.html
So in essence we (QEMU/s390-ccw) have to decide what to use for our
internal purposes. And yes, u64 and this patch is certainly ok. But
we might need to change the patch description
So we should get rid of the __u* in the s390-ccw bios now finally, too.
Signed-off-by: Thomas Huth
---
Based-on: <20230510143925.4094-4-quint...@redhat.com>
pc-bios/s390-ccw/cio.h | 232 ++--
pc-bios/s390-ccw/s390-ccw.h | 4 -
2 files changed, 116 insertions(+), 120 deletions(-)
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index efeb449572..c977a52b50 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -17,10 +17,6 @@ typedef unsigned char u8;
typedef unsigned short u16;
typedef unsigned int u32;
typedef unsigned long long u64;
-typedef unsigned char __u8;
-typedef unsigned short __u16;
-typedef unsigned int __u32;
-typedef unsigned long long __u64;
#define true 1
#define false 0
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index 88a88adfd2..8b18153deb 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -17,32 +17,32 @@
* path management control word
*/
struct pmcw {
-__u32 intparm; /* interruption parameter */
-__u32 qf:1; /* qdio facility */
-__u32 w:1;
-__u32 isc:3;/* interruption subclass */
-__u32 res5:3; /* reserved zeros */
-__u32 ena:1;/* enabled */
-__u32 lm:2; /* limit mode */
-__u32 mme:2;/* measurement-mode enable */
-__u32 mp:1; /* multipath mode */
-__u32 tf:1; /* timing facility */
-__u32 dnv:1;/* device number valid */
-__u32 dev:16; /* device number */
-__u8 lpm; /* logical path mask */
-__u8 pnom; /* path not operational mask */
-__u8 lpum; /* last path used mask */
-__u8 pim; /* path installed mask */
-__u16 mbi; /* measurement-block index */
-__u8 pom; /* path operational mask */
-__u8 pam; /* path available mask */
-__u8 chpid[8]; /* CHPID 0-7 (if available) */
-__u32 unused1:8;/* reserved zeros */
-__u32 st:3; /* subchannel type */
-__u32 unused2:18; /* reserved zeros */
-__u32 mbfc:1; /* measurement block format control */
-__u32 xmwme:1; /* extended measurement word mode enable */
-__u32 csense:1; /* concurrent sense; can be enabled ...*/
+u32 intparm;/* interruption parameter */
+u32 qf:1; /* qdio facility */
+u32 w:1;
+u32 isc:3; /* interruption subclass */
+u32 res5:3; /* reserved zeros */
+u32 ena:1; /* enabled */
+u32 lm:2; /* limit mode */
+u32 mme:2; /* measurement-mode enable */
+u32 mp:1; /* multipath mode */
+u32 tf:1; /* timing facility */
+u32 dnv:1; /* device number valid */
+u32 dev:16; /* device number */
+u8 lpm;/* logical path mask */
+u8 pnom; /* path not operational mask */
+u8 lpum; /* last path used mask */
+u8 pim;/* path installed mask */
+u16 mbi;/* measurement-block index */
+u8 pom;/* path operational mask */
+u8 pam;/* path available mask */
+u8 chpid[8]; /* CHPID 0-7 (if available) */
+u32 unused1:8; /* reserved zeros */
+u32 st:3; /* subchannel type */
+u32 unused2:18; /* reserved zeros */
+u32 mbfc:1; /* measurement block format control */
+u32 xmwme:1;/* extended measurement word mode enable */
+u32 csense:1; /* concurrent sense; can be enabled ...*/
/* ... per MSCH,
Re: [PATCH 3/4] pc-bios/s390-ccw: Move the stack array into start.S
Am 26.06.23 um 15:21 schrieb Thomas Huth: diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S index 29b0a9ece0..47ef6e8aa8 100644 --- a/pc-bios/s390-ccw/start.S +++ b/pc-bios/s390-ccw/start.S @@ -120,3 +120,8 @@ external_new_mask: .quad 0x00018000 io_new_mask: .quad 0x00018000 + +.bss + +.align 16 +.lcomm stack,STACK_SIZE IIRC, the ELF ABI defines the stack to be 8 byte aligned, but 16 certainly does not hurt. Acked-by: Christian Borntraeger
Re: [PATCH 2/4] pc-bios/s390-ccw: Provide space for initial stack frame in start.S
Am 26.06.23 um 15:21 schrieb Thomas Huth: Providing the space of a stack frame is the duty of the caller, so we should reserve 160 bytes before jumping into the main function. Otherwise the main() function might write past the stack array. While we're at it, add a proper STACK_SIZE macro for the stack size instead of using magic numbers (this is also required for the following patch). Signed-off-by: Thomas Huth --- pc-bios/s390-ccw/start.S | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S index d29de09cc6..29b0a9ece0 100644 --- a/pc-bios/s390-ccw/start.S +++ b/pc-bios/s390-ccw/start.S @@ -10,10 +10,12 @@ * directory. */ +#define STACK_SIZE 0x8000 + .globl _start _start: -larl%r15,stack + 0x8000 /* Set up stack */ +larl%r15,stack + STACK_SIZE - 160 /* Set up stack */ Reviewed-by: Christian Borntraeger
Re: [PATCH] pc-bios/s390-ccw/Makefile: Use -z noexecstack to silence linker warning
Am 22.06.23 um 15:08 schrieb Thomas Huth: Recent versions of ld complain when linking the s390-ccw bios: /usr/bin/ld: warning: start.o: missing .note.GNU-stack section implies executable stack /usr/bin/ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker We can silence the warning by telling the linker to mark the stack as not executable. Signed-off-by: Thomas Huth --- pc-bios/s390-ccw/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile index 2e8cc015aa..acfcd1e71a 100644 --- a/pc-bios/s390-ccw/Makefile +++ b/pc-bios/s390-ccw/Makefile @@ -55,7 +55,7 @@ config-cc.mak: Makefile $(call cc-option,-march=z900,-march=z10)) 3> config-cc.mak -include config-cc.mak -LDFLAGS += -Wl,-pie -nostdlib +LDFLAGS += -Wl,-pie -nostdlib -z noexecstack build-all: s390-ccw.img s390-netboot.img In the end this should not matter as the resulting binary is not loaded by an elf loader so this should be fine Acked-by: Christian Borntraeger
Re: s390 private runner CI job timing out
Am 06.04.23 um 14:39 schrieb Peter Maydell: On Thu, 6 Apr 2023 at 13:30, Thomas Huth wrote: The thing is: it shouldn't take that long to build QEMU and run the tests here, theoretically. Some days ago, the job was finishing in 39 minutes: https://gitlab.com/qemu-project/qemu/-/jobs/3973481571 The recent run took 74 minutes: https://gitlab.com/qemu-project/qemu/-/jobs/4066136770 That's almost a factor of two! So there is definitely something strange going on. So that 39 minute run was about 22 minutes compile, 17 minutes test. The 74 minute run was 45 minutes compile, 30 minutes test. The number of compile steps in meson was pretty much the same (10379 vs 10384) in each case. So the most plausible conclusion seems like "the VM mysteriously got slower by nearly a factor of 2", given that the slowdown seems to affect the compile and test stages about equally. The VM has been up for 44 days, so we can rule out "rebooted into a new kernel with some kind of perf bug". Looks like the team is looking into some storage delays at the moment. So it would be good to get some numbers for io wait and steal on the next run to see if this is related to storage or CPU consumption. Maybe collect /proc/stat before and after a run. Or login and use top.
Re: s390 private runner CI job timing out
Am 06.04.23 um 14:39 schrieb Peter Maydell: On Thu, 6 Apr 2023 at 13:30, Thomas Huth wrote: The thing is: it shouldn't take that long to build QEMU and run the tests here, theoretically. Some days ago, the job was finishing in 39 minutes: https://gitlab.com/qemu-project/qemu/-/jobs/3973481571 The recent run took 74 minutes: https://gitlab.com/qemu-project/qemu/-/jobs/4066136770 That's almost a factor of two! So there is definitely something strange going on. So that 39 minute run was about 22 minutes compile, 17 minutes test. The 74 minute run was 45 minutes compile, 30 minutes test. The number of compile steps in meson was pretty much the same (10379 vs 10384) in each case. So the most plausible conclusion seems like "the VM mysteriously got slower by nearly a factor of 2", given that the slowdown seems to affect the compile and test stages about equally. The VM has been up for 44 days, so we can rule out "rebooted into a new kernel with some kind of perf bug". I will ask around if we can get a higher share of the system.
Re: s390 private runner CI job timing out
Am 06.04.23 um 14:30 schrieb Thomas Huth: On 06/04/2023 14.13, Christian Borntraeger wrote: Am 06.04.23 um 14:05 schrieb Peter Maydell: On Thu, 6 Apr 2023 at 12:17, Christian Borntraeger wrote: Am 06.04.23 um 12:44 schrieb Peter Maydell: On Thu, 6 Apr 2023 at 11:40, Christian Borntraeger wrote: Am 06.04.23 um 11:21 schrieb Peter Maydell: Christian, does our S390X machine get a guaranteed amount of CPU, or does it depend on what else is running on the hardware? I think its a shared system with shared CPUs. Can you check the steal time in top or proc? If this is far too high we could ask to give you more weight for that VM. It's idle at the moment and steal time seems to be low (0.0 .. 0.3); I'll try to remember to check next time it's running a job. Do you have /proc/stat ? Yes; hopefully it means more to you than it does to me :-) linux1@qemu01:~$ cat /proc/stat cpu 60904459 604975 15052194 1435958176 17128179 351949 758578 22218760 0 0 cpu0 15022535 146734 3786909 358774818 4283172 98313 237156 5894809 0 0 cpu1 15306890 151164 3746024 358968957 4378864 85629 172492 5434255 0 0 cpu2 15307709 157180 3762691 359141276 4138714 85736 176367 5474594 0 0 cpu3 15267324 149895 3756569 359073124 4327428 82269 172562 5415101 0 0 This is user,nice,system,idle,iowait,irq,softirq,steal,guest,guest_nice So overall there is some (20-30% since the last reboot) steal going on. Not sure if this is the real problem since it is only Ubuntu 20.04. Does a higher timeout make the problem go away? The thing is: it shouldn't take that long to build QEMU and run the tests here, theoretically. Some days ago, the job was finishing in 39 minutes: https://gitlab.com/qemu-project/qemu/-/jobs/3973481571 The recent run took 74 minutes: https://gitlab.com/qemu-project/qemu/-/jobs/4066136770 That's almost a factor of two! So there is definitely something strange going on. But this has 786 instead of 659 tests, no?
Re: s390 private runner CI job timing out
Am 06.04.23 um 14:05 schrieb Peter Maydell: On Thu, 6 Apr 2023 at 12:17, Christian Borntraeger wrote: Am 06.04.23 um 12:44 schrieb Peter Maydell: On Thu, 6 Apr 2023 at 11:40, Christian Borntraeger wrote: Am 06.04.23 um 11:21 schrieb Peter Maydell: Christian, does our S390X machine get a guaranteed amount of CPU, or does it depend on what else is running on the hardware? I think its a shared system with shared CPUs. Can you check the steal time in top or proc? If this is far too high we could ask to give you more weight for that VM. It's idle at the moment and steal time seems to be low (0.0 .. 0.3); I'll try to remember to check next time it's running a job. Do you have /proc/stat ? Yes; hopefully it means more to you than it does to me :-) linux1@qemu01:~$ cat /proc/stat cpu 60904459 604975 15052194 1435958176 17128179 351949 758578 22218760 0 0 cpu0 15022535 146734 3786909 358774818 4283172 98313 237156 5894809 0 0 cpu1 15306890 151164 3746024 358968957 4378864 85629 172492 5434255 0 0 cpu2 15307709 157180 3762691 359141276 4138714 85736 176367 5474594 0 0 cpu3 15267324 149895 3756569 359073124 4327428 82269 172562 5415101 0 0 This is user,nice,system,idle,iowait,irq,softirq,steal,guest,guest_nice So overall there is some (20-30% since the last reboot) steal going on. Not sure if this is the real problem since it is only Ubuntu 20.04. Does a higher timeout make the problem go away?
Re: s390 private runner CI job timing out
Am 06.04.23 um 12:44 schrieb Peter Maydell: On Thu, 6 Apr 2023 at 11:40, Christian Borntraeger wrote: Am 06.04.23 um 11:21 schrieb Peter Maydell: Christian, does our S390X machine get a guaranteed amount of CPU, or does it depend on what else is running on the hardware? I think its a shared system with shared CPUs. Can you check the steal time in top or proc? If this is far too high we could ask to give you more weight for that VM. It's idle at the moment and steal time seems to be low (0.0 .. 0.3); I'll try to remember to check next time it's running a job. Do you have /proc/stat ?
Re: s390 private runner CI job timing out
Am 06.04.23 um 11:21 schrieb Peter Maydell: On Thu, 6 Apr 2023 at 07:57, Thomas Huth wrote: On 05/04/2023 17.15, Peter Maydell wrote: The s390 private runner CI job ubuntu-20.04-s390x-all seems to have started timing out a lot recently. Here's an example where it passed, but with only 53 seconds left on the clock before it would have been killed: https://gitlab.com/qemu-project/qemu/-/jobs/4066136770 It looks like 'make check' was about 30 minutes of the 75 minutes total, and compilation was 45 minutes. Any suggestions for how we can trim this down? (Presumably we could also raise the time limit given that this is a private runner job...) I don't have access to that system, so I can only guess: Did you check whether there are any other processes still running (leftovers from earlier test runs)? I did check for that, as it's been a problem in the past, but no, in this case no other jobs are running in the VM. If not, it's maybe because it is a VM that is running with other VMs in parallel that hog the CPU? In that case, you could contact the owner of the machine and ask whether there is anything that could be done about it. Or simply increase the timeout a little bit more... (our highest timeout in another job is 90 minutes, so I think that would still be OK?). Christian, does our S390X machine get a guaranteed amount of CPU, or does it depend on what else is running on the hardware? I think its a shared system with shared CPUs. Can you check the steal time in top or proc? If this is far too high we could ask to give you more weight for that VM.
Re: [PATCH] s390x/gdb: Split s390-virt.xml
Am 13.03.23 um 22:16 schrieb Ilya Leoshkevich:
TCG emulates ckc, cputm, last_break and prefix, and it's quite useful
to have them during debugging.
KVM provides those as well so I dont get what you are trying to do here. (I
would understand moving out the pfault things into a KVM section)
So move them into the new s390-virt-tcg.xml file.
pp, pfault_token, pfault_select and pfault_compare are not emulated,
so keep them in s390-virt.xml.
Signed-off-by: Ilya Leoshkevich
---
configs/targets/s390x-linux-user.mak | 2 +-
configs/targets/s390x-softmmu.mak| 2 +-
gdb-xml/s390-virt-tcg.xml| 14 +
gdb-xml/s390-virt.xml| 4 --
target/s390x/gdbstub.c | 82 ++--
5 files changed, 69 insertions(+), 35 deletions(-)
create mode 100644 gdb-xml/s390-virt-tcg.xml
diff --git a/configs/targets/s390x-linux-user.mak
b/configs/targets/s390x-linux-user.mak
index e2978248ede..fb3e2b73be7 100644
--- a/configs/targets/s390x-linux-user.mak
+++ b/configs/targets/s390x-linux-user.mak
@@ -2,4 +2,4 @@ TARGET_ARCH=s390x
TARGET_SYSTBL_ABI=common,64
TARGET_SYSTBL=syscall.tbl
TARGET_BIG_ENDIAN=y
-TARGET_XML_FILES= gdb-xml/s390x-core64.xml gdb-xml/s390-acr.xml
gdb-xml/s390-fpr.xml gdb-xml/s390-vx.xml gdb-xml/s390-cr.xml
gdb-xml/s390-virt.xml gdb-xml/s390-gs.xml
+TARGET_XML_FILES= gdb-xml/s390x-core64.xml gdb-xml/s390-acr.xml
gdb-xml/s390-fpr.xml gdb-xml/s390-vx.xml gdb-xml/s390-cr.xml
gdb-xml/s390-virt.xml gdb-xml/s390-virt-tcg.xml gdb-xml/s390-gs.xml
diff --git a/configs/targets/s390x-softmmu.mak
b/configs/targets/s390x-softmmu.mak
index 258b4cf3582..554330d7c85 100644
--- a/configs/targets/s390x-softmmu.mak
+++ b/configs/targets/s390x-softmmu.mak
@@ -1,4 +1,4 @@
TARGET_ARCH=s390x
TARGET_BIG_ENDIAN=y
TARGET_SUPPORTS_MTTCG=y
-TARGET_XML_FILES= gdb-xml/s390x-core64.xml gdb-xml/s390-acr.xml
gdb-xml/s390-fpr.xml gdb-xml/s390-vx.xml gdb-xml/s390-cr.xml
gdb-xml/s390-virt.xml gdb-xml/s390-gs.xml
+TARGET_XML_FILES= gdb-xml/s390x-core64.xml gdb-xml/s390-acr.xml
gdb-xml/s390-fpr.xml gdb-xml/s390-vx.xml gdb-xml/s390-cr.xml
gdb-xml/s390-virt.xml gdb-xml/s390-virt-tcg.xml gdb-xml/s390-gs.xml
diff --git a/gdb-xml/s390-virt-tcg.xml b/gdb-xml/s390-virt-tcg.xml
new file mode 100644
index 000..0f77c9b48c6
--- /dev/null
+++ b/gdb-xml/s390-virt-tcg.xml
@@ -0,0 +1,14 @@
+
+
+
+
+
+
+
+
+
+
diff --git a/gdb-xml/s390-virt.xml b/gdb-xml/s390-virt.xml
index e2e9a7ad3cc..a79c0307682 100644
--- a/gdb-xml/s390-virt.xml
+++ b/gdb-xml/s390-virt.xml
@@ -7,10 +7,6 @@
-
-
-
-
diff --git a/target/s390x/gdbstub.c b/target/s390x/gdbstub.c
index a5d69d0e0bc..111b695dc85 100644
--- a/target/s390x/gdbstub.c
+++ b/target/s390x/gdbstub.c
@@ -200,61 +200,81 @@ static int cpu_write_c_reg(CPUS390XState *env, uint8_t
*mem_buf, int n)
}
}
-/* the values represent the positions in s390-virt.xml */
-#define S390_VIRT_CKC_REGNUM0
-#define S390_VIRT_CPUTM_REGNUM 1
-#define S390_VIRT_BEA_REGNUM2
-#define S390_VIRT_PREFIX_REGNUM 3
-#define S390_VIRT_PP_REGNUM 4
-#define S390_VIRT_PFT_REGNUM5
-#define S390_VIRT_PFS_REGNUM6
-#define S390_VIRT_PFC_REGNUM7
-/* total number of registers in s390-virt.xml */
-#define S390_NUM_VIRT_REGS 8
+/* the values represent the positions in s390-virt-tcg.xml */
+#define S390_VIRT_TCG_CKC_REGNUM0
+#define S390_VIRT_TCG_CPUTM_REGNUM 1
+#define S390_VIRT_TCG_BEA_REGNUM2
+#define S390_VIRT_TCG_PREFIX_REGNUM 3
+/* total number of registers in s390-virt-tcg.xml */
+#define S390_NUM_VIRT_TCG_REGS 4
-static int cpu_read_virt_reg(CPUS390XState *env, GByteArray *mem_buf, int n)
+static int cpu_read_virt_tcg_reg(CPUS390XState *env, GByteArray *mem_buf, int
n)
{
switch (n) {
-case S390_VIRT_CKC_REGNUM:
+case S390_VIRT_TCG_CKC_REGNUM:
return gdb_get_regl(mem_buf, env->ckc);
-case S390_VIRT_CPUTM_REGNUM:
+case S390_VIRT_TCG_CPUTM_REGNUM:
return gdb_get_regl(mem_buf, env->cputm);
-case S390_VIRT_BEA_REGNUM:
+case S390_VIRT_TCG_BEA_REGNUM:
return gdb_get_regl(mem_buf, env->gbea);
-case S390_VIRT_PREFIX_REGNUM:
+case S390_VIRT_TCG_PREFIX_REGNUM:
return gdb_get_regl(mem_buf, env->psa);
-case S390_VIRT_PP_REGNUM:
-return gdb_get_regl(mem_buf, env->pp);
-case S390_VIRT_PFT_REGNUM:
-return gdb_get_regl(mem_buf, env->pfault_token);
-case S390_VIRT_PFS_REGNUM:
-return gdb_get_regl(mem_buf, env->pfault_select);
-case S390_VIRT_PFC_REGNUM:
-return gdb_get_regl(mem_buf, env->pfault_compare);
default:
return 0;
}
}
-static int cpu_write_virt_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
+static int cpu_write_virt_tcg_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
{
switch (n) {
-case S390_VIRT_CKC_REGNUM:
+case S390_VIRT_TCG_CKC_REGNUM:
env->ckc = ldtul_p(mem_buf);
Re: [PATCH] Makefile: allow 'make uninstall'
Am 10.01.23 um 16:13 schrieb Peter Maydell: Meson supports an "uninstall", so we can easily allow it to work by not suppressing the forwarding of it from Make to meson. We originally suppressed this because Meson's 'uninstall' has a hole in it: it will remove everything that is installed by a mechanism meson knows about, but not things installed by "custom install scripts", and there is no "custom uninstall script" mechanism. For QEMU, though, the only thing that was being installed by a custom install script was the LC_MESSAGES files handled by Meson's i18n module, and that code was fixed in Meson commit 487d45c1e5bfff0fbdb4, which is present in Meson 0.60.0 and later. Since we already require a Meson version newer than that, we're now safe to enable 'uninstall', as it will now correctly uninstall everything that was installed. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/109 Signed-off-by: Peter Maydell Always missed that functionality. Thanks.
Re: [PATCH v13 0/7] s390x: CPU Topology
Am 13.12.22 um 14:50 schrieb Christian Borntraeger: Am 12.12.22 um 11:01 schrieb Pierre Morel: On 12/9/22 15:45, Cédric Le Goater wrote: On 12/8/22 10:44, Pierre Morel wrote: Hi, Implementation discussions == CPU models -- Since the S390_FEAT_CONFIGURATION_TOPOLOGY is already in the CPU model for old QEMU we could not activate it as usual from KVM but needed a KVM capability: KVM_CAP_S390_CPU_TOPOLOGY. Checking and enabling this capability enables S390_FEAT_CONFIGURATION_TOPOLOGY. Migration - Once the S390_FEAT_CONFIGURATION_TOPOLOGY is enabled in the source host the STFL(11) is provided to the guest. Since the feature is already in the CPU model of older QEMU, a migration from a new QEMU enabling the topology to an old QEMU will keep STFL(11) enabled making the guest get an exception for illegal operation as soon as it uses the PTF instruction. A VMState keeping track of the S390_FEAT_CONFIGURATION_TOPOLOGY allows to forbid the migration in such a case. Note that the VMState will be used to hold information on the topology once we implement topology change for a running guest. Topology Until we introduce bookss and drawers, polarization and dedication the topology is kept very simple and is specified uniquely by the core_id of the vCPU which is also the vCPU address. Testing === To use the QEMU patches, you will need Linux V6-rc1 or newer, or use the following Linux mainline patches: f5ecfee94493 2022-07-20 KVM: s390: resetting the Topology-Change-Report 24fe0195bc19 2022-07-20 KVM: s390: guest support for topology function 0130337ec45b 2022-07-20 KVM: s390: Cleanup ipte lock access and SIIF fac.. Currently this code is for KVM only, I have no idea if it is interesting to provide a TCG patch. If ever it will be done in another series. Documentation = To have a better understanding of the S390x CPU Topology and its implementation in QEMU you can have a look at the documentation in the last patch of this series. The admin will want to match the host and the guest topology, taking into account that the guest does not recognize multithreading. Consequently, two vCPU assigned to threads of the same real CPU should preferably be assigned to the same socket of the guest machine. Future developments === Two series are actively prepared: - Adding drawers, book, polarization and dedication to the vCPU. - changing the topology with a running guest Since we have time before the next QEMU 8.0 release, could you please send the whole patchset ? Having the full picture would help in taking decisions for downstream also. I am still uncertain about the usefulness of S390Topology object because, as for now, the state can be computed on the fly, the reset can be handled at the machine level directly under s390_machine_reset() and so could migration if the machine had a vmstate (not the case today but quite easy to add). So before merging anything that could be difficult to maintain and/or backport, I would prefer to see it all ! The goal of dedicating an object for topology was to ease the maintenance and portability by using the QEMU object framework. If on contrary it is a problem for maintenance or backport we surely better not use it. Any other opinion? I agree with Cedric. There is no point in a topology object. The state is calculated on the fly depending on the command line. This would change if we ever implement the PTF horizontal/vertical state. But this can then be another CPU state. So I think we could go forward with this patch as a simple patch set that allows to sets a static topology. This makes sense on its own, e.g. if you plan to pin your vCPUs to given host CPUs. Dynamic changes do come with CPU hotplug, not sure what libvirt does with new CPUs in that case during migration. I assume those are re-generated on the target with whatever topology was created on the source. So I guess this will just work, but it would be good if we could test that. A more fine-grained topology (drawer, book) could be added later or upfront. It does require common code and libvirt enhancements, though. Now I have discussed with Pierre and there IS a state that we want to migrate. The topology change state is a guest wide bit that might be still set when topology is changed->bit is set guest is not yet started migration 2 options: 1. a vmstate with that bit and migrate the state 2. always set the topology change bit after migration
Re: [PATCH v13 0/7] s390x: CPU Topology
Am 13.12.22 um 14:57 schrieb Janis Schoetterl-Glausch: On Tue, 2022-12-13 at 14:41 +0100, Christian Borntraeger wrote: Am 12.12.22 um 11:17 schrieb Thomas Huth: On 12/12/2022 11.10, Pierre Morel wrote: On 12/12/22 10:07, Thomas Huth wrote: On 12/12/2022 09.51, Pierre Morel wrote: On 12/9/22 14:32, Thomas Huth wrote: On 08/12/2022 10.44, Pierre Morel wrote: Hi, Implementation discussions == CPU models -- Since the S390_FEAT_CONFIGURATION_TOPOLOGY is already in the CPU model for old QEMU we could not activate it as usual from KVM but needed a KVM capability: KVM_CAP_S390_CPU_TOPOLOGY. Checking and enabling this capability enables S390_FEAT_CONFIGURATION_TOPOLOGY. Migration - Once the S390_FEAT_CONFIGURATION_TOPOLOGY is enabled in the source host the STFL(11) is provided to the guest. Since the feature is already in the CPU model of older QEMU, a migration from a new QEMU enabling the topology to an old QEMU will keep STFL(11) enabled making the guest get an exception for illegal operation as soon as it uses the PTF instruction. I now thought that it is not possible to enable "ctop" on older QEMUs since the don't enable the KVM capability? ... or is it still somehow possible? What did I miss? Thomas Enabling ctop with ctop=on on old QEMU is not possible, this is right. But, if STFL(11) is enable in the source KVM by a new QEMU, I can see that even with -ctop=off the STFL(11) is migrated to the destination. This does not make sense. the cpu model and stfle values are not migrated. This is re-created during startup depending on the command line parameters of -cpu. Thats why source and host have the same command lines for -cpu. And STFLE.11 must not be set on the SOURCE of ctop is off. What about linux? I didn't look to thoroughly at it, but it caches the stfle bits, doesn't it? So if the migration succeeds, even tho it should not, it will look to the guest like the facility is enabled. That is true, but the migration should not succeed in that case unless you use an unsafe way of migrating. And the cpu model was exactly added to block those unsafe way. One thing: -cpu host is unsafe (host-passthrough in libvirt speak). Either use host-model or a fully specified model like z14.2,ctop=on
Re: [PATCH v13 0/7] s390x: CPU Topology
Am 12.12.22 um 11:01 schrieb Pierre Morel: On 12/9/22 15:45, Cédric Le Goater wrote: On 12/8/22 10:44, Pierre Morel wrote: Hi, Implementation discussions == CPU models -- Since the S390_FEAT_CONFIGURATION_TOPOLOGY is already in the CPU model for old QEMU we could not activate it as usual from KVM but needed a KVM capability: KVM_CAP_S390_CPU_TOPOLOGY. Checking and enabling this capability enables S390_FEAT_CONFIGURATION_TOPOLOGY. Migration - Once the S390_FEAT_CONFIGURATION_TOPOLOGY is enabled in the source host the STFL(11) is provided to the guest. Since the feature is already in the CPU model of older QEMU, a migration from a new QEMU enabling the topology to an old QEMU will keep STFL(11) enabled making the guest get an exception for illegal operation as soon as it uses the PTF instruction. A VMState keeping track of the S390_FEAT_CONFIGURATION_TOPOLOGY allows to forbid the migration in such a case. Note that the VMState will be used to hold information on the topology once we implement topology change for a running guest. Topology Until we introduce bookss and drawers, polarization and dedication the topology is kept very simple and is specified uniquely by the core_id of the vCPU which is also the vCPU address. Testing === To use the QEMU patches, you will need Linux V6-rc1 or newer, or use the following Linux mainline patches: f5ecfee94493 2022-07-20 KVM: s390: resetting the Topology-Change-Report 24fe0195bc19 2022-07-20 KVM: s390: guest support for topology function 0130337ec45b 2022-07-20 KVM: s390: Cleanup ipte lock access and SIIF fac.. Currently this code is for KVM only, I have no idea if it is interesting to provide a TCG patch. If ever it will be done in another series. Documentation = To have a better understanding of the S390x CPU Topology and its implementation in QEMU you can have a look at the documentation in the last patch of this series. The admin will want to match the host and the guest topology, taking into account that the guest does not recognize multithreading. Consequently, two vCPU assigned to threads of the same real CPU should preferably be assigned to the same socket of the guest machine. Future developments === Two series are actively prepared: - Adding drawers, book, polarization and dedication to the vCPU. - changing the topology with a running guest Since we have time before the next QEMU 8.0 release, could you please send the whole patchset ? Having the full picture would help in taking decisions for downstream also. I am still uncertain about the usefulness of S390Topology object because, as for now, the state can be computed on the fly, the reset can be handled at the machine level directly under s390_machine_reset() and so could migration if the machine had a vmstate (not the case today but quite easy to add). So before merging anything that could be difficult to maintain and/or backport, I would prefer to see it all ! The goal of dedicating an object for topology was to ease the maintenance and portability by using the QEMU object framework. If on contrary it is a problem for maintenance or backport we surely better not use it. Any other opinion? I agree with Cedric. There is no point in a topology object. The state is calculated on the fly depending on the command line. This would change if we ever implement the PTF horizontal/vertical state. But this can then be another CPU state. So I think we could go forward with this patch as a simple patch set that allows to sets a static topology. This makes sense on its own, e.g. if you plan to pin your vCPUs to given host CPUs. Dynamic changes do come with CPU hotplug, not sure what libvirt does with new CPUs in that case during migration. I assume those are re-generated on the target with whatever topology was created on the source. So I guess this will just work, but it would be good if we could test that. A more fine-grained topology (drawer, book) could be added later or upfront. It does require common code and libvirt enhancements, though.
Re: [PATCH v13 0/7] s390x: CPU Topology
Am 12.12.22 um 11:17 schrieb Thomas Huth: On 12/12/2022 11.10, Pierre Morel wrote: On 12/12/22 10:07, Thomas Huth wrote: On 12/12/2022 09.51, Pierre Morel wrote: On 12/9/22 14:32, Thomas Huth wrote: On 08/12/2022 10.44, Pierre Morel wrote: Hi, Implementation discussions == CPU models -- Since the S390_FEAT_CONFIGURATION_TOPOLOGY is already in the CPU model for old QEMU we could not activate it as usual from KVM but needed a KVM capability: KVM_CAP_S390_CPU_TOPOLOGY. Checking and enabling this capability enables S390_FEAT_CONFIGURATION_TOPOLOGY. Migration - Once the S390_FEAT_CONFIGURATION_TOPOLOGY is enabled in the source host the STFL(11) is provided to the guest. Since the feature is already in the CPU model of older QEMU, a migration from a new QEMU enabling the topology to an old QEMU will keep STFL(11) enabled making the guest get an exception for illegal operation as soon as it uses the PTF instruction. I now thought that it is not possible to enable "ctop" on older QEMUs since the don't enable the KVM capability? ... or is it still somehow possible? What did I miss? Thomas Enabling ctop with ctop=on on old QEMU is not possible, this is right. But, if STFL(11) is enable in the source KVM by a new QEMU, I can see that even with -ctop=off the STFL(11) is migrated to the destination. This does not make sense. the cpu model and stfle values are not migrated. This is re-created during startup depending on the command line parameters of -cpu. Thats why source and host have the same command lines for -cpu. And STFLE.11 must not be set on the SOURCE of ctop is off. Is this with the "host" CPU model or another one? And did you explicitly specify "ctop=off" at the command line, or are you just using the default setting by not specifying it? With explicit cpumodel and using ctop=off like in sudo /usr/local/bin/qemu-system-s390x_master \ -m 512M \ -enable-kvm -smp 4,sockets=4,cores=2,maxcpus=8 \ -cpu z14,ctop=off \ -machine s390-ccw-virtio-7.2,accel=kvm \ ... Ok ... that sounds like a bug somewhere in your patches or in the kernel code ... the guest should never see STFL bit 11 if ctop=off, should it? Correct. If ctop=off then QEMU should disable STFLE.11 for the CPU model.
Re: [PATCH v13 4/7] s390x/cpu_topology: CPU topology migration
Am 08.12.22 um 10:44 schrieb Pierre Morel:
The migration can only take place if both source and destination
of the migration both use or both do not use the CPU topology
facility.
We indicate a change in topology during migration postload for the
case the topology changed between source and destination.
I dont get why we need this? If the target QEMU has topology it should
already create this according to the configuration. WHy do we need a
trigger?
Signed-off-by: Pierre Morel
---
target/s390x/cpu.h| 1 +
hw/s390x/cpu-topology.c | 49 +++
target/s390x/cpu-sysemu.c | 8 +++
3 files changed, 58 insertions(+)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index bc1a7de932..284c708a6c 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -854,6 +854,7 @@ 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);
void s390_cpu_topology_reset(void);
+int s390_cpu_topology_mtcr_set(void);
#ifndef CONFIG_USER_ONLY
unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu);
#else
diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
index f54afcf550..8a2fe041d4 100644
--- a/hw/s390x/cpu-topology.c
+++ b/hw/s390x/cpu-topology.c
@@ -18,6 +18,7 @@
#include "target/s390x/cpu.h"
#include "hw/s390x/s390-virtio-ccw.h"
#include "hw/s390x/cpu-topology.h"
+#include "migration/vmstate.h"
/**
* s390_has_topology
@@ -129,6 +130,53 @@ static void s390_topology_reset(DeviceState *dev)
s390_cpu_topology_reset();
}
+/**
+ * cpu_topology_postload
+ * @opaque: a pointer to the S390Topology
+ * @version_id: version identifier
+ *
+ * We check that the topology is used or is not used
+ * on both side identically.
+ *
+ * If the topology is in use we set the Modified Topology Change Report
+ * on the destination host.
+ */
+static int cpu_topology_postload(void *opaque, int version_id)
+{
+int ret;
+
+/* We do not support CPU Topology, all is good */
+if (!s390_has_topology()) {
+return 0;
+}
+
+/* We support CPU Topology, set the MTCR unconditionally */
+ret = s390_cpu_topology_mtcr_set();
+if (ret) {
+error_report("Failed to set MTCR: %s", strerror(-ret));
+}
+return ret;
+}
+
+/**
+ * cpu_topology_needed:
+ * @opaque: The pointer to the S390Topology
+ *
+ * We always need to know if source and destination use the topology.
+ */
+static bool cpu_topology_needed(void *opaque)
+{
+return s390_has_topology();
+}
+
+const VMStateDescription vmstate_cpu_topology = {
+.name = "cpu_topology",
+.version_id = 1,
+.post_load = cpu_topology_postload,
+.minimum_version_id = 1,
+.needed = cpu_topology_needed,
+};
+
/**
* topology_class_init:
* @oc: Object class
@@ -145,6 +193,7 @@ static void topology_class_init(ObjectClass *oc, void *data)
device_class_set_props(dc, s390_topology_properties);
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
dc->reset = s390_topology_reset;
+dc->vmsd = _cpu_topology;
}
static const TypeInfo cpu_topology_info = {
diff --git a/target/s390x/cpu-sysemu.c b/target/s390x/cpu-sysemu.c
index e27864c5f5..a8e3e6219d 100644
--- a/target/s390x/cpu-sysemu.c
+++ b/target/s390x/cpu-sysemu.c
@@ -319,3 +319,11 @@ void s390_cpu_topology_reset(void)
}
}
}
+
+int s390_cpu_topology_mtcr_set(void)
+{
+if (kvm_enabled()) {
+return kvm_s390_topology_set_mtcr(1);
+}
+return -ENOENT;
+}
Re: [PATCH 1/1] qemu-iotests/stream-under-throttle: do not shutdown QEMU
Am 07.12.22 um 14:14 schrieb Christian Borntraeger:
Without a kernel or boot disk a QEMU on s390 will exit (usually with a
disabled wait state). This breaks the stream-under-throttle test case.
Do not exit qemu if on s390.
Signed-off-by: Christian Borntraeger
---
tests/qemu-iotests/tests/stream-under-throttle | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/qemu-iotests/tests/stream-under-throttle
b/tests/qemu-iotests/tests/stream-under-throttle
index 8d2d9e16840d..c24dfbcaa2f2 100755
--- a/tests/qemu-iotests/tests/stream-under-throttle
+++ b/tests/qemu-iotests/tests/stream-under-throttle
@@ -88,6 +88,8 @@ class TestStreamWithThrottle(iotests.QMPTestCase):
'x-iops-total=1,x-bps-total=104857600')
self.vm.add_blockdev(self.vm.qmp_to_opts(blockdev))
self.vm.add_device('virtio-blk,iothread=iothr0,drive=throttled-node')
+if iotests.qemu_default_machine == 's390-ccw-virtio':
+self.vm.add_args('-no-shutdown')
self.vm.launch()
def tearDown(self) -> None:
ping. I guess, this will come after the release?
Re: [PATCH v3 02/13] tcg/s390x: Remove TCG_REG_TB
Am 07.12.22 um 21:40 schrieb Ilya Leoshkevich: On Wed, 2022-12-07 at 08:55 -0600, Richard Henderson wrote: On 12/7/22 01:45, Thomas Huth wrote: On 06/12/2022 23.22, Richard Henderson wrote: On 12/6/22 13:29, Ilya Leoshkevich wrote: This change doesn't seem to affect that, but what is the minimum supported s390x qemu host? z900? Possibly z990, if I'm reading the gcc processor_flags_table[] correctly; long-displacement-facility is definitely a minimum. We probably should revisit what the minimum for TCG should be, assert those features at startup, and drop the corresponding runtime tests. If we consider the official IBM support statement: https://www.ibm.com/support/pages/system/files/inline-files/IBM%20Mainframe%20Life%20Cycle%20History%20V2.10%20-%20Sept%2013%202022_1.pdf ... that would mean that the z10 and all older machines are not supported anymore. Thanks for the pointer. It would appear that z114 exits support at the end of this month, which would leave z12 as minimum supported cpu. Even assuming z196 gets us extended-immediate, general-insn- extension, load-on-condition, and distinct-operands, which are all quite important to TCG, and constitute almost all of the current runtime checks. The other metric would be matching the set of supported cpus from the set of supported os distributions, but I would be ready to believe z196 is below the minimum there too. r~ I think it should be safe to raise the minimum required hardware for TCG to z196: We recently raised the minimum hardware for the Linux kernel to be z10, so that would be super-safe, but z196 is certainly a sane minimum. * The oldest supported RHEL is v7, it requires z196: https://access.redhat.com/product-life-cycles/ https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/installation_guide/chap-installation-planning-s390 * The oldest supported SLES is v12, it requires z196: https://www.suse.com/de-de/lifecycle/ https://documentation.suse.com/sles/12-SP5/html/SLES-all/cha-zseries.html * The oldest supported Ubuntu is v16.04, it requires zEC12+: https://wiki.ubuntu.com/S390X Best regards, Ilya
Re: [PATCH 1/1] qemu-iotests/stream-under-throttle: do not shutdown QEMU
Am 07.12.22 um 14:23 schrieb Thomas Huth:
On 07/12/2022 14.14, Christian Borntraeger wrote:
Without a kernel or boot disk a QEMU on s390 will exit (usually with a
disabled wait state). This breaks the stream-under-throttle test case.
Do not exit qemu if on s390.
Signed-off-by: Christian Borntraeger
---
tests/qemu-iotests/tests/stream-under-throttle | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/qemu-iotests/tests/stream-under-throttle
b/tests/qemu-iotests/tests/stream-under-throttle
index 8d2d9e16840d..c24dfbcaa2f2 100755
--- a/tests/qemu-iotests/tests/stream-under-throttle
+++ b/tests/qemu-iotests/tests/stream-under-throttle
@@ -88,6 +88,8 @@ class TestStreamWithThrottle(iotests.QMPTestCase):
'x-iops-total=1,x-bps-total=104857600')
self.vm.add_blockdev(self.vm.qmp_to_opts(blockdev))
self.vm.add_device('virtio-blk,iothread=iothr0,drive=throttled-node')
+ if iotests.qemu_default_machine == 's390-ccw-virtio':
+ self.vm.add_args('-no-shutdown')
self.vm.launch()
I guess you could even add that unconditionally for all architectures?
maybe. It might even fix other architecture with the same problem. But I dont
know if thats the case.
So we can start with this fix and then remove the if at a later point in time
if necessary/useful.
Anyway:
Reviewed-by: Thomas Huth
[PATCH 1/1] qemu-iotests/stream-under-throttle: do not shutdown QEMU
Without a kernel or boot disk a QEMU on s390 will exit (usually with a
disabled wait state). This breaks the stream-under-throttle test case.
Do not exit qemu if on s390.
Signed-off-by: Christian Borntraeger
---
tests/qemu-iotests/tests/stream-under-throttle | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/qemu-iotests/tests/stream-under-throttle
b/tests/qemu-iotests/tests/stream-under-throttle
index 8d2d9e16840d..c24dfbcaa2f2 100755
--- a/tests/qemu-iotests/tests/stream-under-throttle
+++ b/tests/qemu-iotests/tests/stream-under-throttle
@@ -88,6 +88,8 @@ class TestStreamWithThrottle(iotests.QMPTestCase):
'x-iops-total=1,x-bps-total=104857600')
self.vm.add_blockdev(self.vm.qmp_to_opts(blockdev))
self.vm.add_device('virtio-blk,iothread=iothr0,drive=throttled-node')
+if iotests.qemu_default_machine == 's390-ccw-virtio':
+self.vm.add_args('-no-shutdown')
self.vm.launch()
def tearDown(self) -> None:
--
2.38.1
Re: [PATCH v2] tests/stream-under-throttle: New test
Am 07.12.22 um 13:56 schrieb Christian Borntraeger:
Am 07.12.22 um 11:31 schrieb Christian Borntraeger:
Am 14.11.22 um 10:52 schrieb Hanna Reitz:
Test streaming a base image into the top image underneath two throttle
nodes. This was reported to make qemu 7.1 hang
(https://gitlab.com/qemu-project/qemu/-/issues/1215), so this serves as
a regression test.
Signed-off-by: Hanna Reitz
---
Based-on: <20221107151321.211175-1-hre...@redhat.com>
v1: https://lists.nongnu.org/archive/html/qemu-block/2022-11/msg00368.html
v2:
- Replace `asyncio.exceptions.TimeoutError` by `asyncio.TimeoutError`:
Stefan reported that the CI does not recognize the former:
https://lists.nongnu.org/archive/html/qemu-block/2022-11/msg00424.html
As far as I understand, the latter was basically moved to become the
former in Python 3.11, and an alias remains, so both are basically
equivalent. I only have 3.10, though, where the documentation says
that both are different, even though using either seems to work fine
(i.e. both catch the timeout there). Not sure about previous
versions, but the CI seems pretty certain about not knowing
`asyncio.exceptions.TimeoutError`, so use `asyncio.TimeoutError`
instead. (Even though that is deprecated in 3.11, but this is not the
first place in the tree to use it, so it should not be too bad.)
---
.../qemu-iotests/tests/stream-under-throttle | 121 ++
.../tests/stream-under-throttle.out | 5 +
2 files changed, 126 insertions(+)
create mode 100755 tests/qemu-iotests/tests/stream-under-throttle
create mode 100644 tests/qemu-iotests/tests/stream-under-throttle.out
As a heads up, I do get the following on s390. I have not yet looked into that:
+EE
+==
+ERROR: test_stream (__main__.TestStreamWithThrottle)
+Do a simple stream beneath the two throttle nodes. Should complete
+--
+Traceback (most recent call last):
+ File "qemu/tests/qemu-iotests/tests/stream-under-throttle", line 110, in
test_stream
+ self.vm.run_job('stream')
+ File "qemu/tests/qemu-iotests/iotests.py", line 986, in run_job
+ result = self.qmp('query-jobs')
+ File "qemu/python/qemu/machine/machine.py", line 646, in qmp
+ ret = self._qmp.cmd(cmd, args=qmp_args)
+ File "qemu/python/qemu/qmp/legacy.py", line 204, in cmd
+ return self.cmd_obj(qmp_cmd)
+ File "qemu/python/qemu/qmp/legacy.py", line 184, in cmd_obj
+ self._qmp._raw(qmp_cmd, assign_id=False),
+ File "qemu/python/qemu/qmp/protocol.py", line 154, in _wrapper
+ raise StateError(emsg, proto.runstate, required_state)
+qemu.qmp.protocol.StateError: QMPClient is disconnecting. Call disconnect() to
return to IDLE state.
+
+==
+ERROR: test_stream (__main__.TestStreamWithThrottle)
+Do a simple stream beneath the two throttle nodes. Should complete
+--
+Traceback (most recent call last):
+ File "qemu/python/qemu/machine/machine.py", line 533, in _soft_shutdown
+ self.qmp('quit')
+ File "qemu/python/qemu/machine/machine.py", line 646, in qmp
+ ret = self._qmp.cmd(cmd, args=qmp_args)
+ File "qemu/python/qemu/qmp/legacy.py", line 204, in cmd
+ return self.cmd_obj(qmp_cmd)
+ File "qemu/python/qemu/qmp/legacy.py", line 184, in cmd_obj
+ self._qmp._raw(qmp_cmd, assign_id=False),
+ File "qemu/python/qemu/qmp/protocol.py", line 154, in _wrapper
+ raise StateError(emsg, proto.runstate, required_state)
+qemu.qmp.protocol.StateError: QMPClient is disconnecting. Call disconnect() to
return to IDLE state.
+
+During handling of the above exception, another exception occurred:
+
+Traceback (most recent call last):
+ File "qemu/python/qemu/machine/machine.py", line 554, in _do_shutdown
+ self._soft_shutdown(timeout)
+ File "qemu/python/qemu/machine/machine.py", line 536, in _soft_shutdown
+ self._close_qmp_connection()
+ File "qemu/python/qemu/machine/machine.py", line 476, in
_close_qmp_connection
+ self._qmp.close()
+ File "qemu/python/qemu/qmp/legacy.py", line 277, in close
+ self._sync(
+ File "qemu/python/qemu/qmp/legacy.py", line 94, in _sync
+ return self._aloop.run_until_complete(
+ File "/usr/lib64/python3.10/asyncio/base_events.py", line 649, in
run_until_complete
+ return future.result()
+ File "/usr/lib64/python3.10/asyncio/tasks.py", line 408, in wait_for
+ return await fut
+ File "qemu/python/qemu/qmp/protocol.py", line 398, in disconnect
+ await self._wait_disconnect()
+ File "qemu/python/qemu/qmp/protocol.py", line 710, in _wait_disconnect
+ await al
Re: [PATCH v2] tests/stream-under-throttle: New test
Am 07.12.22 um 11:31 schrieb Christian Borntraeger:
Am 14.11.22 um 10:52 schrieb Hanna Reitz:
Test streaming a base image into the top image underneath two throttle
nodes. This was reported to make qemu 7.1 hang
(https://gitlab.com/qemu-project/qemu/-/issues/1215), so this serves as
a regression test.
Signed-off-by: Hanna Reitz
---
Based-on: <20221107151321.211175-1-hre...@redhat.com>
v1: https://lists.nongnu.org/archive/html/qemu-block/2022-11/msg00368.html
v2:
- Replace `asyncio.exceptions.TimeoutError` by `asyncio.TimeoutError`:
Stefan reported that the CI does not recognize the former:
https://lists.nongnu.org/archive/html/qemu-block/2022-11/msg00424.html
As far as I understand, the latter was basically moved to become the
former in Python 3.11, and an alias remains, so both are basically
equivalent. I only have 3.10, though, where the documentation says
that both are different, even though using either seems to work fine
(i.e. both catch the timeout there). Not sure about previous
versions, but the CI seems pretty certain about not knowing
`asyncio.exceptions.TimeoutError`, so use `asyncio.TimeoutError`
instead. (Even though that is deprecated in 3.11, but this is not the
first place in the tree to use it, so it should not be too bad.)
---
.../qemu-iotests/tests/stream-under-throttle | 121 ++
.../tests/stream-under-throttle.out | 5 +
2 files changed, 126 insertions(+)
create mode 100755 tests/qemu-iotests/tests/stream-under-throttle
create mode 100644 tests/qemu-iotests/tests/stream-under-throttle.out
As a heads up, I do get the following on s390. I have not yet looked into that:
+EE
+==
+ERROR: test_stream (__main__.TestStreamWithThrottle)
+Do a simple stream beneath the two throttle nodes. Should complete
+--
+Traceback (most recent call last):
+ File "qemu/tests/qemu-iotests/tests/stream-under-throttle", line 110, in
test_stream
+ self.vm.run_job('stream')
+ File "qemu/tests/qemu-iotests/iotests.py", line 986, in run_job
+ result = self.qmp('query-jobs')
+ File "qemu/python/qemu/machine/machine.py", line 646, in qmp
+ ret = self._qmp.cmd(cmd, args=qmp_args)
+ File "qemu/python/qemu/qmp/legacy.py", line 204, in cmd
+ return self.cmd_obj(qmp_cmd)
+ File "qemu/python/qemu/qmp/legacy.py", line 184, in cmd_obj
+ self._qmp._raw(qmp_cmd, assign_id=False),
+ File "qemu/python/qemu/qmp/protocol.py", line 154, in _wrapper
+ raise StateError(emsg, proto.runstate, required_state)
+qemu.qmp.protocol.StateError: QMPClient is disconnecting. Call disconnect() to
return to IDLE state.
+
+==
+ERROR: test_stream (__main__.TestStreamWithThrottle)
+Do a simple stream beneath the two throttle nodes. Should complete
+--
+Traceback (most recent call last):
+ File "qemu/python/qemu/machine/machine.py", line 533, in _soft_shutdown
+ self.qmp('quit')
+ File "qemu/python/qemu/machine/machine.py", line 646, in qmp
+ ret = self._qmp.cmd(cmd, args=qmp_args)
+ File "qemu/python/qemu/qmp/legacy.py", line 204, in cmd
+ return self.cmd_obj(qmp_cmd)
+ File "qemu/python/qemu/qmp/legacy.py", line 184, in cmd_obj
+ self._qmp._raw(qmp_cmd, assign_id=False),
+ File "qemu/python/qemu/qmp/protocol.py", line 154, in _wrapper
+ raise StateError(emsg, proto.runstate, required_state)
+qemu.qmp.protocol.StateError: QMPClient is disconnecting. Call disconnect() to
return to IDLE state.
+
+During handling of the above exception, another exception occurred:
+
+Traceback (most recent call last):
+ File "qemu/python/qemu/machine/machine.py", line 554, in _do_shutdown
+ self._soft_shutdown(timeout)
+ File "qemu/python/qemu/machine/machine.py", line 536, in _soft_shutdown
+ self._close_qmp_connection()
+ File "qemu/python/qemu/machine/machine.py", line 476, in
_close_qmp_connection
+ self._qmp.close()
+ File "qemu/python/qemu/qmp/legacy.py", line 277, in close
+ self._sync(
+ File "qemu/python/qemu/qmp/legacy.py", line 94, in _sync
+ return self._aloop.run_until_complete(
+ File "/usr/lib64/python3.10/asyncio/base_events.py", line 649, in
run_until_complete
+ return future.result()
+ File "/usr/lib64/python3.10/asyncio/tasks.py", line 408, in wait_for
+ return await fut
+ File "qemu/python/qemu/qmp/protocol.py", line 398, in disconnect
+ await self._wait_disconnect()
+ File "qemu/python/qemu/qmp/protocol.py", line 710, in _wait_disconnect
+ await all_defined_tasks # Raise Exceptions from the bottom half.
+
Re: [PATCH v2] tests/stream-under-throttle: New test
Am 14.11.22 um 10:52 schrieb Hanna Reitz:
Test streaming a base image into the top image underneath two throttle
nodes. This was reported to make qemu 7.1 hang
(https://gitlab.com/qemu-project/qemu/-/issues/1215), so this serves as
a regression test.
Signed-off-by: Hanna Reitz
---
Based-on: <20221107151321.211175-1-hre...@redhat.com>
v1: https://lists.nongnu.org/archive/html/qemu-block/2022-11/msg00368.html
v2:
- Replace `asyncio.exceptions.TimeoutError` by `asyncio.TimeoutError`:
Stefan reported that the CI does not recognize the former:
https://lists.nongnu.org/archive/html/qemu-block/2022-11/msg00424.html
As far as I understand, the latter was basically moved to become the
former in Python 3.11, and an alias remains, so both are basically
equivalent. I only have 3.10, though, where the documentation says
that both are different, even though using either seems to work fine
(i.e. both catch the timeout there). Not sure about previous
versions, but the CI seems pretty certain about not knowing
`asyncio.exceptions.TimeoutError`, so use `asyncio.TimeoutError`
instead. (Even though that is deprecated in 3.11, but this is not the
first place in the tree to use it, so it should not be too bad.)
---
.../qemu-iotests/tests/stream-under-throttle | 121 ++
.../tests/stream-under-throttle.out | 5 +
2 files changed, 126 insertions(+)
create mode 100755 tests/qemu-iotests/tests/stream-under-throttle
create mode 100644 tests/qemu-iotests/tests/stream-under-throttle.out
As a heads up, I do get the following on s390. I have not yet looked into that:
+EE
+==
+ERROR: test_stream (__main__.TestStreamWithThrottle)
+Do a simple stream beneath the two throttle nodes. Should complete
+--
+Traceback (most recent call last):
+ File "qemu/tests/qemu-iotests/tests/stream-under-throttle", line 110, in
test_stream
+self.vm.run_job('stream')
+ File "qemu/tests/qemu-iotests/iotests.py", line 986, in run_job
+result = self.qmp('query-jobs')
+ File "qemu/python/qemu/machine/machine.py", line 646, in qmp
+ret = self._qmp.cmd(cmd, args=qmp_args)
+ File "qemu/python/qemu/qmp/legacy.py", line 204, in cmd
+return self.cmd_obj(qmp_cmd)
+ File "qemu/python/qemu/qmp/legacy.py", line 184, in cmd_obj
+self._qmp._raw(qmp_cmd, assign_id=False),
+ File "qemu/python/qemu/qmp/protocol.py", line 154, in _wrapper
+raise StateError(emsg, proto.runstate, required_state)
+qemu.qmp.protocol.StateError: QMPClient is disconnecting. Call disconnect() to
return to IDLE state.
+
+==
+ERROR: test_stream (__main__.TestStreamWithThrottle)
+Do a simple stream beneath the two throttle nodes. Should complete
+--
+Traceback (most recent call last):
+ File "qemu/python/qemu/machine/machine.py", line 533, in _soft_shutdown
+self.qmp('quit')
+ File "qemu/python/qemu/machine/machine.py", line 646, in qmp
+ret = self._qmp.cmd(cmd, args=qmp_args)
+ File "qemu/python/qemu/qmp/legacy.py", line 204, in cmd
+return self.cmd_obj(qmp_cmd)
+ File "qemu/python/qemu/qmp/legacy.py", line 184, in cmd_obj
+self._qmp._raw(qmp_cmd, assign_id=False),
+ File "qemu/python/qemu/qmp/protocol.py", line 154, in _wrapper
+raise StateError(emsg, proto.runstate, required_state)
+qemu.qmp.protocol.StateError: QMPClient is disconnecting. Call disconnect() to
return to IDLE state.
+
+During handling of the above exception, another exception occurred:
+
+Traceback (most recent call last):
+ File "qemu/python/qemu/machine/machine.py", line 554, in _do_shutdown
+self._soft_shutdown(timeout)
+ File "qemu/python/qemu/machine/machine.py", line 536, in _soft_shutdown
+self._close_qmp_connection()
+ File "qemu/python/qemu/machine/machine.py", line 476, in
_close_qmp_connection
+self._qmp.close()
+ File "qemu/python/qemu/qmp/legacy.py", line 277, in close
+self._sync(
+ File "qemu/python/qemu/qmp/legacy.py", line 94, in _sync
+return self._aloop.run_until_complete(
+ File "/usr/lib64/python3.10/asyncio/base_events.py", line 649, in
run_until_complete
+return future.result()
+ File "/usr/lib64/python3.10/asyncio/tasks.py", line 408, in wait_for
+return await fut
+ File "qemu/python/qemu/qmp/protocol.py", line 398, in disconnect
+await self._wait_disconnect()
+ File "qemu/python/qemu/qmp/protocol.py", line 710, in _wait_disconnect
+await all_defined_tasks # Raise Exceptions from the bottom half.
+ File "qemu/python/qemu/qmp/protocol.py", line 861, in _bh_loop_forever
+await async_fn()
+ File "qemu/python/qemu/qmp/protocol.py", line 899, in _bh_recv_message
+msg = await self._recv()
+ File "qemu/python/qemu/qmp/protocol.py", line 1000, in _recv
+
Re: qemu iotest 161 and make check
Am 27.10.22 um 07:54 schrieb Christian Borntraeger:
[...]
diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index 0f1fecc68e..01bdb05575 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -388,7 +388,7 @@ _cleanup_qemu()
kill -KILL ${QEMU_PID} 2>/dev/null
fi
if [ -n "${QEMU_PID}" ]; then
- wait ${QEMU_PID} 2>/dev/null # silent kill
+ wait 2>/dev/null # silent kill
fi
fi
And this also helps. Still trying to find out what clone/fork happens here.
As a new information, the problem only exists on Ubuntu,
I cannot reproduce it with Fedora or RHEL. I also changed
the kernel, its not the reason. As soon as I add tracing
the different timing also makes the problem go away.
Re: [PATCH] virtiofsd: Add `sigreturn` to the seccomp whitelist
Am 29.11.22 um 10:42 schrieb Dr. David Alan Gilbert:
* Marc Hartmayer (mhart...@linux.ibm.com) wrote:
"Dr. David Alan Gilbert" writes:
* Marc Hartmayer (mhart...@linux.ibm.com) wrote:
The virtiofsd currently crashes on s390x. This is because of a
`sigreturn` system call. See audit log below:
type=SECCOMP msg=audit(1669382477.611:459): auid=4294967295 uid=0 gid=0 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 pid=6649 comm="virtiofsd" exe="/usr/libexec/virtiofsd" sig=31
arch=8016 syscall=119 compat=0 ip=0x3fff15f748a code=0x8000AUID="unset" UID="root"
GID="root" ARCH=s390x SYSCALL=sigreturn
I'm curious; doesn't that mean that some signal is being delivered and
you're returning? Which one?
code=0x8000 means that the seccomp action SECCOMP_RET_KILL_PROCESS
is taken => process is killed by a SIGSYS signal (31) [1].
At least, that’s my understanding of this log message.
[1] https://man7.org/linux/man-pages/man2/seccomp.2.html
But isn't that the fallout rather than the cause ? i.e. seccomp
is sending a SIGSYS because the process used sigreturn, my question
is why did the process call sigreturn in the first place - it must
have received a signal to return from?
Good question. virtiofsd seems to prepare itself for
int fuse_set_signal_handlers(struct fuse_session *se)
{
/*
* If we used SIG_IGN instead of the do_nothing function,
* then we would be unable to tell if we set SIG_IGN (and
* thus should reset to SIG_DFL in fuse_remove_signal_handlers)
* or if it was already set to SIG_IGN (and should be left
* untouched.
*/
if (set_one_signal_handler(SIGHUP, exit_handler, 0) == -1 ||
set_one_signal_handler(SIGINT, exit_handler, 0) == -1 ||
set_one_signal_handler(SIGTERM, exit_handler, 0) == -1 ||
set_one_signal_handler(SIGPIPE, do_nothing, 0) == -1) {
return -1;
}
Given that rt_sigreturn was already on the seccomp list it seems
to be expected that those handlers are called.
Re: [PATCH] virtiofsd: Add `sigreturn` to the seccomp whitelist
Am 29.11.22 um 10:52 schrieb Christian Borntraeger:
Am 29.11.22 um 10:42 schrieb Dr. David Alan Gilbert:
* Marc Hartmayer (mhart...@linux.ibm.com) wrote:
"Dr. David Alan Gilbert" writes:
* Marc Hartmayer (mhart...@linux.ibm.com) wrote:
The virtiofsd currently crashes on s390x. This is because of a
`sigreturn` system call. See audit log below:
type=SECCOMP msg=audit(1669382477.611:459): auid=4294967295 uid=0 gid=0 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 pid=6649 comm="virtiofsd" exe="/usr/libexec/virtiofsd" sig=31
arch=8016 syscall=119 compat=0 ip=0x3fff15f748a code=0x8000AUID="unset" UID="root"
GID="root" ARCH=s390x SYSCALL=sigreturn
I'm curious; doesn't that mean that some signal is being delivered and
you're returning? Which one?
code=0x8000 means that the seccomp action SECCOMP_RET_KILL_PROCESS
is taken => process is killed by a SIGSYS signal (31) [1].
At least, that’s my understanding of this log message.
[1] https://man7.org/linux/man-pages/man2/seccomp.2.html
But isn't that the fallout rather than the cause ? i.e. seccomp
is sending a SIGSYS because the process used sigreturn, my question
is why did the process call sigreturn in the first place - it must
have received a signal to return from?
Good question. virtiofsd seems to prepare itself for
int fuse_set_signal_handlers(struct fuse_session *se)
{
/*
* If we used SIG_IGN instead of the do_nothing function,
* then we would be unable to tell if we set SIG_IGN (and
* thus should reset to SIG_DFL in fuse_remove_signal_handlers)
* or if it was already set to SIG_IGN (and should be left
* untouched.
*/
if (set_one_signal_handler(SIGHUP, exit_handler, 0) == -1 ||
set_one_signal_handler(SIGINT, exit_handler, 0) == -1 ||
set_one_signal_handler(SIGTERM, exit_handler, 0) == -1 ||
set_one_signal_handler(SIGPIPE, do_nothing, 0) == -1) {
return -1;
}
Given that rt_sigreturn was already on the seccomp list it seems
to be expected that those handlers are called.
For me, it seems to happen on shutdown:
Stack trace of thread 1:
#0 0x03ffc06f348a __kernel_sigreturn (linux-vdso64.so.1 +
0x48a)
#1 0x03ffc06f3488 __kernel_sigreturn (linux-vdso64.so.1 +
0x488)
#2 0x03ff9af1be96 __GI___futex_abstimed_wait_cancelable64
(libc.so.6 + 0x9be96)
#3 0x03ff9af211b4 __pthread_clockjoin_ex (libc.so.6 +
0xa11b4)
#4 0x03ff9af2106e pthread_join@GLIBC_2.2 (libc.so.6 +
0xa106e)
#5 0x02aa35d2fe36 fv_queue_cleanup_thread (virtiofsd +
0x2fe36)
#6 0x02aa35d3152c stop_all_queues (virtiofsd + 0x3152c)
#7 0x02aa35d2869c main (virtiofsd + 0x2869c)
#8 0x03ff9aeb4872 __libc_start_call_main (libc.so.6 +
0x34872)
#9 0x03ff9aeb4950 __libc_start_main@@GLIBC_2.34 (libc.so.6
+ 0x34950)
#10 0x02aa35d290a0 .annobin_libvhost_user.c_end.startup
(virtiofsd + 0x290a0)
Re: [Virtio-fs] [PATCH] virtiofsd: Add `sigreturn` to the seccomp whitelist
Am 25.11.22 um 17:32 schrieb German Maglione:
On Fri, Nov 25, 2022 at 3:40 PM Marc Hartmayer wrote:
The virtiofsd currently crashes on s390x. This is because of a
`sigreturn` system call. See audit log below:
type=SECCOMP msg=audit(1669382477.611:459): auid=4294967295 uid=0 gid=0 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 pid=6649 comm="virtiofsd" exe="/usr/libexec/virtiofsd" sig=31
arch=8016 syscall=119 compat=0 ip=0x3fff15f748a code=0x8000AUID="unset" UID="root"
GID="root" ARCH=s390x SYSCALL=sigreturn
Signed-off-by: Marc Hartmayer
---
tools/virtiofsd/passthrough_seccomp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virtiofsd/passthrough_seccomp.c
b/tools/virtiofsd/passthrough_seccomp.c
index 888295c073de..0033dab4939e 100644
--- a/tools/virtiofsd/passthrough_seccomp.c
+++ b/tools/virtiofsd/passthrough_seccomp.c
@@ -110,6 +110,7 @@ static const int syscall_allowlist[] = {
#endif
SCMP_SYS(set_robust_list),
SCMP_SYS(setxattr),
+SCMP_SYS(sigreturn),
SCMP_SYS(symlinkat),
SCMP_SYS(syncfs),
SCMP_SYS(time), /* Rarely needed, except on static builds */
--
2.34.1
___
Virtio-fs mailing list
virtio...@redhat.com
https://listman.redhat.com/mailman/listinfo/virtio-fs
Reviewed-by: German Maglione
Should we add this also in the rust version?, I see we don't have it
enabled either.
this is probably a good idea.
Re: [PATCH v1 5/9] hw/virtio: introduce virtio_device_should_start
Am 21.11.22 um 23:37 schrieb Michael S. Tsirkin: [...] qemu-system-x86_64: ../hw/virtio/vhost-vsock-common.c:203: vhost_vsock_common_pre_save: Assertion `!vhost_dev_is_started(>vhost_dev)' failed. 2022-11-15 16:38:46.096+: shutting down, reason=crashed Alex were you able to replicate? Just curious. Ping?
Re: [PATCH v9 00/10] s390x: CPU Topology
Am 02.09.22 um 09:55 schrieb Pierre Morel: Hi, The implementation of the CPU Topology in QEMU has been drastically modified since the last patch series and the number of LOCs has been greatly reduced. Unnecessary objects have been removed, only a single S390Topology object is created to support migration and reset. Also a documentation has been added to the series. To use these patches, you will need Linux V6-rc1 or newer. Mainline patches needed are: f5ecfee94493 2022-07-20 KVM: s390: resetting the Topology-Change-Report 24fe0195bc19 2022-07-20 KVM: s390: guest support for topology function 0130337ec45b 2022-07-20 KVM: s390: Cleanup ipte lock access and SIIF fac.. Currently this code is for KVM only, I have no idea if it is interesting to provide a TCG patch. If ever it will be done in another series. To have a better understanding of the S390x CPU Topology and its implementation in QEMU you can have a look at the documentation in the last patch. New in this series == s390x/cpus: Make absence of multithreading clear This patch makes clear that CPU-multithreading is not supported in the guest. s390x/cpu topology: core_id sets s390x CPU topology This patch uses the core_id to build the container topology and the placement of the CPU inside the container. s390x/cpu topology: reporting the CPU topology to the guest This patch is based on the fact that the CPU type for guests is always IFL, CPUs are always dedicated and the polarity is always horizontal. This may change in the future. hw/core: introducing drawer and books for s390x s390x/cpu: reporting drawers and books topology to the guest These two patches extend the topology handling to add two new containers levels above sockets: books and drawers. The subject of the last patches is clear enough (I hope). Regards, Pierre Pierre Morel (10): s390x/cpus: Make absence of multithreading clear s390x/cpu topology: core_id sets s390x CPU topology s390x/cpu topology: reporting the CPU topology to the guest hw/core: introducing drawer and books for s390x s390x/cpu: reporting drawers and books topology to the guest s390x/cpu_topology: resetting the Topology-Change-Report s390x/cpu_topology: CPU topology migration target/s390x: interception of PTF instruction s390x/cpu_topology: activating CPU topology Do we really need a machine property? As far as I can see, old QEMU cannot activate the ctop facility with old and new kernel unless it enables CAP_S390_CPU_TOPOLOGY. I do get oldqemu -cpu z14,ctop=on qemu-system-s390x: Some features requested in the CPU model are not available in the configuration: ctop With the newer QEMU we can. So maybe we can simply have a topology (and then a cpu model feature) in new QEMUs and non in old. the cpu model would then also fence migration from enabled to disabled.
Re: [PATCH v1 5/9] hw/virtio: introduce virtio_device_should_start
Am 15.11.22 um 17:40 schrieb Christian Borntraeger:
Am 15.11.22 um 17:05 schrieb Alex Bennée:
Christian Borntraeger writes:
Am 15.11.22 um 15:31 schrieb Alex Bennée:
"Michael S. Tsirkin" writes:
On Mon, Nov 14, 2022 at 06:15:30PM +0100, Christian Borntraeger wrote:
Am 14.11.22 um 18:10 schrieb Michael S. Tsirkin:
On Mon, Nov 14, 2022 at 05:55:09PM +0100, Christian Borntraeger wrote:
Am 14.11.22 um 17:37 schrieb Michael S. Tsirkin:
On Mon, Nov 14, 2022 at 05:18:53PM +0100, Christian Borntraeger wrote:
Am 08.11.22 um 10:23 schrieb Alex Bennée:
The previous fix to virtio_device_started revealed a problem in its
use by both the core and the device code. The core code should be able
to handle the device "starting" while the VM isn't running to handle
the restoration of migration state. To solve this dual use introduce a
new helper for use by the vhost-user backends who all use it to feed a
should_start variable.
We can also pick up a change vhost_user_blk_set_status while we are at
it which follows the same pattern.
Fixes: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started)
Fixes: 27ba7b027f (hw/virtio: add boilerplate for vhost-user-gpio device)
Signed-off-by: Alex Bennée
Cc: "Michael S. Tsirkin"
Hmmm, is this
commit 259d69c00b67c02a67f3bdbeeea71c2c0af76c35
Author: Alex Bennée
AuthorDate: Mon Nov 7 12:14:07 2022 +
Commit: Michael S. Tsirkin
CommitDate: Mon Nov 7 14:08:18 2022 -0500
hw/virtio: introduce virtio_device_should_start
and older version?
This is what got merged:
https://lore.kernel.org/r/20221107121407.1010913-1-alex.bennee%40linaro.org
This patch was sent after I merged the RFC.
I think the only difference is the commit log but I might be missing
something.
This does not seem to fix the regression that I have reported.
This was applied on top of 9f6bcfd99f which IIUC does, right?
QEMU master still fails for me for suspend/resume to disk:
#0 0x03ff8e3980a6 in __pthread_kill_implementation () at /lib64/libc.so.6
#1 0x03ff8e348580 in raise () at /lib64/libc.so.6
#2 0x03ff8e32b5c0 in abort () at /lib64/libc.so.6
#3 0x03ff8e3409da in __assert_fail_base () at /lib64/libc.so.6
#4 0x03ff8e340a4e in () at /lib64/libc.so.6
#5 0x02aa1ffa8966 in vhost_vsock_common_pre_save
(opaque=) at
../hw/virtio/vhost-vsock-common.c:203
#6 0x02aa1fe5e0ee in vmstate_save_state_v
(f=f@entry=0x2aa21bdc170, vmsd=0x2aa204ac5f0
, opaque=0x2aa21bac9f8,
vmdesc=vmdesc@entry=0x3fddc08eb30,
version_id=version_id@entry=0) at ../migration/vmstate.c:329
#7 0x02aa1fe5ebf8 in vmstate_save_state
(f=f@entry=0x2aa21bdc170, vmsd=,
opaque=, vmdesc_id=vmdesc_id@entry=0x3fddc08eb30)
at ../migration/vmstate.c:317
#8 0x02aa1fe75bd0 in vmstate_save (f=f@entry=0x2aa21bdc170,
se=se@entry=0x2aa21bdbe90, vmdesc=vmdesc@entry=0x3fddc08eb30) at
../migration/savevm.c:908
#9 0x02aa1fe79584 in
qemu_savevm_state_complete_precopy_non_iterable
(f=f@entry=0x2aa21bdc170, in_postcopy=in_postcopy@entry=false,
inactivate_disks=inactivate_disks@entry=true)
at ../migration/savevm.c:1393
#10 0x02aa1fe79a96 in qemu_savevm_state_complete_precopy
(f=0x2aa21bdc170, iterable_only=iterable_only@entry=false,
inactivate_disks=inactivate_disks@entry=true) at
../migration/savevm.c:1459
#11 0x02aa1fe6d6ee in migration_completion (s=0x2aa218ef600) at
../migration/migration.c:3314
#12 migration_iteration_run (s=0x2aa218ef600) at ../migration/migration.c:3761
#13 migration_thread (opaque=opaque@entry=0x2aa218ef600) at
../migration/migration.c:3989
#14 0x02aa201f0b8c in qemu_thread_start (args=) at
../util/qemu-thread-posix.c:505
#15 0x03ff8e396248 in start_thread () at /lib64/libc.so.6
#16 0x03ff8e41183e in thread_start () at /lib64/libc.so.6
Michael, your previous branch did work if I recall correctly.
That one was failing under github CI though (for reasons we didn't
really address, such as disconnect during stop causing a recursive
call to stop, but there you are).
Even the double revert of everything?
I don't remember at this point.
So how do we proceed now?
I'm hopeful Alex will come up with a fix.
I need to replicate the failing test for that. Which test is
failing?
Pretty much the same as before. guest with vsock, managedsave and
restore.
If this isn't in our test suite I'm going to need exact steps.
Just get any libvirt guest, add
to your libvirt xml. Start the guest (with the new xml).
Run virsh managedsave - qemu crashes. On x86 and s390.
the libvirt log:
/home/cborntra/REPOS/qemu/build/x86_64-softmmu/qemu-system-x86_64 \
-name guest=f36,debug-threads=on \
-S \
-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain-1-f36/master-key.aes"}'
\
-machine pc-i440fx-7.2,usb=off,dump-guest-core=off,memory-backend=pc.ra
Re: [PATCH v1 5/9] hw/virtio: introduce virtio_device_should_start
Am 15.11.22 um 17:05 schrieb Alex Bennée: Christian Borntraeger writes: Am 15.11.22 um 15:31 schrieb Alex Bennée: "Michael S. Tsirkin" writes: On Mon, Nov 14, 2022 at 06:15:30PM +0100, Christian Borntraeger wrote: Am 14.11.22 um 18:10 schrieb Michael S. Tsirkin: On Mon, Nov 14, 2022 at 05:55:09PM +0100, Christian Borntraeger wrote: Am 14.11.22 um 17:37 schrieb Michael S. Tsirkin: On Mon, Nov 14, 2022 at 05:18:53PM +0100, Christian Borntraeger wrote: Am 08.11.22 um 10:23 schrieb Alex Bennée: The previous fix to virtio_device_started revealed a problem in its use by both the core and the device code. The core code should be able to handle the device "starting" while the VM isn't running to handle the restoration of migration state. To solve this dual use introduce a new helper for use by the vhost-user backends who all use it to feed a should_start variable. We can also pick up a change vhost_user_blk_set_status while we are at it which follows the same pattern. Fixes: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started) Fixes: 27ba7b027f (hw/virtio: add boilerplate for vhost-user-gpio device) Signed-off-by: Alex Bennée Cc: "Michael S. Tsirkin" Hmmm, is this commit 259d69c00b67c02a67f3bdbeeea71c2c0af76c35 Author: Alex Bennée AuthorDate: Mon Nov 7 12:14:07 2022 + Commit: Michael S. Tsirkin CommitDate: Mon Nov 7 14:08:18 2022 -0500 hw/virtio: introduce virtio_device_should_start and older version? This is what got merged: https://lore.kernel.org/r/20221107121407.1010913-1-alex.bennee%40linaro.org This patch was sent after I merged the RFC. I think the only difference is the commit log but I might be missing something. This does not seem to fix the regression that I have reported. This was applied on top of 9f6bcfd99f which IIUC does, right? QEMU master still fails for me for suspend/resume to disk: #0 0x03ff8e3980a6 in __pthread_kill_implementation () at /lib64/libc.so.6 #1 0x03ff8e348580 in raise () at /lib64/libc.so.6 #2 0x03ff8e32b5c0 in abort () at /lib64/libc.so.6 #3 0x03ff8e3409da in __assert_fail_base () at /lib64/libc.so.6 #4 0x03ff8e340a4e in () at /lib64/libc.so.6 #5 0x02aa1ffa8966 in vhost_vsock_common_pre_save (opaque=) at ../hw/virtio/vhost-vsock-common.c:203 #6 0x02aa1fe5e0ee in vmstate_save_state_v (f=f@entry=0x2aa21bdc170, vmsd=0x2aa204ac5f0 , opaque=0x2aa21bac9f8, vmdesc=vmdesc@entry=0x3fddc08eb30, version_id=version_id@entry=0) at ../migration/vmstate.c:329 #7 0x02aa1fe5ebf8 in vmstate_save_state (f=f@entry=0x2aa21bdc170, vmsd=, opaque=, vmdesc_id=vmdesc_id@entry=0x3fddc08eb30) at ../migration/vmstate.c:317 #8 0x02aa1fe75bd0 in vmstate_save (f=f@entry=0x2aa21bdc170, se=se@entry=0x2aa21bdbe90, vmdesc=vmdesc@entry=0x3fddc08eb30) at ../migration/savevm.c:908 #9 0x02aa1fe79584 in qemu_savevm_state_complete_precopy_non_iterable (f=f@entry=0x2aa21bdc170, in_postcopy=in_postcopy@entry=false, inactivate_disks=inactivate_disks@entry=true) at ../migration/savevm.c:1393 #10 0x02aa1fe79a96 in qemu_savevm_state_complete_precopy (f=0x2aa21bdc170, iterable_only=iterable_only@entry=false, inactivate_disks=inactivate_disks@entry=true) at ../migration/savevm.c:1459 #11 0x02aa1fe6d6ee in migration_completion (s=0x2aa218ef600) at ../migration/migration.c:3314 #12 migration_iteration_run (s=0x2aa218ef600) at ../migration/migration.c:3761 #13 migration_thread (opaque=opaque@entry=0x2aa218ef600) at ../migration/migration.c:3989 #14 0x02aa201f0b8c in qemu_thread_start (args=) at ../util/qemu-thread-posix.c:505 #15 0x03ff8e396248 in start_thread () at /lib64/libc.so.6 #16 0x03ff8e41183e in thread_start () at /lib64/libc.so.6 Michael, your previous branch did work if I recall correctly. That one was failing under github CI though (for reasons we didn't really address, such as disconnect during stop causing a recursive call to stop, but there you are). Even the double revert of everything? I don't remember at this point. So how do we proceed now? I'm hopeful Alex will come up with a fix. I need to replicate the failing test for that. Which test is failing? Pretty much the same as before. guest with vsock, managedsave and restore. If this isn't in our test suite I'm going to need exact steps. Just get any libvirt guest, add to your libvirt xml. Start the guest (with the new xml). Run virsh managedsave - qemu crashes. On x86 and s390.
Re: [PATCH v1 5/9] hw/virtio: introduce virtio_device_should_start
Am 15.11.22 um 15:31 schrieb Alex Bennée: "Michael S. Tsirkin" writes: On Mon, Nov 14, 2022 at 06:15:30PM +0100, Christian Borntraeger wrote: Am 14.11.22 um 18:10 schrieb Michael S. Tsirkin: On Mon, Nov 14, 2022 at 05:55:09PM +0100, Christian Borntraeger wrote: Am 14.11.22 um 17:37 schrieb Michael S. Tsirkin: On Mon, Nov 14, 2022 at 05:18:53PM +0100, Christian Borntraeger wrote: Am 08.11.22 um 10:23 schrieb Alex Bennée: The previous fix to virtio_device_started revealed a problem in its use by both the core and the device code. The core code should be able to handle the device "starting" while the VM isn't running to handle the restoration of migration state. To solve this dual use introduce a new helper for use by the vhost-user backends who all use it to feed a should_start variable. We can also pick up a change vhost_user_blk_set_status while we are at it which follows the same pattern. Fixes: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started) Fixes: 27ba7b027f (hw/virtio: add boilerplate for vhost-user-gpio device) Signed-off-by: Alex Bennée Cc: "Michael S. Tsirkin" Hmmm, is this commit 259d69c00b67c02a67f3bdbeeea71c2c0af76c35 Author: Alex Bennée AuthorDate: Mon Nov 7 12:14:07 2022 + Commit: Michael S. Tsirkin CommitDate: Mon Nov 7 14:08:18 2022 -0500 hw/virtio: introduce virtio_device_should_start and older version? This is what got merged: https://lore.kernel.org/r/20221107121407.1010913-1-alex.bennee%40linaro.org This patch was sent after I merged the RFC. I think the only difference is the commit log but I might be missing something. This does not seem to fix the regression that I have reported. This was applied on top of 9f6bcfd99f which IIUC does, right? QEMU master still fails for me for suspend/resume to disk: #0 0x03ff8e3980a6 in __pthread_kill_implementation () at /lib64/libc.so.6 #1 0x03ff8e348580 in raise () at /lib64/libc.so.6 #2 0x03ff8e32b5c0 in abort () at /lib64/libc.so.6 #3 0x03ff8e3409da in __assert_fail_base () at /lib64/libc.so.6 #4 0x03ff8e340a4e in () at /lib64/libc.so.6 #5 0x02aa1ffa8966 in vhost_vsock_common_pre_save (opaque=) at ../hw/virtio/vhost-vsock-common.c:203 #6 0x02aa1fe5e0ee in vmstate_save_state_v (f=f@entry=0x2aa21bdc170, vmsd=0x2aa204ac5f0 , opaque=0x2aa21bac9f8, vmdesc=vmdesc@entry=0x3fddc08eb30, version_id=version_id@entry=0) at ../migration/vmstate.c:329 #7 0x02aa1fe5ebf8 in vmstate_save_state (f=f@entry=0x2aa21bdc170, vmsd=, opaque=, vmdesc_id=vmdesc_id@entry=0x3fddc08eb30) at ../migration/vmstate.c:317 #8 0x02aa1fe75bd0 in vmstate_save (f=f@entry=0x2aa21bdc170, se=se@entry=0x2aa21bdbe90, vmdesc=vmdesc@entry=0x3fddc08eb30) at ../migration/savevm.c:908 #9 0x02aa1fe79584 in qemu_savevm_state_complete_precopy_non_iterable (f=f@entry=0x2aa21bdc170, in_postcopy=in_postcopy@entry=false, inactivate_disks=inactivate_disks@entry=true) at ../migration/savevm.c:1393 #10 0x02aa1fe79a96 in qemu_savevm_state_complete_precopy (f=0x2aa21bdc170, iterable_only=iterable_only@entry=false, inactivate_disks=inactivate_disks@entry=true) at ../migration/savevm.c:1459 #11 0x02aa1fe6d6ee in migration_completion (s=0x2aa218ef600) at ../migration/migration.c:3314 #12 migration_iteration_run (s=0x2aa218ef600) at ../migration/migration.c:3761 #13 migration_thread (opaque=opaque@entry=0x2aa218ef600) at ../migration/migration.c:3989 #14 0x02aa201f0b8c in qemu_thread_start (args=) at ../util/qemu-thread-posix.c:505 #15 0x03ff8e396248 in start_thread () at /lib64/libc.so.6 #16 0x03ff8e41183e in thread_start () at /lib64/libc.so.6 Michael, your previous branch did work if I recall correctly. That one was failing under github CI though (for reasons we didn't really address, such as disconnect during stop causing a recursive call to stop, but there you are). Even the double revert of everything? I don't remember at this point. So how do we proceed now? I'm hopeful Alex will come up with a fix. I need to replicate the failing test for that. Which test is failing? Pretty much the same as before. guest with vsock, managedsave and restore.
Re: [PATCH v1 5/9] hw/virtio: introduce virtio_device_should_start
Am 15.11.22 um 12:25 schrieb Michael S. Tsirkin:
On Tue, Nov 15, 2022 at 09:18:27AM +0100, Christian Borntraeger wrote:
Am 14.11.22 um 18:20 schrieb Michael S. Tsirkin:
On Mon, Nov 14, 2022 at 06:15:30PM +0100, Christian Borntraeger wrote:
Am 14.11.22 um 18:10 schrieb Michael S. Tsirkin:
On Mon, Nov 14, 2022 at 05:55:09PM +0100, Christian Borntraeger wrote:
Am 14.11.22 um 17:37 schrieb Michael S. Tsirkin:
On Mon, Nov 14, 2022 at 05:18:53PM +0100, Christian Borntraeger wrote:
Am 08.11.22 um 10:23 schrieb Alex Bennée:
The previous fix to virtio_device_started revealed a problem in its
use by both the core and the device code. The core code should be able
to handle the device "starting" while the VM isn't running to handle
the restoration of migration state. To solve this dual use introduce a
new helper for use by the vhost-user backends who all use it to feed a
should_start variable.
We can also pick up a change vhost_user_blk_set_status while we are at
it which follows the same pattern.
Fixes: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started)
Fixes: 27ba7b027f (hw/virtio: add boilerplate for vhost-user-gpio device)
Signed-off-by: Alex Bennée
Cc: "Michael S. Tsirkin"
Hmmm, is this
commit 259d69c00b67c02a67f3bdbeeea71c2c0af76c35
Author: Alex Bennée
AuthorDate: Mon Nov 7 12:14:07 2022 +
Commit: Michael S. Tsirkin
CommitDate: Mon Nov 7 14:08:18 2022 -0500
hw/virtio: introduce virtio_device_should_start
and older version?
This is what got merged:
https://lore.kernel.org/r/20221107121407.1010913-1-alex.bennee%40linaro.org
This patch was sent after I merged the RFC.
I think the only difference is the commit log but I might be missing
something.
This does not seem to fix the regression that I have reported.
This was applied on top of 9f6bcfd99f which IIUC does, right?
QEMU master still fails for me for suspend/resume to disk:
#0 0x03ff8e3980a6 in __pthread_kill_implementation () at /lib64/libc.so.6
#1 0x03ff8e348580 in raise () at /lib64/libc.so.6
#2 0x03ff8e32b5c0 in abort () at /lib64/libc.so.6
#3 0x03ff8e3409da in __assert_fail_base () at /lib64/libc.so.6
#4 0x03ff8e340a4e in () at /lib64/libc.so.6
#5 0x02aa1ffa8966 in vhost_vsock_common_pre_save (opaque=)
at ../hw/virtio/vhost-vsock-common.c:203
#6 0x02aa1fe5e0ee in vmstate_save_state_v
(f=f@entry=0x2aa21bdc170, vmsd=0x2aa204ac5f0
, opaque=0x2aa21bac9f8,
vmdesc=vmdesc@entry=0x3fddc08eb30, version_id=version_id@entry=0) at
../migration/vmstate.c:329
#7 0x02aa1fe5ebf8 in vmstate_save_state (f=f@entry=0x2aa21bdc170, vmsd=, opaque=, vmdesc_id=vmdesc_id@entry=0x3fddc08eb30) at
../migration/vmstate.c:317
#8 0x02aa1fe75bd0 in vmstate_save (f=f@entry=0x2aa21bdc170,
se=se@entry=0x2aa21bdbe90, vmdesc=vmdesc@entry=0x3fddc08eb30) at
../migration/savevm.c:908
#9 0x02aa1fe79584 in qemu_savevm_state_complete_precopy_non_iterable
(f=f@entry=0x2aa21bdc170, in_postcopy=in_postcopy@entry=false,
inactivate_disks=inactivate_disks@entry=true)
at ../migration/savevm.c:1393
#10 0x02aa1fe79a96 in qemu_savevm_state_complete_precopy (f=0x2aa21bdc170,
iterable_only=iterable_only@entry=false,
inactivate_disks=inactivate_disks@entry=true) at ../migration/savevm.c:1459
#11 0x02aa1fe6d6ee in migration_completion (s=0x2aa218ef600) at
../migration/migration.c:3314
#12 migration_iteration_run (s=0x2aa218ef600) at ../migration/migration.c:3761
#13 migration_thread (opaque=opaque@entry=0x2aa218ef600) at
../migration/migration.c:3989
#14 0x02aa201f0b8c in qemu_thread_start (args=) at
../util/qemu-thread-posix.c:505
#15 0x03ff8e396248 in start_thread () at /lib64/libc.so.6
#16 0x03ff8e41183e in thread_start () at /lib64/libc.so.6
Michael, your previous branch did work if I recall correctly.
That one was failing under github CI though (for reasons we didn't
really address, such as disconnect during stop causing a recursive
call to stop, but there you are).
Even the double revert of everything?
I don't remember at this point.
So how do we proceed now?
I'm hopeful Alex will come up with a fix.
The initial fix changed to qemu/master does still work for me
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index a973811cbfc6..fb3072838119 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -411,14 +411,14 @@ static inline bool virtio_device_started(VirtIODevice
*vdev, uint8_t status)
*/
static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t
status)
{
-if (vdev->use_started) {
-return vdev->started;
-}
-
if (!vdev->vm_running) {
return false;
}
+if (vdev->use_started) {
+return vdev->started;
+}
+
return status & VIRTIO_CONFIG_S_DRIVER_OK;
}
Triggers failure on gitlab unfortunately:
https://gitlab.com/mstredhat/qemu/-/jobs/3323768122
S
Re: [PATCH v1 5/9] hw/virtio: introduce virtio_device_should_start
Am 14.11.22 um 18:20 schrieb Michael S. Tsirkin:
On Mon, Nov 14, 2022 at 06:15:30PM +0100, Christian Borntraeger wrote:
Am 14.11.22 um 18:10 schrieb Michael S. Tsirkin:
On Mon, Nov 14, 2022 at 05:55:09PM +0100, Christian Borntraeger wrote:
Am 14.11.22 um 17:37 schrieb Michael S. Tsirkin:
On Mon, Nov 14, 2022 at 05:18:53PM +0100, Christian Borntraeger wrote:
Am 08.11.22 um 10:23 schrieb Alex Bennée:
The previous fix to virtio_device_started revealed a problem in its
use by both the core and the device code. The core code should be able
to handle the device "starting" while the VM isn't running to handle
the restoration of migration state. To solve this dual use introduce a
new helper for use by the vhost-user backends who all use it to feed a
should_start variable.
We can also pick up a change vhost_user_blk_set_status while we are at
it which follows the same pattern.
Fixes: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started)
Fixes: 27ba7b027f (hw/virtio: add boilerplate for vhost-user-gpio device)
Signed-off-by: Alex Bennée
Cc: "Michael S. Tsirkin"
Hmmm, is this
commit 259d69c00b67c02a67f3bdbeeea71c2c0af76c35
Author: Alex Bennée
AuthorDate: Mon Nov 7 12:14:07 2022 +
Commit: Michael S. Tsirkin
CommitDate: Mon Nov 7 14:08:18 2022 -0500
hw/virtio: introduce virtio_device_should_start
and older version?
This is what got merged:
https://lore.kernel.org/r/20221107121407.1010913-1-alex.bennee%40linaro.org
This patch was sent after I merged the RFC.
I think the only difference is the commit log but I might be missing
something.
This does not seem to fix the regression that I have reported.
This was applied on top of 9f6bcfd99f which IIUC does, right?
QEMU master still fails for me for suspend/resume to disk:
#0 0x03ff8e3980a6 in __pthread_kill_implementation () at /lib64/libc.so.6
#1 0x03ff8e348580 in raise () at /lib64/libc.so.6
#2 0x03ff8e32b5c0 in abort () at /lib64/libc.so.6
#3 0x03ff8e3409da in __assert_fail_base () at /lib64/libc.so.6
#4 0x03ff8e340a4e in () at /lib64/libc.so.6
#5 0x02aa1ffa8966 in vhost_vsock_common_pre_save (opaque=)
at ../hw/virtio/vhost-vsock-common.c:203
#6 0x02aa1fe5e0ee in vmstate_save_state_v
(f=f@entry=0x2aa21bdc170, vmsd=0x2aa204ac5f0
, opaque=0x2aa21bac9f8,
vmdesc=vmdesc@entry=0x3fddc08eb30, version_id=version_id@entry=0) at
../migration/vmstate.c:329
#7 0x02aa1fe5ebf8 in vmstate_save_state (f=f@entry=0x2aa21bdc170, vmsd=, opaque=, vmdesc_id=vmdesc_id@entry=0x3fddc08eb30) at
../migration/vmstate.c:317
#8 0x02aa1fe75bd0 in vmstate_save (f=f@entry=0x2aa21bdc170,
se=se@entry=0x2aa21bdbe90, vmdesc=vmdesc@entry=0x3fddc08eb30) at
../migration/savevm.c:908
#9 0x02aa1fe79584 in qemu_savevm_state_complete_precopy_non_iterable
(f=f@entry=0x2aa21bdc170, in_postcopy=in_postcopy@entry=false,
inactivate_disks=inactivate_disks@entry=true)
at ../migration/savevm.c:1393
#10 0x02aa1fe79a96 in qemu_savevm_state_complete_precopy (f=0x2aa21bdc170,
iterable_only=iterable_only@entry=false,
inactivate_disks=inactivate_disks@entry=true) at ../migration/savevm.c:1459
#11 0x02aa1fe6d6ee in migration_completion (s=0x2aa218ef600) at
../migration/migration.c:3314
#12 migration_iteration_run (s=0x2aa218ef600) at ../migration/migration.c:3761
#13 migration_thread (opaque=opaque@entry=0x2aa218ef600) at
../migration/migration.c:3989
#14 0x02aa201f0b8c in qemu_thread_start (args=) at
../util/qemu-thread-posix.c:505
#15 0x03ff8e396248 in start_thread () at /lib64/libc.so.6
#16 0x03ff8e41183e in thread_start () at /lib64/libc.so.6
Michael, your previous branch did work if I recall correctly.
That one was failing under github CI though (for reasons we didn't
really address, such as disconnect during stop causing a recursive
call to stop, but there you are).
Even the double revert of everything?
I don't remember at this point.
So how do we proceed now?
I'm hopeful Alex will come up with a fix.
The initial fix changed to qemu/master does still work for me
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index a973811cbfc6..fb3072838119 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -411,14 +411,14 @@ static inline bool virtio_device_started(VirtIODevice
*vdev, uint8_t status)
*/
static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t
status)
{
-if (vdev->use_started) {
-return vdev->started;
-}
-
if (!vdev->vm_running) {
return false;
}
+if (vdev->use_started) {
+return vdev->started;
+}
+
return status & VIRTIO_CONFIG_S_DRIVER_OK;
}
Re: [PATCH v1 5/9] hw/virtio: introduce virtio_device_should_start
Am 14.11.22 um 18:20 schrieb Michael S. Tsirkin: [...] This does not seem to fix the regression that I have reported. This was applied on top of 9f6bcfd99f which IIUC does, right? Just dobble checked, 9f6bcfd99f was the patch that created the original problem, no?
Re: [PATCH v1 5/9] hw/virtio: introduce virtio_device_should_start
Am 14.11.22 um 18:10 schrieb Michael S. Tsirkin: On Mon, Nov 14, 2022 at 05:55:09PM +0100, Christian Borntraeger wrote: Am 14.11.22 um 17:37 schrieb Michael S. Tsirkin: On Mon, Nov 14, 2022 at 05:18:53PM +0100, Christian Borntraeger wrote: Am 08.11.22 um 10:23 schrieb Alex Bennée: The previous fix to virtio_device_started revealed a problem in its use by both the core and the device code. The core code should be able to handle the device "starting" while the VM isn't running to handle the restoration of migration state. To solve this dual use introduce a new helper for use by the vhost-user backends who all use it to feed a should_start variable. We can also pick up a change vhost_user_blk_set_status while we are at it which follows the same pattern. Fixes: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started) Fixes: 27ba7b027f (hw/virtio: add boilerplate for vhost-user-gpio device) Signed-off-by: Alex Bennée Cc: "Michael S. Tsirkin" Hmmm, is this commit 259d69c00b67c02a67f3bdbeeea71c2c0af76c35 Author: Alex Bennée AuthorDate: Mon Nov 7 12:14:07 2022 + Commit: Michael S. Tsirkin CommitDate: Mon Nov 7 14:08:18 2022 -0500 hw/virtio: introduce virtio_device_should_start and older version? This is what got merged: https://lore.kernel.org/r/20221107121407.1010913-1-alex.bennee%40linaro.org This patch was sent after I merged the RFC. I think the only difference is the commit log but I might be missing something. This does not seem to fix the regression that I have reported. This was applied on top of 9f6bcfd99f which IIUC does, right? QEMU master still fails for me for suspend/resume to disk: #0 0x03ff8e3980a6 in __pthread_kill_implementation () at /lib64/libc.so.6 #1 0x03ff8e348580 in raise () at /lib64/libc.so.6 #2 0x03ff8e32b5c0 in abort () at /lib64/libc.so.6 #3 0x03ff8e3409da in __assert_fail_base () at /lib64/libc.so.6 #4 0x03ff8e340a4e in () at /lib64/libc.so.6 #5 0x02aa1ffa8966 in vhost_vsock_common_pre_save (opaque=) at ../hw/virtio/vhost-vsock-common.c:203 #6 0x02aa1fe5e0ee in vmstate_save_state_v (f=f@entry=0x2aa21bdc170, vmsd=0x2aa204ac5f0 , opaque=0x2aa21bac9f8, vmdesc=vmdesc@entry=0x3fddc08eb30, version_id=version_id@entry=0) at ../migration/vmstate.c:329 #7 0x02aa1fe5ebf8 in vmstate_save_state (f=f@entry=0x2aa21bdc170, vmsd=, opaque=, vmdesc_id=vmdesc_id@entry=0x3fddc08eb30) at ../migration/vmstate.c:317 #8 0x02aa1fe75bd0 in vmstate_save (f=f@entry=0x2aa21bdc170, se=se@entry=0x2aa21bdbe90, vmdesc=vmdesc@entry=0x3fddc08eb30) at ../migration/savevm.c:908 #9 0x02aa1fe79584 in qemu_savevm_state_complete_precopy_non_iterable (f=f@entry=0x2aa21bdc170, in_postcopy=in_postcopy@entry=false, inactivate_disks=inactivate_disks@entry=true) at ../migration/savevm.c:1393 #10 0x02aa1fe79a96 in qemu_savevm_state_complete_precopy (f=0x2aa21bdc170, iterable_only=iterable_only@entry=false, inactivate_disks=inactivate_disks@entry=true) at ../migration/savevm.c:1459 #11 0x02aa1fe6d6ee in migration_completion (s=0x2aa218ef600) at ../migration/migration.c:3314 #12 migration_iteration_run (s=0x2aa218ef600) at ../migration/migration.c:3761 #13 migration_thread (opaque=opaque@entry=0x2aa218ef600) at ../migration/migration.c:3989 #14 0x02aa201f0b8c in qemu_thread_start (args=) at ../util/qemu-thread-posix.c:505 #15 0x03ff8e396248 in start_thread () at /lib64/libc.so.6 #16 0x03ff8e41183e in thread_start () at /lib64/libc.so.6 Michael, your previous branch did work if I recall correctly. That one was failing under github CI though (for reasons we didn't really address, such as disconnect during stop causing a recursive call to stop, but there you are). Even the double revert of everything? So how do we proceed now?
Re: [PATCH v1 5/9] hw/virtio: introduce virtio_device_should_start
Am 14.11.22 um 17:37 schrieb Michael S. Tsirkin: On Mon, Nov 14, 2022 at 05:18:53PM +0100, Christian Borntraeger wrote: Am 08.11.22 um 10:23 schrieb Alex Bennée: The previous fix to virtio_device_started revealed a problem in its use by both the core and the device code. The core code should be able to handle the device "starting" while the VM isn't running to handle the restoration of migration state. To solve this dual use introduce a new helper for use by the vhost-user backends who all use it to feed a should_start variable. We can also pick up a change vhost_user_blk_set_status while we are at it which follows the same pattern. Fixes: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started) Fixes: 27ba7b027f (hw/virtio: add boilerplate for vhost-user-gpio device) Signed-off-by: Alex Bennée Cc: "Michael S. Tsirkin" Hmmm, is this commit 259d69c00b67c02a67f3bdbeeea71c2c0af76c35 Author: Alex Bennée AuthorDate: Mon Nov 7 12:14:07 2022 + Commit: Michael S. Tsirkin CommitDate: Mon Nov 7 14:08:18 2022 -0500 hw/virtio: introduce virtio_device_should_start and older version? This is what got merged: https://lore.kernel.org/r/20221107121407.1010913-1-alex.bennee%40linaro.org This patch was sent after I merged the RFC. I think the only difference is the commit log but I might be missing something. This does not seem to fix the regression that I have reported. This was applied on top of 9f6bcfd99f which IIUC does, right? QEMU master still fails for me for suspend/resume to disk: #0 0x03ff8e3980a6 in __pthread_kill_implementation () at /lib64/libc.so.6 #1 0x03ff8e348580 in raise () at /lib64/libc.so.6 #2 0x03ff8e32b5c0 in abort () at /lib64/libc.so.6 #3 0x03ff8e3409da in __assert_fail_base () at /lib64/libc.so.6 #4 0x03ff8e340a4e in () at /lib64/libc.so.6 #5 0x02aa1ffa8966 in vhost_vsock_common_pre_save (opaque=) at ../hw/virtio/vhost-vsock-common.c:203 #6 0x02aa1fe5e0ee in vmstate_save_state_v (f=f@entry=0x2aa21bdc170, vmsd=0x2aa204ac5f0 , opaque=0x2aa21bac9f8, vmdesc=vmdesc@entry=0x3fddc08eb30, version_id=version_id@entry=0) at ../migration/vmstate.c:329 #7 0x02aa1fe5ebf8 in vmstate_save_state (f=f@entry=0x2aa21bdc170, vmsd=, opaque=, vmdesc_id=vmdesc_id@entry=0x3fddc08eb30) at ../migration/vmstate.c:317 #8 0x02aa1fe75bd0 in vmstate_save (f=f@entry=0x2aa21bdc170, se=se@entry=0x2aa21bdbe90, vmdesc=vmdesc@entry=0x3fddc08eb30) at ../migration/savevm.c:908 #9 0x02aa1fe79584 in qemu_savevm_state_complete_precopy_non_iterable (f=f@entry=0x2aa21bdc170, in_postcopy=in_postcopy@entry=false, inactivate_disks=inactivate_disks@entry=true) at ../migration/savevm.c:1393 #10 0x02aa1fe79a96 in qemu_savevm_state_complete_precopy (f=0x2aa21bdc170, iterable_only=iterable_only@entry=false, inactivate_disks=inactivate_disks@entry=true) at ../migration/savevm.c:1459 #11 0x02aa1fe6d6ee in migration_completion (s=0x2aa218ef600) at ../migration/migration.c:3314 #12 migration_iteration_run (s=0x2aa218ef600) at ../migration/migration.c:3761 #13 migration_thread (opaque=opaque@entry=0x2aa218ef600) at ../migration/migration.c:3989 #14 0x02aa201f0b8c in qemu_thread_start (args=) at ../util/qemu-thread-posix.c:505 #15 0x03ff8e396248 in start_thread () at /lib64/libc.so.6 #16 0x03ff8e41183e in thread_start () at /lib64/libc.so.6 Michael, your previous branch did work if I recall correctly.
Re: [PATCH v1 5/9] hw/virtio: introduce virtio_device_should_start
Am 08.11.22 um 10:23 schrieb Alex Bennée: The previous fix to virtio_device_started revealed a problem in its use by both the core and the device code. The core code should be able to handle the device "starting" while the VM isn't running to handle the restoration of migration state. To solve this dual use introduce a new helper for use by the vhost-user backends who all use it to feed a should_start variable. We can also pick up a change vhost_user_blk_set_status while we are at it which follows the same pattern. Fixes: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started) Fixes: 27ba7b027f (hw/virtio: add boilerplate for vhost-user-gpio device) Signed-off-by: Alex Bennée Cc: "Michael S. Tsirkin" Hmmm, is this commit 259d69c00b67c02a67f3bdbeeea71c2c0af76c35 Author: Alex Bennée AuthorDate: Mon Nov 7 12:14:07 2022 + Commit: Michael S. Tsirkin CommitDate: Mon Nov 7 14:08:18 2022 -0500 hw/virtio: introduce virtio_device_should_start and older version? This does not seem to fix the regression that I have reported.
Re: [RFC PATCH] virtio: re-order vm_running and use_started checks
Am 04.11.22 um 17:51 schrieb Christian Borntraeger: Am 04.11.22 um 17:14 schrieb Michael S. Tsirkin: On Fri, Nov 04, 2022 at 04:59:35PM +0100, Christian Borntraeger wrote: Am 04.11.22 um 16:56 schrieb Michael S. Tsirkin: On Fri, Oct 14, 2022 at 02:21:08PM +0100, Alex Bennée wrote: During migration the virtio device state can be restored before we restart the VM. As no devices can be running while the VM is paused it makes sense to bail out early in that case. This returns the order introduced in: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started) to what virtio-sock was doing longhand. Signed-off-by: Alex Bennée Cc: Christian Borntraeger What happens now: with this applied I get: https://gitlab.com/mitsirkin/qemu/-/pipelines/685829158/failures ― ✀ ― stderr: qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-reconnect,path=/tmp/vhost-test-QLKXU1/reconnect.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-QLKXU1/reconnect.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-connect-fail,path=/tmp/vhost-test-L9Q6U1/connect-fail.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-L9Q6U1/connect-fail.sock,server=on qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: Failed to read msg header. Read 0 instead of 12. Original request 1. qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: vhost_backend_init failed: Protocol error qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: failed to init vhost_net for queue 0 qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-L9Q6U1/connect-fail.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-flags-mismatch,path=/tmp/vhost-test-3MO5U1/flags-mismatch.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-3MO5U1/flags-mismatch.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 52. qemu-system-arm: vhost_set_mem_table failed: Invalid argument (22) qemu-system-arm: unable to start vhost net: 22: falling back on userspace virtio vhost lacks feature mask 0x4000 for backend qemu-system-arm: failed to init vhost_net for queue 0 qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 2 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 3 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost_set_vring_call failed: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost_set_vring_call failed: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed
Re: [RFC PATCH] virtio: re-order vm_running and use_started checks
Am 04.11.22 um 17:14 schrieb Michael S. Tsirkin: On Fri, Nov 04, 2022 at 04:59:35PM +0100, Christian Borntraeger wrote: Am 04.11.22 um 16:56 schrieb Michael S. Tsirkin: On Fri, Oct 14, 2022 at 02:21:08PM +0100, Alex Bennée wrote: During migration the virtio device state can be restored before we restart the VM. As no devices can be running while the VM is paused it makes sense to bail out early in that case. This returns the order introduced in: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started) to what virtio-sock was doing longhand. Signed-off-by: Alex Bennée Cc: Christian Borntraeger What happens now: with this applied I get: https://gitlab.com/mitsirkin/qemu/-/pipelines/685829158/failures ― ✀ ― stderr: qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-reconnect,path=/tmp/vhost-test-QLKXU1/reconnect.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-QLKXU1/reconnect.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-connect-fail,path=/tmp/vhost-test-L9Q6U1/connect-fail.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-L9Q6U1/connect-fail.sock,server=on qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: Failed to read msg header. Read 0 instead of 12. Original request 1. qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: vhost_backend_init failed: Protocol error qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: failed to init vhost_net for queue 0 qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-L9Q6U1/connect-fail.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-flags-mismatch,path=/tmp/vhost-test-3MO5U1/flags-mismatch.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-3MO5U1/flags-mismatch.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 52. qemu-system-arm: vhost_set_mem_table failed: Invalid argument (22) qemu-system-arm: unable to start vhost net: 22: falling back on userspace virtio vhost lacks feature mask 0x4000 for backend qemu-system-arm: failed to init vhost_net for queue 0 qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 2 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 3 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost_set_vring_call failed: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost_set_vring_call failed: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -5: Input/output error (5) qemu-system-arm: ../hw
Re: [RFC PATCH] virtio: re-order vm_running and use_started checks
Am 04.11.22 um 16:56 schrieb Michael S. Tsirkin: On Fri, Oct 14, 2022 at 02:21:08PM +0100, Alex Bennée wrote: During migration the virtio device state can be restored before we restart the VM. As no devices can be running while the VM is paused it makes sense to bail out early in that case. This returns the order introduced in: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started) to what virtio-sock was doing longhand. Signed-off-by: Alex Bennée Cc: Christian Borntraeger What happens now: with this applied I get: https://gitlab.com/mitsirkin/qemu/-/pipelines/685829158/failures ― ✀ ― stderr: qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-reconnect,path=/tmp/vhost-test-QLKXU1/reconnect.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-QLKXU1/reconnect.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-connect-fail,path=/tmp/vhost-test-L9Q6U1/connect-fail.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-L9Q6U1/connect-fail.sock,server=on qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: Failed to read msg header. Read 0 instead of 12. Original request 1. qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: vhost_backend_init failed: Protocol error qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: failed to init vhost_net for queue 0 qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-L9Q6U1/connect-fail.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-flags-mismatch,path=/tmp/vhost-test-3MO5U1/flags-mismatch.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-3MO5U1/flags-mismatch.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 52. qemu-system-arm: vhost_set_mem_table failed: Invalid argument (22) qemu-system-arm: unable to start vhost net: 22: falling back on userspace virtio vhost lacks feature mask 0x4000 for backend qemu-system-arm: failed to init vhost_net for queue 0 qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 2 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 3 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost_set_vring_call failed: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost_set_vring_call failed: Invalid argument (22) qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -5: Input/output error (5) qemu-system-arm: ../hw/virtio/virtio-bus.c:211: void virtio_bus_release_ioeventfd(VirtioBusState *): Assertion `bus->ioeventfd_grabbed != 0' fai
Re: [PATCH 1/1] tcg: add perfmap and jitdump
Am 12.10.22 um 07:18 schrieb Ilya Leoshkevich: Add ability to dump /tmp/perf-.map and jit-.dump. The first one allows the perf tool to map samples to each individual translation block. The second one adds the ability to resolve symbol names, line numbers and inspect JITed code. Example of use: perf record qemu-x86_64 -perfmap ./a.out perf report or perf record -k 1 qemu-x86_64 -jitdump ./a.out perf inject -j -i perf.data -o perf.data.jitted perf report -i perf.data.jitted Co-developed-by: Vanderson M. do Rosario Co-developed-by: Alex Bennée Signed-off-by: Ilya Leoshkevich I think this would be awesome to have. I know our performance people do use this for Java a lot.
Re: qemu iotest 161 and make check
Am 31.03.22 um 10:25 schrieb Christian Borntraeger:
Am 31.03.22 um 09:44 schrieb Christian Borntraeger:
Am 21.02.22 um 11:27 schrieb Christian Borntraeger:
Am 10.02.22 um 18:44 schrieb Vladimir Sementsov-Ogievskiy:
10.02.2022 20:13, Thomas Huth wrote:
On 10/02/2022 15.51, Christian Borntraeger wrote:
Am 10.02.22 um 15:47 schrieb Vladimir Sementsov-Ogievskiy:
10.02.2022 10:57, Christian Borntraeger wrote:
Hello,
I do see spurious failures of 161 in our CI, but only when I use
make check with parallelism (-j).
I have not yet figured out which other testcase could interfere
@@ -34,6 +34,8 @@
*** Commit and then change an option on the backing file
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+qemu-img: TEST_DIR/t.IMGFMT.base: Failed to get "write" lock
FWIW, qemu_lock_fd_test returns -11 (EAGAIN)
and raw_check_lock_bytes spits this error.
And its coming from here (ret is 0)
int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive)
{
int ret;
struct flock fl = {
.l_whence = SEEK_SET,
.l_start = start,
.l_len = len,
.l_type = exclusive ? F_WRLCK : F_RDLCK,
};
qemu_probe_lock_ops();
ret = fcntl(fd, fcntl_op_getlk, );
if (ret == -1) {
return -errno;
} else {
-> return fl.l_type == F_UNLCK ? 0 : -EAGAIN;
}
}
Is this just some overload situation that we do not recover because we do not
handle EAGAIN any special.
Restarted my investigation. Looks like the file lock from qemu is not fully
cleaned up when the process is gone.
Something like
diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index 0f1fecc68e..b28a6c187c 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -403,4 +403,5 @@ _cleanup_qemu()
unset QEMU_IN[$i]
unset QEMU_OUT[$i]
done
+sleep 0.5
}
makes the problem go away.
Looks like we do use the OFD variant of the file lock, so any clone, fork etc
will keep the lock.
So I tested the following:
diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index 0f1fecc68e..01bdb05575 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -388,7 +388,7 @@ _cleanup_qemu()
kill -KILL ${QEMU_PID} 2>/dev/null
fi
if [ -n "${QEMU_PID}" ]; then
-wait ${QEMU_PID} 2>/dev/null # silent kill
+wait 2>/dev/null # silent kill
fi
fi
And this also helps. Still trying to find out what clone/fork happens here.
[PATCH] MAINTAINERS: target/s390x/: add Ilya as reviewer
Ilya has volunteered to review TCG patches for s390x. Signed-off-by: Christian Borntraeger --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index e3d5b7e09c46..ae5e8c8ecbb6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -305,6 +305,7 @@ F: target/rx/ S390 TCG CPUs M: Richard Henderson M: David Hildenbrand +R: Ilya Leoshkevich S: Maintained F: target/s390x/ F: target/s390x/tcg -- 2.37.3
Re: [RFC PATCH] virtio: re-order vm_running and use_started checks
Am 14.10.22 um 15:21 schrieb Alex Bennée:
During migration the virtio device state can be restored before we
restart the VM. As no devices can be running while the VM is paused it
makes sense to bail out early in that case.
This returns the order introduced in:
9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started)
to what virtio-sock was doing longhand.
Signed-off-by: Alex Bennée
Cc: Christian Borntraeger
Tested-by: Christian Borntraeger
---
include/hw/virtio/virtio.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index f41b4a7e64..ebb58feaac 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -385,14 +385,14 @@ static inline bool virtio_is_big_endian(VirtIODevice
*vdev)
static inline bool virtio_device_started(VirtIODevice *vdev, uint8_t status)
{
-if (vdev->use_started) {
-return vdev->started;
-}
-
if (!vdev->vm_running) {
return false;
}
+if (vdev->use_started) {
+return vdev->started;
+}
+
return status & VIRTIO_CONFIG_S_DRIVER_OK;
}
Re: Regression save/restore of vsock: (was [PULL 07/55] hw/virtio: move vm_running check to virtio_device_started)
Am 14.10.22 um 13:07 schrieb Alex Bennée:
Christian Borntraeger writes:
Am 14.10.22 um 09:30 schrieb Christian Borntraeger:
Am 10.10.22 um 19:29 schrieb Michael S. Tsirkin:
From: Alex Bennée
All the boilerplate virtio code does the same thing (or should at
least) of checking to see if the VM is running before attempting to
start VirtIO. Push the logic up to the common function to avoid
getting a copy and paste wrong.
Signed-off-by: Alex Bennée
Message-Id: <20220802095010.3330793-11-alex.ben...@linaro.org>
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
This results in a regression for our s390x CI when doing
save/restore of guests with vsock:
#1 0x03ff9a248580 raise (libc.so.6 + 0x48580)
#2 0x03ff9a22b5c0 abort (libc.so.6 + 0x2b5c0)
#3 0x03ff9a2409da __assert_fail_base (libc.so.6 +
0x409da)
#4 0x03ff9a240a4e __assert_fail (libc.so.6 + 0x40a4e)
#5 0x02aa2d69a066 vhost_vsock_common_pre_save
(qemu-system-s390x + 0x39a066)
#6 0x02aa2d55570e vmstate_save_state_v
(qemu-system-s390x + 0x25570e)
#7 0x02aa2d556218 vmstate_save_state (qemu-system-s390x
+ 0x256218)
#8 0x02aa2d570ba4
qemu_savevm_state_complete_precopy_non_iterable (qemu-system-s390x +
0x270ba4)
#9 0x02aa2d5710b6 qemu_savevm_state_complete_precopy
(qemu-system-s390x + 0x2710b6)
#10 0x02aa2d564d0e migration_completion
(qemu-system-s390x + 0x264d0e)
#11 0x02aa2d8db25c qemu_thread_start (qemu-system-s390x +
0x5db25c)
#12 0x03ff9a296248 start_thread (libc.so.6 + 0x96248)
#13 0x03ff9a31183e thread_start (libc.so.6 + 0x11183e)
Something like
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 7dc3c7393122..b4d056ae6f01 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -73,6 +73,10 @@ static void vhost_vsock_set_status(VirtIODevice *vdev,
uint8_t status)
bool should_start = virtio_device_started(vdev, status);
int ret;
+if (!vdev->vm_running) {
+should_start = false;
+}
+
if (vhost_dev_is_started(>vhost_dev) == should_start) {
return;
}
helps.
The problem seems to be that virtio_device_started does ignore
vm_running when use_start is set.
Wouldn't it make more sense to re-order the check there, something like:
static inline bool virtio_device_started(VirtIODevice *vdev, uint8_t status)
{
if (!vdev->vm_running) {
return false;
}
if (vdev->use_started) {
return vdev->started;
}
return status & VIRTIO_CONFIG_S_DRIVER_OK;
}
That does work as well. (and it restores the original ordering so that makes
sense).
Is the problem that vdev->started gets filled during the migration but
because the VM isn't running yet we can never actually run?
I dont know.
Re: Regression save/restore of vsock: (was [PULL 07/55] hw/virtio: move vm_running check to virtio_device_started)
Am 14.10.22 um 10:37 schrieb Alex Bennée:
Christian Borntraeger writes:
Am 10.10.22 um 19:29 schrieb Michael S. Tsirkin:
From: Alex Bennée
All the boilerplate virtio code does the same thing (or should at
least) of checking to see if the VM is running before attempting to
start VirtIO. Push the logic up to the common function to avoid
getting a copy and paste wrong.
Signed-off-by: Alex Bennée
Message-Id: <20220802095010.3330793-11-alex.ben...@linaro.org>
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
This results in a regression for our s390x CI when doing save/restore of guests
with vsock:
#1 0x03ff9a248580 raise (libc.so.6 + 0x48580)
#2 0x03ff9a22b5c0 abort (libc.so.6 + 0x2b5c0)
#3 0x03ff9a2409da __assert_fail_base (libc.so.6 + 0x409da)
#4 0x03ff9a240a4e __assert_fail (libc.so.6 + 0x40a4e)
#5 0x02aa2d69a066 vhost_vsock_common_pre_save
(qemu-system-s390x + 0x39a066)
#6 0x02aa2d55570e vmstate_save_state_v (qemu-system-s390x
+ 0x25570e)
#7 0x02aa2d556218 vmstate_save_state (qemu-system-s390x +
0x256218)
#8 0x02aa2d570ba4
qemu_savevm_state_complete_precopy_non_iterable (qemu-system-s390x +
0x270ba4)
#9 0x02aa2d5710b6 qemu_savevm_state_complete_precopy
(qemu-system-s390x + 0x2710b6)
#10 0x02aa2d564d0e migration_completion (qemu-system-s390x
+ 0x264d0e)
#11 0x02aa2d8db25c qemu_thread_start (qemu-system-s390x +
0x5db25c)
#12 0x03ff9a296248 start_thread (libc.so.6 + 0x96248)
#13 0x03ff9a31183e thread_start (libc.so.6 + 0x11183e)
Which test does this break?
migrate to file and restore.
Looking at the change the only thing I can think of is there is a subtle
change in the order of checks because if the device is set as
use_started we return the result regardless of vm or config state:
if (vdev->use_started) {
return vdev->started;
}
Could some printfs confirm that?
Right. The problem is we now ignore the vm state and thus run into the
assertion in vhost_vsock_common_pre_save.
Removing the asserting then results in virtio errors, which really indicates
that the device must not be started.
Re: Regression save/restore of vsock: (was [PULL 07/55] hw/virtio: move vm_running check to virtio_device_started)
Am 14.10.22 um 09:30 schrieb Christian Borntraeger:
Am 10.10.22 um 19:29 schrieb Michael S. Tsirkin:
From: Alex Bennée
All the boilerplate virtio code does the same thing (or should at
least) of checking to see if the VM is running before attempting to
start VirtIO. Push the logic up to the common function to avoid
getting a copy and paste wrong.
Signed-off-by: Alex Bennée
Message-Id: <20220802095010.3330793-11-alex.ben...@linaro.org>
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
This results in a regression for our s390x CI when doing save/restore of guests
with vsock:
#1 0x03ff9a248580 raise (libc.so.6 + 0x48580)
#2 0x03ff9a22b5c0 abort (libc.so.6 + 0x2b5c0)
#3 0x03ff9a2409da __assert_fail_base (libc.so.6 + 0x409da)
#4 0x03ff9a240a4e __assert_fail (libc.so.6 + 0x40a4e)
#5 0x02aa2d69a066 vhost_vsock_common_pre_save
(qemu-system-s390x + 0x39a066)
#6 0x02aa2d55570e vmstate_save_state_v (qemu-system-s390x
+ 0x25570e)
#7 0x02aa2d556218 vmstate_save_state (qemu-system-s390x +
0x256218)
#8 0x02aa2d570ba4
qemu_savevm_state_complete_precopy_non_iterable (qemu-system-s390x + 0x270ba4)
#9 0x02aa2d5710b6 qemu_savevm_state_complete_precopy
(qemu-system-s390x + 0x2710b6)
#10 0x02aa2d564d0e migration_completion (qemu-system-s390x
+ 0x264d0e)
#11 0x02aa2d8db25c qemu_thread_start (qemu-system-s390x +
0x5db25c)
#12 0x03ff9a296248 start_thread (libc.so.6 + 0x96248)
#13 0x03ff9a31183e thread_start (libc.so.6 + 0x11183e)
Something like
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 7dc3c7393122..b4d056ae6f01 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -73,6 +73,10 @@ static void vhost_vsock_set_status(VirtIODevice *vdev,
uint8_t status)
bool should_start = virtio_device_started(vdev, status);
int ret;
+if (!vdev->vm_running) {
+should_start = false;
+}
+
if (vhost_dev_is_started(>vhost_dev) == should_start) {
return;
}
helps.
The problem seems to be that virtio_device_started does ignore vm_running when
use_start is set.
Regression save/restore of vsock: (was [PULL 07/55] hw/virtio: move vm_running check to virtio_device_started)
Am 10.10.22 um 19:29 schrieb Michael S. Tsirkin: From: Alex Bennée All the boilerplate virtio code does the same thing (or should at least) of checking to see if the VM is running before attempting to start VirtIO. Push the logic up to the common function to avoid getting a copy and paste wrong. Signed-off-by: Alex Bennée Message-Id: <20220802095010.3330793-11-alex.ben...@linaro.org> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin This results in a regression for our s390x CI when doing save/restore of guests with vsock: #1 0x03ff9a248580 raise (libc.so.6 + 0x48580) #2 0x03ff9a22b5c0 abort (libc.so.6 + 0x2b5c0) #3 0x03ff9a2409da __assert_fail_base (libc.so.6 + 0x409da) #4 0x03ff9a240a4e __assert_fail (libc.so.6 + 0x40a4e) #5 0x02aa2d69a066 vhost_vsock_common_pre_save (qemu-system-s390x + 0x39a066) #6 0x02aa2d55570e vmstate_save_state_v (qemu-system-s390x + 0x25570e) #7 0x02aa2d556218 vmstate_save_state (qemu-system-s390x + 0x256218) #8 0x02aa2d570ba4 qemu_savevm_state_complete_precopy_non_iterable (qemu-system-s390x + 0x270ba4) #9 0x02aa2d5710b6 qemu_savevm_state_complete_precopy (qemu-system-s390x + 0x2710b6) #10 0x02aa2d564d0e migration_completion (qemu-system-s390x + 0x264d0e) #11 0x02aa2d8db25c qemu_thread_start (qemu-system-s390x + 0x5db25c) #12 0x03ff9a296248 start_thread (libc.so.6 + 0x96248) #13 0x03ff9a31183e thread_start (libc.so.6 + 0x11183e)
[PATCH 1/1] s390x/tcg: Fix opcode for lzrf
Fix the opcode for Load and Zero Rightmost Byte (32). Cc: qemu-sta...@nongnu.org Reported-by: Nathan Chancellor Tested-by: Nathan Chancellor Signed-off-by: Christian Borntraeger --- target/s390x/tcg/insn-data.def | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/s390x/tcg/insn-data.def b/target/s390x/tcg/insn-data.def index 6d2cfe5fa2b7..6382ceabfcfa 100644 --- a/target/s390x/tcg/insn-data.def +++ b/target/s390x/tcg/insn-data.def @@ -466,7 +466,7 @@ C(0xe39f, LAT, RXY_a, LAT, 0, m2_32u, r1, 0, lat, 0) C(0xe385, LGAT,RXY_a, LAT, 0, a2, r1, 0, lgat, 0) /* LOAD AND ZERO RIGHTMOST BYTE */ -C(0xe3eb, LZRF,RXY_a, LZRB, 0, m2_32u, new, r1_32, lzrb, 0) +C(0xe33b, LZRF,RXY_a, LZRB, 0, m2_32u, new, r1_32, lzrb, 0) C(0xe32a, LZRG,RXY_a, LZRB, 0, m2_64, r1, 0, lzrb, 0) /* LOAD LOGICAL AND ZERO RIGHTMOST BYTE */ C(0xe33a, LLZRGF, RXY_a, LZRB, 0, m2_32u, r1, 0, lzrb, 0) -- 2.37.1
Re: [PATCH v3 1/1] os-posix: asynchronous teardown for shutdown on Linux
Am 11.08.22 um 14:27 schrieb Daniel P. Berrangé:
[...]
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4743,6 +4743,23 @@ HXCOMM Internal use
DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL)
DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL)
+#ifdef __linux__
+DEF("async-teardown", 0, QEMU_OPTION_asyncteardown,
+"-async-teardown enable asynchronous teardown\n",
+QEMU_ARCH_ALL)
+#endif
+SRST
+``-async-teardown``
+Enable asynchronous teardown. A new teardown process will be
+created at startup, using clone. The teardown process will share
+the address space of the main qemu process, and wait for the main
+process to terminate. At that point, the teardown process will
+also exit. This allows qemu to terminate quickly if the guest was
+huge, leaving the teardown of the address space to the teardown
+process. Since the teardown process shares the same cgroups as the
+main qemu process, accounting is performed correctly.
+ERST
+
DEF("msg", HAS_ARG, QEMU_OPTION_msg,
"-msg [timestamp[=on|off]][,guest-name=[on|off]]\n"
"control error message format\n"
It occurrs to me that we've got a general goal of getting away from
adding new top level command line arguments. Most of the time there's
an obvious existing place to put them, but I'm really not sure
where this particular option would fit ?
it isn't tied to any aspect of the VM backend configuration nor
hardware frontends.
The closest match is the lifecycle action option (-no-shutdown)
which were merged into a -action arg, but even that's a bit of a
stretch.
Markus/Paolo: do you have suggestions ?
Also extending this to libvirt, would it make sense to add this to the event
list
as
with async/sync
This might give an indication where to put it in qemu.
Re: [PATCH v4 2/2] target/s390x: support SHA-512 extensions
Am 04.08.22 um 08:51 schrieb Harald Freudenberger: On 2022-08-03 14:14, Jason A. Donenfeld wrote: Hi David, On Wed, Aug 03, 2022 at 01:55:21PM +0200, David Hildenbrand wrote: On 02.08.22 21:00, Jason A. Donenfeld wrote: > In order to fully support MSA_EXT_5, we have to also support the SHA-512 > special instructions. So implement those. > > The implementation began as something TweetNacl-like, and then was > adjusted to be useful here. It's not very beautiful, but it is quite > short and compact, which is what we're going for. > Do we have to worry about copyright/authorship of the original code or did you write that from scratch? I actually don't really remember how much of that is leftover from tweetnacl and how much I've rewritten - I've had some variant of this code or another kicking around in various projects and repos for a long time. But the tweetnacl stuff is public domain to begin with, so all good. Are we properly handling the length register (r2 + 1) in the 24-bit/31-bit addressing mode? Similarly, are we properly handling updates to the message register (r2) depending on the addressing mode? Ugh, probably not... I didn't do any of the deposit_64 stuff. I guess I'll look into that. It's worth noting that we might want to implement (also for PRNO-TRNG): "The operation is ended when all source bytes in the second operand have been pro- cessed (called normal completion), or when a CPU- determined number of blocks that is less than the length of the second operand have been processed (called partial completion). The CPU-determined number of blocks depends on the model, and may be a different number each time the instruction is exe- cuted. The CPU-determined number of blocks is usu- ally nonzero. In certain unusual situations, this number may be zero, and condition code 3 may be set with no progress." Otherwise, a large length can make us loop quite a while in QEMU, without the chance to deliver any other interrupts. Hmm, okay. Looking at the Linux code, I see: s.even = (unsigned long)src; s.odd = (unsigned long)src_len; asm volatile( " lgr 0,%[fc]\n" " lgr 1,%[pba]\n" "0: .insn rre,%[opc] << 16,0,%[src]\n" " brc 1,0b\n" /* handle partial completion */ : [src] "+" (s.pair) : [fc] "d" (func), [pba] "d" ((unsigned long)(param)), [opc] "i" (CPACF_KIMD) : "cc", "memory", "0", "1"); So I guess that means it'll just loop until it's done? Or do I need to return "1" from HELPER(msa)? Jason Hm, you don't really want to implement some kind of particial complete. Qemu is an emulation and you would have to implement some kind of fragmenting this based on machine generation. For my feeling this is way too overengineered. Btw. as there came the request to handle the 24-bit/31-bit addressing correctly. Is Qemu 32 bit supported ? We do not support the esa390 mode, but the 24/31 bit _addressing_ modes are totally valid to be used in zarch mode (with sam31 for example). The kernel does that for example for some diagnoses under z/VM. Nobody in problem state should probably do that, but its possible.
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
Am 02.08.22 um 16:53 schrieb David Hildenbrand:
On 02.08.22 16:01, Christian Borntraeger wrote:
Am 02.08.22 um 15:54 schrieb David Hildenbrand:
On 02.08.22 15:26, Christian Borntraeger wrote:
Am 20.07.22 um 14:08 schrieb Jason A. Donenfeld:
In order for hosts running inside of TCG to initialize the kernel's
random number generator, we should support the PRNO_TRNG instruction,
backed in the usual way with the qemu_guest_getrandom helper. This is
confirmed working on Linux 5.19-rc6.
Cc: Thomas Huth
Cc: David Hildenbrand
Cc: Richard Henderson
Cc: Cornelia Huck
Cc: Harald Freudenberger
Cc: Holger Dengler
Signed-off-by: Jason A. Donenfeld
[...]
+case 114:
+if (r1 & 1 || !r1 || r2 & 1 || !r2)
+tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+fill_buf_random(env, ra, >regs[r1], >regs[r1 + 1]);
+fill_buf_random(env, ra, >regs[r2], >regs[r2 + 1]);
+break;
I think I agree with Harald that some aspects are missing.
Linux does not seem to check, but we should also modify the query function to
indicate the availability of 114.
As the msa helper deals with many instructions
...
target/s390x/tcg/insn-data.def:D(0xb91e, KMAC,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMAC)
target/s390x/tcg/insn-data.def:D(0xb928, PCKMO, RRE, MSA3, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PCKMO)
target/s390x/tcg/insn-data.def:D(0xb92a, KMF, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMF)
target/s390x/tcg/insn-data.def:D(0xb92b, KMO, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMO)
target/s390x/tcg/insn-data.def:D(0xb92c, PCC, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PCC)
target/s390x/tcg/insn-data.def:D(0xb92d, KMCTR, RRF_b, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMCTR)
target/s390x/tcg/insn-data.def:D(0xb92e, KM, RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KM)
target/s390x/tcg/insn-data.def:D(0xb92f, KMC, RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMC)
target/s390x/tcg/insn-data.def:D(0xb929, KMA, RRF_b, MSA8, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMA)
target/s390x/tcg/insn-data.def:D(0xb93c, PPNO,RRE, MSA5, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PPNO)
target/s390x/tcg/insn-data.def:D(0xb93e, KIMD,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KIMD)
target/s390x/tcg/insn-data.def:D(0xb93f, KLMD,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KLMD)
...
and in theory other instructions might also have 114 we should at least check
that this is ppno/prno.
Or we split out a prno helper from the msa helper.
Doesn't
s390_get_feat_block(type, subfunc);
if (!test_be_bit(fc, subfunc)) {
tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
}
check that? As long as we don't implement 114 for any other instruction.
that should properly fence off the other instructions.
Right that would help. We should still take care of the query function.
s390_get_feat_block() should already take care of that as well, no?
Ah right, yes it fills subfunc. So yes, that should do the trick. Sorry for the
noise.
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
Am 02.08.22 um 15:54 schrieb David Hildenbrand:
On 02.08.22 15:26, Christian Borntraeger wrote:
Am 20.07.22 um 14:08 schrieb Jason A. Donenfeld:
In order for hosts running inside of TCG to initialize the kernel's
random number generator, we should support the PRNO_TRNG instruction,
backed in the usual way with the qemu_guest_getrandom helper. This is
confirmed working on Linux 5.19-rc6.
Cc: Thomas Huth
Cc: David Hildenbrand
Cc: Richard Henderson
Cc: Cornelia Huck
Cc: Harald Freudenberger
Cc: Holger Dengler
Signed-off-by: Jason A. Donenfeld
[...]
+case 114:
+if (r1 & 1 || !r1 || r2 & 1 || !r2)
+tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+fill_buf_random(env, ra, >regs[r1], >regs[r1 + 1]);
+fill_buf_random(env, ra, >regs[r2], >regs[r2 + 1]);
+break;
I think I agree with Harald that some aspects are missing.
Linux does not seem to check, but we should also modify the query function to
indicate the availability of 114.
As the msa helper deals with many instructions
...
target/s390x/tcg/insn-data.def:D(0xb91e, KMAC,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMAC)
target/s390x/tcg/insn-data.def:D(0xb928, PCKMO, RRE, MSA3, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PCKMO)
target/s390x/tcg/insn-data.def:D(0xb92a, KMF, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMF)
target/s390x/tcg/insn-data.def:D(0xb92b, KMO, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMO)
target/s390x/tcg/insn-data.def:D(0xb92c, PCC, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PCC)
target/s390x/tcg/insn-data.def:D(0xb92d, KMCTR, RRF_b, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMCTR)
target/s390x/tcg/insn-data.def:D(0xb92e, KM, RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KM)
target/s390x/tcg/insn-data.def:D(0xb92f, KMC, RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMC)
target/s390x/tcg/insn-data.def:D(0xb929, KMA, RRF_b, MSA8, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMA)
target/s390x/tcg/insn-data.def:D(0xb93c, PPNO,RRE, MSA5, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PPNO)
target/s390x/tcg/insn-data.def:D(0xb93e, KIMD,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KIMD)
target/s390x/tcg/insn-data.def:D(0xb93f, KLMD,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KLMD)
...
and in theory other instructions might also have 114 we should at least check
that this is ppno/prno.
Or we split out a prno helper from the msa helper.
Doesn't
s390_get_feat_block(type, subfunc);
if (!test_be_bit(fc, subfunc)) {
tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
}
check that? As long as we don't implement 114 for any other instruction.
that should properly fence off the other instructions.
Right that would help. We should still take care of the query function.
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
Am 20.07.22 um 14:08 schrieb Jason A. Donenfeld: In order for hosts running inside of TCG to initialize the kernel's random number generator, we should support the PRNO_TRNG instruction, backed in the usual way with the qemu_guest_getrandom helper. This is confirmed working on Linux 5.19-rc6. Cc: Thomas Huth Cc: David Hildenbrand Cc: Richard Henderson Cc: Cornelia Huck Cc: Harald Freudenberger Cc: Holger Dengler Signed-off-by: Jason A. Donenfeld [...] +case 114: +if (r1 & 1 || !r1 || r2 & 1 || !r2) +tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra); +fill_buf_random(env, ra, >regs[r1], >regs[r1 + 1]); +fill_buf_random(env, ra, >regs[r2], >regs[r2 + 1]); +break; I think I agree with Harald that some aspects are missing. Linux does not seem to check, but we should also modify the query function to indicate the availability of 114. As the msa helper deals with many instructions ... target/s390x/tcg/insn-data.def:D(0xb91e, KMAC,RRE, MSA, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMAC) target/s390x/tcg/insn-data.def:D(0xb928, PCKMO, RRE, MSA3, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_PCKMO) target/s390x/tcg/insn-data.def:D(0xb92a, KMF, RRE, MSA4, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMF) target/s390x/tcg/insn-data.def:D(0xb92b, KMO, RRE, MSA4, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMO) target/s390x/tcg/insn-data.def:D(0xb92c, PCC, RRE, MSA4, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_PCC) target/s390x/tcg/insn-data.def:D(0xb92d, KMCTR, RRF_b, MSA4, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMCTR) target/s390x/tcg/insn-data.def:D(0xb92e, KM, RRE, MSA, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KM) target/s390x/tcg/insn-data.def:D(0xb92f, KMC, RRE, MSA, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMC) target/s390x/tcg/insn-data.def:D(0xb929, KMA, RRF_b, MSA8, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMA) target/s390x/tcg/insn-data.def:D(0xb93c, PPNO,RRE, MSA5, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_PPNO) target/s390x/tcg/insn-data.def:D(0xb93e, KIMD,RRE, MSA, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KIMD) target/s390x/tcg/insn-data.def:D(0xb93f, KLMD,RRE, MSA, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KLMD) ... and in theory other instructions might also have 114 we should at least check that this is ppno/prno. Or we split out a prno helper from the msa helper.
[PATCH v2] s390x/cpumodel: add stfl197 processor-activity-instrumentation extension 1
Add stfle 197 (processor-activity-instrumentation extension 1) to the
gen16 default model and fence it off for 7.1 and older.
Signed-off-by: Christian Borntraeger
Reviewed-by: David Hildenbrand
---
v1->v2:
- this is on top of "hw: Add compat machines for 7.2" from Cornelia Huck
(please queue afterwards)
- move fencing to 7.1
hw/s390x/s390-virtio-ccw.c | 1 +
target/s390x/cpu_features_def.h.inc | 1 +
target/s390x/gen-features.c | 2 ++
3 files changed, 4 insertions(+)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index bf1b36d824..9a2467c889 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -804,6 +804,7 @@ DEFINE_CCW_MACHINE(7_2, "7.2", true);
static void ccw_machine_7_1_instance_options(MachineState *machine)
{
ccw_machine_7_2_instance_options(machine);
+s390_cpudef_featoff_greater(16, 1, S390_FEAT_PAIE);
}
static void ccw_machine_7_1_class_options(MachineClass *mc)
diff --git a/target/s390x/cpu_features_def.h.inc
b/target/s390x/cpu_features_def.h.inc
index 3603e5fb12..e3cfe63735 100644
--- a/target/s390x/cpu_features_def.h.inc
+++ b/target/s390x/cpu_features_def.h.inc
@@ -114,6 +114,7 @@ DEF_FEAT(VECTOR_PACKED_DECIMAL_ENH2, "vxpdeh2", STFL, 192,
"Vector-Packed-Decima
DEF_FEAT(BEAR_ENH, "beareh", STFL, 193, "BEAR-enhancement facility")
DEF_FEAT(RDP, "rdp", STFL, 194, "Reset-DAT-protection facility")
DEF_FEAT(PAI, "pai", STFL, 196, "Processor-Activity-Instrumentation facility")
+DEF_FEAT(PAIE, "paie", STFL, 197, "Processor-Activity-Instrumentation
extension-1")
/* Features exposed via SCLP SCCB Byte 80 - 98 (bit numbers relative to
byte-80) */
DEF_FEAT(SIE_GSLS, "gsls", SCLP_CONF_CHAR, 40, "SIE:
Guest-storage-limit-suppression facility")
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index ad140184b9..1558c52626 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -575,6 +575,7 @@ static uint16_t full_GEN16_GA1[] = {
S390_FEAT_BEAR_ENH,
S390_FEAT_RDP,
S390_FEAT_PAI,
+S390_FEAT_PAIE,
};
@@ -669,6 +670,7 @@ static uint16_t default_GEN16_GA1[] = {
S390_FEAT_BEAR_ENH,
S390_FEAT_RDP,
S390_FEAT_PAI,
+S390_FEAT_PAIE,
};
/* QEMU (CPU model) features */
--
2.34.1
Re: [PATCH] s390x/cpumodel: add stfl197 processor-activity-instrumentation extension 1
Am 26.07.22 um 22:00 schrieb David Hildenbrand:
On 26.07.22 21:48, Christian Borntraeger wrote:
Add stfle 197 (processor-activity-instrumentation extension 1) to the
gen16 default model and fence it off for 7.0 and older.
QEMU is already in soft-freeze. I assume you want to get this still into
7.1. (decision not in my hands :) )
Right, 7.1 and 7.2 are both valid options.
Anyhow, if a re-target to the next release is required or not
Reviewed-by: David Hildenbrand
Signed-off-by: Christian Borntraeger
---
hw/s390x/s390-virtio-ccw.c | 1 +
target/s390x/cpu_features_def.h.inc | 1 +
target/s390x/gen-features.c | 2 ++
3 files changed, 4 insertions(+)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index cc3097bfee80..6268aa5d0888 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -806,6 +806,7 @@ static void ccw_machine_7_0_instance_options(MachineState
*machine)
static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V7_0 };
ccw_machine_7_1_instance_options(machine);
+s390_cpudef_featoff_greater(16, 1, S390_FEAT_PAIE);
s390_set_qemu_cpu_model(0x8561, 15, 1, qemu_cpu_feat);
}
diff --git a/target/s390x/cpu_features_def.h.inc b/target/s390x/cpu_features_def.h.inc
index 3603e5fb12c6..e3cfe637354b 100644
--- a/target/s390x/cpu_features_def.h.inc
+++ b/target/s390x/cpu_features_def.h.inc
@@ -114,6 +114,7 @@ DEF_FEAT(VECTOR_PACKED_DECIMAL_ENH2, "vxpdeh2", STFL, 192,
"Vector-Packed-Decima
DEF_FEAT(BEAR_ENH, "beareh", STFL, 193, "BEAR-enhancement facility")
DEF_FEAT(RDP, "rdp", STFL, 194, "Reset-DAT-protection facility")
DEF_FEAT(PAI, "pai", STFL, 196, "Processor-Activity-Instrumentation facility")
+DEF_FEAT(PAIE, "paie", STFL, 197, "Processor-Activity-Instrumentation
extension-1")
/* Features exposed via SCLP SCCB Byte 80 - 98 (bit numbers relative to byte-80) */
DEF_FEAT(SIE_GSLS, "gsls", SCLP_CONF_CHAR, 40, "SIE:
Guest-storage-limit-suppression facility")
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index ad140184b903..1558c5262616 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -575,6 +575,7 @@ static uint16_t full_GEN16_GA1[] = {
S390_FEAT_BEAR_ENH,
S390_FEAT_RDP,
S390_FEAT_PAI,
+S390_FEAT_PAIE,
};
@@ -669,6 +670,7 @@ static uint16_t default_GEN16_GA1[] = {
S390_FEAT_BEAR_ENH,
S390_FEAT_RDP,
S390_FEAT_PAI,
+S390_FEAT_PAIE,
};
/* QEMU (CPU model) features */
[PATCH] s390x/cpumodel: add stfl197 processor-activity-instrumentation extension 1
Add stfle 197 (processor-activity-instrumentation extension 1) to the
gen16 default model and fence it off for 7.0 and older.
Signed-off-by: Christian Borntraeger
---
hw/s390x/s390-virtio-ccw.c | 1 +
target/s390x/cpu_features_def.h.inc | 1 +
target/s390x/gen-features.c | 2 ++
3 files changed, 4 insertions(+)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index cc3097bfee80..6268aa5d0888 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -806,6 +806,7 @@ static void ccw_machine_7_0_instance_options(MachineState
*machine)
static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V7_0 };
ccw_machine_7_1_instance_options(machine);
+s390_cpudef_featoff_greater(16, 1, S390_FEAT_PAIE);
s390_set_qemu_cpu_model(0x8561, 15, 1, qemu_cpu_feat);
}
diff --git a/target/s390x/cpu_features_def.h.inc
b/target/s390x/cpu_features_def.h.inc
index 3603e5fb12c6..e3cfe637354b 100644
--- a/target/s390x/cpu_features_def.h.inc
+++ b/target/s390x/cpu_features_def.h.inc
@@ -114,6 +114,7 @@ DEF_FEAT(VECTOR_PACKED_DECIMAL_ENH2, "vxpdeh2", STFL, 192,
"Vector-Packed-Decima
DEF_FEAT(BEAR_ENH, "beareh", STFL, 193, "BEAR-enhancement facility")
DEF_FEAT(RDP, "rdp", STFL, 194, "Reset-DAT-protection facility")
DEF_FEAT(PAI, "pai", STFL, 196, "Processor-Activity-Instrumentation facility")
+DEF_FEAT(PAIE, "paie", STFL, 197, "Processor-Activity-Instrumentation
extension-1")
/* Features exposed via SCLP SCCB Byte 80 - 98 (bit numbers relative to
byte-80) */
DEF_FEAT(SIE_GSLS, "gsls", SCLP_CONF_CHAR, 40, "SIE:
Guest-storage-limit-suppression facility")
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index ad140184b903..1558c5262616 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -575,6 +575,7 @@ static uint16_t full_GEN16_GA1[] = {
S390_FEAT_BEAR_ENH,
S390_FEAT_RDP,
S390_FEAT_PAI,
+S390_FEAT_PAIE,
};
@@ -669,6 +670,7 @@ static uint16_t default_GEN16_GA1[] = {
S390_FEAT_BEAR_ENH,
S390_FEAT_RDP,
S390_FEAT_PAI,
+S390_FEAT_PAIE,
};
/* QEMU (CPU model) features */
--
2.36.1
Re: [PATCH] multifd: Copy pages before compressing them with zlib
Am 05.07.22 um 18:16 schrieb Dr. David Alan Gilbert: * Peter Maydell (peter.mayd...@linaro.org) wrote: On Mon, 4 Jul 2022 at 17:43, Ilya Leoshkevich wrote: zlib_send_prepare() compresses pages of a running VM. zlib does not make any thread-safety guarantees with respect to changing deflate() input concurrently with deflate() [1]. One can observe problems due to this with the IBM zEnterprise Data Compression accelerator capable zlib [2]. When the hardware acceleration is enabled, migration/multifd/tcp/plain/zlib test fails intermittently [3] due to sliding window corruption. The accelerator's architecture explicitly discourages concurrent accesses [4]: Page 26-57, "Other Conditions": As observed by this CPU, other CPUs, and channel programs, references to the parameter block, first, second, and third operands may be multiple-access references, accesses to these storage locations are not necessarily block-concurrent, and the sequence of these accesses or references is undefined. Mark Adler pointed out that vanilla zlib performs double fetches under certain circumstances as well [5], therefore we need to copy data before passing it to deflate(). [1] https://zlib.net/manual.html [2] https://github.com/madler/zlib/pull/410 [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-03/msg03988.html [4] http://publibfp.dhe.ibm.com/epubs/pdf/a227832c.pdf [5] https://gitlab.com/qemu-project/qemu/-/issues/1099 Is this [5] the wrong link? It's to our issue tracker, not zlib's or a zlib mailing list thread, and it doesn't contain any messages from Mark Adler. Looking at Mark's message, I'm not seeing that it was cc'd to the lists. I did however ask him to update zlib's docs to describe the requirement. Can you maybe forward the message here?
Re: [PATCH 1/2] pc-bios/s390-ccw/virtio-blkdev: Simplify/fix virtio_ipl_disk_is_valid()
Am 24.06.22 um 10:50 schrieb Thomas Huth:
The s390-ccw bios fails to boot if the boot disk is a virtio-blk
disk with a sector size of 4096. For example:
dasdfmt -b 4096 -d cdl -y -p -M quick /dev/dasdX
fdasd -a /dev/dasdX
install a guest onto /dev/dasdX1 using virtio-blk
qemu-system-s390x -nographic -hda /dev/dasdX1
Interestingly enough a real DASD (dasdX and not dasdX1) did work in the
past and I also successfully uses an NVMe disk. So I guess the NVMe
was 512 byte sector size then?
The bios then bails out with:
! Cannot read block 0 !
Looking at virtio_ipl_disk_is_valid() and especially the function
virtio_disk_is_scsi(), it does not really make sense that we expect
only such a limited disk geometry (like a block size of 512) for
out boot disks. Let's relax the check and allow everything that
remotely looks like a sane disk.
I agree that we should accept as much list-directed IPL formats as possible.
I have not fully looked into your changes though.
Signed-off-by: Thomas Huth
---
pc-bios/s390-ccw/virtio.h| 2 --
pc-bios/s390-ccw/virtio-blkdev.c | 41 ++--
2 files changed, 7 insertions(+), 36 deletions(-)
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index 19fceb6495..07fdcabd9f 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -186,8 +186,6 @@ void virtio_assume_scsi(void);
void virtio_assume_eckd(void);
void virtio_assume_iso9660(void);
-extern bool virtio_disk_is_scsi(void);
-extern bool virtio_disk_is_eckd(void);
extern bool virtio_ipl_disk_is_valid(void);
extern int virtio_get_block_size(void);
extern uint8_t virtio_get_heads(void);
diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
index 7d35050292..b5506bb29d 100644
--- a/pc-bios/s390-ccw/virtio-blkdev.c
+++ b/pc-bios/s390-ccw/virtio-blkdev.c
@@ -166,46 +166,19 @@ void virtio_assume_eckd(void)
virtio_eckd_sectors_for_block_size(vdev->config.blk.blk_size);
}
-bool virtio_disk_is_scsi(void)
-{
-VDev *vdev = virtio_get_device();
-
-if (vdev->guessed_disk_nature == VIRTIO_GDN_SCSI) {
-return true;
-}
-switch (vdev->senseid.cu_model) {
-case VIRTIO_ID_BLOCK:
-return (vdev->config.blk.geometry.heads == 255)
-&& (vdev->config.blk.geometry.sectors == 63)
-&& (virtio_get_block_size() == VIRTIO_SCSI_BLOCK_SIZE);
-case VIRTIO_ID_SCSI:
-return true;
-}
-return false;
-}
-
-bool virtio_disk_is_eckd(void)
+bool virtio_ipl_disk_is_valid(void)
{
+int blksize = virtio_get_block_size();
VDev *vdev = virtio_get_device();
-const int block_size = virtio_get_block_size();
-if (vdev->guessed_disk_nature == VIRTIO_GDN_DASD) {
+if (vdev->guessed_disk_nature == VIRTIO_GDN_SCSI ||
+vdev->guessed_disk_nature == VIRTIO_GDN_DASD) {
return true;
}
-switch (vdev->senseid.cu_model) {
-case VIRTIO_ID_BLOCK:
-return (vdev->config.blk.geometry.heads == 15)
-&& (vdev->config.blk.geometry.sectors ==
-virtio_eckd_sectors_for_block_size(block_size));
-case VIRTIO_ID_SCSI:
-return false;
-}
-return false;
-}
-bool virtio_ipl_disk_is_valid(void)
-{
-return virtio_disk_is_scsi() || virtio_disk_is_eckd();
+return (vdev->senseid.cu_model == VIRTIO_ID_BLOCK ||
+vdev->senseid.cu_model == VIRTIO_ID_SCSI) &&
+ blksize >= 512 && blksize <= 4096;
}
int virtio_get_block_size(void)
Re: [PATCH 1/2] pc-bios/s390-ccw/virtio: Set missing status bits while initializing
Am 23.06.22 um 09:11 schrieb Thomas Huth:
According chapter "3.1.1 Driver Requirements: Device Initialization"
of the Virtio specification (v1.1), a driver for a device has to set
the ACKNOWLEDGE and DRIVER bits in the status field after resetting
the device. The s390-ccw bios skipped these steps so far and seems
like QEMU never cared. Anyway, it's better to follow the spec, so
let's set these bits now in the right spots, too.
I have not tested that but I agree with this
Acked-by: Christian Borntraeger
Signed-off-by: Thomas Huth
---
pc-bios/s390-ccw/virtio.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index 5d2c6e3381..4e85a2eb82 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -220,7 +220,7 @@ int virtio_run(VDev *vdev, int vqid, VirtioCmd *cmd)
void virtio_setup_ccw(VDev *vdev)
{
int i, rc, cfg_size = 0;
-unsigned char status = VIRTIO_CONFIG_S_DRIVER_OK;
+uint8_t status;
struct VirtioFeatureDesc {
uint32_t features;
uint8_t index;
@@ -234,6 +234,10 @@ void virtio_setup_ccw(VDev *vdev)
run_ccw(vdev, CCW_CMD_VDEV_RESET, NULL, 0, false);
+status = VIRTIO_CONFIG_S_ACKNOWLEDGE;
+rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, , sizeof(status), false);
+IPL_assert(rc == 0, "Could not write ACKNOWLEDGE status to host");
+
switch (vdev->senseid.cu_model) {
case VIRTIO_ID_NET:
vdev->nr_vqs = 2;
@@ -253,6 +257,11 @@ void virtio_setup_ccw(VDev *vdev)
default:
panic("Unsupported virtio device\n");
}
+
+status |= VIRTIO_CONFIG_S_DRIVER;
+rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, , sizeof(status), false);
+IPL_assert(rc == 0, "Could not write DRIVER status to host");
+
IPL_assert(
run_ccw(vdev, CCW_CMD_READ_CONF, >config, cfg_size, false) == 0,
"Could not get block device configuration");
@@ -291,9 +300,10 @@ void virtio_setup_ccw(VDev *vdev)
run_ccw(vdev, CCW_CMD_SET_VQ, , sizeof(info), false) == 0,
"Cannot set VQ info");
}
-IPL_assert(
-run_ccw(vdev, CCW_CMD_WRITE_STATUS, , sizeof(status), false) ==
0,
-"Could not write status to host");
+
+status |= VIRTIO_CONFIG_S_DRIVER_OK;
+rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, , sizeof(status), false);
+IPL_assert(rc == 0, "Could not write DRIVER_OK status to host");
}
bool virtio_is_supported(SubChannelId schid)
Re: [PATCH 2/2] target/s390x: kvm: Honor storage keys during emulation
Am 24.05.22 um 12:43 schrieb Thomas Huth:
On 19/05/2022 15.53, Janis Schoetterl-Glausch wrote:
On 5/19/22 12:05, Thomas Huth wrote:
On 06/05/2022 17.39, Janis Schoetterl-Glausch wrote:
Storage key controlled protection is currently not honored when
emulating instructions.
If available, enable key protection for the MEM_OP ioctl, thereby
enabling it for the s390_cpu_virt_mem_* functions, when using kvm.
As a result, the emulation of the following instructions honors storage
keys:
* CLP
The Synch I/O CLP command would need special handling in order
to support storage keys, but is currently not supported.
* CHSC
Performing commands asynchronously would require special
handling, but commands are currently always synchronous.
* STSI
* TSCH
Must (and does) not change channel if terminated due to
protection.
* MSCH
Suppressed on protection, works because fetching instruction.
* SSCH
Suppressed on protection, works because fetching instruction.
* STSCH
* STCRW
Suppressed on protection, this works because no partial store is
possible, because the operand cannot span multiple pages.
* PCISTB
* MPCIFC
* STPCIFC
Signed-off-by: Janis Schoetterl-Glausch
---
target/s390x/kvm/kvm.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 53098bf541..7bd8db0e7b 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -151,12 +151,15 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[]
= {
static int cap_sync_regs;
static int cap_async_pf;
static int cap_mem_op;
+static int cap_mem_op_extension;
static int cap_s390_irq;
static int cap_ri;
static int cap_hpage_1m;
static int cap_vcpu_resets;
static int cap_protected;
+static bool mem_op_storage_key_support;
+
static int active_cmma;
static int kvm_s390_query_mem_limit(uint64_t *memory_limit)
@@ -354,6 +357,8 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
+ cap_mem_op_extension = kvm_check_extension(s,
KVM_CAP_S390_MEM_OP_EXTENSION);
+ mem_op_storage_key_support = cap_mem_op_extension > 0;
Ah, so KVM_CAP_S390_MEM_OP_EXTENSION is a "version number", not a boolean flag?
... ok, now I've finally understood that ... ;-)
Yeah, potentially having a bunch of memop capabilities didn't seem nice to me.
We can remove extensions if, when introducing an extension, we define that
version x supports functionality y, z...,
but for the storage keys I've written in api.rst that it's supported if the cap
> 0.
So we'd need a new cap if we want to get rid of the skey extension and still
support some other extension,
but that doesn't seem particularly likely.
Oh well, never say that ... we've seen it in the past, that sometimes we want
to get rid of features again, and if they don't have a separate feature flag
bit somewhere, it's getting very ugly to disable them again.
So since we don't have merged this patch yet, and thus we don't have a public
userspace program using this interface yet, this is our last chance to redefine
this interface before we might regret it later.
I'm in strong favor of treating the KVM_CAP_S390_MEM_OP_EXTENSION as a flag
field instead of a version number. What do others think? Christian? Halil?
Its too late for that. This is part of 5.18.
Re: [PATCH v5 00/11] s390x/tcg: Implement Vector-Enhancements Facility 2
Am 23.03.22 um 14:57 schrieb David Miller:
Implement Vector-Enhancements Facility 2 for s390x
resolves: https://gitlab.com/qemu-project/qemu/-/issues/738
implements:
VECTOR LOAD ELEMENTS REVERSED (VLER)
VECTOR LOAD BYTE REVERSED ELEMENTS (VLBR)
VECTOR LOAD BYTE REVERSED ELEMENT (VLEBRH, VLEBRF, VLEBRG)
VECTOR LOAD BYTE REVERSED ELEMENT AND ZERO (VLLEBRZ)
VECTOR LOAD BYTE REVERSED ELEMENT AND REPLICATE (VLBRREP)
VECTOR STORE ELEMENTS REVERSED (VSTER)
VECTOR STORE BYTE REVERSED ELEMENTS (VSTBR)
VECTOR STORE BYTE REVERSED ELEMENTS (VSTEBRH, VSTEBRF, VSTEBRG)
VECTOR SHIFT LEFT DOUBLE BY BIT (VSLD)
VECTOR SHIFT RIGHT DOUBLE BY BIT(VSRD)
VECTOR STRING SEARCH(VSTRS)
modifies:
VECTOR FP CONVERT FROM FIXED(VCFPS)
VECTOR FP CONVERT FROM LOGICAL (VCFPL)
VECTOR FP CONVERT TO FIXED (VCSFP)
VECTOR FP CONVERT TO LOGICAL(VCLFP)
VECTOR SHIFT LEFT (VSL)
VECTOR SHIFT RIGHT ARITHMETIC (VSRA)
VECTOR SHIFT RIGHT LOGICAL (VSRL)
David Miller (9):
tcg: Implement tcg_gen_{h,w}swap_{i32,i64}
target/s390x: vxeh2: vector convert short/32b
target/s390x: vxeh2: vector string search
target/s390x: vxeh2: Update for changes to vector shifts
target/s390x: vxeh2: vector shift double by bit
target/s390x: vxeh2: vector {load, store} elements reversed
target/s390x: vxeh2: vector {load, store} byte reversed elements
target/s390x: vxeh2: vector {load, store} byte reversed element
target/s390x: add S390_FEAT_VECTOR_ENH2 to qemu CPU model
tests/tcg/s390x: Tests for Vector Enhancements Facility 2
target/s390x: Fix writeback to v1 in helper_vstl
Richard Henderson (2):
tcg: Implement tcg_gen_{h,w}swap_{i32,i64}
target/s390x: Fix writeback to v1 in helper_vstl
I guess we can now re-do this series against 7.1-devel (qemu/master) which does
have the machine compat changes. Apart from that this should be ready now?
Re: [PATCH v4 10/11] tests/tcg/s390x: Tests for Vector Enhancements Facility 2
Am 01.04.22 um 17:02 schrieb David Miller: vrr is almost a perfect match (it is for this, larger than imm4 would need to be split). .long : this would be uglier. use enough to be filled with nops after ? or use a 32b and 16b instead if it's in .text it should make no difference. I will let Richard or David decide what they prefer.
Re: [PATCH v4 10/11] tests/tcg/s390x: Tests for Vector Enhancements Facility 2
Am 01.04.22 um 04:15 schrieb David Miller:
Hi,
There is some issue with instruction sub/alt encodings not matching,
but I worked around it easily.
I'm dropping the updated patch for the tests in here.
I know I should resend the entire patch series as a higher version
really, and will do so.
I'm hoping someone can tell me if it's ok to use .insn vrr in place
of vri(-d) as it doesn't match vri.
[https://sourceware.org/binutils/docs-2.37/as/s390-Formats.html]
.insn doesn't deal with sub encodings and there is no good alternative
that I know of.
example:
/* vri-d as vrr */
asm volatile(".insn vrr, 0xE786, %[v1], %[v2], %[v3], 0, %[I], 0\n"
: [v1] "=v" (v1->v)
: [v2] "v" (v2->v)
, [v3] "v" (v3->v)
, [I] "i" (I & 7));
Patch is attached
Yes, vri sucks and does not work with vrsd. Maybe just use .long which is
probably
better than using a "wrong" format.
Opinions?
Re: qemu iotest 161 and make check
Am 31.03.22 um 09:44 schrieb Christian Borntraeger:
Am 21.02.22 um 11:27 schrieb Christian Borntraeger:
Am 10.02.22 um 18:44 schrieb Vladimir Sementsov-Ogievskiy:
10.02.2022 20:13, Thomas Huth wrote:
On 10/02/2022 15.51, Christian Borntraeger wrote:
Am 10.02.22 um 15:47 schrieb Vladimir Sementsov-Ogievskiy:
10.02.2022 10:57, Christian Borntraeger wrote:
Hello,
I do see spurious failures of 161 in our CI, but only when I use
make check with parallelism (-j).
I have not yet figured out which other testcase could interfere
@@ -34,6 +34,8 @@
*** Commit and then change an option on the backing file
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+qemu-img: TEST_DIR/t.IMGFMT.base: Failed to get "write" lock
FWIW, qemu_lock_fd_test returns -11 (EAGAIN)
and raw_check_lock_bytes spits this error.
And its coming from here (ret is 0)
int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive)
{
int ret;
struct flock fl = {
.l_whence = SEEK_SET,
.l_start = start,
.l_len= len,
.l_type = exclusive ? F_WRLCK : F_RDLCK,
};
qemu_probe_lock_ops();
ret = fcntl(fd, fcntl_op_getlk, );
if (ret == -1) {
return -errno;
} else {
->return fl.l_type == F_UNLCK ? 0 : -EAGAIN;
}
}
Is this just some overload situation that we do not recover because we do not
handle EAGAIN any special.
Re: qemu iotest 161 and make check
Am 21.02.22 um 11:27 schrieb Christian Borntraeger: Am 10.02.22 um 18:44 schrieb Vladimir Sementsov-Ogievskiy: 10.02.2022 20:13, Thomas Huth wrote: On 10/02/2022 15.51, Christian Borntraeger wrote: Am 10.02.22 um 15:47 schrieb Vladimir Sementsov-Ogievskiy: 10.02.2022 10:57, Christian Borntraeger wrote: Hello, I do see spurious failures of 161 in our CI, but only when I use make check with parallelism (-j). I have not yet figured out which other testcase could interfere @@ -34,6 +34,8 @@ *** Commit and then change an option on the backing file Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576 +qemu-img: TEST_DIR/t.IMGFMT.base: Failed to get "write" lock FWIW, qemu_lock_fd_test returns -11 (EAGAIN) and raw_check_lock_bytes spits this error. Is this just some overload situation that we do not recover because we do not handle EAGAIN any special.
Re: [PATCH] multifd: Copy pages before compressing them with zlib
Peter, Alex this is the fallout of Ilyas analysis of the s390x migration issue
that triggered the DFLTCC workaround.
Am 29.03.22 um 17:21 schrieb Ilya Leoshkevich:
zlib_send_prepare() compresses pages of a running VM. zlib does not
make any thread-safety guarantees with respect to changing deflate()
input concurrently with deflate() [1].
One can observe problems due to this with the IBM zEnterprise Data
Compression accelerator capable zlib [2]. When the hardware
acceleration is enabled, migration/multifd/tcp/zlib test fails
intermittently [3] due to sliding window corruption.
At the moment this problem occurs only with this accelerator, since
its architecture explicitly discourages concurrent accesses [4]:
Page 26-57, "Other Conditions":
As observed by this CPU, other CPUs, and channel
programs, references to the parameter block, first,
second, and third operands may be multiple-access
references, accesses to these storage locations are
not necessarily block-concurrent, and the sequence
of these accesses or references is undefined.
Still, it might affect other platforms due to a future zlib update.
Therefore, copy the page being compressed into a private buffer before
passing it to zlib.
[1] https://zlib.net/manual.html
[2] https://github.com/madler/zlib/pull/410
[3] https://lists.nongnu.org/archive/html/qemu-devel/2022-03/msg03988.html
[4] http://publibfp.dhe.ibm.com/epubs/pdf/a227832c.pdf
Signed-off-by: Ilya Leoshkevich
---
migration/multifd-zlib.c | 35 ++-
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
index 3a7ae44485..b6b22b7d1f 100644
--- a/migration/multifd-zlib.c
+++ b/migration/multifd-zlib.c
@@ -27,6 +27,8 @@ struct zlib_data {
uint8_t *zbuff;
/* size of compressed buffer */
uint32_t zbuff_len;
+/* uncompressed buffer */
+uint8_t buf[];
};
/* Multifd zlib compression */
@@ -43,9 +45,18 @@ struct zlib_data {
*/
static int zlib_send_setup(MultiFDSendParams *p, Error **errp)
{
-struct zlib_data *z = g_new0(struct zlib_data, 1);
-z_stream *zs = >zs;
+/* This is the maximum size of the compressed buffer */
+uint32_t zbuff_len = compressBound(MULTIFD_PACKET_SIZE);
+size_t buf_len = qemu_target_page_size();
+struct zlib_data *z;
+z_stream *zs;
+z = g_try_malloc0(sizeof(struct zlib_data) + buf_len + zbuff_len);
+if (!z) {
+error_setg(errp, "multifd %u: out of memory for zlib_data", p->id);
+return -1;
+}
+zs = >zs;
zs->zalloc = Z_NULL;
zs->zfree = Z_NULL;
zs->opaque = Z_NULL;
@@ -54,15 +65,8 @@ static int zlib_send_setup(MultiFDSendParams *p, Error
**errp)
error_setg(errp, "multifd %u: deflate init failed", p->id);
return -1;
}
-/* This is the maxium size of the compressed buffer */
-z->zbuff_len = compressBound(MULTIFD_PACKET_SIZE);
-z->zbuff = g_try_malloc(z->zbuff_len);
-if (!z->zbuff) {
-deflateEnd(>zs);
-g_free(z);
-error_setg(errp, "multifd %u: out of memory for zbuff", p->id);
-return -1;
-}
+z->zbuff_len = zbuff_len;
+z->zbuff = z->buf + buf_len;
p->data = z;
return 0;
}
@@ -80,7 +84,6 @@ static void zlib_send_cleanup(MultiFDSendParams *p, Error
**errp)
struct zlib_data *z = p->data;
deflateEnd(>zs);
-g_free(z->zbuff);
z->zbuff = NULL;
g_free(p->data);
p->data = NULL;
@@ -114,8 +117,14 @@ static int zlib_send_prepare(MultiFDSendParams *p, Error
**errp)
flush = Z_SYNC_FLUSH;
}
+/*
+ * Since the VM might be running, the page may be changing concurrently
+ * with compression. zlib does not guarantee that this is safe,
+ * therefore copy the page before calling deflate().
+ */
+memcpy(z->buf, p->pages->block->host + p->normal[i], page_size);
zs->avail_in = page_size;
-zs->next_in = p->pages->block->host + p->normal[i];
+zs->next_in = z->buf;
zs->avail_out = available;
zs->next_out = z->zbuff + out_size;
Re: [PULL 00/18] migration queue
Am 08.03.22 um 19:47 schrieb Dr. David Alan Gilbert: * Philippe Mathieu-Daudé (philippe.mathieu.da...@gmail.com) wrote: On 3/3/22 15:46, Peter Maydell wrote: On Wed, 2 Mar 2022 at 18:32, Dr. David Alan Gilbert (git) wrote: From: "Dr. David Alan Gilbert" The following changes since commit 64ada298b98a51eb2512607f6e6180cb330c47b1: Merge remote-tracking branch 'remotes/legoater/tags/pull-ppc-20220302' into staging (2022-03-02 12:38:46 +) are available in the Git repository at: https://gitlab.com/dagrh/qemu.git tags/pull-migration-20220302b for you to fetch changes up to 18621987027b1800f315fb9e29967e7b5398ef6f: migration: Remove load_state_old and minimum_version_id_old (2022-03-02 18:20:45 +) Migration/HMP/Virtio pull 2022-03-02 A bit of a mix this time: * Minor fixes from myself, Hanna, and Jack * VNC password rework by Stefan and Fabian * Postcopy changes from Peter X that are the start of a larger series to come * Removing the prehistoic load_state_old code from Peter M I'm seeing an error on the s390x runner: ▶ 26/547 ERROR:../tests/qtest/migration-test.c:276:check_guests_ram: assertion failed: (bad == 0) ERROR 26/547 qemu:qtest+qtest-i386 / qtest-i386/migration-testERROR 78.87s killed by signal 6 SIGABRT https://app.travis-ci.com/gitlab/qemu-project/qemu/jobs/562515884#L7848 Yeh, thuth mentioned that, it seems to only be s390 which is odd. I'm not seeing anything obviously architecture dependent in that set, or for that matter that plays with the ram migration stream much. Is this reliable enough that someone with a tame s390 could bisect? I just asked Peter to try with DFLTCC=0 to disable the hardware acceleration. Maybe the zlib library still has a bug? (We are not aware of any problem right now). In case DFLTCC makes a difference, this would be something for Ilya to look at.
Re: [PATCH 3/3] tests/tcg/s390x: Test BRASL and BRCL with large negative offsets
Am 11.03.22 um 21:32 schrieb Richard Henderson: On 3/11/22 10:49, Ilya Leoshkevich wrote: + size_t length = 0x10006; + unsigned char *buf; + int i; + + buf = mmap(NULL, length, PROT_READ | PROT_WRITE | PROT_EXEC, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + assert(buf != MAP_FAILED); I'm thinking exit success here, as such a large allocation may well fail depending on the host. What about using MAP_NORESERVE ?
Re: [PATCH v6 1/4] s390x/tcg: Implement Miscellaneous-Instruction-Extensions Facility 3 for the s390x
Am 23.02.22 um 23:29 schrieb David Miller: Yes I'm adding to this patch, I haven't quite figured out where to put them, they are inline to various things in the patch themselves so I'm putting in the cover letter under the patch they go to. I hope that's correct. You usually put it under your Signed-off-by: line in the patch. I think Thomas can fixup when applying. Thanks - David Miller On Wed, Feb 23, 2022 at 8:40 AM Christian Borntraeger wrote: Am 18.02.22 um 00:17 schrieb David Miller: resolves: https://gitlab.com/qemu-project/qemu/-/issues/737 implements: AND WITH COMPLEMENT (NCRK, NCGRK) NAND (NNRK, NNGRK) NOT EXCLUSIVE OR (NXRK, NXGRK) NOR (NORK, NOGRK) OR WITH COMPLEMENT(OCRK, OCGRK) SELECT(SELR, SELGR) SELECT HIGH (SELFHR) MOVE RIGHT TO LEFT(MVCRL) POPULATION COUNT (POPCNT) Signed-off-by: David Miller For your next patches, feel free to add previous Reviewed-by: tags so that others can see what review has already happened.
Re: [PATCH v6 1/4] s390x/tcg: Implement Miscellaneous-Instruction-Extensions Facility 3 for the s390x
Am 18.02.22 um 00:17 schrieb David Miller: resolves: https://gitlab.com/qemu-project/qemu/-/issues/737 implements: AND WITH COMPLEMENT (NCRK, NCGRK) NAND (NNRK, NNGRK) NOT EXCLUSIVE OR (NXRK, NXGRK) NOR (NORK, NOGRK) OR WITH COMPLEMENT(OCRK, OCGRK) SELECT(SELR, SELGR) SELECT HIGH (SELFHR) MOVE RIGHT TO LEFT(MVCRL) POPULATION COUNT (POPCNT) Signed-off-by: David Miller For your next patches, feel free to add previous Reviewed-by: tags so that others can see what review has already happened.
Re: [RFC PATCH] gitlab: upgrade the job definition for s390x to 20.04
Am 22.02.22 um 00:06 schrieb Alex Bennée:
The new s390x machine has more of everything including the OS. As
18.04 will soon be going we might as well get onto something moderately
modern.
Signed-off-by: Alex Bennée
Cc: Christian Borntraeger
Cc: Peter Maydell
Looks sane,
Acked-by: Christian Borntraeger
---
.gitlab-ci.d/custom-runners.yml | 2 +-
...18.04-s390x.yml => ubuntu-20.04-s390x.yml} | 28 +--
2 files changed, 15 insertions(+), 15 deletions(-)
rename .gitlab-ci.d/custom-runners/{ubuntu-18.04-s390x.yml =>
ubuntu-20.04-s390x.yml} (87%)
diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
index 056c374619..3e76a2034a 100644
--- a/.gitlab-ci.d/custom-runners.yml
+++ b/.gitlab-ci.d/custom-runners.yml
@@ -14,6 +14,6 @@ variables:
GIT_STRATEGY: clone
include:
- - local: '/.gitlab-ci.d/custom-runners/ubuntu-18.04-s390x.yml'
+ - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml'
- local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml'
- local: '/.gitlab-ci.d/custom-runners/centos-stream-8-x86_64.yml'
diff --git a/.gitlab-ci.d/custom-runners/ubuntu-18.04-s390x.yml
b/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
similarity index 87%
rename from .gitlab-ci.d/custom-runners/ubuntu-18.04-s390x.yml
rename to .gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
index f39d874a1e..0333872113 100644
--- a/.gitlab-ci.d/custom-runners/ubuntu-18.04-s390x.yml
+++ b/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
@@ -1,12 +1,12 @@
-# All ubuntu-18.04 jobs should run successfully in an environment
+# All ubuntu-20.04 jobs should run successfully in an environment
# setup by the scripts/ci/setup/build-environment.yml task
-# "Install basic packages to build QEMU on Ubuntu 18.04/20.04"
+# "Install basic packages to build QEMU on Ubuntu 20.04/20.04"
-ubuntu-18.04-s390x-all-linux-static:
+ubuntu-20.04-s390x-all-linux-static:
needs: []
stage: build
tags:
- - ubuntu_18.04
+ - ubuntu_20.04
- s390x
rules:
- if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~
/^staging/'
@@ -21,11 +21,11 @@ ubuntu-18.04-s390x-all-linux-static:
- make --output-sync -j`nproc` check V=1
- make --output-sync -j`nproc` check-tcg V=1
-ubuntu-18.04-s390x-all:
+ubuntu-20.04-s390x-all:
needs: []
stage: build
tags:
- - ubuntu_18.04
+ - ubuntu_20.04
- s390x
rules:
- if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~
/^staging/'
@@ -37,11 +37,11 @@ ubuntu-18.04-s390x-all:
- make --output-sync -j`nproc`
- make --output-sync -j`nproc` check V=1
-ubuntu-18.04-s390x-alldbg:
+ubuntu-20.04-s390x-alldbg:
needs: []
stage: build
tags:
- - ubuntu_18.04
+ - ubuntu_20.04
- s390x
rules:
- if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~
/^staging/'
@@ -58,11 +58,11 @@ ubuntu-18.04-s390x-alldbg:
- make --output-sync -j`nproc`
- make --output-sync -j`nproc` check V=1
-ubuntu-18.04-s390x-clang:
+ubuntu-20.04-s390x-clang:
needs: []
stage: build
tags:
- - ubuntu_18.04
+ - ubuntu_20.04
- s390x
rules:
- if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~
/^staging/'
@@ -78,11 +78,11 @@ ubuntu-18.04-s390x-clang:
- make --output-sync -j`nproc`
- make --output-sync -j`nproc` check V=1
-ubuntu-18.04-s390x-tci:
+ubuntu-20.04-s390x-tci:
needs: []
stage: build
tags:
- - ubuntu_18.04
+ - ubuntu_20.04
- s390x
rules:
- if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~
/^staging/'
@@ -97,11 +97,11 @@ ubuntu-18.04-s390x-tci:
- ../configure --disable-libssh --enable-tcg-interpreter
- make --output-sync -j`nproc`
-ubuntu-18.04-s390x-notcg:
+ubuntu-20.04-s390x-notcg:
needs: []
stage: build
tags:
- - ubuntu_18.04
+ - ubuntu_20.04
- s390x
rules:
- if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~
/^staging/'
Re: qemu iotest 161 and make check
Am 10.02.22 um 18:44 schrieb Vladimir Sementsov-Ogievskiy:
10.02.2022 20:13, Thomas Huth wrote:
On 10/02/2022 15.51, Christian Borntraeger wrote:
Am 10.02.22 um 15:47 schrieb Vladimir Sementsov-Ogievskiy:
10.02.2022 10:57, Christian Borntraeger wrote:
Hello,
I do see spurious failures of 161 in our CI, but only when I use
make check with parallelism (-j).
I have not yet figured out which other testcase could interfere
@@ -34,6 +34,8 @@
*** Commit and then change an option on the backing file
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+qemu-img: TEST_DIR/t.IMGFMT.base: Failed to get "write" lock
+Is another process using the image [TEST_DIR/t.IMGFMT.base]?
Formatting 'TEST_DIR/t.IMGFMT.int', fmt=IMGFMT size=1048576
backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
backing_file=TEST_DIR/t.IMGFMT.int backing_fmt=IMGFMT
{ 'execute': 'qmp_capabilities' }
any ideas?
Hmm, interesting.. Is it always 161 and always exactly this diff?
Its always 161 and only 161. I would need to check if its always the same error.
First, this place in 161 is usual: we just create and image, like in many other
tests.
Second, why _make_test_img trigger "Failed to get write lock"? It should just
create an image. Hmm. And probably starts QSD if protocol is fuse. So, that start of QSD
may probably fail.. Is that the case? What is image format and protocol used in test run?
But anyway, tests running in parallel should not break each other as each test
has own TEST_DIR and SOCK_DIR..
Unless you run into the issue that Hanna described here:
https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01735.html
Yes, we can't execute same test several times (for different formats) in
parallel.. But that's about any test, not only 161.
And I don't think that it's currently possible that we run same test in
parallel several times somewhere, do we? In tests/check-block.sh we have a
sequential loop through $format_list ..
FWIW, I was able to bisect this and it came in with
bcda7b178fde7797f476e3b066fe5fc76bfa1c43 is the first bad commit
commit bcda7b178fde7797f476e3b066fe5fc76bfa1c43
Author: Vladimir Sementsov-Ogievskiy
Date: Thu Dec 23 19:39:33 2021 +0100
check-block.sh: passthrough -jN flag of make to -j N flag of check
This improves performance of running iotests during "make -jN check".
Signed-off-by: Vladimir Sementsov-Ogievskiy
Message-Id: <20211223183933.1497037-1-vsement...@virtuozzo.com>
Signed-off-by: Paolo Bonzini
tests/check-block.sh | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
With
make check-block -j 100
it reproduced pretty quickly for me.
Re: [PATCH v5 1/3] s390x/tcg: Implement Miscellaneous-Instruction-Extensions Facility 3 for the s390x
Am 16.02.22 um 21:34 schrieb David Miller:
resolves: https://gitlab.com/qemu-project/qemu/-/issues/737
implements:
AND WITH COMPLEMENT (NCRK, NCGRK)
NAND (NNRK, NNGRK)
NOT EXCLUSIVE OR (NXRK, NXGRK)
NOR (NORK, NOGRK)
OR WITH COMPLEMENT(OCRK, OCGRK)
SELECT(SELR, SELGR)
SELECT HIGH (SELFHR)
MOVE RIGHT TO LEFT(MVCRL)
POPULATION COUNT (POPCNT)
Signed-off-by: David Miller
---
target/s390x/gen-features.c| 1 +
target/s390x/helper.h | 1 +
target/s390x/tcg/insn-data.def | 30 +--
target/s390x/tcg/mem_helper.c | 20 +
target/s390x/tcg/translate.c | 53 --
5 files changed, 100 insertions(+), 5 deletions(-)
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 7cb1a6ec10..a3f30f69d9 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -740,6 +740,7 @@ static uint16_t qemu_LATEST[] = {
/* add all new definitions before this point */
static uint16_t qemu_MAX[] = {
+S390_FEAT_MISC_INSTRUCTION_EXT3,
/* generates a dependency warning, leave it out for now */
S390_FEAT_MSA_EXT_5,
};
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 271b081e8c..69f69cf718 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -4,6 +4,7 @@ DEF_HELPER_FLAGS_4(nc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(oc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(xc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(mvc, TCG_CALL_NO_WG, void, env, i32, i64, i64)
+DEF_HELPER_FLAGS_4(mvcrl, TCG_CALL_NO_WG, void, env, i64, i64, i64)
DEF_HELPER_FLAGS_4(mvcin, TCG_CALL_NO_WG, void, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(clc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
DEF_HELPER_3(mvcl, i32, env, i32, i32)
diff --git a/target/s390x/tcg/insn-data.def b/target/s390x/tcg/insn-data.def
index 1c3e115712..efb1d5bc19 100644
--- a/target/s390x/tcg/insn-data.def
+++ b/target/s390x/tcg/insn-data.def
@@ -105,6 +105,9 @@
D(0xa507, NILL,RI_a, Z, r1_o, i2_16u, r1, 0, andi, 0, 0x1000)
D(0x9400, NI, SI,Z, la1, i2_8u, new, 0, ni, nz64, MO_UB)
D(0xeb54, NIY, SIY, LD, la1, i2_8u, new, 0, ni, nz64, MO_UB)
+/* AND WITH COMPLEMENT */
+C(0xb9f5, NCRK,RRF_a, MIE3, r2, r3, new, r1_32, andc, nz32)
+C(0xb9e5, NCGRK, RRF_a, MIE3, r2, r3, r1, 0, andc, nz64)
/* BRANCH AND LINK */
C(0x0500, BALR,RR_a, Z, 0, r2_nz, r1, 0, bal, 0)
@@ -640,6 +643,8 @@
C(0xeb8e, MVCLU, RSY_a, E2, 0, a2, 0, 0, mvclu, 0)
/* MOVE NUMERICS */
C(0xd100, MVN, SS_a, Z, la1, a2, 0, 0, mvn, 0)
+/* MOVE RIGHT TO LEFT */
+C(0xe50a, MVCRL, SSE, MIE3, la1, a2, 0, 0, mvcrl, 0)
/* MOVE PAGE */
C(0xb254, MVPG,RRE, Z, 0, 0, 0, 0, mvpg, 0)
/* MOVE STRING */
@@ -707,6 +712,16 @@
F(0xed0f, MSEB,RXF, Z, e1, m2_32u, new, e1, mseb, 0, IF_BFP)
F(0xed1f, MSDB,RXF, Z, f1, m2_64, new, f1, msdb, 0, IF_BFP)
+/* NAND */
+C(0xb974, NNRK,RRF_a, MIE3, r2, r3, new, r1_32, nand, nz32)
+C(0xb964, NNGRK, RRF_a, MIE3, r2, r3, r1, 0, nand, nz64)
+/* NOR */
+C(0xb976, NORK,RRF_a, MIE3, r2, r3, new, r1_32, nor, nz32)
+C(0xb966, NOGRK, RRF_a, MIE3, r2, r3, r1, 0, nor, nz64)
+/* NOT EXCLUSIVE OR */
+C(0xb977, NXRK,RRF_a, MIE3, r2, r3, new, r1_32, nxor, nz32)
+C(0xb967, NXGRK, RRF_a, MIE3, r2, r3, r1, 0, nxor, nz64)
+
/* OR */
C(0x1600, OR, RR_a, Z, r1, r2, new, r1_32, or, nz32)
C(0xb9f6, ORK, RRF_a, DO, r2, r3, new, r1_32, or, nz32)
@@ -725,6 +740,9 @@
D(0xa50b, OILL,RI_a, Z, r1_o, i2_16u, r1, 0, ori, 0, 0x1000)
D(0x9600, OI, SI,Z, la1, i2_8u, new, 0, oi, nz64, MO_UB)
D(0xeb56, OIY, SIY, LD, la1, i2_8u, new, 0, oi, nz64, MO_UB)
+/* OR WITH COMPLEMENT */
+C(0xb975, OCRK,RRF_a, MIE3, r2, r3, new, r1_32, orc, nz32)
+C(0xb965, OCGRK, RRF_a, MIE3, r2, r3, r1, 0, orc, nz64)
/* PACK */
/* Really format SS_b, but we pack both lengths into one argument
@@ -735,6 +753,9 @@
/* PACK UNICODE */
C(0xe100, PKU, SS_f, E2, la1, a2, 0, 0, pku, 0)
+/* POPULATION COUNT */
+C(0xb9e1, POPCNT, RRF_c, PC, 0, r2_o, r1, 0, popcnt, nz64)
+
/* PREFETCH */
/* Implemented as nops of course. */
C(0xe336, PFD, RXY_b, GIE, 0, 0, 0, 0, 0, 0)
@@ -743,9 +764,6 @@
/* Implemented as nop of course. */
C(0xb2e8, PPA, RRF_c, PPA, 0, 0, 0, 0, 0, 0)
-/* POPULATION COUNT */
-C(0xb9e1, POPCNT, RRE, PC, 0, r2_o, r1, 0, popcnt, nz64)
-
/* ROTATE LEFT SINGLE LOGICAL */
C(0xeb1d, RLL, RSY_a, Z, r3_o, sh, new, r1_32, rll32, 0)
C(0xeb1c, RLLG,RSY_a, Z, r3_o, sh, r1, 0, rll64, 0)
@@ -765,6 +783,12 @@
/* SEARCH STRING UNICODE */
C(0xb9be, SRSTU, RRE, ETF3, 0, 0, 0, 0, srstu, 0)
+/*
Re: [PATCH v3 3/3] s390x/tcg/tests: Tests for Miscellaneous-Instruction-Extensions Facility 3
Am 16.02.22 um 10:17 schrieb David Hildenbrand: On 15.02.22 21:27, David Miller wrote: tests/tcg/s390x/mie3-compl.c: [N]*K instructions tests/tcg/s390x/mie3-mvcrl.c: MVCRL instruction tests/tcg/s390x/mie3-sel.c: SELECT instruction Signed-off-by: David Miller --- tests/tcg/s390x/Makefile.target | 2 +- tests/tcg/s390x/mie3-compl.c| 56 + tests/tcg/s390x/mie3-mvcrl.c| 31 ++ tests/tcg/s390x/mie3-sel.c | 42 + 4 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/s390x/mie3-compl.c create mode 100644 tests/tcg/s390x/mie3-mvcrl.c create mode 100644 tests/tcg/s390x/mie3-sel.c diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target index 1a7238b4eb..16b9d45307 100644 --- a/tests/tcg/s390x/Makefile.target +++ b/tests/tcg/s390x/Makefile.target @@ -1,6 +1,6 @@ S390X_SRC=$(SRC_PATH)/tests/tcg/s390x VPATH+=$(S390X_SRC) -CFLAGS+=-march=zEC12 -m64 +CFLAGS+=-march=z15 -m64 Unfortunately, this makes our docker builds unhappy -- fail. I assume the compiler in the container is outdated. $ make run-tcg-tests-s390x-linux-user changing dir to build for make "run-tcg-tests-s390x-linux-user"... make[1]: Entering directory '/home/dhildenb/git/qemu/build' GIT ui/keycodemapdb tests/fp/berkeley-testfloat-3 tests/fp/berkeley-softfloat-3 dtc capstone slirp BUILD debian10 BUILD debian-s390x-cross BUILD TCG tests for s390x-linux-user CHECK debian10 CHECK debian-s390x-cross BUILD s390x-linux-user guest-tests with docker qemu/debian-s390x-cross s390x-linux-gnu-gcc: error: unrecognized argument in option '-march=z15' s390x-linux-gnu-gcc: note: valid arguments to '-march=' are: arch10 arch11 arch12 arch3 arch5 arch6 arch7 arch8 arch9 g5 g6 native z10 z13 z14 z196 z9-109 z9-ec z900 z990 zEC12; did you mean 'z10'? Maybe debian11 could, work. @Thomas do you have any idea if we could get this to work with '-march=z15' or should we work around that by manually encoding the relevant instructions instead? Yes, the problem is that binutils reject new mnemomics for older -march settings. The Linux kernel handles this by using .insn instead of the mnemonic. This will also allow to compile the testcase with older compilers/binutils. So something like "ncrk 0, 3, 2" -> ".insn rrf,0xb9f5,0,3,2,0"
Re: qemu iotest 161 and make check
Am 10.02.22 um 15:47 schrieb Vladimir Sementsov-Ogievskiy:
10.02.2022 10:57, Christian Borntraeger wrote:
Hello,
I do see spurious failures of 161 in our CI, but only when I use
make check with parallelism (-j).
I have not yet figured out which other testcase could interfere
@@ -34,6 +34,8 @@
*** Commit and then change an option on the backing file
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+qemu-img: TEST_DIR/t.IMGFMT.base: Failed to get "write" lock
+Is another process using the image [TEST_DIR/t.IMGFMT.base]?
Formatting 'TEST_DIR/t.IMGFMT.int', fmt=IMGFMT size=1048576
backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
backing_file=TEST_DIR/t.IMGFMT.int backing_fmt=IMGFMT
{ 'execute': 'qmp_capabilities' }
any ideas?
Hmm, interesting.. Is it always 161 and always exactly this diff?
Seems to be always this diff.
Re: qemu iotest 161 and make check
Am 10.02.22 um 15:47 schrieb Vladimir Sementsov-Ogievskiy:
10.02.2022 10:57, Christian Borntraeger wrote:
Hello,
I do see spurious failures of 161 in our CI, but only when I use
make check with parallelism (-j).
I have not yet figured out which other testcase could interfere
@@ -34,6 +34,8 @@
*** Commit and then change an option on the backing file
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+qemu-img: TEST_DIR/t.IMGFMT.base: Failed to get "write" lock
+Is another process using the image [TEST_DIR/t.IMGFMT.base]?
Formatting 'TEST_DIR/t.IMGFMT.int', fmt=IMGFMT size=1048576
backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
backing_file=TEST_DIR/t.IMGFMT.int backing_fmt=IMGFMT
{ 'execute': 'qmp_capabilities' }
any ideas?
Hmm, interesting.. Is it always 161 and always exactly this diff?
Its always 161 and only 161. I would need to check if its always the same error.
First, this place in 161 is usual: we just create and image, like in many other
tests.
Second, why _make_test_img trigger "Failed to get write lock"? It should just
create an image. Hmm. And probably starts QSD if protocol is fuse. So, that start of QSD
may probably fail.. Is that the case? What is image format and protocol used in test run?
But anyway, tests running in parallel should not break each other as each test
has own TEST_DIR and SOCK_DIR..
qemu iotest 161 and make check
Hello,
I do see spurious failures of 161 in our CI, but only when I use
make check with parallelism (-j).
I have not yet figured out which other testcase could interfere
@@ -34,6 +34,8 @@
*** Commit and then change an option on the backing file
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+qemu-img: TEST_DIR/t.IMGFMT.base: Failed to get "write" lock
+Is another process using the image [TEST_DIR/t.IMGFMT.base]?
Formatting 'TEST_DIR/t.IMGFMT.int', fmt=IMGFMT size=1048576
backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
backing_file=TEST_DIR/t.IMGFMT.int backing_fmt=IMGFMT
{ 'execute': 'qmp_capabilities' }
any ideas?
Christian
Re: [RFC PATCH] MAINTAINERS: Add myself to s390 I/O areas
Am 12.01.22 um 17:40 schrieb Eric Farman: After the recent restructuring, I'd like to volunteer to help in some of the s390 I/O areas. Built on "[PATCH RFC v2] MAINTAINERS: split out s390x sections" Signed-off-by: Eric Farman Acked-by: Christian Borntraeger Thanks a lot Eric --- MAINTAINERS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 5d37b0eec5..343f43e83d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1521,6 +1521,7 @@ S390 Machines S390 Virtio-ccw M: Halil Pasic M: Christian Borntraeger +M: Eric Farman S: Supported F: hw/s390x/ F: include/hw/s390x/ @@ -1551,6 +1552,7 @@ L: qemu-s3...@nongnu.org S390 channel subsystem M: Halil Pasic M: Christian Borntraeger +M: Eric Farman S: Supported F: hw/s390x/ccw-device.[ch] F: hw/s390x/css.c @@ -1975,6 +1977,7 @@ T: git https://github.com/stefanha/qemu.git block virtio-ccw M: Cornelia Huck M: Halil Pasic +M: Eric Farman S: Supported F: hw/s390x/virtio-ccw*.[hc] F: hw/s390x/vhost-vsock-ccw.c
Re: [PATCH RFC] MAINTAINERS: split out s390x sections
Am 20.12.21 um 12:54 schrieb Cornelia Huck: Split out some more specialized devices etc., so that we can build smarter lists of people to be put on cc: in the future. Signed-off-by: Cornelia Huck Acked-by: Christian Borntraeger That should help to get additional maintainers (in add-on patches) added. Letsa go with this split - we can fix and improve things anytime. --- As discussed offlist. Some notes: - The new sections have inherited the maintainers of the sections they have been split out of (except where people had already volunteered). That's easy to change, obviously, and I hope that the cc: list already contains people who might have interest in volunteering for some sections. - I may not have gotten the F: patterns correct, please double check. - I'm also not sure about where in the MAINTAINERS file the new sections should go; if you have a better idea, please speak up. - Also, if you have better ideas regarding the sections, please speak up as well :) - Pull requests will probably continue the same way as now (i.e. patches picked up at the top level and then sent, except for some things like tcg which may go separately.) Not sure if it would make sense to try out the submaintainer pull request model again, I don't think it made life easier in the past, and now we have the b4 tool to pick patches easily anyway. It might be a good idea to check which of the tree locations should stay, or if we want to have new ones. --- MAINTAINERS | 86 ++--- 1 file changed, 75 insertions(+), 11 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 9a8d1bdf727d..d1916f075386 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -297,7 +297,6 @@ M: David Hildenbrand S: Maintained F: target/s390x/ F: target/s390x/tcg -F: target/s390x/cpu_models_*.[ch] F: hw/s390x/ F: disas/s390.c F: tests/tcg/s390x/ @@ -396,16 +395,10 @@ M: Halil Pasic M: Christian Borntraeger S: Supported F: target/s390x/kvm/ -F: target/s390x/ioinst.[ch] F: target/s390x/machine.c F: target/s390x/sigp.c -F: target/s390x/cpu_features*.[ch] -F: target/s390x/cpu_models.[ch] F: hw/s390x/pv.c F: include/hw/s390x/pv.h -F: hw/intc/s390_flic.c -F: hw/intc/s390_flic_kvm.c -F: include/hw/s390x/s390_flic.h F: gdb-xml/s390*.xml T: git https://github.com/borntraeger/qemu.git s390-next L: qemu-s3...@nongnu.org @@ -1529,12 +1522,8 @@ S390 Virtio-ccw M: Halil Pasic M: Christian Borntraeger S: Supported -F: hw/char/sclp*.[hc] -F: hw/char/terminal3270.c F: hw/s390x/ F: include/hw/s390x/ -F: hw/watchdog/wdt_diag288.c -F: include/hw/watchdog/wdt_diag288.h F: configs/devices/s390x-softmmu/default.mak F: tests/avocado/machine_s390_ccw_virtio.py T: git https://github.com/borntraeger/qemu.git s390-next @@ -1559,6 +1548,80 @@ F: hw/s390x/s390-pci* F: include/hw/s390x/s390-pci* L: qemu-s3...@nongnu.org +S390 channel subsystem +M: Halil Pasic +M: Christian Borntraeger +S: Supported +F: hw/s390x/ccw-device.[ch] +F: hw/s390x/css.c +F: hw/s390x/css-bridge.c +F: include/hw/s390x/css.h +F: include/hw/s390x/css-bridge.h +F: include/hw/s390x/ioinst.h +F: target/s390x/ioinst.c +L: qemu-s3...@nongnu.org + +3270 device +M: Halil Pasic +M: Christian Borntraeger +S: Odd fixes +F: include/hw/s390x/3270-ccw.h +F: hw/char/terminal3270.c +F: hw/s390x/3270-ccw.c +L: qemu-s3...@nongnu.org + +diag 288 watchdog +M: Halil Pasic +M: Christian Borntraeger +S: Supported +F: hw/watchdog/wdt_diag288.c +F: include/hw/watchdog/wdt_diag288.h +L: qemu-s3...@nongnu.org + +S390 CPU models +M: David Hildenbrand +S: Maintained +F: target/s390x/cpu_features*.[ch] +F: target/s390x/cpu_models.[ch] +L: qemu-s3...@nongnu.org + +S390 storage key device +M: Halil Pasic +M: Christian Borntraeger +S: Supported +F: hw/s390x/storage-keys.h +F: hw/390x/s390-skeys*.c +L: qemu-s3...@nongnu.org + +S390 storage attribute device +M: Halil Pasic +M: Christian Borntraeger +S: Supported +F: hw/s390x/storage-attributes.h +F: hw/s390/s390-stattrib*.c +L: qemu-s3...@nongnu.org + +S390 SCLP-backed devices +M: Halil Pasic +M: Christian Borntraeger +S: Supported +F: include/hw/s390x/event-facility.h +F: include/hw/s390x/sclp.h +F: hw/char/sclp*.[hc] +F: hw/s390x/event-facility.c +F: hw/s390x/sclp*.c +L: qemu-s3...@nongnu.org + +S390 floating interrupt controller +M: Halil Pasic +M: Christian Borntraeger +M: David Hildenbrand +S: Supported +F: hw/intc/s390_flic.c +F: hw/intc/s390_flic_kvm.c +F: include/hw/s390x/s390_flic.h +L: qemu-s3...@nongnu.org + X86 Machines PC @@ -1957,6 +2020,7 @@ M: Halil Pasic S: Supported F: hw/s390x/virtio-ccw*.[hc] F: hw/s390x/vhost-vsock-ccw.c +F: hw/s390x/vhost-user-fs-ccw.c T: git https://gitlab.com/cohuck/qemu.git s390-next T: git https://github.com/borntraeger/qemu.git s390-next L: qemu-s3...@nongnu.org
Re: [PATCH v2] hw: Add compat machines for 7.0
Am 17.12.21 um 15:39 schrieb Cornelia Huck:
Add 7.0 machine types for arm/i440fx/q35/s390x/spapr.
Acked-by: Cédric Le Goater
Reviewed-by: Juan Quintela
Signed-off-by: Cornelia Huck
s390 parts
Reviewed-by: Christian Borntraeger
---
v1->v2: fix typo in i386 function chaining (thanks danpb!)
---
hw/arm/virt.c | 9 -
hw/core/machine.c | 3 +++
hw/i386/pc.c | 3 +++
hw/i386/pc_piix.c | 14 +-
hw/i386/pc_q35.c | 13 -
hw/ppc/spapr.c | 15 +--
hw/s390x/s390-virtio-ccw.c | 14 +-
include/hw/boards.h| 3 +++
include/hw/i386/pc.h | 3 +++
9 files changed, 71 insertions(+), 6 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 6bce595aba20..4593fea1ce8a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2856,10 +2856,17 @@ static void machvirt_machine_init(void)
}
type_init(machvirt_machine_init);
+static void virt_machine_7_0_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(7, 0)
+
static void virt_machine_6_2_options(MachineClass *mc)
{
+virt_machine_7_0_options(mc);
+compat_props_add(mc->compat_props, hw_compat_6_2, hw_compat_6_2_len);
}
-DEFINE_VIRT_MACHINE_AS_LATEST(6, 2)
+DEFINE_VIRT_MACHINE(6, 2)
static void virt_machine_6_1_options(MachineClass *mc)
{
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 53a99abc5605..a9c15479fe1d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -37,6 +37,9 @@
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-pci.h"
+GlobalProperty hw_compat_6_2[] = {};
+const size_t hw_compat_6_2_len = G_N_ELEMENTS(hw_compat_6_2);
+
GlobalProperty hw_compat_6_1[] = {
{ "vhost-user-vsock-device", "seqpacket", "off" },
{ "nvme-ns", "shared", "off" },
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a2ef40ecbc24..fccde2ef39f6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -94,6 +94,9 @@
#include "trace.h"
#include CONFIG_DEVICES
+GlobalProperty pc_compat_6_2[] = {};
+const size_t pc_compat_6_2_len = G_N_ELEMENTS(pc_compat_6_2);
+
GlobalProperty pc_compat_6_1[] = {
{ TYPE_X86_CPU, "hv-version-id-build", "0x1bbc" },
{ TYPE_X86_CPU, "hv-version-id-major", "0x0006" },
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 223dd3e05d15..19991902761e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -413,7 +413,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
}
-static void pc_i440fx_6_2_machine_options(MachineClass *m)
+static void pc_i440fx_7_0_machine_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_i440fx_machine_options(m);
@@ -422,6 +422,18 @@ static void pc_i440fx_6_2_machine_options(MachineClass *m)
pcmc->default_cpu_version = 1;
}
+DEFINE_I440FX_MACHINE(v7_0, "pc-i440fx-7.0", NULL,
+ pc_i440fx_7_0_machine_options);
+
+static void pc_i440fx_6_2_machine_options(MachineClass *m)
+{
+pc_i440fx_7_0_machine_options(m);
+m->alias = NULL;
+m->is_default = false;
+compat_props_add(m->compat_props, hw_compat_6_2, hw_compat_6_2_len);
+compat_props_add(m->compat_props, pc_compat_6_2, pc_compat_6_2_len);
+}
+
DEFINE_I440FX_MACHINE(v6_2, "pc-i440fx-6.2", NULL,
pc_i440fx_6_2_machine_options);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index e1e100316d93..2e981f436ce5 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -360,7 +360,7 @@ static void pc_q35_machine_options(MachineClass *m)
m->max_cpus = 288;
}
-static void pc_q35_6_2_machine_options(MachineClass *m)
+static void pc_q35_7_0_machine_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_q35_machine_options(m);
@@ -368,6 +368,17 @@ static void pc_q35_6_2_machine_options(MachineClass *m)
pcmc->default_cpu_version = 1;
}
+DEFINE_Q35_MACHINE(v7_0, "pc-q35-7.0", NULL,
+ pc_q35_7_0_machine_options);
+
+static void pc_q35_6_2_machine_options(MachineClass *m)
+{
+pc_q35_7_0_machine_options(m);
+m->alias = NULL;
+compat_props_add(m->compat_props, hw_compat_6_2, hw_compat_6_2_len);
+compat_props_add(m->compat_props, pc_compat_6_2, pc_compat_6_2_len);
+}
+
DEFINE_Q35_MACHINE(v6_2, "pc-q35-6.2", NULL,
pc_q35_6_2_machine_options);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3b5fd749be89..837342932586 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4665,15 +4665,26 @@ static void
spapr_machine_latest_class_options(MachineClass *mc)
}\
typ
Re: [PATCH 00/12] s390x/pci: zPCI interpretation support
Am 15.12.21 um 16:53 schrieb Matthew Rosato: On 12/15/21 2:35 AM, Pierre Morel wrote: On 12/7/21 22:04, Matthew Rosato wrote: Note: The first 3 patches of this series are included as pre-reqs, but should be pulled via a separate series. Also, patch 5 is needed to support 5.16+ linux header-sync and was already done by Paolo but not merged yet so is thus included here as well. For QEMU, the majority of the work in enabling instruction interpretation is handled via new VFIO ioctls to SET the appropriate interpretation and interrupt forwarding modes, and to GET the function handle to use for interpretive execution. This series implements these new ioctls, as well as adding a new, optional 'intercept' parameter to zpci to request interpretation support not be used as well as an 'intassist' parameter to determine whether or not the firmware assist will be used for interrupt delivery or whether the host will be responsible for delivering all interrupts. In which circumstances do we have an added value by not using interrupt delivered by firmware? Disabling it can be a tool to debug and assist in problem determination, but that's about the only scenario I can think of where you would intentionally want to disable intassist. Perhaps then it's not worth leaving in place. I would leave it in in case we run into problems. Things like the nomio parameter for the kernel have proven to be useful.
Re: [PATCH 06/12] target/s390x: add zpci-interp to cpu models
Am 07.12.21 um 22:04 schrieb Matthew Rosato:
The zpci-interp feature is used to specify whether zPCI interpretation is
to be used for this guest.
Signed-off-by: Matthew Rosato
---
target/s390x/cpu_features_def.h.inc | 1 +
target/s390x/gen-features.c | 2 ++
target/s390x/kvm/kvm.c | 1 +
3 files changed, 4 insertions(+)
diff --git a/target/s390x/cpu_features_def.h.inc
b/target/s390x/cpu_features_def.h.inc
index e86662bb3b..4ade3182aa 100644
--- a/target/s390x/cpu_features_def.h.inc
+++ b/target/s390x/cpu_features_def.h.inc
@@ -146,6 +146,7 @@ DEF_FEAT(SIE_CEI, "cei", SCLP_CPU, 43, "SIE:
Conditional-external-interception f
DEF_FEAT(DAT_ENH_2, "dateh2", MISC, 0, "DAT-enhancement facility 2")
DEF_FEAT(CMM, "cmm", MISC, 0, "Collaborative-memory-management facility")
DEF_FEAT(AP, "ap", MISC, 0, "AP instructions installed")
+DEF_FEAT(ZPCI_INTERP, "zpci-interp", MISC, 0, "zPCI interpretation")
/* Features exposed via the PLO instruction. */
DEF_FEAT(PLO_CL, "plo-cl", PLO, 0, "PLO Compare and load (32 bit in general
registers)")
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 7cb1a6ec10..7005d22415 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -554,6 +554,7 @@ static uint16_t full_GEN14_GA1[] = {
S390_FEAT_HPMA2,
S390_FEAT_SIE_KSS,
S390_FEAT_GROUP_MULTIPLE_EPOCH_PTFF,
+S390_FEAT_ZPCI_INTERP,
};
#define full_GEN14_GA2 EmptyFeat
@@ -650,6 +651,7 @@ static uint16_t default_GEN14_GA1[] = {
S390_FEAT_GROUP_MSA_EXT_8,
S390_FEAT_MULTIPLE_EPOCH,
S390_FEAT_GROUP_MULTIPLE_EPOCH_PTFF,
+S390_FEAT_ZPCI_INTERP,
};
For the default model you need to be careful.
Is this in any way guest visible? then you definitely need to fence this
off for older QEMU versions so that when you migrate with older QEMUs
See the s390_cpudef_featoff_greater calls in hw/s390x/s390-virtio-ccw.c
I know its more of a theoretical aspect, since PCI currently forbids migration
but we should try to have the cpu model consistent I guess.
#define default_GEN14_GA2 EmptyFeat
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 5b1fdb55c4..b13d78f988 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -2290,6 +2290,7 @@ static int kvm_to_feat[][2] = {
{ KVM_S390_VM_CPU_FEAT_PFMFI, S390_FEAT_SIE_PFMFI},
{ KVM_S390_VM_CPU_FEAT_SIGPIF, S390_FEAT_SIE_SIGPIF},
{ KVM_S390_VM_CPU_FEAT_KSS, S390_FEAT_SIE_KSS},
+{ KVM_S390_VM_CPU_FEAT_ZPCI_INTERP, S390_FEAT_ZPCI_INTERP },
};
static int query_cpu_feat(S390FeatBitmap features)
Re: [RFC PATCH] s390: kvm: reduce frequency of CPU syncs of diag318 info
Am 02.12.21 um 21:54 schrieb Collin Walling:
On 11/23/21 01:14, Christian Borntraeger wrote:
Am 22.11.21 um 23:33 schrieb Collin Walling:
DIAGNOSE 0318 is invoked only once during IPL. As such, the
diag318 info will only change once initially and during resets.
Let's only sync the register to convey the info to KVM if and
only if the diag318 info has changed. Only set the dirty bit
flag for diag318 whenever it must be updated.
Is this really necessary? In my logs the sync only happens for larger
changes (like reset) operations and then yes we log again.
But I think it is ok to see such a log entry in these rare events.
Albeit a micro-optimization, I don't see why the diag318 dirtied bit
must to be set during /every/ sync. It makes more sense to set the flag
after the register was actually modified.
My point is that it is NOT set during /every/ sync.
We only set it when level != KVM_PUT_RUNTIME_STATE (see kvm_arch_put_registers)
And this basically never happens during normal runtime, only for major events
like reset.
The migration handler will invoke the set_diag318 helper on
post_load to ensure the info is set on the destination machine.
Signed-off-by: Collin Walling
---
target/s390x/kvm/kvm.c | 5 -
target/s390x/machine.c | 14 ++
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 6acf14d5ec..6a141399f7 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -599,11 +599,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_ETOKEN;
}
- if (can_sync_regs(cs, KVM_SYNC_DIAG318)) {
- cs->kvm_run->s.regs.diag318 = env->diag318_info;
- cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
- }
-
/* Finally the prefix */
if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
cs->kvm_run->s.regs.prefix = env->psa;
diff --git a/target/s390x/machine.c b/target/s390x/machine.c
index 37a076858c..a5d113ce3a 100644
--- a/target/s390x/machine.c
+++ b/target/s390x/machine.c
@@ -234,6 +234,19 @@ const VMStateDescription vmstate_etoken = {
}
};
+static int diag318_post_load(void *opaque, int version_id)
+{
+ S390CPU *cpu = opaque;
+ CPUState *cs = CPU(cpu);
+ CPUS390XState *env = _CPU(cs)->env;
+
+ if (kvm_enabled()) {
+ kvm_s390_set_diag318(cs, env->diag318_info);
+ }
+
+ return 0;
+}
+
static bool diag318_needed(void *opaque)
{
return s390_has_feat(S390_FEAT_DIAG_318);
@@ -243,6 +256,7 @@ const VMStateDescription vmstate_diag318 = {
.name = "cpu/diag318",
.version_id = 1,
.minimum_version_id = 1,
+ .post_load = diag318_post_load,
.needed = diag318_needed,
.fields = (VMStateField[]) {
VMSTATE_UINT64(env.diag318_info, S390CPU),
Re: [PATCH] s390x/ipl: support extended kernel command line size
Am 22.11.21 um 12:29 schrieb Marc Hartmayer:
In the past s390 used a fixed command line length of 896 bytes. This has changed
with the Linux commit 5ecb2da660ab ("s390: support command lines longer than 896
bytes"). There is now a parm area indicating the maximum command line size. This
parm area has always been initialized to zero, so with older kernels this field
would read zero and we must then assume that only 896 bytes are available.
Acked-by: Viktor Mihajlovski
Signed-off-by: Marc Hartmayer
Reviewed-by: Christian Borntraeger
---
hw/s390x/ipl.c | 23 ---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 7ddca0127fc2..092c66b3f9f1 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -37,8 +37,9 @@
#define KERN_IMAGE_START0x01UL
#define LINUX_MAGIC_ADDR0x010008UL
+#define KERN_PARM_AREA_SIZE_ADDR0x010430UL
#define KERN_PARM_AREA 0x010480UL
-#define KERN_PARM_AREA_SIZE 0x000380UL
+#define LEGACY_KERN_PARM_AREA_SIZE 0x000380UL
#define INITRD_START0x80UL
#define INITRD_PARM_START 0x010408UL
#define PARMFILE_START 0x001000UL
@@ -110,6 +111,21 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t
srcaddr)
return srcaddr + dstaddr;
}
+static uint64_t get_max_kernel_cmdline_size(void)
+{
+uint64_t *size_ptr = rom_ptr(KERN_PARM_AREA_SIZE_ADDR, sizeof(*size_ptr));
+
+if (size_ptr) {
+uint64_t size;
+
+size = be64_to_cpu(*size_ptr);
+if (size != 0) {
+return size;
+}
+}
+return LEGACY_KERN_PARM_AREA_SIZE;
+}
+
static void s390_ipl_realize(DeviceState *dev, Error **errp)
{
MachineState *ms = MACHINE(qdev_get_machine());
@@ -197,10 +213,11 @@ static void s390_ipl_realize(DeviceState *dev, Error
**errp)
ipl->start_addr = KERN_IMAGE_START;
/* Overwrite parameters in the kernel image, which are "rom" */
if (parm_area) {
-if (cmdline_size > KERN_PARM_AREA_SIZE) {
+uint64_t max_cmdline_size = get_max_kernel_cmdline_size();
+if (cmdline_size > max_cmdline_size) {
error_setg(errp,
"kernel command line exceeds maximum size: %zu >
%lu",
- cmdline_size, KERN_PARM_AREA_SIZE);
+ cmdline_size, max_cmdline_size);
return;
}
[PATCH v2 1/1] MAINTAINERS: update email address of Christian Borntraeger
My borntrae...@de.ibm.com email is just a forwarder to the linux.ibm.com address. Let us remove the extra hop to avoid a potential source of errors. While at it, add the relevant email addresses to mailmap. Signed-off-by: Christian Borntraeger --- .mailmap| 1 + MAINTAINERS | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.mailmap b/.mailmap index 8beb2f95ae28..c45d1c530144 100644 --- a/.mailmap +++ b/.mailmap @@ -50,6 +50,7 @@ Aleksandar Rikalo Aleksandar Rikalo Alexander Graf Anthony Liguori Anthony Liguori +Christian Borntraeger Filip Bozuta Frederic Konrad Greg Kurz diff --git a/MAINTAINERS b/MAINTAINERS index d3879aa3c12c..e19d88ca9960 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -393,7 +393,7 @@ F: target/ppc/kvm.c S390 KVM CPUs M: Halil Pasic -M: Christian Borntraeger +M: Christian Borntraeger S: Supported F: target/s390x/kvm/ F: target/s390x/ioinst.[ch] @@ -1527,7 +1527,7 @@ S390 Machines - S390 Virtio-ccw M: Halil Pasic -M: Christian Borntraeger +M: Christian Borntraeger S: Supported F: hw/char/sclp*.[hc] F: hw/char/terminal3270.c @@ -1541,7 +1541,7 @@ T: git https://github.com/borntraeger/qemu.git s390-next L: qemu-s3...@nongnu.org S390-ccw boot -M: Christian Borntraeger +M: Christian Borntraeger M: Thomas Huth S: Supported F: hw/s390x/ipl.* -- 2.31.1
