Re: [Qemu-devel] [PATCH 4/4] target-arm: add minimal dump-guest-memory support
On Thu, Jun 28, 2012 at 05:46:02PM +0100, Peter Maydell wrote: On 20 June 2012 18:28, Rabin Vincent ra...@rab.in wrote: Add a minimal dump-guest-memory support for ARM. The -p option is not supported and we don't add any QEMU-specific notes. So what does this patch give us? This commit message is pretty short and I couldn't find a cover message for the patchset... It makes the dump-guest-memory command work for arm-softmmu. The resulting core dump can be analysed with a tool such as the crash utility. Signed-off-by: Rabin Vincent ra...@rab.in --- configure | 4 +-- target-arm/Makefile.objs | 2 +- target-arm/arch_dump.c | 59 ++ target-arm/arch_memory_mapping.c | 13 + 4 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 target-arm/arch_dump.c create mode 100644 target-arm/arch_memory_mapping.c diff --git a/configure b/configure index b68c0ca..a20ad19 100755 --- a/configure +++ b/configure @@ -3727,7 +3727,7 @@ case $target_arch2 in fi esac case $target_arch2 in - i386|x86_64) + arm|i386|x86_64) echo CONFIG_HAVE_GET_MEMORY_MAPPING=y $config_target_mak esac if test $target_arch2 = ppc64 -a $fdt = yes; then @@ -3746,7 +3746,7 @@ if test $target_softmmu = yes ; then echo subdir-$target: subdir-libcacard $config_host_mak fi case $target_arch2 in - i386|x86_64) + arm|i386|x86_64) echo CONFIG_HAVE_CORE_DUMP=y $config_target_mak esac fi diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs index f447c4f..837b374 100644 --- a/target-arm/Makefile.objs +++ b/target-arm/Makefile.objs @@ -1,5 +1,5 @@ obj-y += arm-semi.o -obj-$(CONFIG_SOFTMMU) += machine.o +obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o obj-y += translate.o op_helper.o helper.o cpu.o obj-y += neon_helper.o iwmmxt_helper.o diff --git a/target-arm/arch_dump.c b/target-arm/arch_dump.c new file mode 100644 index 000..47a7e40 --- /dev/null +++ b/target-arm/arch_dump.c @@ -0,0 +1,59 @@ +#include cpu.h +#include cpu-all.h +#include dump.h +#include elf.h + +typedef struct { + char pad1[24]; + uint32_t pid; + char pad2[44]; + uint32_t regs[18]; + char pad3[4]; +} arm_elf_prstatus; I'm guessing this is following some specification's structure layout; what specification? struct elf_prstatus from the Linux kernel's include/linux/elfcore.h. + +int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env, + int cpuid, void *opaque) Should these APIs really be taking a CPUArchState* rather rather than an ARMCPU* ? (Andreas?) No idea. Cc'ing Wen, who added the APIs. +{ + return -1; +} + +int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env, + int cpuid, void *opaque) +{ + arm_elf_prstatus prstatus; + + memset(prstatus, 0, sizeof(prstatus)); + memcpy((prstatus.regs), env-regs, sizeof(env-regs)); This looks a bit odd -- env-regs[] is a 16 word array but prstatus.regs is 18 words. What are the last two words for? CPSR and orig_r0. orig_r0 is not useful, but I think we can save the CPSR in there. + prstatus.pid = cpuid; + + return dump_write_elf_note(ELFCLASS32, CORE, NT_PRSTATUS, + prstatus, sizeof(prstatus), + f, opaque); +} + +int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env, + void *opaque) +{ + return -1; +} + +int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env, + void *opaque) +{ + return 0; +} + +int cpu_get_dump_info(ArchDumpInfo *info) +{ + info-d_machine = EM_ARM; + info-d_endian = ELFDATA2LSB; ...even for big endian ARM? I'll use TARGET_WORDS_BIGENDIAN to check. Though it appears we don't have a armbe-softmmu? + info-d_class = ELFCLASS32; + + return 0; +} + +ssize_t cpu_get_note_size(int class, int machine, int nr_cpus) +{ + return nr_cpus * dump_get_note_size(ELFCLASS32, CORE, + sizeof(arm_elf_prstatus)); +} diff --git a/target-arm/arch_memory_mapping.c b/target-arm/arch_memory_mapping.c new file mode 100644 index 000..eeaaf09 --- /dev/null +++ b/target-arm/arch_memory_mapping.c @@ -0,0 +1,13 @@ +#include cpu.h +#include cpu-all.h +#include memory_mapping.h + +bool cpu_paging_enabled(CPUArchState *env) +{ + return 0; +} + +int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env) +{ + return -1; +} Why do we need these null implementations and why do they work better than the default ones in
Re: [Qemu-devel] [PATCH 01/12] savevm: Use a struct to pass all handlers
On 06/28/2012 10:21 PM, Juan Quintela wrote: This would make easier to add more operations in the next patches. Signed-off-by: Juan Quintela quint...@redhat.com --- savevm.c | 54 +- vmstate.h |7 +++ 2 files changed, 32 insertions(+), 29 deletions(-) diff --git a/savevm.c b/savevm.c index a15c163..73626d4 100644 --- a/savevm.c +++ b/savevm.c @@ -1171,10 +1171,7 @@ typedef struct SaveStateEntry { int alias_id; int version_id; int section_id; -SaveSetParamsHandler *set_params; -SaveLiveStateHandler *save_live_state; -SaveStateHandler *save_state; -LoadStateHandler *load_state; +SaveVMHandlers *ops; const VMStateDescription *vmsd; void *opaque; CompatEntry *compat; @@ -1237,10 +1234,11 @@ int register_savevm_live(DeviceState *dev, se = g_malloc0(sizeof(SaveStateEntry)); se-version_id = version_id; se-section_id = global_section_id++; -se-set_params = set_params; -se-save_live_state = save_live_state; -se-save_state = save_state; -se-load_state = load_state; +se-ops = g_malloc0(sizeof(SaveVMHandlers)); +se-ops-set_params = set_params; +se-ops-save_live_state = save_live_state; +se-ops-save_state = save_state; +se-ops-load_state = load_state; se-opaque = opaque; se-vmsd = NULL; se-no_migrate = 0; @@ -1309,6 +1307,7 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque) if (se-compat) { g_free(se-compat); } +g_free(se-ops); g_free(se); } } @@ -1327,9 +1326,6 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id, se = g_malloc0(sizeof(SaveStateEntry)); se-version_id = vmsd-version_id; se-section_id = global_section_id++; -se-save_live_state = NULL; -se-save_state = NULL; -se-load_state = NULL; se-opaque = opaque; se-vmsd = vmsd; se-alias_id = alias_id; @@ -1524,7 +1520,7 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id) { if (!se-vmsd) { /* Old style */ -return se-load_state(f, se-opaque, version_id); +return se-ops-load_state(f, se-opaque, version_id); } return vmstate_load_state(f, se-vmsd, se-opaque, version_id); } @@ -1532,7 +1528,7 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id) static void vmstate_save(QEMUFile *f, SaveStateEntry *se) { if (!se-vmsd) { /* Old style */ -se-save_state(f, se-opaque); +se-ops-save_state(f, se-opaque); return; } vmstate_save_state(f,se-vmsd, se-opaque); @@ -1569,10 +1565,10 @@ int qemu_savevm_state_begin(QEMUFile *f, int ret; QTAILQ_FOREACH(se, savevm_handlers, entry) { -if(se-set_params == NULL) { +if (!se-ops || !se-ops-set_params) { continue; } -se-set_params(params, se-opaque); +se-ops-set_params(params, se-opaque); } qemu_put_be32(f, QEMU_VM_FILE_MAGIC); @@ -1581,9 +1577,9 @@ int qemu_savevm_state_begin(QEMUFile *f, QTAILQ_FOREACH(se, savevm_handlers, entry) { int len; -if (se-save_live_state == NULL) +if (!se-ops || !se-ops-save_live_state) { continue; - +} /* Section type */ qemu_put_byte(f, QEMU_VM_SECTION_START); qemu_put_be32(f, se-section_id); @@ -1596,7 +1592,7 @@ int qemu_savevm_state_begin(QEMUFile *f, qemu_put_be32(f, se-instance_id); qemu_put_be32(f, se-version_id); -ret = se-save_live_state(f, QEMU_VM_SECTION_START, se-opaque); +ret = se-ops-save_live_state(f, QEMU_VM_SECTION_START, se-opaque); if (ret 0) { qemu_savevm_state_cancel(f); return ret; @@ -1623,9 +1619,9 @@ int qemu_savevm_state_iterate(QEMUFile *f) int ret = 1; QTAILQ_FOREACH(se, savevm_handlers, entry) { -if (se-save_live_state == NULL) +if (!se-ops || !se-ops-save_live_state) { continue; - +} if (qemu_file_rate_limit(f)) { return 0; } @@ -1634,7 +1630,7 @@ int qemu_savevm_state_iterate(QEMUFile *f) qemu_put_byte(f, QEMU_VM_SECTION_PART); qemu_put_be32(f, se-section_id); -ret = se-save_live_state(f, QEMU_VM_SECTION_PART, se-opaque); +ret = se-ops-save_live_state(f, QEMU_VM_SECTION_PART, se-opaque); trace_savevm_section_end(se-section_id); if (ret = 0) { @@ -1663,15 +1659,15 @@ int qemu_savevm_state_complete(QEMUFile *f) cpu_synchronize_all_states(); QTAILQ_FOREACH(se, savevm_handlers, entry) { -if (se-save_live_state == NULL) +
Re: [Qemu-devel] [PATCH 02/12] savevm: Live migration handlers register the struct directly
On 06/28/2012 10:22 PM, Juan Quintela wrote: Notice that the live migration users never unregister, so no problem about freeing the ops structure. Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c |9 +++-- block-migration.c | 10 -- migration.h |4 ++-- savevm.c | 18 +++--- vl.c |3 +-- vmstate.h |5 + 6 files changed, 26 insertions(+), 23 deletions(-) diff --git a/arch_init.c b/arch_init.c index 7ce074e..0b7e31f 100644 --- a/arch_init.c +++ b/arch_init.c @@ -298,7 +298,7 @@ static void migration_end(void) #define MAX_WAIT 50 /* ms, half buffered_file limit */ -int ram_save_live(QEMUFile *f, int stage, void *opaque) +static int ram_save_live(QEMUFile *f, int stage, void *opaque) { ram_addr_t addr; uint64_t bytes_transferred_last; @@ -436,7 +436,7 @@ static inline void *host_from_stream_offset(QEMUFile *f, return NULL; } -int ram_load(QEMUFile *f, void *opaque, int version_id) +static int ram_load(QEMUFile *f, void *opaque, int version_id) { ram_addr_t addr; int flags, ret = 0; @@ -533,6 +533,11 @@ done: return ret; } +SaveVMHandlers savevm_ram_handlers = { +.save_live_state = ram_save_live, +.load_state = ram_load, +}; + #ifdef HAS_AUDIO struct soundhw { const char *name; diff --git a/block-migration.c b/block-migration.c index b95b4e1..00151a0 100644 --- a/block-migration.c +++ b/block-migration.c @@ -709,11 +709,17 @@ static void block_set_params(const MigrationParams *params, void *opaque) block_mig_state.blk_enable |= params-shared; } +SaveVMHandlers savevm_block_handlers = { +.set_params = block_set_params, +.save_live_state = block_save_live, +.load_state = block_load, +}; + void blk_mig_init(void) { QSIMPLEQ_INIT(block_mig_state.bmds_list); QSIMPLEQ_INIT(block_mig_state.blk_list); -register_savevm_live(NULL, block, 0, 1, block_set_params, - block_save_live, NULL, block_load, block_mig_state); +register_savevm_live(NULL, block, 0, 1, savevm_block_handlers, + block_mig_state); } diff --git a/migration.h b/migration.h index 3990771..98bceda 100644 --- a/migration.h +++ b/migration.h @@ -18,6 +18,7 @@ #include qemu-common.h #include notify.h #include error.h +#include vmstate.h struct MigrationParams { int blk; @@ -81,8 +82,7 @@ uint64_t ram_bytes_remaining(void); uint64_t ram_bytes_transferred(void); uint64_t ram_bytes_total(void); -int ram_save_live(QEMUFile *f, int stage, void *opaque); -int ram_load(QEMUFile *f, void *opaque, int version_id); +extern SaveVMHandlers savevm_ram_handlers; /** * @migrate_add_blocker - prevent migration from proceeding diff --git a/savevm.c b/savevm.c index 73626d4..a451be2 100644 --- a/savevm.c +++ b/savevm.c @@ -1223,10 +1223,7 @@ int register_savevm_live(DeviceState *dev, const char *idstr, int instance_id, int version_id, - SaveSetParamsHandler *set_params, - SaveLiveStateHandler *save_live_state, - SaveStateHandler *save_state, - LoadStateHandler *load_state, + SaveVMHandlers *ops, void *opaque) { SaveStateEntry *se; @@ -1234,16 +1231,12 @@ int register_savevm_live(DeviceState *dev, se = g_malloc0(sizeof(SaveStateEntry)); se-version_id = version_id; se-section_id = global_section_id++; -se-ops = g_malloc0(sizeof(SaveVMHandlers)); -se-ops-set_params = set_params; -se-ops-save_live_state = save_live_state; -se-ops-save_state = save_state; -se-ops-load_state = load_state; +se-ops = ops; se-opaque = opaque; se-vmsd = NULL; se-no_migrate = 0; /* if this is a live_savem then set is_ram */ -if (save_live_state != NULL) { +if (ops-save_live_state != NULL) { se-is_ram = 1; } @@ -1282,8 +1275,11 @@ int register_savevm(DeviceState *dev, LoadStateHandler *load_state, void *opaque) { +SaveVMHandlers *ops = g_malloc0(sizeof(SaveVMHandlers)); +ops-save_state = save_state; +ops-load_state = load_state; return register_savevm_live(dev, idstr, instance_id, version_id, -NULL, NULL, save_state, load_state, opaque); +ops, opaque); } void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque) diff --git a/vl.c b/vl.c index 1329c30..b1f31e8 100644 --- a/vl.c +++ b/vl.c @@ -3438,8 +3438,7 @@ int main(int argc, char **argv, char **envp) default_drive(default_sdcard, snapshot, machine-use_scsi, IF_SD, 0,
Re: [Qemu-devel] qemu-kvm-1.0.1 - unable to exit if vcpu is in infinite loop
On 06/28/2012 10:27 PM, Peter Lieven wrote: Am 28.06.2012 um 18:32 schrieb Avi Kivity: On 06/28/2012 07:29 PM, Peter Lieven wrote: Yes. A signal is sent, and KVM returns from the guest to userspace on pending signals. is there a description available how this process exactly works? The kernel part is in vcpu_enter_guest(), see the check for signal_pending(). But this hasn't seen changes for quite a long while. Thank you, i will have a look. I noticed a few patches that where submitted during the last year, maybe one of them is related: Switch SIG_IPI to SIGUSR1 Fix signal handling of SIG_IPI when io-thread is enabled In the first commit there is mentioned a 32-on-64-bit Linux kernel bug is there any reference to that? http://web.archiveorange.com/archive/v/1XS1vwGSFLyYygwTXg1K. Are you running 32-on-64? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Using qemu to profile ARM binaries
I'm trying to edit the qemu source code so I can use qemu as a profiler for a benchmark of ARM programs. A good start would be counting loads, stores, int ops, float ops and branch instructions used by each of the binary files. I have two (related) questions: 1. Where in the qemu implementation would be best to count instructions? My first thought is to count TCG instructions before they are sent for translation into host code, although it could maybe be done before translation from ARM to TCG instructions as well. Doesn't -d option help you? 2. What's the corresponding folder/file(s) in the source code where I can add code for profiling? target-arm/* would be the place since you're running ARM binary, tcg/ARCH/* could be another place depends on what machine you're running QEMU on. Regards, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] [PATCH 03/12] savevm: remove SaveSetParamsHandler
On 06/28/2012 10:22 PM, Juan Quintela wrote: It was used only once, just unfold. Signed-off-by: Juan Quintela quint...@redhat.com --- vmstate.h |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vmstate.h b/vmstate.h index 4bce53b..5e1a7cc 100644 --- a/vmstate.h +++ b/vmstate.h @@ -26,13 +26,12 @@ #ifndef QEMU_VMSTATE_H #define QEMU_VMSTATE_H 1 -typedef void SaveSetParamsHandler(const MigrationParams *params, void * opaque); typedef void SaveStateHandler(QEMUFile *f, void *opaque); typedef int SaveLiveStateHandler(QEMUFile *f, int stage, void *opaque); typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id); typedef struct SaveVMHandlers { -SaveSetParamsHandler *set_params; +void (*set_params)(const MigrationParams *params, void * opaque); SaveStateHandler *save_state; SaveLiveStateHandler *save_live_state; LoadStateHandler *load_state; Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 04/12] savevm: remove SaveLiveStateHandler
On 06/28/2012 10:22 PM, Juan Quintela wrote: It was used only once, just unfold. Signed-off-by: Juan Quintela quint...@redhat.com --- vmstate.h |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vmstate.h b/vmstate.h index 5e1a7cc..0e24834 100644 --- a/vmstate.h +++ b/vmstate.h @@ -27,13 +27,12 @@ #define QEMU_VMSTATE_H 1 typedef void SaveStateHandler(QEMUFile *f, void *opaque); -typedef int SaveLiveStateHandler(QEMUFile *f, int stage, void *opaque); typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id); typedef struct SaveVMHandlers { void (*set_params)(const MigrationParams *params, void * opaque); SaveStateHandler *save_state; -SaveLiveStateHandler *save_live_state; +int (*save_live_state)(QEMUFile *f, int stage, void *opaque); LoadStateHandler *load_state; } SaveVMHandlers; Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [RFC V2 PATCH 4/4] virtio-net: add multiqueue support
On Mon, Jun 25, 2012 at 06:04:49PM +0800, Jason Wang wrote: This patch let the virtio-net can transmit and recevie packets through multiuple VLANClientStates and abstract them as multiple virtqueues to guest. A new parameter 'queues' were introduced to specify the number of queue pairs. The main goal for vhost support is to let the multiqueue could be used without changes in vhost code. So each vhost_net structure were used to track a single VLANClientState and two virtqueues in the past. As multiple VLANClientState were stored in the NICState, we can infer the correspond VLANClientState from this and queue_index easily. Signed-off-by: Jason Wang jasow...@redhat.com Can this patch be split up? 1. extend vhost API to allow multiqueue and minimally tweak virtio 2. add real multiqueue for virtio Hmm? --- hw/vhost.c | 58 --- hw/vhost.h |1 hw/vhost_net.c |7 + hw/vhost_net.h |2 hw/virtio-net.c | 461 +-- hw/virtio-net.h |3 6 files changed, 355 insertions(+), 177 deletions(-) diff --git a/hw/vhost.c b/hw/vhost.c index 43664e7..6318bb2 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -620,11 +620,12 @@ static int vhost_virtqueue_init(struct vhost_dev *dev, { target_phys_addr_t s, l, a; int r; +int vhost_vq_index = (idx 2 ? idx - 1 : idx) % dev-nvqs; struct vhost_vring_file file = { -.index = idx, + .index = vhost_vq_index }; struct vhost_vring_state state = { -.index = idx, +.index = vhost_vq_index }; struct VirtQueue *vvq = virtio_get_queue(vdev, idx); @@ -670,11 +671,12 @@ static int vhost_virtqueue_init(struct vhost_dev *dev, goto fail_alloc_ring; } -r = vhost_virtqueue_set_addr(dev, vq, idx, dev-log_enabled); +r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev-log_enabled); if (r 0) { r = -errno; goto fail_alloc; } + file.fd = event_notifier_get_fd(virtio_queue_get_host_notifier(vvq)); r = ioctl(dev-control, VHOST_SET_VRING_KICK, file); if (r) { @@ -715,7 +717,7 @@ static void vhost_virtqueue_cleanup(struct vhost_dev *dev, unsigned idx) { struct vhost_vring_state state = { -.index = idx, +.index = (idx 2 ? idx - 1 : idx) % dev-nvqs, }; int r; r = ioctl(dev-control, VHOST_GET_VRING_BASE, state); @@ -829,7 +831,9 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) } for (i = 0; i hdev-nvqs; ++i) { -r = vdev-binding-set_host_notifier(vdev-binding_opaque, i, true); +r = vdev-binding-set_host_notifier(vdev-binding_opaque, + hdev-start_idx + i, + true); if (r 0) { fprintf(stderr, vhost VQ %d notifier binding failed: %d\n, i, -r); goto fail_vq; @@ -839,7 +843,9 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) return 0; fail_vq: while (--i = 0) { -r = vdev-binding-set_host_notifier(vdev-binding_opaque, i, false); +r = vdev-binding-set_host_notifier(vdev-binding_opaque, + hdev-start_idx + i, + false); if (r 0) { fprintf(stderr, vhost VQ %d notifier cleanup error: %d\n, i, -r); fflush(stderr); @@ -860,7 +866,9 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) int i, r; for (i = 0; i hdev-nvqs; ++i) { -r = vdev-binding-set_host_notifier(vdev-binding_opaque, i, false); +r = vdev-binding-set_host_notifier(vdev-binding_opaque, + hdev-start_idx + i, + false); if (r 0) { fprintf(stderr, vhost VQ %d notifier cleanup failed: %d\n, i, -r); fflush(stderr); @@ -874,15 +882,17 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) { int i, r; if (!vdev-binding-set_guest_notifiers) { -fprintf(stderr, binding does not support guest notifiers\n); +fprintf(stderr, binding does not support guest notifier\n); r = -ENOSYS; goto fail; } -r = vdev-binding-set_guest_notifiers(vdev-binding_opaque, true); -if (r 0) { -fprintf(stderr, Error binding guest notifier: %d\n, -r); -goto fail_notifiers; +if (hdev-start_idx == 0) { +r = vdev-binding-set_guest_notifiers(vdev-binding_opaque, true); +if (r 0) { +fprintf(stderr, Error binding guest notifier: %d\n, -r); +goto fail_notifiers; +} } r = vhost_dev_set_features(hdev,
Re: [Qemu-devel] [PATCH 05/12] savevm: Refactor cancel operation in its own operation
On 06/28/2012 10:22 PM, Juan Quintela wrote: Intead of abusing stage with value -1. Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c | 11 ++- block-migration.c | 10 ++ savevm.c |4 ++-- vmstate.h |1 + 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/arch_init.c b/arch_init.c index 0b7e31f..36e19b0 100644 --- a/arch_init.c +++ b/arch_init.c @@ -296,6 +296,11 @@ static void migration_end(void) memory_global_dirty_log_stop(); } +static void ram_migration_cancel(void *opaque) +{ +migration_end(); +} + #define MAX_WAIT 50 /* ms, half buffered_file limit */ static int ram_save_live(QEMUFile *f, int stage, void *opaque) @@ -306,11 +311,6 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque) int ret; int i; -if (stage 0) { -migration_end(); -return 0; -} - memory_global_sync_dirty_bitmap(get_system_memory()); if (stage == 1) { @@ -536,6 +536,7 @@ done: SaveVMHandlers savevm_ram_handlers = { .save_live_state = ram_save_live, .load_state = ram_load, +.cancel = ram_migration_cancel, }; #ifdef HAS_AUDIO diff --git a/block-migration.c b/block-migration.c index 00151a0..cd8a8dd 100644 --- a/block-migration.c +++ b/block-migration.c @@ -536,6 +536,11 @@ static void blk_mig_cleanup(void) } } +static void block_migration_cancel(void *opaque) +{ +blk_mig_cleanup(); +} + static int block_save_live(QEMUFile *f, int stage, void *opaque) { int ret; @@ -543,10 +548,6 @@ static int block_save_live(QEMUFile *f, int stage, void *opaque) DPRINTF(Enter save live stage %d submitted %d transferred %d\n, stage, block_mig_state.submitted, block_mig_state.transferred); -if (stage 0) { -blk_mig_cleanup(); -return 0; -} if (block_mig_state.blk_enable != 1) { /* no need to migrate storage */ @@ -713,6 +714,7 @@ SaveVMHandlers savevm_block_handlers = { .set_params = block_set_params, .save_live_state = block_save_live, .load_state = block_load, +.cancel = block_migration_cancel, }; void blk_mig_init(void) diff --git a/savevm.c b/savevm.c index a451be2..888c5a2 100644 --- a/savevm.c +++ b/savevm.c @@ -1703,8 +1703,8 @@ void qemu_savevm_state_cancel(QEMUFile *f) SaveStateEntry *se; QTAILQ_FOREACH(se, savevm_handlers, entry) { -if (se-ops se-ops-save_live_state) { -se-ops-save_live_state(f, -1, se-opaque); +if (se-ops se-ops-cancel) { +se-ops-cancel(se-opaque); } } } diff --git a/vmstate.h b/vmstate.h index 0e24834..1dd42f5 100644 --- a/vmstate.h +++ b/vmstate.h @@ -33,6 +33,7 @@ typedef struct SaveVMHandlers { void (*set_params)(const MigrationParams *params, void * opaque); SaveStateHandler *save_state; int (*save_live_state)(QEMUFile *f, int stage, void *opaque); +void (*cancel)(void *opaque); LoadStateHandler *load_state; } SaveVMHandlers; Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] Bad mail header?
Stefan Weil s...@weilnetz.de wrote: Am 28.06.2012 21:21, schrieb Juan Quintela: This would make easier to add more operations in the next patches. Signed-off-by: Juan Quintela quint...@redhat.com --- savevm.c | 54 +- vmstate.h | 7 +++ 2 files changed, 32 insertions(+), 29 deletions(-) Hi Juan, Hi stefan this mail and the other mails from this patch series trigger an alert in my mail filter: Just before we start, I just hate email (TM) :-(. X-Amavis-Alert: BAD HEADER SECTION Duplicate header field: References I am looking, at the patches generated by gfpm alias just have one of each. Looking into my INBOX. You are right, I also have duplicate In-Reply-To and Reference fields. No clue if the problem is with git or mailer. If it makes any difference, this is git: (trasno *)$ rpm -qa git git-1.7.10.4-1.fc17.x86_64 The following extract from the mail header shows that there is really a duplication of the fields In-Reply-To and References: From: Juan Quintela quint...@redhat.com To: qemu-devel@nongnu.org Date: Thu, 28 Jun 2012 21:21:59 +0200 Message-Id: 39651e275488caec46861cb5b11ba1c7961a429c.1340910651.git.quint...@redhat.com In-Reply-To: cover.1340910651.git.quint...@redhat.com References: cover.1340910651.git.quint...@redhat.com In-Reply-To: cover.1340910651.git.quint...@redhat.com References: cover.1340910651.git.quint...@redhat.com X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: owass...@redhat.com Subject: [Qemu-devel] [PATCH 01/12] savevm: Use a struct to pass all handlers This is not a new issue, and there are lots of other mail senders which also send mails with duplications. I noticed that recently and would like to understand how it happens. Maybe some tool (git-send-email?) which is used for sending mails is buggy. Do you remember how you created the patch mails? alias gfpm='/usr/bin/git format-patch --signoff --find-renames --thread--cover-letter alias gsend-qemu='/usr/bin/git send-email --smtp-server=smtp.corp.redhat.com --bcc=quint...@redhat.com --from=Juan Quintela quint...@redhat.com --suppress-cc=self --to=qemu-devel@nongnu.org' cd $QEMU_DIR gfpm master -o /tmp/folder/ edit cover letter until happy gsend-qemu /tmp/folder/*patch I have always send the emails this way (this is why I have the alias). Which mailer did you use? Command line, redhat server is zimbra (I think it has postfix underneath, but I would not trust my memory so much). Do you also see the duplication in your local copy of that mail? As said, the generated file with git-format-patch, only have one header field of each, but the mail that I receive have it duplicated. Do other users also see the duplicate lines in the full mail header? Or is this a bug in my MTA? I also see them, just that I don't get the warning from my mailer. Anyone has any clue? I think that the commands that I use are the normal way to send multi-patches series? And what is more, nobody complained in the past. Regards, Later, Juan.
Re: [Qemu-devel] [PATCH 02/12] savevm: Live migration handlers register the struct directly
Orit Wasserman owass...@redhat.com wrote: On 06/28/2012 10:22 PM, Juan Quintela wrote: Notice that the live migration users never unregister, so no problem about freeing the ops structure. void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque) diff --git a/vl.c b/vl.c index 1329c30..b1f31e8 100644 --- a/vl.c +++ b/vl.c @@ -3438,8 +3438,7 @@ int main(int argc, char **argv, char **envp) default_drive(default_sdcard, snapshot, machine-use_scsi, IF_SD, 0, SD_OPTS); -register_savevm_live(NULL, ram, 0, 4, NULL, ram_save_live, NULL, - ram_load, NULL); +register_savevm_live(NULL, ram, 0, 4, savevm_ram_handlers, NULL); Juan, Can't we move it migration.c (migrate_init and qemu_start_incoming migration)? this will remove to use an extern for savevm_ram_handlers. savevm_ram_handlers are exported from arch_init.c, what do we win if we move it? Furthermore, I add more functions to this extructure later in the series, so not exporting the structure and having to export more functions what bring to us? Thanks, Juan.
Re: [Qemu-devel] [PATCH 06/12] savevm: introduce is_active method
Igor Mitsyanko i.mitsya...@gmail.com wrote: On 6/28/2012 11:22 PM, Juan Quintela wrote: Enable the creation of a method to tell migration if that section is active and should be migrate. We use it for blk-migration, that is normally not active. We don't create the method for RAM, as setups without RAM are very strange O:-) Signed-off-by: Juan Quintela quint...@redhat.com @@ -1576,6 +1576,11 @@ int qemu_savevm_state_begin(QEMUFile *f, if (!se-ops || !se-ops-save_live_state) { continue; } +if (se-ops se-ops-is_active) { +if (!se-ops-is_active(se-opaque)) { If we got here then we know for sure that se-ops != NULL,and then nested condition could be simplified to if (se-ops-is_active !se-ops-is_active(se-opaque)). Same for qemu_savevm_state_iterate() amd qemu_savevm_state_complete() I tried to maintain consistency with the previous test, i.e. all have the same structure. I am still not sure which one is better :-() The approach that I did put the things in the same order, yours, remove two lines and one operand. Will think about that one, thanks. Later, Juan.
Re: [Qemu-devel] Bad mail header?
On 1 July 2012 11:16, Juan Quintela quint...@redhat.com wrote: Stefan Weil s...@weilnetz.de wrote: this mail and the other mails from this patch series trigger an alert in my mail filter: X-Amavis-Alert: BAD HEADER SECTION Duplicate header field: References You are right, I also have duplicate In-Reply-To and Reference fields. No clue if the problem is with git or mailer. It seems to have been reported as a Debian bug against git a while back: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=593542 but the git-send-email manpage suggests that upstream would like to consider this as user error: # It is up to the user to ensure that no In-Reply-To header already # exists when git send-email is asked to add it (especially note that # git format-patch can be configured to do the threading itself). # Failure to do so may not produce the expected result in the # recipient’s MUA. I think what this boils down to is don't tell both format-patch and send-email to do threading. send-email threads by default, and format-patch doesn't thread by default, so the path of least resistance is probably not to pass '--thread' to format-patch. (Alternatively you could tell format-patch --thread and send-email --no-thread, but I don't know why you'd want to do that that way around.) -- PMM
[Qemu-devel] QEMU question: is eventfd not thread safe?
QEMU from qemu.org/master (not qemu-kvm), linux host 3.4, ppc64. Shortly the problem is in the host kernel: closing a file in one thread does not interrupt select() waiting on the same file description in another thread. Longer story is: I'll use VFIO as an example as I hit this when I was debugging it but VFIO itself has nothing to do with the issue. The bug looks like: I start the guest with MSI-capable PCI card passed via VFIO. The guest does dhclient from .bashrc and this dhclient does not receive anything till I press any key. I did not see it for a while as I always start the guest and then typed dhclient manually in the guest console. What happens: VFIO initializes INTx eventfd via event_notifier_init() (returns fd=XX) and qemu_set_fd_handler() before the guest starts. Then QEMU starts the guest and enters a loop is os_host_main_loop_wait() which stays in select() until something happens. From the host kernel prospective, the XX fd was allocated, struct file* (P1) with eventfd specific private_data allocated and initialized. select() added a file refcounter (called get_file() in __pollwait()) and started waiting on file* P1 but not on fd's number XX (what is mmm weird but ok). The guest starts and tries to initialize MSI for the PCI card passed through. The guest does PCI config write and this write creates a second thread in QEMU. In this thread QEMU-VFIO closes fd XX which makes the host kernel release fd=XX, release the corresponding file* P1 (fput() in filp_close()) but this does not free this P1 as there is select() waiting on it. eventfd_release() could wake it up but it is called when its refcounter becomes 0 and this won't happen till select() interrupts. Doing MSI init stuff, QEMU-VFIO calls the same event_notifier_init() (returns recycled fd=XX what is correct but confuses) and qemu_set_fd_handler() which adds a handler but select() does not pick it up. The eventfd() (called by event_notifier_init()) allocates new struct file *P2 in the kernel, QEMU-VFIO passes this fd=XX to the kernel's VFIO. When MSI interrupt comes to the host kernel, the VFIO interrupt handler calls eventfd_signal() on the correct file* P2. However do_select() in the kernel does not interrupt to deliver eventfd event as it is still waiting on P1 - nobody interrupted select(). It can only interrupt on stdin events (like typing keys) and INTx interrupt (which does not happen as MSI is enabled). So we need to sync eventfd()/close() calls in one thread with select() in another. Below is the patch which simply sends SIGUSR2 to the main thread (the sample patch is below). Another solution could be adding new IO handler to wake select() up. Or to do something with the kernel but I am not sure what - implementing file_operations::flush for eventfd to do wakeup did not help and I did not dig any further. Does it make sense? What do I miss? What would be the right solution? --- iohandler.c |1 + main-loop.c | 17 + 2 files changed, 18 insertions(+) diff --git a/iohandler.c b/iohandler.c index 3c74de6..54f4c48 100644 --- a/iohandler.c +++ b/iohandler.c @@ -77,6 +77,7 @@ int qemu_set_fd_handler2(int fd, ioh-fd_write = fd_write; ioh-opaque = opaque; ioh-deleted = 0; +kill(getpid(), SIGUSR2); } return 0; } diff --git a/main-loop.c b/main-loop.c index eb3b6e6..f65e048 100644 --- a/main-loop.c +++ b/main-loop.c @@ -199,10 +199,27 @@ static int qemu_signal_init(void) } #endif +static void sigusr2_print(int signal) +{ +} + +static void sigusr2_init(void) +{ +struct sigaction action; + +memset(action, 0, sizeof(action)); +sigfillset(action.sa_mask); +action.sa_handler = sigusr2_print; +action.sa_flags = 0; +sigaction(SIGUSR2, action, NULL); +} + int main_loop_init(void) { int ret; +sigusr2_init(); + qemu_mutex_lock_iothread(); ret = qemu_signal_init(); if (ret) { -- 1.7.10
Re: [Qemu-devel] [PATCH 02/12] savevm: Live migration handlers register the struct directly
On 07/01/2012 01:20 PM, Juan Quintela wrote: Orit Wasserman owass...@redhat.com wrote: On 06/28/2012 10:22 PM, Juan Quintela wrote: Notice that the live migration users never unregister, so no problem about freeing the ops structure. void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque) diff --git a/vl.c b/vl.c index 1329c30..b1f31e8 100644 --- a/vl.c +++ b/vl.c @@ -3438,8 +3438,7 @@ int main(int argc, char **argv, char **envp) default_drive(default_sdcard, snapshot, machine-use_scsi, IF_SD, 0, SD_OPTS); -register_savevm_live(NULL, ram, 0, 4, NULL, ram_save_live, NULL, - ram_load, NULL); +register_savevm_live(NULL, ram, 0, 4, savevm_ram_handlers, NULL); Juan, Can't we move it migration.c (migrate_init and qemu_start_incoming migration)? this will remove to use an extern for savevm_ram_handlers. savevm_ram_handlers are exported from arch_init.c, what do we win if we move it? Furthermore, I add more functions to this extructure later in the series, so not exporting the structure and having to export more functions what bring to us? well it shouldn't be in vl.c , there is no reason (correct me if I'm wrong) to register ram handlers if there is no migration ... Orit Thanks, Juan.
Re: [Qemu-devel] [PATCH 07/12] savevm: split save_live_setup from save_live_state
On 06/28/2012 10:22 PM, Juan Quintela wrote: This patch splits stage 1 to its own function for both save_live users, ram and block. It is just a copy of the function, removing the parts of the other stages. Optimizations would came later. Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c | 86 +++-- block-migration.c | 35 +- savevm.c |4 +-- vmstate.h |1 + 4 files changed, 95 insertions(+), 31 deletions(-) diff --git a/arch_init.c b/arch_init.c index 36e19b0..b296e17 100644 --- a/arch_init.c +++ b/arch_init.c @@ -303,44 +303,85 @@ static void ram_migration_cancel(void *opaque) #define MAX_WAIT 50 /* ms, half buffered_file limit */ -static int ram_save_live(QEMUFile *f, int stage, void *opaque) +static int ram_save_setup(QEMUFile *f, void *opaque) { ram_addr_t addr; -uint64_t bytes_transferred_last; +RAMBlock *block; double bwidth = 0; int ret; int i; memory_global_sync_dirty_bitmap(get_system_memory()); -if (stage == 1) { -RAMBlock *block; -bytes_transferred = 0; -last_block = NULL; -last_offset = 0; -sort_ram_list(); - -/* Make sure all dirty bits are set */ -QLIST_FOREACH(block, ram_list.blocks, next) { -for (addr = 0; addr block-length; addr += TARGET_PAGE_SIZE) { -if (!memory_region_get_dirty(block-mr, addr, TARGET_PAGE_SIZE, - DIRTY_MEMORY_MIGRATION)) { -memory_region_set_dirty(block-mr, addr, TARGET_PAGE_SIZE); -} +bytes_transferred = 0; +last_block = NULL; +last_offset = 0; +sort_ram_list(); + +/* Make sure all dirty bits are set */ +QLIST_FOREACH(block, ram_list.blocks, next) { +for (addr = 0; addr block-length; addr += TARGET_PAGE_SIZE) { +if (!memory_region_get_dirty(block-mr, addr, TARGET_PAGE_SIZE, + DIRTY_MEMORY_MIGRATION)) { +memory_region_set_dirty(block-mr, addr, TARGET_PAGE_SIZE); } } +} -memory_global_dirty_log_start(); +memory_global_dirty_log_start(); + +qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE); + +QLIST_FOREACH(block, ram_list.blocks, next) { +qemu_put_byte(f, strlen(block-idstr)); +qemu_put_buffer(f, (uint8_t *)block-idstr, strlen(block-idstr)); +qemu_put_be64(f, block-length); +} + +bwidth = qemu_get_clock_ns(rt_clock); -qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE); +i = 0; +while ((ret = qemu_file_rate_limit(f)) == 0) { +int bytes_sent; -QLIST_FOREACH(block, ram_list.blocks, next) { -qemu_put_byte(f, strlen(block-idstr)); -qemu_put_buffer(f, (uint8_t *)block-idstr, strlen(block-idstr)); -qemu_put_be64(f, block-length); +bytes_sent = ram_save_block(f); +bytes_transferred += bytes_sent; +if (bytes_sent == 0) { /* no more blocks */ +break; } +/* we want to check in the 1st loop, just in case it was the 1st time + and we had to sync the dirty bitmap. + qemu_get_clock_ns() is a bit expensive, so we only check each some + iterations +*/ +if ((i 63) == 0) { +uint64_t t1 = (qemu_get_clock_ns(rt_clock) - bwidth) / 100; +if (t1 MAX_WAIT) { +DPRINTF(big wait: %ld milliseconds, %d iterations\n, t1, i); +break; +} +} +i++; } +if (ret 0) { +return ret; +} + +qemu_put_be64(f, RAM_SAVE_FLAG_EOS); + +return 0; +} + +static int ram_save_live(QEMUFile *f, int stage, void *opaque) +{ +uint64_t bytes_transferred_last; +double bwidth = 0; +int ret; +int i; + +memory_global_sync_dirty_bitmap(get_system_memory()); + bytes_transferred_last = bytes_transferred; bwidth = qemu_get_clock_ns(rt_clock); @@ -534,6 +575,7 @@ done: } SaveVMHandlers savevm_ram_handlers = { +.save_live_setup = ram_save_setup, .save_live_state = ram_save_live, .load_state = ram_load, .cancel = ram_migration_cancel, diff --git a/block-migration.c b/block-migration.c index 6d37dc1..fc3d1f4 100644 --- a/block-migration.c +++ b/block-migration.c @@ -541,20 +541,40 @@ static void block_migration_cancel(void *opaque) blk_mig_cleanup(); } -static int block_save_live(QEMUFile *f, int stage, void *opaque) +static int block_save_setup(QEMUFile *f, void *opaque) { int ret; -DPRINTF(Enter save live stage %d submitted %d transferred %d\n, -stage, block_mig_state.submitted,
Re: [Qemu-devel] [PATCH 11/12] ram: iterate phase
On 6/28/2012 11:22 PM, Juan Quintela wrote: We only need to synchronize the bitmap when the number of dirty pages is low. Not every time that we call the function. Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c |9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/arch_init.c b/arch_init.c index fe843a7..8299c15 100644 --- a/arch_init.c +++ b/arch_init.c @@ -348,8 +348,6 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) int i; uint64_t expected_time; -memory_global_sync_dirty_bitmap(get_system_memory()); - bytes_transferred_last = bytes_transferred; bwidth = qemu_get_clock_ns(rt_clock); @@ -397,7 +395,12 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) DPRINTF(ram_save_live: expected(%ld) = max(%ld)?\n, expected_time, migrate_max_downtime()); -return expected_time = migrate_max_downtime(); +if (expected_time = migrate_max_downtime()) { +memory_global_sync_dirty_bitmap(get_system_memory()); + +return expected_time = migrate_max_downtime(); Shouldn't expected_time be recalculated after memory_global_sync_dirty_bitmap()? +} +return 0; } static int ram_save_complete(QEMUFile *f, void *opaque)
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
On Sun, Jul 01, 2012 at 09:06:20PM +1000, Alexey Kardashevskiy wrote: QEMU from qemu.org/master (not qemu-kvm), linux host 3.4, ppc64. Shortly the problem is in the host kernel: closing a file in one thread does not interrupt select() waiting on the same file description in another thread. Longer story is: I'll use VFIO as an example as I hit this when I was debugging it but VFIO itself has nothing to do with the issue. The bug looks like: I start the guest with MSI-capable PCI card passed via VFIO. The guest does dhclient from .bashrc and this dhclient does not receive anything till I press any key. I did not see it for a while as I always start the guest and then typed dhclient manually in the guest console. What happens: VFIO initializes INTx eventfd via event_notifier_init() (returns fd=XX) and qemu_set_fd_handler() before the guest starts. Then QEMU starts the guest and enters a loop is os_host_main_loop_wait() which stays in select() until something happens. From the host kernel prospective, the XX fd was allocated, struct file* (P1) with eventfd specific private_data allocated and initialized. select() added a file refcounter (called get_file() in __pollwait()) and started waiting on file* P1 but not on fd's number XX (what is mmm weird but ok). The guest starts and tries to initialize MSI for the PCI card passed through. The guest does PCI config write and this write creates a second thread in QEMU. Why does this create a thread btw? In this thread QEMU-VFIO closes fd XX which makes the host kernel release fd=XX, release the corresponding file* P1 (fput() in filp_close()) but this does not free this P1 as there is select() waiting on it. Doesn't qemu remove an fd handler before closing the fd? If not that's the bug right there. eventfd_release() could wake it up but it is called when its refcounter becomes 0 and this won't happen till select() interrupts. Doing MSI init stuff, QEMU-VFIO calls the same event_notifier_init() (returns recycled fd=XX what is correct but confuses) and qemu_set_fd_handler() which adds a handler but select() does not pick it up. The eventfd() (called by event_notifier_init()) allocates new struct file *P2 in the kernel, QEMU-VFIO passes this fd=XX to the kernel's VFIO. When MSI interrupt comes to the host kernel, the VFIO interrupt handler calls eventfd_signal() on the correct file* P2. However do_select() in the kernel does not interrupt to deliver eventfd event as it is still waiting on P1 - nobody interrupted select(). It can only interrupt on stdin events (like typing keys) and INTx interrupt (which does not happen as MSI is enabled). So we need to sync eventfd()/close() calls in one thread with select() in another. Below is the patch which simply sends SIGUSR2 to the main thread (the sample patch is below). Another solution could be adding new IO handler to wake select() up. Or to do something with the kernel but I am not sure what - implementing file_operations::flush for eventfd to do wakeup did not help and I did not dig any further. Does it make sense? What do I miss? What would be the right solution? --- iohandler.c |1 + main-loop.c | 17 + 2 files changed, 18 insertions(+) diff --git a/iohandler.c b/iohandler.c index 3c74de6..54f4c48 100644 --- a/iohandler.c +++ b/iohandler.c @@ -77,6 +77,7 @@ int qemu_set_fd_handler2(int fd, ioh-fd_write = fd_write; ioh-opaque = opaque; ioh-deleted = 0; +kill(getpid(), SIGUSR2); } return 0; } diff --git a/main-loop.c b/main-loop.c index eb3b6e6..f65e048 100644 --- a/main-loop.c +++ b/main-loop.c @@ -199,10 +199,27 @@ static int qemu_signal_init(void) } #endif +static void sigusr2_print(int signal) +{ +} + +static void sigusr2_init(void) +{ +struct sigaction action; + +memset(action, 0, sizeof(action)); +sigfillset(action.sa_mask); +action.sa_handler = sigusr2_print; +action.sa_flags = 0; +sigaction(SIGUSR2, action, NULL); +} + int main_loop_init(void) { int ret; +sigusr2_init(); + qemu_mutex_lock_iothread(); ret = qemu_signal_init(); if (ret) { -- 1.7.10
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
On 01/07/12 22:43, Michael S. Tsirkin wrote: On Sun, Jul 01, 2012 at 09:06:20PM +1000, Alexey Kardashevskiy wrote: QEMU from qemu.org/master (not qemu-kvm), linux host 3.4, ppc64. Shortly the problem is in the host kernel: closing a file in one thread does not interrupt select() waiting on the same file description in another thread. Longer story is: I'll use VFIO as an example as I hit this when I was debugging it but VFIO itself has nothing to do with the issue. The bug looks like: I start the guest with MSI-capable PCI card passed via VFIO. The guest does dhclient from .bashrc and this dhclient does not receive anything till I press any key. I did not see it for a while as I always start the guest and then typed dhclient manually in the guest console. What happens: VFIO initializes INTx eventfd via event_notifier_init() (returns fd=XX) and qemu_set_fd_handler() before the guest starts. Then QEMU starts the guest and enters a loop is os_host_main_loop_wait() which stays in select() until something happens. From the host kernel prospective, the XX fd was allocated, struct file* (P1) with eventfd specific private_data allocated and initialized. select() added a file refcounter (called get_file() in __pollwait()) and started waiting on file* P1 but not on fd's number XX (what is mmm weird but ok). The guest starts and tries to initialize MSI for the PCI card passed through. The guest does PCI config write and this write creates a second thread in QEMU. Why does this create a thread btw? It is the thread where the guest is running I guess (?). This is the backtrace of this second thread: (gdb) bt #0 vfio_enable_msi (vdev=0x110200f0) at /home/aik/qemu-impreza/hw/ppc/../vfio_pci.c:699 #1 0x1036c948 in vfio_pci_write_config (pdev=0x110200f0, addr=0xd2, val=0x81, len=0x2) at /home/aik/qemu-impreza/hw/ppc/../vfio_pci.c:979 #2 0x101b1388 in pci_host_config_write_common (pci_dev=0x110200f0, addr=0xd2, limit=0x100, val=0x81, len=0x2) at /home/aik/qemu-impreza/hw/pci_host.c:54 #3 0x1035d70c in finish_write_pci_config (spapr=0x10efa860, buid=0x2, addr=0xd2, size=0x2, val=0x81, rets=0xd64758) at /home/aik/qemu-impreza/hw/ppc/../spapr_pci.c:170 #4 0x1035d874 in rtas_ibm_write_pci_config (spapr=0x10efa860, token=0x2010, nargs=0x5, args=0xd64744, nret=0x1, rets=0xd64758) at /home/aik/qemu-impreza/hw/ppc/../spapr_pci.c:194 #5 0x10360bcc in spapr_rtas_call (spapr=0x10efa860, token=0x2010, nargs=0x5, args=0xd64744, nret=0x1, rets=0xd64758) at /home/aik/qemu-impreza/hw/ppc/../spapr_rtas.c:218 #6 0x10358150 in h_rtas (env=0xfffb733f040, spapr=0x10efa860, opcode=0xf000, args=0xfffb7fea030) at /home/aik/qemu-impreza/hw/ppc/../spapr_hcall.c:560 #7 0x10358c4c in spapr_hypercall (env=0xfffb733f040, opcode=0xf000, args=0xfffb7fea030) at /home/aik/qemu-impreza/hw/ppc/../spapr_hcall.c:734 #8 0x103dab2c in kvm_arch_handle_exit (env=0xfffb733f040, run=0xfffb7fea000) at /home/aik/qemu-impreza/target-ppc/kvm.c:769 #9 0x103a8a94 in kvm_cpu_exec (env=0xfffb733f040) at /home/aik/qemu-impreza/kvm-all.c:1536 #10 0x102ede24 in qemu_kvm_cpu_thread_fn (arg=0xfffb733f040) at /home/aik/qemu-impreza/cpus.c:752 #11 0x0fffb7ab19f0 in .start_thread () from /lib64/libpthread.so.0 #12 0x0fffb7706774 in .__clone () from /lib64/libc.so.6 In this thread QEMU-VFIO closes fd XX which makes the host kernel release fd=XX, release the corresponding file* P1 (fput() in filp_close()) but this does not free this P1 as there is select() waiting on it. Doesn't qemu remove an fd handler before closing the fd? If not that's the bug right there. QEMU does not literally remove it but it marks the event as deleted and actually removes it much later from qemu_iohandler_poll() which is called after os_host_main_loop_wait(). Removal is done by calling qemu_set_fd_handler(fd, NULL, NULL, vdev); But even if it did remove it, what would it change? eventfd_release() could wake it up but it is called when its refcounter becomes 0 and this won't happen till select() interrupts. Doing MSI init stuff, QEMU-VFIO calls the same event_notifier_init() (returns recycled fd=XX what is correct but confuses) and qemu_set_fd_handler() which adds a handler but select() does not pick it up. The eventfd() (called by event_notifier_init()) allocates new struct file *P2 in the kernel, QEMU-VFIO passes this fd=XX to the kernel's VFIO. When MSI interrupt comes to the host kernel, the VFIO interrupt handler calls eventfd_signal() on the correct file* P2. However do_select() in the kernel does not interrupt to deliver eventfd event as it is still waiting on P1 - nobody interrupted select(). It can only interrupt on stdin events (like typing keys) and INTx interrupt (which does not happen as MSI is enabled). So we need to sync eventfd()/close() calls in one thread with
Re: [Qemu-devel] [RFC] [PATCHv2 2/2] Adding basic calls to libseccomp in vl.c
Il 18/06/2012 23:53, Corey Bryant ha scritto: Can each thread have separate seccomp whitelists? For example CPU threads should not need pretty much anything but the I/O thread needs I/O. No, seccomp filters are defined and enforced at the process level. Perhaps we can add (at the kernel level) a way for seccomp filters to examine the current tid. Paolo
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
Il 01/07/2012 13:06, Alexey Kardashevskiy ha scritto: Doing MSI init stuff, QEMU-VFIO calls the same event_notifier_init() (returns recycled fd=XX what is correct but confuses) and qemu_set_fd_handler() which adds a handler but select() does not pick it up. This sounds like a missing qemu_notify_event(). There was a recent thread on a similar problem with block/iscsi.c. Paolo
Re: [Qemu-devel] [PATCH v2 00/17] introduce OptsVisitor, rebase -net/-netdev parsing
Il 13/06/2012 12:54, Paolo Bonzini ha scritto: Il 13/06/2012 10:22, Laszlo Ersek ha scritto: Inspired by [1], the first half of this series attempts to implement a new visitor that should clean up defining and processing command line options. For a more detailed description, please see [PATCH 05/17] qapi: introduce OptsVisitor. The second half converts -net/-netdev parsing to the new visitor. v1-v2: - Insert a small patch between v1 01/16 and v1 02/16 in order to generate C types for fixed-width integers. - Tighten / clean up integer types in Netdev options and in OptsVisitor. - NetLegacy::name is optional. - Changes are marked below individually and described separately. - (Rebase to current master.) Looks good, thanks! Luiz, what's the state of this? Paolo
Re: [Qemu-devel] [PATCH 3/4] usb: add usb attached scsi emulation
Il 20/06/2012 14:41, Gerd Hoffmann ha scritto: Special thanks go to Paolo for bringing the qemu scsi emulation into shape, so this can be added nicely without having to touch a single line of scsi code. But we can touch it and make it even better. :) Do you need a release_req SCSIBusInfo method that is called with the hba_private when a request refcount goes to zero, so that you can remove usb_uas_ref_request and usb_uas_unref_request? (and replace it with scsi_req_ref/unref in usb_uas_handle_data, I think). Paolo
Re: [Qemu-devel] [PATCH] build: introduce target CONFIG_ variables and use them for kvm
Il 23/06/2012 12:30, Peter Maydell ha scritto: Can't it go in hw/arm/kvm (mimicking the final desired place, which will be target-arm/hw/kvm)? And hw/kvm can be moved to hw/i386/kvm, or we can leave it there for now until we're ready to move it to target-i386/hw/kvm. Why's the final desired place target-arm/hw/kvm ? That doesn't make much sense to me... Doesn't it have some dependency on target-arm/kvm.c? Paolo
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
On 01/07/12 23:32, Paolo Bonzini wrote: Il 01/07/2012 13:06, Alexey Kardashevskiy ha scritto: Doing MSI init stuff, QEMU-VFIO calls the same event_notifier_init() (returns recycled fd=XX what is correct but confuses) and qemu_set_fd_handler() which adds a handler but select() does not pick it up. This sounds like a missing qemu_notify_event(). There was a recent thread on a similar problem with block/iscsi.c. Oh, right, that helps too when place in qemu_set_fd_handler2(). -- Alexey
Re: [Qemu-devel] [PATCH] build: introduce target CONFIG_ variables and use them for kvm
On 1 July 2012 14:37, Paolo Bonzini pbonz...@redhat.com wrote: Il 23/06/2012 12:30, Peter Maydell ha scritto: Can't it go in hw/arm/kvm (mimicking the final desired place, which will be target-arm/hw/kvm)? And hw/kvm can be moved to hw/i386/kvm, or we can leave it there for now until we're ready to move it to target-i386/hw/kvm. Why's the final desired place target-arm/hw/kvm ? That doesn't make much sense to me... Doesn't it have some dependency on target-arm/kvm.c? Well, it does at the moment, but I'm not entirely sure it should (this is on my list of things to look at). I would expect that insert interrupt into the KVM kernel irqchip should be a generic interface the same way that KVM hooks into cpu_interrupt(), only it doesn't seem to be handled that way for eg PPC. Having device models in hw/ make direct calls to per-target KVM functions in target-*/kvm.c seems like a bit of a layering violation to me. -- PMM
Re: [Qemu-devel] [PATCH] build: introduce target CONFIG_ variables and use them for kvm
Il 29/06/2012 14:31, Andreas Färber ha scritto: Ping? I can't figure out if this discussion got wedged (in which case, how should it be unwedged?) or if people were eventually happy with this patch... My guess is we're waiting for Paolo to return from vacation and to comment. Basically the patch is correct but the open issue is how we want to structure the directories - do we want hw/kvm/ to contain multiple architectures' files, or do we want to separate devices by architecture. There's reasons for both once it works technically. If Anthony split off the kvm/ change it might be less controversial. I don't really care much about that, I hope it's temporary anyway. :) However, one change is necessary: the config-all-devices.mak should also include the arch defines (or if you don't like the naming, we could have another config-all-arches.mak file). It should not be hard with grep (perhaps after renaming the variable should probably be named CONFIG_ARCH_$ARCH). With this change, the patch is perfectly fine! Paolo
Re: [Qemu-devel] [PATCH] w32: Fix broken build (missing include file)
Il 16/06/2012 20:48, Artyom Tarasenko ha scritto: But still, it's not possible for all contributions, right? To sum this up: GPL v2+ are allowed non-GPL contributions are allowed Yes, as long as they are GPLv2- and v3-compatible. So for example no Apache license (just a theoretical case, but worth mentioning for completeness), because it's not GPLv2-compatible. I'm not aware of any license that is GPLv2-compatible but not GPLv3, but there must be one hiding somewhere. :) Paolo GPL v2- are not allowed.
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
On 01/07/12 23:40, Alexey Kardashevskiy wrote: On 01/07/12 23:32, Paolo Bonzini wrote: Il 01/07/2012 13:06, Alexey Kardashevskiy ha scritto: Doing MSI init stuff, QEMU-VFIO calls the same event_notifier_init() (returns recycled fd=XX what is correct but confuses) and qemu_set_fd_handler() which adds a handler but select() does not pick it up. This sounds like a missing qemu_notify_event(). There was a recent thread on a similar problem with block/iscsi.c. Oh, right, that helps too when place in qemu_set_fd_handler2(). Like this. Right place? --- iohandler.c |1 + 1 file changed, 1 insertion(+) diff --git a/iohandler.c b/iohandler.c index 3c74de6..dea4355 100644 --- a/iohandler.c +++ b/iohandler.c @@ -77,6 +77,7 @@ int qemu_set_fd_handler2(int fd, ioh-fd_write = fd_write; ioh-opaque = opaque; ioh-deleted = 0; +qemu_notify_event(); } return 0; } -- 1.7.10
Re: [Qemu-devel] [RFC PATCH 3/3] block: gluster as block backend
Il 18/06/2012 19:35, Stefan Hajnoczi ha scritto: +/* Use O_DSYNC for write-through caching, no flags for write-back caching, + * and O_DIRECT for no caching. */ +if ((bdrv_flags BDRV_O_NOCACHE)) +s-open_flags |= O_DIRECT; +if (!(bdrv_flags BDRV_O_CACHE_WB)) +s-open_flags |= O_DSYNC; Paolo has changed this recently, you might need to use bs-enable_write_cache instead. At the protocol (i.e. low-level backend) level you don't need to do anything really, if you implement bdrv_flush_to_disk correctly. Looking at BDRV_O_CACHE_WB will do no harm, it's just dead code. Paolo
Re: [Qemu-devel] [RFC PATCH 3/3] block: gluster as block backend
Il 19/06/2012 13:05, Stefan Hajnoczi ha scritto: I picked up this logic from block/raw-posix.c:raw_open_common(). Don't see anything related to bs-enable_write_cache there. Will find out more about bs-enable_write_cache. If you fetch the latest qemu.git and check bdrv_open_common() there is new code that stashes BDRV_O_CACHE_WB in bs-enable_write_cache and then opens the actual block driver with BDRV_O_CACHE_WB set. You can use bdrv_enable_write_cache() to test the original flag. Yes, but you shouldn't do this when opening. You should always open for writeback. Paolo
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
Il 01/07/2012 16:46, Alexey Kardashevskiy ha scritto: On 01/07/12 23:40, Alexey Kardashevskiy wrote: On 01/07/12 23:32, Paolo Bonzini wrote: Il 01/07/2012 13:06, Alexey Kardashevskiy ha scritto: Doing MSI init stuff, QEMU-VFIO calls the same event_notifier_init() (returns recycled fd=XX what is correct but confuses) and qemu_set_fd_handler() which adds a handler but select() does not pick it up. This sounds like a missing qemu_notify_event(). There was a recent thread on a similar problem with block/iscsi.c. Oh, right, that helps too when place in qemu_set_fd_handler2(). Like this. Right place? Yes, please resend as a toplevel message (i.e. not deep in a thread) with my Reviewed-by. Paolo --- iohandler.c |1 + 1 file changed, 1 insertion(+) diff --git a/iohandler.c b/iohandler.c index 3c74de6..dea4355 100644 --- a/iohandler.c +++ b/iohandler.c @@ -77,6 +77,7 @@ int qemu_set_fd_handler2(int fd, ioh-fd_write = fd_write; ioh-opaque = opaque; ioh-deleted = 0; +qemu_notify_event(); } return 0; }
[Qemu-devel] [Bug 1019467] Re: Text mode cursor doesn't blink
** Changed in: qemu Assignee: Javier Donoso (jedc375) = (unassigned) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1019467 Title: Text mode cursor doesn't blink Status in QEMU: New Bug description: Sorry for doing this, but QEMU should support text mode cursor blinking. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1019467/+subscriptions
Re: [Qemu-devel] [PATCH] Exynos4: added RTC device
Il 29/06/2012 14:26, Andreas Färber ha scritto: Oh, I see. Should we place this device to hw/Makefile.objs in v2? That would've been nice, but I'll do it as a follow-up now. Yes, so we can also use Anthony's new CONFIG_ARCH_ARM (introducing CONFIG_EXYNOS can be done later). Paolo
Re: [Qemu-devel] [PATCH] target-arm: Fix some copy-and-paste errors in cp register names
On 6/28/2012 5:42 PM, Peter Maydell wrote: Fix a couple of cases where cp register names were copy-and-pasted. These are harmless since we don't use the name for anything (except debugging convenience) but could be confusing. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/helper.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 2309923..7a9ad8d 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -216,9 +216,9 @@ static const ARMCPRegInfo v6_cp_reginfo[] = { .access = PL1_W, .type = ARM_CP_NOP }, { .name = ISB, .cp = 15, .crn = 7, .crm = 5, .opc1 = 0, .opc2 = 4, .access = PL0_W, .type = ARM_CP_NOP }, -{ .name = ISB, .cp = 15, .crn = 7, .crm = 10, .opc1 = 0, .opc2 = 4, +{ .name = DSB, .cp = 15, .crn = 7, .crm = 10, .opc1 = 0, .opc2 = 4, .access = PL0_W, .type = ARM_CP_NOP }, -{ .name = ISB, .cp = 15, .crn = 7, .crm = 10, .opc1 = 0, .opc2 = 5, +{ .name = DMB, .cp = 15, .crn = 7, .crm = 10, .opc1 = 0, .opc2 = 5, .access = PL0_W, .type = ARM_CP_NOP }, { .name = IFAR, .cp = 15, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 2, .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.c6_insn), @@ -346,7 +346,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { */ { .name = DBGDRAR, .cp = 14, .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 0, .access = PL0_R, .type = ARM_CP_CONST, .resetvalue = 0 }, -{ .name = DBGDRAR, .cp = 14, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 0, +{ .name = DBGDSAR, .cp = 14, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 0, .access = PL0_R, .type = ARM_CP_CONST, .resetvalue = 0 }, /* the old v6 WFI, UNPREDICTABLE in v7 but we choose to NOP */ { .name = NOP, .cp = 15, .crn = 7, .crm = 0, .opc1 = 0, .opc2 = 4, just like it named in architecture reference manual Reviewed-by: Igor Mitsyanko i.mitsya...@samsung.com
Re: [Qemu-devel] qemu-kvm-1.0.1 - unable to exit if vcpu is in infinite loop
Am 01.07.2012 um 10:19 schrieb Avi Kivity: On 06/28/2012 10:27 PM, Peter Lieven wrote: Am 28.06.2012 um 18:32 schrieb Avi Kivity: On 06/28/2012 07:29 PM, Peter Lieven wrote: Yes. A signal is sent, and KVM returns from the guest to userspace on pending signals. is there a description available how this process exactly works? The kernel part is in vcpu_enter_guest(), see the check for signal_pending(). But this hasn't seen changes for quite a long while. Thank you, i will have a look. I noticed a few patches that where submitted during the last year, maybe one of them is related: Switch SIG_IPI to SIGUSR1 Fix signal handling of SIG_IPI when io-thread is enabled In the first commit there is mentioned a 32-on-64-bit Linux kernel bug is there any reference to that? http://web.archiveorange.com/archive/v/1XS1vwGSFLyYygwTXg1K. Are you running 32-on-64? I think the issue occurs when running a 32-bit guest on a 64-bit system. Afaik, the isolinux loader where is see the race is 32-bit altough it is a 64-bit ubuntu lts cd image. The second case where i have seen the race is on shutdown of a Windows 2000 Server which is also 32-bit. Peter -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH] eventfd: making it rhread safe
QEMU uses IO handlers to run select() in the main loop. The handlers list is managed by qemu_set_fd_handler() helper which works fine when called from the main thread as it is called not when select() is waiting. However sometime we need to update the handlers list from another thread. For that the main loop's select() needs to be restarted with the updated list. The patch adds the qemu_notify_event() call to interrupt select() and make wrapping code to restart select() with the updated IO handlers list. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru Reviewed-by: Paolo Bonzini pbonz...@redhat.com --- iohandler.c |1 + 1 file changed, 1 insertion(+) diff --git a/iohandler.c b/iohandler.c index 3c74de6..dea4355 100644 --- a/iohandler.c +++ b/iohandler.c @@ -77,6 +77,7 @@ int qemu_set_fd_handler2(int fd, ioh-fd_write = fd_write; ioh-opaque = opaque; ioh-deleted = 0; +qemu_notify_event(); } return 0; } -- 1.7.10
[Qemu-devel] [PATCH] target-arm: Fix CP15 based WFI
The coprocessor register rework broke cp15 based WFI instructions. We incorrectly fall through the normal register write case, which incorrectly adds a forced block termination. We've already done a special version of this (DISAS_WFI), so return immediately. Signed-off-by: Paul Brook p...@codesourcery.com --- target-arm/translate.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index a2a0ecd..f39b9ca 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -6236,7 +6236,7 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn) } gen_set_pc_im(s-pc); s-is_jmp = DISAS_WFI; -break; +return 0; default: break; } -- 1.7.10
Re: [Qemu-devel] SMP for PReP architecture
On 29.06.2012, at 15:55, Eli Lewis wrote: Hi all, I would like to use qemu to emulate a muliticore PowerPC PReP machine but it see that the SMP is not supported for the PReP architecture. In fact, running: qemu-system-ppc -M prep -smp 2 It returns this message: Number of SMP cpus requested (2), exceeds max cpus supported by machine `prep' (1) Does anyone know if this functionality is under development or if there is a patch that solves this problem? CC'ing Andreas, the PREP maintainer. My rough estimate would be a simple no, unless you do it though :). Why would you need emulated SMP on PReP? Alex
Re: [Qemu-devel] [PATCH] target-arm: Fix CP15 based WFI
On 1 July 2012 20:59, Paul Brook p...@codesourcery.com wrote: The coprocessor register rework broke cp15 based WFI instructions. We incorrectly fall through the normal register write case, which incorrectly adds a forced block termination. We've already done a special version of this (DISAS_WFI), so return immediately. Signed-off-by: Paul Brook p...@codesourcery.com Reviewed-by: Peter Maydell peter.mayd...@linaro.org Thanks for the catch. -- PMM
Re: [Qemu-devel] [RFC PATCH 0/4] virtio-rng and RngBackend infrastructure (v2)
This series depends on my QOM -object series that I just posted. In Amit's thread on virtio-rng, danpb mentioned that we really ought to have a proper RNG backend infrastructure and of course he's correct on that. Now that we have QOM, I wanted to demonstrate how we can use QOM to construct a complete backend without adding any new infrastructure. I've now implemented a urandom and egd backend and tested them. I think the first three patches are ready to go. I never really understood why this exists in the first place. It's a simple readonly charcter device. IMHO you should be using virtio-serial. This is virtio-console v.s. virtio-serial all over again. The only thing close to a reason I've heard is that guest OS is incompetent and can't source random rata from a serial device. Even accepting the pointless guest device, I see absolutely no reason to have special infrastructure for this within qemu. Character devices do everything you need. Creating annother read stream of data API is needless duplication and only going to reintroduce bugs we already fixed in the character device layer. Paul
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
Doesn't qemu remove an fd handler before closing the fd? If not that's the bug right there. No, it's just marked deleted, removal is deferred. But that doesn't change the fact that you need to wake up select. Ie. What happens is: - eventfd gets you fd #x - thread 1 selects() on it which sleeps - thread 2 closes (x) - thread 2 does eventfd and gets fd #x (same number) - new eventfd gets signalled, but doesn't wake up select which internally is still waiting on the old file descriptor. The reason for that is that select() (and poll()) internally in the kernel do a get_file() on the fd (as a result of eventfd-poll calling poll_wait()) and keeps a reference to the struct file. So the fd itself can be freed from the table by close, but the struct file remains around (f_count is elevated), thus the close does not calls eventfd-release (that only happens on the last close, ie, after select times out or returns for another reason). I think this was even overlooked in the design of eventfd itself, ie, it tries to wakeup all waiters in its release() function but that will not be called as long as either select or poll is waiting due to the elevated refcount. The right solution at this stage if for qemu to kick select() out of its slumber when it closes the fd, a signal does the job. Cheers, Ben. eventfd_release() could wake it up but it is called when its refcounter becomes 0 and this won't happen till select() interrupts. Doing MSI init stuff, QEMU-VFIO calls the same event_notifier_init() (returns recycled fd=XX what is correct but confuses) and qemu_set_fd_handler() which adds a handler but select() does not pick it up. The eventfd() (called by event_notifier_init()) allocates new struct file *P2 in the kernel, QEMU-VFIO passes this fd=XX to the kernel's VFIO. When MSI interrupt comes to the host kernel, the VFIO interrupt handler calls eventfd_signal() on the correct file* P2. However do_select() in the kernel does not interrupt to deliver eventfd event as it is still waiting on P1 - nobody interrupted select(). It can only interrupt on stdin events (like typing keys) and INTx interrupt (which does not happen as MSI is enabled). So we need to sync eventfd()/close() calls in one thread with select() in another. Below is the patch which simply sends SIGUSR2 to the main thread (the sample patch is below). Another solution could be adding new IO handler to wake select() up. Or to do something with the kernel but I am not sure what - implementing file_operations::flush for eventfd to do wakeup did not help and I did not dig any further. Does it make sense? What do I miss? What would be the right solution? --- iohandler.c |1 + main-loop.c | 17 + 2 files changed, 18 insertions(+) diff --git a/iohandler.c b/iohandler.c index 3c74de6..54f4c48 100644 --- a/iohandler.c +++ b/iohandler.c @@ -77,6 +77,7 @@ int qemu_set_fd_handler2(int fd, ioh-fd_write = fd_write; ioh-opaque = opaque; ioh-deleted = 0; +kill(getpid(), SIGUSR2); } return 0; } diff --git a/main-loop.c b/main-loop.c index eb3b6e6..f65e048 100644 --- a/main-loop.c +++ b/main-loop.c @@ -199,10 +199,27 @@ static int qemu_signal_init(void) } #endif +static void sigusr2_print(int signal) +{ +} + +static void sigusr2_init(void) +{ +struct sigaction action; + +memset(action, 0, sizeof(action)); +sigfillset(action.sa_mask); +action.sa_handler = sigusr2_print; +action.sa_flags = 0; +sigaction(SIGUSR2, action, NULL); +} + int main_loop_init(void) { int ret; +sigusr2_init(); + qemu_mutex_lock_iothread(); ret = qemu_signal_init(); if (ret) { -- 1.7.10
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
diff --git a/iohandler.c b/iohandler.c index 3c74de6..54f4c48 100644 --- a/iohandler.c +++ b/iohandler.c @@ -77,6 +77,7 @@ int qemu_set_fd_handler2(int fd, ioh-fd_write = fd_write; ioh-opaque = opaque; ioh-deleted = 0; +kill(getpid(), SIGUSR2); } return 0; } That probably wants to be a pthread_kill targetted at the main loop. +static void sigusr2_print(int signal) +{ +} + +static void sigusr2_init(void) +{ +struct sigaction action; + +memset(action, 0, sizeof(action)); +sigfillset(action.sa_mask); +action.sa_handler = sigusr2_print; +action.sa_flags = 0; +sigaction(SIGUSR2, action, NULL); +} + Won't that conflict with the business in coroutine-sigaltstack.c ? Hrm... looking at it, it looks like it will save/restore the handler, so that should be good. Still, one might want to wrap that into something, like qemu_wake_main_loop(); Cheers, Ben. int main_loop_init(void) { int ret; +sigusr2_init(); + qemu_mutex_lock_iothread(); ret = qemu_signal_init(); if (ret) { -- 1.7.10
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
On 02/07/12 09:07, Benjamin Herrenschmidt wrote: diff --git a/iohandler.c b/iohandler.c index 3c74de6..54f4c48 100644 --- a/iohandler.c +++ b/iohandler.c @@ -77,6 +77,7 @@ int qemu_set_fd_handler2(int fd, ioh-fd_write = fd_write; ioh-opaque = opaque; ioh-deleted = 0; +kill(getpid(), SIGUSR2); } return 0; } That probably wants to be a pthread_kill targetted at the main loop. +static void sigusr2_print(int signal) +{ +} + +static void sigusr2_init(void) +{ +struct sigaction action; + +memset(action, 0, sizeof(action)); +sigfillset(action.sa_mask); +action.sa_handler = sigusr2_print; +action.sa_flags = 0; +sigaction(SIGUSR2, action, NULL); +} + Won't that conflict with the business in coroutine-sigaltstack.c ? The code which touches SIGUSR2 does not compile on power. Hrm... looking at it, it looks like it will save/restore the handler, so that should be good. Still, one might want to wrap that into something, like qemu_wake_main_loop(); I already posted another patch with qemu_notify_event() in this mail thread later :) Cheers, Ben. int main_loop_init(void) { int ret; +sigusr2_init(); + qemu_mutex_lock_iothread(); ret = qemu_signal_init(); if (ret) { -- 1.7.10 -- Alexey
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
Hi, As Paolo said, I hit this with block/iscsi.c a few months back. Then about a month back I recall something that looks alkmost identical to this hit the IDE driver on the KVM list. ( At least the symptoms looked just like my symptoms, and the KVM guys managed to bisect it down to the exact same patch that I did that would expose the bug in block.iscsi.c, namely the lack of calling qemu_notify_event() ) If we have hit a problem 3 times recently due to developers not realizing the importance of calling qemu_notify_event(), maybe the API is suboptimal. Would it be possible to change the set-event-handlers functions to automatically call qemu_notify_event() when the descriptos change? To eliminate the need to call this function at all ? regards ronnie sahlberg On Mon, Jul 2, 2012 at 10:06 AM, Alexey Kardashevskiy a...@ozlabs.ru wrote: On 02/07/12 09:07, Benjamin Herrenschmidt wrote: diff --git a/iohandler.c b/iohandler.c index 3c74de6..54f4c48 100644 --- a/iohandler.c +++ b/iohandler.c @@ -77,6 +77,7 @@ int qemu_set_fd_handler2(int fd, ioh-fd_write = fd_write; ioh-opaque = opaque; ioh-deleted = 0; + kill(getpid(), SIGUSR2); } return 0; } That probably wants to be a pthread_kill targetted at the main loop. +static void sigusr2_print(int signal) +{ +} + +static void sigusr2_init(void) +{ + struct sigaction action; + + memset(action, 0, sizeof(action)); + sigfillset(action.sa_mask); + action.sa_handler = sigusr2_print; + action.sa_flags = 0; + sigaction(SIGUSR2, action, NULL); +} + Won't that conflict with the business in coroutine-sigaltstack.c ? The code which touches SIGUSR2 does not compile on power. Hrm... looking at it, it looks like it will save/restore the handler, so that should be good. Still, one might want to wrap that into something, like qemu_wake_main_loop(); I already posted another patch with qemu_notify_event() in this mail thread later :) Cheers, Ben. int main_loop_init(void) { int ret; + sigusr2_init(); + qemu_mutex_lock_iothread(); ret = qemu_signal_init(); if (ret) { -- 1.7.10 -- Alexey
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
On Mon, 2012-07-02 at 10:06 +1000, Alexey Kardashevskiy wrote: Won't that conflict with the business in coroutine-sigaltstack.c ? The code which touches SIGUSR2 does not compile on power. Oh, we don't get the altstack coroutine stuff ? interesting... Hrm... looking at it, it looks like it will save/restore the handler, so that should be good. Still, one might want to wrap that into something, like qemu_wake_main_loop(); I already posted another patch with qemu_notify_event() in this mail thread later :) Ok. Thanks. Cheers, Ben. Cheers, Ben. int main_loop_init(void) { int ret; +sigusr2_init(); + qemu_mutex_lock_iothread(); ret = qemu_signal_init(); if (ret) { -- 1.7.10
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
On Mon, 2012-07-02 at 10:42 +1000, ronnie sahlberg wrote: Would it be possible to change the set-event-handlers functions to automatically call qemu_notify_event() when the descriptos change? To eliminate the need to call this function at all ? That definitely sounds like the right thing to do. Cheers, Ben.
Re: [Qemu-devel] QEMU question: is eventfd not thread safe?
On Mon, 2012-07-02 at 10:06 +1000, Alexey Kardashevskiy wrote: I already posted another patch with qemu_notify_event() in this mail thread later :) Yup, just saw that, for some reason I got dropped from the CC list. BTW. I told you there must be an existing mechanism for that :-) Cheers, Ben.
Re: [Qemu-devel] Request VFIO inclusion in linux-next
On 27/06/12 22:37, Dan Carpenter wrote: On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote: Hi, VFIO has been kicking around for well over a year now and has been posted numerous times for review. The pre-requirements are finally available in linux-next (or will be in the 20120626 build) so I'd like to request a new branch be included in linux-next with a goal of being accepted into v3.6. Could you run Sparse over the driver? http://lwn.net/Articles/205624/ It reports a bunch of endian problems. Some are definitely bugs like: *prev |= cpu_to_le32((u32)epos 20); What is wrong here? -- Alexey
Re: [Qemu-devel] [RFC] [PATCHv2 2/2] Adding basic calls to libseccomp in vl.c
On Sun, Jul 1, 2012 at 8:25 AM, Paolo Bonzini pbonz...@redhat.com wrote: Il 18/06/2012 23:53, Corey Bryant ha scritto: Can each thread have separate seccomp whitelists? For example CPU threads should not need pretty much anything but the I/O thread needs I/O. No, seccomp filters are defined and enforced at the process level. Perhaps we can add (at the kernel level) a way for seccomp filters to examine the current tid. seccomp filters are attached to the task_struct and apply per thread or per process since they both get their own task_structs. (For Linux, process==thread with shared resources.) Filter programs are also inherited across clone/fork, so it's possible to install a global filter program which applies which is inherited during thread creation, then apply per-thread refinements by stacking on additional filters (at the cost of additional evaluation time). hth! will
Re: [Qemu-devel] Request VFIO inclusion in linux-next
On Mon, 2012-07-02 at 13:41 +1000, Alexey Kardashevskiy wrote: On 27/06/12 22:37, Dan Carpenter wrote: On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote: Hi, VFIO has been kicking around for well over a year now and has been posted numerous times for review. The pre-requirements are finally available in linux-next (or will be in the 20120626 build) so I'd like to request a new branch be included in linux-next with a goal of being accepted into v3.6. Could you run Sparse over the driver? http://lwn.net/Articles/205624/ It reports a bunch of endian problems. Some are definitely bugs like: *prev |= cpu_to_le32((u32)epos 20); What is wrong here? I believe the only thing wrong here was that prev was a u32* instead of a __le32*. The new version in my tree has much better endian annotation after going through all the sparse errors. The only bug I found in the cleanup was the handling of rbar. It was missing the le32_to_cpu as we copied it out of vconfig. This is later used with pci_user_write_config_dword, so it needs to be in native endian. Thanks, Alex
Re: [Qemu-devel] [PATCH] msi/msix: added API to set MSI message address and data
Ping? On 22/06/12 11:15, Alexey Kardashevskiy wrote: Added (msi|msix)_set_message() function for whoever might want to use them. Currently msi_notify()/msix_notify() write to these vectors to signal the guest about an interrupt so the correct values have to written there by the guest or QEMU. For example, POWER guest never initializes MSI/MSIX vectors, instead it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on POWER we have to initialize MSI/MSIX message from QEMU. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- hw/msi.c | 13 + hw/msi.h |1 + hw/msix.c |9 + hw/msix.h |2 ++ 4 files changed, 25 insertions(+) diff --git a/hw/msi.c b/hw/msi.c index 5233204..cc6102f 100644 --- a/hw/msi.c +++ b/hw/msi.c @@ -105,6 +105,19 @@ static inline uint8_t msi_pending_off(const PCIDevice* dev, bool msi64bit) return dev-msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : PCI_MSI_PENDING_32); } +void msi_set_message(PCIDevice *dev, MSIMessage msg) +{ +uint16_t flags = pci_get_word(dev-config + msi_flags_off(dev)); +bool msi64bit = flags PCI_MSI_FLAGS_64BIT; + +if (msi64bit) { +pci_set_quad(dev-config + msi_address_lo_off(dev), msg.address); +} else { +pci_set_long(dev-config + msi_address_lo_off(dev), msg.address); +} +pci_set_word(dev-config + msi_data_off(dev, msi64bit), msg.data); +} + bool msi_enabled(const PCIDevice *dev) { return msi_present(dev) diff --git a/hw/msi.h b/hw/msi.h index 75747ab..6ec1f99 100644 --- a/hw/msi.h +++ b/hw/msi.h @@ -31,6 +31,7 @@ struct MSIMessage { extern bool msi_supported; +void msi_set_message(PCIDevice *dev, MSIMessage msg); bool msi_enabled(const PCIDevice *dev); int msi_init(struct PCIDevice *dev, uint8_t offset, unsigned int nr_vectors, bool msi64bit, bool msi_per_vector_mask); diff --git a/hw/msix.c b/hw/msix.c index ded3c55..5f7d6d3 100644 --- a/hw/msix.c +++ b/hw/msix.c @@ -45,6 +45,15 @@ static MSIMessage msix_get_message(PCIDevice *dev, unsigned vector) return msg; } +void msix_set_message(PCIDevice *dev, int vector, struct MSIMessage msg) +{ +uint8_t *table_entry = dev-msix_table_page + vector * PCI_MSIX_ENTRY_SIZE; + +pci_set_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR, msg.address); +pci_set_long(table_entry + PCI_MSIX_ENTRY_DATA, msg.data); +table_entry[PCI_MSIX_ENTRY_VECTOR_CTRL] = ~PCI_MSIX_ENTRY_CTRL_MASKBIT; +} + /* Add MSI-X capability to the config space for the device. */ /* Given a bar and its size, add MSI-X table on top of it * and fill MSI-X capability in the config space. diff --git a/hw/msix.h b/hw/msix.h index 50aee82..26a437e 100644 --- a/hw/msix.h +++ b/hw/msix.h @@ -4,6 +4,8 @@ #include qemu-common.h #include pci.h +void msix_set_message(PCIDevice *dev, int vector, MSIMessage msg); + int msix_init(PCIDevice *pdev, unsigned short nentries, MemoryRegion *bar, unsigned bar_nr, unsigned bar_size); -- Alexey
[Qemu-devel] [Qemu-ppc][PATCH v5 2/4] Add one new file vga-pci.h
From: Li Zhang zhlci...@linux.vnet.ibm.com Functions pci_vga_init() and pci_cirrus_vga_init() are declared in pc.h. That prevents other platforms (e.g. sPAPR) to use them. This patch is to create one new file vga-pci.h and move the declarations to vga-pci.h, so that they can be shared by all the platforms. Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- hw/cirrus_vga.c |2 +- hw/pc.h |4 hw/vga-pci.c|2 +- hw/vga-pci.h| 13 + 4 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 hw/vga-pci.h diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index 623dd68..3e8e5bb 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -27,11 +27,11 @@ * available at http://home.worldonline.dk/~finth/ */ #include hw.h -#include pc.h #include pci.h #include console.h #include vga_int.h #include loader.h +#include vga-pci.h /* * TODO: diff --git a/hw/pc.h b/hw/pc.h index 31ccb6f..e4db071 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -189,14 +189,10 @@ static inline DeviceState *isa_vga_init(ISABus *bus) return dev-qdev; } -DeviceState *pci_vga_init(PCIBus *bus); int isa_vga_mm_init(target_phys_addr_t vram_base, target_phys_addr_t ctrl_base, int it_shift, MemoryRegion *address_space); -/* cirrus_vga.c */ -DeviceState *pci_cirrus_vga_init(PCIBus *bus); - /* ne2000.c */ static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo *nd) { diff --git a/hw/vga-pci.c b/hw/vga-pci.c index 37dc019..4872056 100644 --- a/hw/vga-pci.c +++ b/hw/vga-pci.c @@ -23,12 +23,12 @@ */ #include hw.h #include console.h -#include pc.h #include pci.h #include vga_int.h #include pixel_ops.h #include qemu-timer.h #include loader.h +#include vga-pci.h typedef struct PCIVGAState { PCIDevice dev; diff --git a/hw/vga-pci.h b/hw/vga-pci.h new file mode 100644 index 000..e272deb --- /dev/null +++ b/hw/vga-pci.h @@ -0,0 +1,13 @@ +#ifndef VGA_PCI_H +#define VGA_PCI_H + +#include qemu-common.h +#include qdev.h + +/* vga-pci.c */ +DeviceState *pci_vga_init(PCIBus *bus); + +/* cirrus_vga.c */ +DeviceState *pci_cirrus_vga_init(PCIBus *bus); + +#endif -- 1.7.9.5
[Qemu-devel] [Qemu-ppc][PATCH v5 4/4] spapr: Add support for -vga option
From: Li Zhang zhlci...@linux.vnet.ibm.com Also instanciate the USB keyboard and mouse when that option is used (you can still use -device to create individual devices without all the defaults) Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- hw/spapr.c | 29 - 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/hw/spapr.c b/hw/spapr.c index 973de1b..3648cad 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -45,6 +45,8 @@ #include kvm.h #include kvm_ppc.h #include pci.h +#include vga-pci.h +#include usb.h #include exec-memory.h @@ -82,6 +84,7 @@ #define PHANDLE_XICP0x sPAPREnvironment *spapr; +bool spapr_has_graphics; qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num, enum xics_irq_type type) @@ -257,6 +260,9 @@ static void *spapr_create_fdt_skel(const char *cpu_model, _FDT((fdt_property(fdt, qemu,boot-kernel, kprop, sizeof(kprop; } _FDT((fdt_property_string(fdt, qemu,boot-device, boot_device))); +_FDT((fdt_property_cell(fdt, qemu,graphic-width, graphic_width))); +_FDT((fdt_property_cell(fdt, qemu,graphic-height, graphic_height))); +_FDT((fdt_property_cell(fdt, qemu,graphic-depth, graphic_depth))); _FDT((fdt_end_node(fdt))); @@ -503,7 +509,9 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr, } } -spapr_populate_chosen_stdout(fdt, spapr-vio_bus); +if (!spapr_has_graphics) { +spapr_populate_chosen_stdout(fdt, spapr-vio_bus); +} _FDT((fdt_pack(fdt))); @@ -556,6 +564,16 @@ static void spapr_cpu_reset(void *opaque) cpu_reset(CPU(cpu)); } +static int spapr_vga_init(PCIBus *pci_bus) +{ +if (std_vga_enabled) { +pci_vga_init(pci_bus); +} else { +return 0; +} +return 1; +} + /* pSeries LPAR / sPAPR hardware init */ static void ppc_spapr_init(ram_addr_t ram_size, const char *boot_device, @@ -712,6 +730,11 @@ static void ppc_spapr_init(ram_addr_t ram_size, spapr_vscsi_create(spapr-vio_bus); } +/* Graphics */ +if (spapr_vga_init(QLIST_FIRST(spapr-phbs)-host_state.bus)) { +spapr_has_graphics = true; +} + machine_opts = qemu_opts_find(qemu_find_opts(machine), 0); if (machine_opts) { add_usb = qemu_opt_get_bool(machine_opts, usb, true); @@ -720,6 +743,10 @@ static void ppc_spapr_init(ram_addr_t ram_size, if (add_usb) { pci_create_simple(QLIST_FIRST(spapr-phbs)-host_state.bus, -1, pci-ohci); +if (spapr_has_graphics) { +usbdevice_create(keyboard); +usbdevice_create(mouse); +} } if (rma_size (MIN_RMA_SLOF 20)) { fprintf(stderr, qemu: pSeries SLOF firmware requires = -- 1.7.9.5
[Qemu-devel] [Qemu-ppc][PATCH v5 0/4] Add USB option and enable vga on spapr
From: Li Zhang zhlci...@linux.vnet.ibm.com v1 - v2: * Convert USB option from char to bool. v2 - v3: * Remove global USB option * Get USB option with qemu_opt_get_bool(). * Send vga patch for sPAPR which requires USB enabled. v3 - v4: * Fix some English grammar and coding style faults * Replace usb_on with add_usb to be more functional. * Remove vga init functions' declearations from pc.h, and add one new file vga-pci.h for the declearations. * Cleanup pc.h on some platforms and add vga-pci.h * Remove graphic devices which are not supported on sPAPR platform. They will be added back when they are supported. v4 - v5: * Correct several English words * Add header file qemu-comman.h in vga-pci.h, which defines PCIBus and DeviceState types. Move #endif to the end of the vga-pci.h and remove the trailing white line. Benjamin Herrenschmidt (1): spapr: Add support for -vga option Li Zhang (3): Add usb option in machine options Add one new file vga-pci.h Cleanup pc.h on other platforms hw/alpha_pci.c|1 + hw/cirrus_vga.c |2 +- hw/mips_malta.c |1 + hw/pc.c |1 + hw/pc.h |4 hw/ppc_newworld.c |2 +- hw/ppc_oldworld.c |2 +- hw/ppc_prep.c |1 + hw/spapr.c| 40 +++- hw/sun4u.c|1 + hw/vga-pci.c |2 +- hw/vga-pci.h | 13 + qemu-config.c |4 13 files changed, 65 insertions(+), 9 deletions(-) create mode 100644 hw/vga-pci.h -- 1.7.9.5
[Qemu-devel] [Qemu-ppc][PATCH v5 3/4] Cleanup pc.h on other platforms
From: Li Zhang zhlci...@linux.vnet.ibm.com The declarations of pci_vga_init() and pci_cirrus_vga_init() are moved to vga-pci.h to be called by all the platforms. So it's necessary to cleanup pc.h on the platforms other than PC which include the file and add vga-pci.h on all the plaforms to call vga related functions. This patch is to cleanup pc.h and add vga-pci.h. Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- hw/alpha_pci.c|1 + hw/mips_malta.c |1 + hw/pc.c |1 + hw/ppc_newworld.c |2 +- hw/ppc_oldworld.c |2 +- hw/ppc_prep.c |1 + hw/sun4u.c|1 + 7 files changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/alpha_pci.c b/hw/alpha_pci.c index 6735577..ea546f8 100644 --- a/hw/alpha_pci.c +++ b/hw/alpha_pci.c @@ -11,6 +11,7 @@ #include qemu-log.h #include sysemu.h #include vmware_vga.h +#include vga-pci.h /* PCI IO reads/writes, to byte-word addressable memory. */ diff --git a/hw/mips_malta.c b/hw/mips_malta.c index 351c88e..ad23f26 100644 --- a/hw/mips_malta.c +++ b/hw/mips_malta.c @@ -48,6 +48,7 @@ #include blockdev.h #include exec-memory.h #include sysbus.h /* SysBusDevice */ +#include vga-pci.h //#define DEBUG_BOARD_INIT diff --git a/hw/pc.c b/hw/pc.c index c7e9ab3..f43e817 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -48,6 +48,7 @@ #include memory.h #include exec-memory.h #include arch_init.h +#include vga-pci.h /* output Bochs bios info messages */ //#define DEBUG_BIOS diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c index 4e2a6e6..e95cfe8 100644 --- a/hw/ppc_newworld.c +++ b/hw/ppc_newworld.c @@ -52,7 +52,6 @@ #include adb.h #include mac_dbdma.h #include nvram.h -#include pc.h #include pci.h #include net.h #include sysemu.h @@ -68,6 +67,7 @@ #include hw/usb.h #include blockdev.h #include exec-memory.h +#include vga-pci.h #define MAX_IDE_BUS 2 #define CFG_ADDR 0xf510 diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c index f2c6908..1dcd8a6 100644 --- a/hw/ppc_oldworld.c +++ b/hw/ppc_oldworld.c @@ -29,7 +29,6 @@ #include adb.h #include mac_dbdma.h #include nvram.h -#include pc.h #include sysemu.h #include net.h #include isa.h @@ -44,6 +43,7 @@ #include kvm_ppc.h #include blockdev.h #include exec-memory.h +#include vga-pci.h #define MAX_IDE_BUS 2 #define CFG_ADDR 0xf510 diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c index be2b268..7a87616 100644 --- a/hw/ppc_prep.c +++ b/hw/ppc_prep.c @@ -39,6 +39,7 @@ #include blockdev.h #include arch_init.h #include exec-memory.h +#include vga-pci.h //#define HARD_DEBUG_PPC_IO //#define DEBUG_PPC_IO diff --git a/hw/sun4u.c b/hw/sun4u.c index 137a7c6..07cd042 100644 --- a/hw/sun4u.c +++ b/hw/sun4u.c @@ -39,6 +39,7 @@ #include elf.h #include blockdev.h #include exec-memory.h +#include vga-pci.h //#define DEBUG_IRQ //#define DEBUG_EBUS -- 1.7.9.5
[Qemu-devel] [Qemu-ppc][PATCH v5 1/4] Add usb option in machine options
From: Li Zhang zhlci...@linux.vnet.ibm.com pSeries machine needs to enable USB to add a USB keyboard or USB mouse. -usb option won't be used in the future, and machine options are a better way to enable USB. So this patch is to add USB option to machine options (-machine type=pseries,usb=on/off) to enable/disable USB controller. And machines will get the machine option and create a USB controller if USB is on. By the way, USB is on by default on pSeries machine. So USB controller should be turned off explicitly through the command line for -nodefault case as the following: -machine type=pseries,usb=off. Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com Reviewed-by: Andreas Färber afaer...@suse.de --- hw/spapr.c| 11 +++ qemu-config.c |4 2 files changed, 15 insertions(+) diff --git a/hw/spapr.c b/hw/spapr.c index 81c9343..973de1b 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -575,6 +575,8 @@ static void ppc_spapr_init(ram_addr_t ram_size, long load_limit, rtas_limit, fw_size; long pteg_shift = 17; char *filename; +QemuOpts *machine_opts; +bool add_usb = true; spapr = g_malloc0(sizeof(*spapr)); QLIST_INIT(spapr-phbs); @@ -710,6 +712,15 @@ static void ppc_spapr_init(ram_addr_t ram_size, spapr_vscsi_create(spapr-vio_bus); } +machine_opts = qemu_opts_find(qemu_find_opts(machine), 0); +if (machine_opts) { +add_usb = qemu_opt_get_bool(machine_opts, usb, true); +} + +if (add_usb) { +pci_create_simple(QLIST_FIRST(spapr-phbs)-host_state.bus, + -1, pci-ohci); +} if (rma_size (MIN_RMA_SLOF 20)) { fprintf(stderr, qemu: pSeries SLOF firmware requires = %ldM guest RMA (Real Mode Area memory)\n, MIN_RMA_SLOF); diff --git a/qemu-config.c b/qemu-config.c index 5c3296b..b86ee36 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -595,6 +595,10 @@ static QemuOptsList qemu_machine_opts = { .name = dt_compatible, .type = QEMU_OPT_STRING, .help = Overrides the \compatible\ property of the dt root node, +},{ +.name = usb, +.type = QEMU_OPT_BOOL, +.help = Set on/off to enable/disable usb, }, { /* End of list */ } }, -- 1.7.9.5