From 96acfb1f2552c24af6b3ed886daabe2bd3ceff2d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 1 Jun 2021 09:54:10 +0200 Subject: [PATCH 01/13] meson: allow optional dependencies for block modules Right now all dependencies for block modules are passed to module_ss.add(when: ...), so they are mandatory. In the next patch we will need to add a libm dependency to a module, but libm does not exist on all systems. So, modify the creation of module_ss and modsrc so that dependencies can also be passed to module_ss.add(if_true: ...). While touching the array, remove the useless dependency of the curl module on glib. glib is always linked in QEMU and in fact all other block modules also need it, but they don't have to specify it. Signed-off-by: Paolo Bonzini --- block/meson.build | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/block/meson.build b/block/meson.build index e687c54dbc..9e3388f633 100644 --- a/block/meson.build +++ b/block/meson.build @@ -71,19 +71,19 @@ block_modules = {} modsrc = [] foreach m : [ - [curl, 'curl', [curl, glib], 'curl.c'], - [glusterfs, 'gluster', glusterfs, 'gluster.c'], - [libiscsi, 'iscsi', libiscsi, 'iscsi.c'], - [libnfs, 'nfs', libnfs, 'nfs.c'], - [libssh, 'ssh', libssh, 'ssh.c'], - [rbd, 'rbd', rbd, 'rbd.c'], + [curl, 'curl', files('curl.c')], + [glusterfs, 'gluster', files('gluster.c')], + [libiscsi, 'iscsi', files('iscsi.c')], + [libnfs, 'nfs', files('nfs.c')], + [libssh, 'ssh', files('ssh.c')], + [rbd, 'rbd', files('rbd.c')], ] if m[0].found() - if enable_modules - modsrc += files(m[3]) - endif module_ss = ss.source_set() - module_ss.add(when: m[2], if_true: files(m[3])) + module_ss.add(when: m[0], if_true: m[2]) + if enable_modules + modsrc += module_ss.all_sources() + endif block_modules += {m[1] : module_ss} endif endforeach From 7fa1c63553242ad557c26dafd01e828ff1507c64 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 1 Jun 2021 10:00:48 +0200 Subject: [PATCH 02/13] iscsi: link libm into the module Depending on the configuration of QEMU, some binaries might not need libm at all. In that case libiscsi, which uses exp(), will fail to load. Link it in the module explicitly. Reported-by: Yi Sun Signed-off-by: Paolo Bonzini --- block/meson.build | 2 +- meson.build | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/block/meson.build b/block/meson.build index 9e3388f633..01861e1545 100644 --- a/block/meson.build +++ b/block/meson.build @@ -73,7 +73,7 @@ modsrc = [] foreach m : [ [curl, 'curl', files('curl.c')], [glusterfs, 'gluster', files('gluster.c')], - [libiscsi, 'iscsi', files('iscsi.c')], + [libiscsi, 'iscsi', [files('iscsi.c'), libm]], [libnfs, 'nfs', files('nfs.c')], [libssh, 'ssh', files('ssh.c')], [rbd, 'rbd', files('rbd.c')], diff --git a/meson.build b/meson.build index a45f1a844f..913cf2a41a 100644 --- a/meson.build +++ b/meson.build @@ -163,7 +163,7 @@ if targetos != 'linux' and get_option('multiprocess').enabled() endif multiprocess_allowed = targetos == 'linux' and not get_option('multiprocess').disabled() -m = cc.find_library('m', required: false) +libm = cc.find_library('m', required: false) util = cc.find_library('util', required: false) winmm = [] socket = [] @@ -1899,7 +1899,7 @@ util_ss.add_all(trace_ss) util_ss = util_ss.apply(config_all, strict: false) libqemuutil = static_library('qemuutil', sources: util_ss.sources() + stub_ss.sources() + genh, - dependencies: [util_ss.dependencies(), m, glib, socket, malloc, pixman]) + dependencies: [util_ss.dependencies(), libm, glib, socket, malloc, pixman]) qemuutil = declare_dependency(link_with: libqemuutil, sources: genh + version_res) From 29c3d213f4ad69688638330728cff1a8769d7415 Mon Sep 17 00:00:00 2001 From: Brad Smith Date: Thu, 1 Apr 2021 13:34:00 -0400 Subject: [PATCH 03/13] oslib-posix: Remove OpenBSD workaround for fcntl("/dev/null", F_SETFL, O_NONBLOCK) failure OpenBSD prior to 6.3 required a workaround to utilize fcntl(F_SETFL) on memory devices. Since modern verions of OpenBSD that are only officialy supported and buildable on do not have this issue I am garbage collecting this workaround. Signed-off-by: Brad Smith Message-Id: Signed-off-by: Paolo Bonzini --- util/oslib-posix.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/util/oslib-posix.c b/util/oslib-posix.c index 36820fec16..7b4bec1402 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -273,17 +273,6 @@ int qemu_try_set_nonblock(int fd) return -errno; } if (fcntl(fd, F_SETFL, f | O_NONBLOCK) == -1) { -#ifdef __OpenBSD__ - /* - * Previous to OpenBSD 6.3, fcntl(F_SETFL) is not permitted on - * memory devices and sets errno to ENODEV. - * It's OK if we fail to set O_NONBLOCK on devices like /dev/null, - * because they will never block anyway. - */ - if (errno == ENODEV) { - return 0; - } -#endif return -errno; } return 0; From 28f6aa1178581c3647819e1abc4905899d97d3a2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 1 Jun 2021 15:31:38 +0200 Subject: [PATCH 04/13] target/i386: tcg: fix segment register offsets for 16-bit TSS The TSS offsets in the manuals have only 2-byte slots for the segment registers. QEMU incorrectly uses 4-byte slots, so that SS overlaps the LDT selector. Resolves: #382 Reported-by: Peter Maydell Signed-off-by: Paolo Bonzini --- target/i386/tcg/seg_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c index 2f6cdc8239..547b959689 100644 --- a/target/i386/tcg/seg_helper.c +++ b/target/i386/tcg/seg_helper.c @@ -281,7 +281,7 @@ static void switch_tss_ra(CPUX86State *env, int tss_selector, retaddr) | 0xffff0000; } for (i = 0; i < 4; i++) { - new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x22 + i * 4), + new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x22 + i * 2), retaddr); } new_ldt = cpu_lduw_kernel_ra(env, tss_base + 0x2a, retaddr); @@ -349,7 +349,7 @@ static void switch_tss_ra(CPUX86State *env, int tss_selector, cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 6 * 2), env->regs[R_ESI], retaddr); cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 7 * 2), env->regs[R_EDI], retaddr); for (i = 0; i < 4; i++) { - cpu_stw_kernel_ra(env, env->tr.base + (0x22 + i * 4), + cpu_stw_kernel_ra(env, env->tr.base + (0x22 + i * 2), env->segs[i].selector, retaddr); } } From a5505f6b5b6f72eb21be7567fc1ef3ae2d5b3281 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 1 Jun 2021 15:34:26 +0200 Subject: [PATCH 05/13] target/i386: tcg: fix loading of registers from 16-bit TSS According to the manual, the high 16-bit of the registers are preserved when switching to a 16-bit task. Implement this in switch_tss_ra. Signed-off-by: Paolo Bonzini --- target/i386/tcg/seg_helper.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c index 547b959689..2112c5fc51 100644 --- a/target/i386/tcg/seg_helper.c +++ b/target/i386/tcg/seg_helper.c @@ -277,8 +277,7 @@ static void switch_tss_ra(CPUX86State *env, int tss_selector, new_eip = cpu_lduw_kernel_ra(env, tss_base + 0x0e, retaddr); new_eflags = cpu_lduw_kernel_ra(env, tss_base + 0x10, retaddr); for (i = 0; i < 8; i++) { - new_regs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x12 + i * 2), - retaddr) | 0xffff0000; + new_regs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x12 + i * 2), retaddr); } for (i = 0; i < 4; i++) { new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x22 + i * 2), @@ -391,19 +390,17 @@ static void switch_tss_ra(CPUX86State *env, int tss_selector, env->eip = new_eip; eflags_mask = TF_MASK | AC_MASK | ID_MASK | IF_MASK | IOPL_MASK | VM_MASK | RF_MASK | NT_MASK; - if (!(type & 8)) { - eflags_mask &= 0xffff; + if (type & 8) { + cpu_load_eflags(env, new_eflags, eflags_mask); + for (i = 0; i < 8; i++) { + env->regs[i] = new_regs[i]; + } + } else { + cpu_load_eflags(env, new_eflags, eflags_mask & 0xffff); + for (i = 0; i < 8; i++) { + env->regs[i] = (env->regs[i] & 0xffff0000) | new_regs[i]; + } } - cpu_load_eflags(env, new_eflags, eflags_mask); - /* XXX: what to do in 16 bit case? */ - env->regs[R_EAX] = new_regs[0]; - env->regs[R_ECX] = new_regs[1]; - env->regs[R_EDX] = new_regs[2]; - env->regs[R_EBX] = new_regs[3]; - env->regs[R_ESP] = new_regs[4]; - env->regs[R_EBP] = new_regs[5]; - env->regs[R_ESI] = new_regs[6]; - env->regs[R_EDI] = new_regs[7]; if (new_eflags & VM_MASK) { for (i = 0; i < 6; i++) { load_seg_vm(env, i, new_segs[i]); From 1b627f389f9da48aa8f28808770a731c1e09c338 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 1 Jun 2021 15:36:00 +0200 Subject: [PATCH 06/13] target/i386: tcg: fix switching from 16-bit to 32-bit tasks or vice versa The format of the task state segment is governed by bit 3 in the descriptor type field. On a task switch, the format for saving is given by the current value of TR's type field, while the format for loading is given by the new descriptor. Signed-off-by: Paolo Bonzini --- target/i386/tcg/seg_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c index 2112c5fc51..3ed20ca31d 100644 --- a/target/i386/tcg/seg_helper.c +++ b/target/i386/tcg/seg_helper.c @@ -319,7 +319,7 @@ static void switch_tss_ra(CPUX86State *env, int tss_selector, } /* save the current state in the old TSS */ - if (type & 8) { + if (old_type & 8) { /* 32 bit */ cpu_stl_kernel_ra(env, env->tr.base + 0x20, next_eip, retaddr); cpu_stl_kernel_ra(env, env->tr.base + 0x24, old_eflags, retaddr); From e18a6ec8c4516f2c2b973f452207e74c1b608414 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Tue, 1 Jun 2021 20:55:11 -0700 Subject: [PATCH 07/13] target/i386: Fix decode of cr8 A recent cleanup did not recognize that there are two ways to encode cr8: one via the LOCK and the other via REX. Fixes: 7eff2e7c Resolves: https://gitlab.com/qemu-project/qemu/-/issues/380 Signed-off-by: Richard Henderson Message-Id: <20210602035511.96834-1-richard.henderson@linaro.org> Signed-off-by: Paolo Bonzini --- target/i386/tcg/translate.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index 834186bcae..a7f5c0c8f2 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -8091,6 +8091,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu) case 2: case 3: case 4: + case 8: break; default: goto unknown_op; From 6b731a96aa743a4d384197acb4a6038efbb492b4 Mon Sep 17 00:00:00 2001 From: Kit Westneat Date: Thu, 3 Jun 2021 14:20:22 +0000 Subject: [PATCH 08/13] tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test Add test for issue #345 Signed-off-by: Kit Westneat Message-Id: <20210603142022.676395-1-kit.westneat@gmail.com> Signed-off-by: Paolo Bonzini --- tests/qtest/virtio-scsi-test.c | 51 ++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/qtest/virtio-scsi-test.c b/tests/qtest/virtio-scsi-test.c index 1b7ecc1c8f..8ceb12aacd 100644 --- a/tests/qtest/virtio-scsi-test.c +++ b/tests/qtest/virtio-scsi-test.c @@ -200,6 +200,42 @@ static void test_unaligned_write_same(void *obj, void *data, qvirtio_scsi_pci_free(vs); } +/* Test UNMAP with a large LBA, issue #345 */ +static void test_unmap_large_lba(void *obj, void *data, + QGuestAllocator *t_alloc) +{ + QVirtioSCSI *scsi = obj; + QVirtioSCSIQueues *vs; + const uint8_t unmap[VIRTIO_SCSI_CDB_SIZE] = { + 0x42, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0x00 + }; + + /* + * Default null-co device size is 2**30 + * LBA 0x7fff is ~ 1/8 into device, with 4k blocks + * if check_lba_range incorrectly using 512 bytes, will trigger sense error + */ + uint8_t unmap_params[0x18] = { + 0x00, 0x16, /* unmap data length */ + 0x00, 0x10, /* unmap block descriptor data length */ + 0x00, 0x00, 0x00, 0x00, /* reserved */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7f, 0xff, /* LBA */ + 0x00, 0x00, 0x03, 0xff, /* sector count */ + 0x00, 0x00, 0x00, 0x00, /* reserved */ + }; + struct virtio_scsi_cmd_resp resp; + + alloc = t_alloc; + vs = qvirtio_scsi_init(scsi->vdev); + + virtio_scsi_do_command(vs, unmap, NULL, 0, unmap_params, + sizeof(unmap_params), &resp); + g_assert_cmphex(resp.response, ==, 0); + g_assert_cmphex(resp.status, !=, CHECK_CONDITION); + + qvirtio_scsi_pci_free(vs); +} + static void test_write_to_cdrom(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -293,6 +329,17 @@ static void *virtio_scsi_setup(GString *cmd_line, void *arg) return arg; } +static void *virtio_scsi_setup_4k(GString *cmd_line, void *arg) +{ + g_string_append(cmd_line, + " -drive file=blkdebug::null-co://," + "file.image.read-zeroes=on," + "if=none,id=dr1,format=raw " + "-device scsi-hd,drive=dr1,lun=0,scsi-id=1" + ",logical_block_size=4k,physical_block_size=4k"); + return arg; +} + static void *virtio_scsi_setup_cd(GString *cmd_line, void *arg) { g_string_append(cmd_line, @@ -323,6 +370,10 @@ static void register_virtio_scsi_test(void) qos_add_test("unaligned-write-same", "virtio-scsi", test_unaligned_write_same, &opts); + opts.before = virtio_scsi_setup_4k; + qos_add_test("large-lba-unmap", "virtio-scsi", + test_unmap_large_lba, &opts); + opts.before = virtio_scsi_setup_cd; qos_add_test("write-to-cdrom", "virtio-scsi", test_write_to_cdrom, &opts); From 662175b91ff2c0d56f709345b0bf9534ec2a218d Mon Sep 17 00:00:00 2001 From: Claudio Fontana Date: Thu, 3 Jun 2021 14:30:00 +0200 Subject: [PATCH 09/13] i386: reorder call to cpu_exec_realizefn i386 realizefn code is sensitive to ordering, and recent commits aimed at refactoring it, splitting accelerator-specific code, broke assumptions which need to be fixed. We need to: * process hyper-v enlightements first, as they assume features not to be expanded * only then, expand features * after expanding features, attempt to check them and modify them in the accel-specific realizefn code called by cpu_exec_realizefn(). * after the framework has been called via cpu_exec_realizefn, the code can check for what has or hasn't been set by accel-specific code, or extend its results, ie: - check and evenually set code_urev default - modify cpu->mwait after potentially being set from host CPUID. - finally check for phys_bits assuming all user and accel-specific adjustments have already been taken into account. Fixes: f5cc5a5c ("i386: split cpu accelerators from cpu.c"...) Fixes: 30565f10 ("cpu: call AccelCPUClass::cpu_realizefn in"...) Cc: Eduardo Habkost Cc: Vitaly Kuznetsov Cc: Paolo Bonzini Signed-off-by: Claudio Fontana Message-Id: <20210603123001.17843-2-cfontana@suse.de> Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 79 +++++++++++++++++++++++++-------------- target/i386/kvm/kvm-cpu.c | 12 +++++- 2 files changed, 61 insertions(+), 30 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index e0ba36cc23..9c47daa409 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6089,39 +6089,17 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) Error *local_err = NULL; static bool ht_warned; - /* Process Hyper-V enlightenments */ - x86_cpu_hyperv_realize(cpu); - - cpu_exec_realizefn(cs, &local_err); - if (local_err != NULL) { - error_propagate(errp, local_err); - return; - } - - if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) { - g_autofree char *name = x86_cpu_class_get_model_name(xcc); - error_setg(&local_err, "CPU model '%s' requires KVM or HVF", name); - goto out; - } - - if (cpu->ucode_rev == 0) { - /* The default is the same as KVM's. */ - if (IS_AMD_CPU(env)) { - cpu->ucode_rev = 0x01000065; - } else { - cpu->ucode_rev = 0x100000000ULL; - } - } - - /* mwait extended info: needed for Core compatibility */ - /* We always wake on interrupt even if host does not have the capability */ - cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE; - if (cpu->apic_id == UNASSIGNED_APIC_ID) { error_setg(errp, "apic-id property was not initialized properly"); return; } + /* + * Process Hyper-V enlightenments. + * Note: this currently has to happen before the expansion of CPU features. + */ + x86_cpu_hyperv_realize(cpu); + x86_cpu_expand_features(cpu, &local_err); if (local_err) { goto out; @@ -6146,11 +6124,56 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) & CPUID_EXT2_AMD_ALIASES); } + /* + * note: the call to the framework needs to happen after feature expansion, + * but before the checks/modifications to ucode_rev, mwait, phys_bits. + * These may be set by the accel-specific code, + * and the results are subsequently checked / assumed in this function. + */ + cpu_exec_realizefn(cs, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); + return; + } + + if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) { + g_autofree char *name = x86_cpu_class_get_model_name(xcc); + error_setg(&local_err, "CPU model '%s' requires KVM or HVF", name); + goto out; + } + + if (cpu->ucode_rev == 0) { + /* + * The default is the same as KVM's. Note that this check + * needs to happen after the evenual setting of ucode_rev in + * accel-specific code in cpu_exec_realizefn. + */ + if (IS_AMD_CPU(env)) { + cpu->ucode_rev = 0x01000065; + } else { + cpu->ucode_rev = 0x100000000ULL; + } + } + + /* + * mwait extended info: needed for Core compatibility + * We always wake on interrupt even if host does not have the capability. + * + * requires the accel-specific code in cpu_exec_realizefn to + * have already acquired the CPUID data into cpu->mwait. + */ + cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE; + /* For 64bit systems think about the number of physical bits to present. * ideally this should be the same as the host; anything other than matching * the host can cause incorrect guest behaviour. * QEMU used to pick the magic value of 40 bits that corresponds to * consumer AMD devices but nothing else. + * + * Note that this code assumes features expansion has already been done + * (as it checks for CPUID_EXT2_LM), and also assumes that potential + * phys_bits adjustments to match the host have been already done in + * accel-specific code in cpu_exec_realizefn. */ if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) { if (cpu->phys_bits && diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c index 5235bce8dc..00369c2000 100644 --- a/target/i386/kvm/kvm-cpu.c +++ b/target/i386/kvm/kvm-cpu.c @@ -26,10 +26,18 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp) /* * The realize order is important, since x86_cpu_realize() checks if * nothing else has been set by the user (or by accelerators) in - * cpu->ucode_rev and cpu->phys_bits. + * cpu->ucode_rev and cpu->phys_bits, and updates the CPUID results in + * mwait.ecx. + * This accel realization code also assumes cpu features are already expanded. * * realize order: - * kvm_cpu -> host_cpu -> x86_cpu + * + * x86_cpu_realize(): + * -> x86_cpu_expand_features() + * -> cpu_exec_realizefn(): + * -> accel_cpu_realizefn() + * kvm_cpu_realizefn() -> host_cpu_realizefn() + * -> check/update ucode_rev, phys_bits, mwait */ if (cpu->max_features) { if (enable_cpu_pm && kvm_has_waitpkg()) { From 4db4385a7ab6512e9af08305f5725b26c8a980ee Mon Sep 17 00:00:00 2001 From: Claudio Fontana Date: Thu, 3 Jun 2021 14:30:01 +0200 Subject: [PATCH 10/13] i386: run accel_cpu_instance_init as post_init This fixes host and max cpu initialization, by running the accel cpu initialization only after all instance init functions are called for all X86 cpu subclasses. The bug this is fixing is related to the "max" and "host" i386 cpu subclasses, which set cpu->max_features, which is then used at cpu realization time. In order to properly split the accel-specific max features code that needs to be executed at cpu instance initialization time, we cannot call the accel cpu initialization at the end of the x86 base class initialization, or we will have no way to specialize "max features" cpu behavior, overriding the "max" cpu class defaults, and checking for the "max features" flag itself. This patch moves the accel-specific cpu instance initialization to after all x86 cpu instance code has been executed, including subclasses, so that proper initialization of cpu "host" and "max" can be restored. Fixes: f5cc5a5c ("i386: split cpu accelerators from cpu.c,"...) Cc: Eduardo Habkost Cc: Paolo Bonzini Signed-off-by: Claudio Fontana Message-Id: <20210603123001.17843-3-cfontana@suse.de> Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 9c47daa409..a9fe1662d3 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6401,6 +6401,11 @@ static void x86_cpu_register_feature_bit_props(X86CPUClass *xcc, x86_cpu_register_bit_prop(xcc, name, w, bitnr); } +static void x86_cpu_post_initfn(Object *obj) +{ + accel_cpu_instance_init(CPU(obj)); +} + static void x86_cpu_initfn(Object *obj) { X86CPU *cpu = X86_CPU(obj); @@ -6452,9 +6457,6 @@ static void x86_cpu_initfn(Object *obj) if (xcc->model) { x86_cpu_load_model(cpu, xcc->model); } - - /* if required, do accelerator-specific cpu initializations */ - accel_cpu_instance_init(CPU(obj)); } static int64_t x86_cpu_get_arch_id(CPUState *cs) @@ -6799,6 +6801,8 @@ static const TypeInfo x86_cpu_type_info = { .parent = TYPE_CPU, .instance_size = sizeof(X86CPU), .instance_init = x86_cpu_initfn, + .instance_post_init = x86_cpu_post_initfn, + .abstract = true, .class_size = sizeof(X86CPUClass), .class_init = x86_cpu_common_class_init, From 37701411397c7b7d709ae92abd347cc593940ee5 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 24 May 2021 06:57:50 -0400 Subject: [PATCH 11/13] qemu-config: parse configuration files to a QDict Change the parser to put the values into a QDict and pass them to a callback. qemu_config_parse's QemuOpts creation is itself turned into a callback function. This is useful for -readconfig to support keyval-based options; getting a QDict from the parser removes a roundtrip from QDict to QemuOpts and then back to QDict. Unfortunately there is a disadvantage in that semantic errors will point to the last line of the group, because the entries of the QDict do not have a location attached. Cc: Kevin Wolf Cc: Markus Armbruster Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini Message-Id: <20210524105752.3318299-2-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- include/qemu/config-file.h | 7 ++- softmmu/vl.c | 4 +- util/qemu-config.c | 98 ++++++++++++++++++++++++++------------ 3 files changed, 76 insertions(+), 33 deletions(-) diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h index 0500b3668d..f605423321 100644 --- a/include/qemu/config-file.h +++ b/include/qemu/config-file.h @@ -1,6 +1,8 @@ #ifndef QEMU_CONFIG_FILE_H #define QEMU_CONFIG_FILE_H +typedef void QEMUConfigCB(const char *group, QDict *qdict, void *opaque, Error **errp); + void qemu_load_module_for_opts(const char *group); QemuOptsList *qemu_find_opts(const char *group); QemuOptsList *qemu_find_opts_err(const char *group, Error **errp); @@ -14,7 +16,10 @@ void qemu_config_write(FILE *fp); int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp); -int qemu_read_config_file(const char *filename, Error **errp); +/* A default callback for qemu_read_config_file(). */ +void qemu_config_do_parse(const char *group, QDict *qdict, void *opaque, Error **errp); + +int qemu_read_config_file(const char *filename, QEMUConfigCB *f, Error **errp); /* Parse QDict options as a replacement for a config file (allowing multiple enumerated (0..(n-1)) configuration "sections") */ diff --git a/softmmu/vl.c b/softmmu/vl.c index 6054f6f0b9..47dfdd704f 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2133,7 +2133,7 @@ static void qemu_read_default_config_file(Error **errp) int ret; g_autofree char *file = get_relocated_path(CONFIG_QEMU_CONFDIR "/qemu.conf"); - ret = qemu_read_config_file(file, errp); + ret = qemu_read_config_file(file, qemu_config_do_parse, errp); if (ret < 0) { if (ret == -ENOENT) { error_free(*errp); @@ -3399,7 +3399,7 @@ void qemu_init(int argc, char **argv, char **envp) qemu_plugin_opt_parse(optarg, &plugin_list); break; case QEMU_OPTION_readconfig: - qemu_read_config_file(optarg, &error_fatal); + qemu_read_config_file(optarg, qemu_config_do_parse, &error_fatal); break; case QEMU_OPTION_spice: olist = qemu_find_opts_err("spice", NULL); diff --git a/util/qemu-config.c b/util/qemu-config.c index 34974c4b47..374f3bc460 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -2,6 +2,7 @@ #include "block/qdict.h" /* for qdict_extract_subqdict() */ #include "qapi/error.h" #include "qapi/qapi-commands-misc.h" +#include "qapi/qmp/qerror.h" #include "qapi/qmp/qdict.h" #include "qapi/qmp/qlist.h" #include "qemu/error-report.h" @@ -351,19 +352,19 @@ void qemu_config_write(FILE *fp) } /* Returns number of config groups on success, -errno on error */ -int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp) +static int qemu_config_foreach(FILE *fp, QEMUConfigCB *cb, void *opaque, + const char *fname, Error **errp) { - char line[1024], group[64], id[64], arg[64], value[1024]; + char line[1024], prev_group[64], group[64], arg[64], value[1024]; Location loc; - QemuOptsList *list = NULL; Error *local_err = NULL; - QemuOpts *opts = NULL; + QDict *qdict = NULL; int res = -EINVAL, lno = 0; int count = 0; loc_push_none(&loc); while (fgets(line, sizeof(line), fp) != NULL) { - loc_set_file(fname, ++lno); + ++lno; if (line[0] == '\n') { /* skip empty lines */ continue; @@ -372,39 +373,39 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error * /* comment */ continue; } - if (sscanf(line, "[%63s \"%63[^\"]\"]", group, id) == 2) { - /* group with id */ - list = find_list(lists, group, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto out; + if (line[0] == '[') { + QDict *prev = qdict; + if (sscanf(line, "[%63s \"%63[^\"]\"]", group, value) == 2) { + qdict = qdict_new(); + qdict_put_str(qdict, "id", value); + count++; + } else if (sscanf(line, "[%63[^]]]", group) == 1) { + qdict = qdict_new(); + count++; } - opts = qemu_opts_create(list, id, 1, NULL); - count++; - continue; - } - if (sscanf(line, "[%63[^]]]", group) == 1) { - /* group without id */ - list = find_list(lists, group, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto out; + if (qdict != prev) { + if (prev) { + cb(prev_group, prev, opaque, &local_err); + qobject_unref(prev); + if (local_err) { + error_propagate(errp, local_err); + goto out; + } + } + strcpy(prev_group, group); + continue; } - opts = qemu_opts_create(list, NULL, 0, &error_abort); - count++; - continue; } + loc_set_file(fname, lno); value[0] = '\0'; if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2 || sscanf(line, " %63s = \"\"", arg) == 1) { /* arg = value */ - if (opts == NULL) { + if (qdict == NULL) { error_setg(errp, "no group defined"); goto out; } - if (!qemu_opt_set(opts, arg, value, errp)) { - goto out; - } + qdict_put_str(qdict, arg, value); continue; } error_setg(errp, "parse error"); @@ -417,11 +418,48 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error * } res = count; out: + if (qdict) { + cb(group, qdict, opaque, errp); + qobject_unref(qdict); + } loc_pop(&loc); return res; } -int qemu_read_config_file(const char *filename, Error **errp) +void qemu_config_do_parse(const char *group, QDict *qdict, void *opaque, Error **errp) +{ + QemuOptsList **lists = opaque; + const char *id = qdict_get_try_str(qdict, "id"); + QemuOptsList *list; + QemuOpts *opts; + const QDictEntry *unrecognized; + + list = find_list(lists, group, errp); + if (!list) { + return; + } + + opts = qemu_opts_create(list, id, 1, errp); + if (!opts) { + return; + } + if (!qemu_opts_absorb_qdict(opts, qdict, errp)) { + qemu_opts_del(opts); + return; + } + unrecognized = qdict_first(qdict); + if (unrecognized) { + error_setg(errp, QERR_INVALID_PARAMETER, unrecognized->key); + qemu_opts_del(opts); + } +} + +int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp) +{ + return qemu_config_foreach(fp, qemu_config_do_parse, lists, fname, errp); +} + +int qemu_read_config_file(const char *filename, QEMUConfigCB *cb, Error **errp) { FILE *f = fopen(filename, "r"); int ret; @@ -431,7 +469,7 @@ int qemu_read_config_file(const char *filename, Error **errp) return -errno; } - ret = qemu_config_parse(f, vm_config_groups, filename, errp); + ret = qemu_config_foreach(f, cb, vm_config_groups, filename, errp); fclose(f); return ret; } From c0d4aa82f895af67cbf7772324e05605e22b4162 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 24 May 2021 06:57:51 -0400 Subject: [PATCH 12/13] vl: plumb keyval-based options into -readconfig Let -readconfig support parsing command line options into QDict or QemuOpts. This will be used to add back support for objects in -readconfig. Cc: Markus Armbruster Cc: qemu-stable@nongnu.org Reviewed-by: Kevin Wolf Signed-off-by: Paolo Bonzini Message-Id: <20210524105752.3318299-3-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- include/block/qdict.h | 2 - include/qapi/qmp/qdict.h | 3 ++ softmmu/vl.c | 83 ++++++++++++++++++++++++++++------------ 3 files changed, 62 insertions(+), 26 deletions(-) diff --git a/include/block/qdict.h b/include/block/qdict.h index d8cb502d7d..ced2acfb92 100644 --- a/include/block/qdict.h +++ b/include/block/qdict.h @@ -20,8 +20,6 @@ void qdict_join(QDict *dest, QDict *src, bool overwrite); void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start); void qdict_array_split(QDict *src, QList **dst); int qdict_array_entries(QDict *src, const char *subqdict); -QObject *qdict_crumple(const QDict *src, Error **errp); -void qdict_flatten(QDict *qdict); typedef struct QDictRenames { const char *from; diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h index 9934539c1b..d5b5430e21 100644 --- a/include/qapi/qmp/qdict.h +++ b/include/qapi/qmp/qdict.h @@ -64,4 +64,7 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key); QDict *qdict_clone_shallow(const QDict *src); +QObject *qdict_crumple(const QDict *src, Error **errp); +void qdict_flatten(QDict *qdict); + #endif /* QDICT_H */ diff --git a/softmmu/vl.c b/softmmu/vl.c index 47dfdd704f..5e8240b9d8 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -121,6 +121,7 @@ #include "qapi/qapi-commands-misc.h" #include "qapi/qapi-visit-qom.h" #include "qapi/qapi-commands-ui.h" +#include "qapi/qmp/qdict.h" #include "qapi/qmp/qerror.h" #include "sysemu/iothread.h" #include "qemu/guest-random.h" @@ -2127,13 +2128,53 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp) return 0; } +/* + * Return whether configuration group @group is stored in QemuOpts, or + * recorded as one or more QDicts by qemu_record_config_group. + */ +static bool is_qemuopts_group(const char *group) +{ + return true; +} + +static void qemu_record_config_group(const char *group, QDict *dict, + bool from_json, Error **errp) +{ + abort(); +} + +/* + * Parse non-QemuOpts config file groups, pass the rest to + * qemu_config_do_parse. + */ +static void qemu_parse_config_group(const char *group, QDict *qdict, + void *opaque, Error **errp) +{ + QObject *crumpled; + if (is_qemuopts_group(group)) { + qemu_config_do_parse(group, qdict, opaque, errp); + return; + } + + crumpled = qdict_crumple(qdict, errp); + if (!crumpled) { + return; + } + if (qobject_type(crumpled) != QTYPE_QDICT) { + assert(qobject_type(crumpled) == QTYPE_QLIST); + error_setg(errp, "Lists cannot be at top level of a configuration section"); + return; + } + qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp); +} + static void qemu_read_default_config_file(Error **errp) { ERRP_GUARD(); int ret; g_autofree char *file = get_relocated_path(CONFIG_QEMU_CONFDIR "/qemu.conf"); - ret = qemu_read_config_file(file, qemu_config_do_parse, errp); + ret = qemu_read_config_file(file, qemu_parse_config_group, errp); if (ret < 0) { if (ret == -ENOENT) { error_free(*errp); @@ -2142,9 +2183,8 @@ static void qemu_read_default_config_file(Error **errp) } } -static int qemu_set_option(const char *str) +static void qemu_set_option(const char *str, Error **errp) { - Error *local_err = NULL; char group[64], id[64], arg[64]; QemuOptsList *list; QemuOpts *opts; @@ -2152,27 +2192,23 @@ static int qemu_set_option(const char *str) rc = sscanf(str, "%63[^.].%63[^.].%63[^=]%n", group, id, arg, &offset); if (rc < 3 || str[offset] != '=') { - error_report("can't parse: \"%s\"", str); - return -1; + error_setg(errp, "can't parse: \"%s\"", str); + return; } - list = qemu_find_opts(group); - if (list == NULL) { - return -1; + if (!is_qemuopts_group(group)) { + error_setg(errp, "-set is not supported with %s", group); + } else { + list = qemu_find_opts_err(group, errp); + if (list) { + opts = qemu_opts_find(list, id); + if (!opts) { + error_setg(errp, "there is no %s \"%s\" defined", group, id); + return; + } + qemu_opt_set(opts, arg, str + offset + 1, errp); + } } - - opts = qemu_opts_find(list, id); - if (!opts) { - error_report("there is no %s \"%s\" defined", - list->name, id); - return -1; - } - - if (!qemu_opt_set(opts, arg, str + offset + 1, &local_err)) { - error_report_err(local_err); - return -1; - } - return 0; } static void user_register_global_props(void) @@ -2766,8 +2802,7 @@ void qemu_init(int argc, char **argv, char **envp) } break; case QEMU_OPTION_set: - if (qemu_set_option(optarg) != 0) - exit(1); + qemu_set_option(optarg, &error_fatal); break; case QEMU_OPTION_global: if (qemu_global_option(optarg) != 0) @@ -3399,7 +3434,7 @@ void qemu_init(int argc, char **argv, char **envp) qemu_plugin_opt_parse(optarg, &plugin_list); break; case QEMU_OPTION_readconfig: - qemu_read_config_file(optarg, qemu_config_do_parse, &error_fatal); + qemu_read_config_file(optarg, qemu_parse_config_group, &error_fatal); break; case QEMU_OPTION_spice: olist = qemu_find_opts_err("spice", NULL); From 49e987695a1873a769a823604f9065aa88e00c55 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 24 May 2021 06:57:52 -0400 Subject: [PATCH 13/13] vl: plug -object back into -readconfig Commit bc2f4fcb1d ("qom: move user_creatable_add_opts logic to vl.c and QAPIfy it", 2021-03-19) switched the creation of objects from qemu_opts_foreach to a bespoke QTAILQ in preparation for supporting JSON syntax in -object. Unfortunately in doing so it lost support for [object] stanzas in configuration files and also for "-set object.ID.KEY=VAL". The latter is hard to re-establish and probably best solved by deprecating -set. This patch uses the infrastructure introduced by the previous two patches in order to parse QOM objects correctly from configuration files. Cc: Markus Armbruster Cc: qemu-stable@nongnu.org Reviewed-by: Kevin Wolf Signed-off-by: Paolo Bonzini Message-Id: <20210524105752.3318299-4-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- softmmu/vl.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/softmmu/vl.c b/softmmu/vl.c index 5e8240b9d8..326c1e9080 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -1722,9 +1722,15 @@ static void object_option_foreach_add(bool (*type_opt_predicate)(const char *)) } } +static void object_option_add_visitor(Visitor *v) +{ + ObjectOption *opt = g_new0(ObjectOption, 1); + visit_type_ObjectOptions(v, NULL, &opt->opts, &error_fatal); + QTAILQ_INSERT_TAIL(&object_opts, opt, next); +} + static void object_option_parse(const char *optarg) { - ObjectOption *opt; QemuOpts *opts; const char *type; Visitor *v; @@ -1752,11 +1758,8 @@ static void object_option_parse(const char *optarg) v = opts_visitor_new(opts); } - opt = g_new0(ObjectOption, 1); - visit_type_ObjectOptions(v, NULL, &opt->opts, &error_fatal); + object_option_add_visitor(v); visit_free(v); - - QTAILQ_INSERT_TAIL(&object_opts, opt, next); } /* @@ -2134,13 +2137,22 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp) */ static bool is_qemuopts_group(const char *group) { + if (g_str_equal(group, "object")) { + return false; + } return true; } static void qemu_record_config_group(const char *group, QDict *dict, bool from_json, Error **errp) { - abort(); + if (g_str_equal(group, "object")) { + Visitor *v = qobject_input_visitor_new_keyval(QOBJECT(dict)); + object_option_add_visitor(v); + visit_free(v); + } else { + abort(); + } } /*