[Qemu-devel] [PATCH] boot-order-test: Add tests for PowerMacs
They set the boot device via fw_cfg, which is then translated to a boot path of hd or cd in OpenBIOS. Signed-off-by: Andreas Färber afaer...@suse.de Cc: Markus Armbruster arm...@redhat.com Cc: Alexander Graf ag...@suse.de Cc: qemu-...@nongnu.org --- tests/Makefile |2 ++ tests/boot-order-test.c | 66 ++- 2 Dateien geändert, 67 Zeilen hinzugefügt(+), 1 Zeile entfernt(-) diff --git a/tests/Makefile b/tests/Makefile index 1305642..f84d466 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -76,6 +76,8 @@ gcov-files-sparc-y += hw/m48t59.c gcov-files-sparc64-y += hw/m48t59.c check-qtest-arm-y = tests/tmp105-test$(EXESUF) gcov-files-arm-y += hw/tmp105.c +check-qtest-ppc-y += tests/boot-order-test$(EXESUF) +check-qtest-ppc64-y += tests/boot-order-test$(EXESUF) GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c index 60412ad..84dd694 100644 --- a/tests/boot-order-test.c +++ b/tests/boot-order-test.c @@ -14,8 +14,10 @@ * Covers only PC. Better than nothing. Improvements welcome. */ +#include string.h #include glib.h #include libqtest.h +#include qemu/bswap.h static void test_cmos_byte(int reg, int expected) { @@ -61,11 +63,73 @@ static void test_pc_boot_order(void) 0, 0x02, 0x30, 0x12); } +#define G3BEIGE_CFG_ADDR 0xf510 +#define MAC99_CFG_ADDR 0xf510 + +#define NO_QEMU_PROTOS +#include hw/fw_cfg.h +#undef NO_QEMU_PROTOS + +static void powermac_fw_cfg_read(bool newworld, uint16_t cmd, + uint8_t *buf, unsigned int len) +{ +unsigned int i; + +writew(newworld ? MAC99_CFG_ADDR : G3BEIGE_CFG_ADDR, cmd); +for (i = 0; i len; i++) { +buf[i] = readb((newworld ? MAC99_CFG_ADDR : G3BEIGE_CFG_ADDR) + 2); +} +} + +static uint16_t powermac_fw_cfg_read16(bool newworld, uint16_t cmd) +{ +uint16_t value; + +powermac_fw_cfg_read(newworld, cmd, (uint8_t *)value, sizeof(value)); +return le16_to_cpu(value); +} + +static void test_powermac_with_args(bool newworld, const char *extra_args, +uint16_t expected_boot, +uint16_t expected_reboot) +{ +char *args = g_strdup_printf(-nodefaults -display none -machine %s %s, + newworld ? mac99 : g3beige, extra_args); +uint16_t actual; +qtest_start(args); +actual = powermac_fw_cfg_read16(newworld, FW_CFG_BOOT_DEVICE); +g_assert_cmphex(actual, ==, expected_boot); +qmp({ 'execute': 'system_reset' }); +actual = powermac_fw_cfg_read16(newworld, FW_CFG_BOOT_DEVICE); +g_assert_cmphex(actual, ==, expected_reboot); +qtest_quit(global_qtest); +g_free(args); +} + +static void test_powermac_boot_order(void) +{ +int i; + +for (i = 0; i 2; i++) { +bool newworld = (i == 1); + +test_powermac_with_args(newworld, , 'c', 'c'); +test_powermac_with_args(newworld, -boot c, 'c', 'c'); +test_powermac_with_args(newworld, -boot d, 'd', 'd'); +} +} + int main(int argc, char *argv[]) { +const char *arch = qtest_get_arch(); + g_test_init(argc, argv, NULL); -qtest_add_func(boot-order/pc, test_pc_boot_order); +if (strcmp(arch, i386) == 0 || strcmp(arch, x86_64) == 0) { +qtest_add_func(boot-order/pc, test_pc_boot_order); +} else if (strcmp(arch, ppc) == 0 || strcmp(arch, ppc64) == 0) { +qtest_add_func(boot-order/powermac, test_powermac_boot_order); +} return g_test_run(); } -- 1.7.10.4
Re: [Qemu-devel] [PATCH v2 8/8] qtest: Add boot order test
Am 22.02.2013 18:20, schrieb Markus Armbruster: diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c new file mode 100644 index 000..60412ad --- /dev/null +++ b/tests/boot-order-test.c [...] +static void test_pc_with_args(const char *args, uint8_t boot1, uint8_t boot2, + uint8_t reboot1, uint8_t reboot2) +{ +char *extra_args = g_strdup_printf(-nodefaults -display none -S %s, + args); Why do you add -S here? qtest is an accel mode of its own and should never execute anything. Also in my code I found it more logical to add extra args to args rather than the reverse, not just because of 80 chars limit when trying to add -machine argument. ;) I assume q35 is reusing pc code? Or should it be tested as well? (Having any of our qtests cover it would be nice.) +qtest_start(extra_args); +test_cmos(boot1, boot2); +qtest_qmp(global_qtest, { 'execute': 'system_reset' }); qmp()? +test_cmos(reboot1, reboot2); +qtest_quit(global_qtest); +g_free(extra_args); +} [snip] Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RFC PATCH 3/3] KVM: s390: Hook up ioeventfds.
On Fri, Feb 22, 2013 at 08:22:08AM +0100, Cornelia Huck wrote: On Thu, 21 Feb 2013 22:42:41 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 21, 2013 at 07:14:31PM +0100, Cornelia Huck wrote: On Thu, 21 Feb 2013 18:34:59 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 21, 2013 at 04:21:43PM +0100, Cornelia Huck wrote: On Thu, 21 Feb 2013 16:39:05 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Thu, Feb 21, 2013 at 02:13:00PM +0100, Cornelia Huck wrote: As s390 doesn't use memory writes for virtio notifcations, create a special kind of ioeventfd instead that hooks up into diagnose 0x500 (kvm hypercall) with function code 3 (virtio-ccw notification). Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com Do we really have to put virtio specific stuff into kvm? How about we add generic functionality to match GPRs on a hypercall and signal an eventfd? Worth a try implementing that. Also, it's a bit unfortunate that this doesn't use the io bus datastructure, long term the linked list handling might become a bottleneck, using shared code this could maybe benefit from performance optimizations there. The linked list stuff was more like an initial implementation that could be improved later. io bus data structure currently has the ability to match on two 64 bit fields (addr/datamatch) and a signed 32 bit one (length). Isn't this sufficient for your purposes? How about sticking subchannel id in address, vq in data match and using io bus? I can give that a try. (I must admit that I didn't look at the iobus stuff in detail.) BTW maybe we could do this for the user interface too, while I'm not 100% sure it's the cleanest thing to do (or will work), it would certainly minimize the patchset. You mean integrating with the generic interface and dropping the new ARCH flag? Not sure about the flag but we could use the general structure without an arch-specific format, if that's a good fit. So I have something that seems to do what I want. I'll see if I can morph it into something presentable tomorrow. diff --git a/arch/s390/kvm/Kconfig b/arch/s390/kvm/Kconfig index b58dd86..3c43e30 100644 --- a/arch/s390/kvm/Kconfig +++ b/arch/s390/kvm/Kconfig @@ -22,6 +22,7 @@ config KVM select PREEMPT_NOTIFIERS select ANON_INODES select HAVE_KVM_CPU_RELAX_INTERCEPT + select HAVE_KVM_EVENTFD ---help--- Support hosting paravirtualized guest machines using the SIE virtualization capability on the mainframe. This should work diff --git a/arch/s390/kvm/Makefile b/arch/s390/kvm/Makefile index 3975722..8fe9d65 100644 --- a/arch/s390/kvm/Makefile +++ b/arch/s390/kvm/Makefile @@ -6,7 +6,7 @@ # it under the terms of the GNU General Public License (version 2 only) # as published by the Free Software Foundation. -common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o) +common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o eventfd.o) ccflags-y := -Ivirt/kvm -Iarch/s390/kvm diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c index a390687..7fc195e 100644 --- a/arch/s390/kvm/diag.c +++ b/arch/s390/kvm/diag.c @@ -104,6 +104,20 @@ static int __diag_ipl_functions(struct kvm_vcpu *vcpu) return -EREMOTE; } +static int __diag_virtio_hypercall(struct kvm_vcpu *vcpu) +{ + int ret, idx; + u64 vq = vcpu-run-s.regs.gprs[3]; + + idx = srcu_read_lock(vcpu-kvm-srcu); + ret = kvm_io_bus_write(vcpu-kvm, KVM_CSS_BUS, + vcpu-run-s.regs.gprs[2], + vcpu-run-s.regs.gprs[1], + vq); Hmm, do you really need three gprs? If not, we could just pass len == 8, which would be cleaner. Also might as well drop the vq variable, no? If I want to pass generic hypercalls, I need to pass the subcode (in gpr 1) in the len variable. If all we'll ever need is the virtio-ccw notify hypercall, gprs 2 and 3 are enough. Would perhaps make the common code less hacky, but we'd lose (unneeded?) flexibility. I see. I'm fine with either way, but I note the code above can overflow int len, which is likely not what you want. + srcu_read_unlock(vcpu-kvm-srcu, idx); + return ret; +} + int kvm_s390_handle_diag(struct kvm_vcpu *vcpu) { int code = (vcpu-arch.sie_block-ipb 0xfff) 16; @@ -118,6 +132,8 @@ int kvm_s390_handle_diag(struct kvm_vcpu *vcpu) return __diag_time_slice_end_directed(vcpu); case 0x308: return __diag_ipl_functions(vcpu); + case 0x500: + return __diag_virtio_hypercall(vcpu); default: return -EOPNOTSUPP; }
Re: [Qemu-devel] [RFC PATCH v2 0/4] kvm: Make ioeventfd usable on s390.
On Fri, Feb 22, 2013 at 01:09:45PM +0100, Cornelia Huck wrote: Here's the second attempt at implementing ioeventfd for s390. Rather than the architecture-specific functions used in v1, we now try to integrate with the kvm_io_device infrastructure. Calls to diagnose 500 subcode 3 are now mapped to _write. These devices are created on a new KVM_CSS_BUS when using a new flag KVM_IOEVENTFD_FLAG_CSS. addr and datamatch are (ab)used to contain the subchannel id and the virtqueue. OK so a question: qemu has: #define KVM_S390_VIRTIO_NOTIFY 0 #define KVM_S390_VIRTIO_RESET 1 #define KVM_S390_VIRTIO_SET_STATUS 2 #define KVM_S390_VIRTIO_CCW_NOTIFY 3 So this is really s390_virtio_ccw not generally css? Maybe should say so in all headers. we want to attach other hypercalls or carry more payload. Another limitation is the limit of 1000 io devices per bus, which we would hit easily with a few hundred devices, but that should be fixable. v1 - v2: - Move irqfd initialization from a module init function to kvm_init, eliminating the need for a second module for kvm/s390. - Use kvm_io_device for s390 css devices. Cornelia Huck (4): KVM: Initialize irqfd from kvm_init(). KVM: Introduce KVM_CSS_BUS. KVM: ioeventfd for s390 css devices. KVM: s390: Wire up ioeventfd. Documentation/virtual/kvm/api.txt | 7 +++ arch/s390/kvm/Kconfig | 1 + arch/s390/kvm/Makefile| 2 +- arch/s390/kvm/diag.c | 25 + arch/s390/kvm/kvm-s390.c | 1 + include/linux/kvm_host.h | 14 ++ include/uapi/linux/kvm.h | 2 ++ virt/kvm/eventfd.c| 15 --- virt/kvm/kvm_main.c | 6 ++ 9 files changed, 65 insertions(+), 8 deletions(-) -- 1.7.12.4
Re: [Qemu-devel] [RFC PATCH v2 4/4] KVM: s390: Wire up ioeventfd.
On Fri, Feb 22, 2013 at 01:09:49PM +0100, Cornelia Huck wrote: Enable ioeventfd support on s390 and hook up diagnose 500 virtio-ccw notifications. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- arch/s390/kvm/Kconfig| 1 + arch/s390/kvm/Makefile | 2 +- arch/s390/kvm/diag.c | 25 + arch/s390/kvm/kvm-s390.c | 1 + 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/arch/s390/kvm/Kconfig b/arch/s390/kvm/Kconfig index b58dd86..3c43e30 100644 --- a/arch/s390/kvm/Kconfig +++ b/arch/s390/kvm/Kconfig @@ -22,6 +22,7 @@ config KVM select PREEMPT_NOTIFIERS select ANON_INODES select HAVE_KVM_CPU_RELAX_INTERCEPT + select HAVE_KVM_EVENTFD ---help--- Support hosting paravirtualized guest machines using the SIE virtualization capability on the mainframe. This should work diff --git a/arch/s390/kvm/Makefile b/arch/s390/kvm/Makefile index 3975722..8fe9d65 100644 --- a/arch/s390/kvm/Makefile +++ b/arch/s390/kvm/Makefile @@ -6,7 +6,7 @@ # it under the terms of the GNU General Public License (version 2 only) # as published by the Free Software Foundation. -common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o) +common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o eventfd.o) ccflags-y := -Ivirt/kvm -Iarch/s390/kvm diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c index a390687..13cf74a 100644 --- a/arch/s390/kvm/diag.c +++ b/arch/s390/kvm/diag.c @@ -104,6 +104,29 @@ static int __diag_ipl_functions(struct kvm_vcpu *vcpu) return -EREMOTE; } +static int __diag_virtio_hypercall(struct kvm_vcpu *vcpu) +{ + int ret, idx; + + /* No virtio-ccw notification? Get out quickly. */ + if (!vcpu-kvm-arch.css_support || + (vcpu-run-s.regs.gprs[1] != 3)) + return -EOPNOTSUPP; Looks like we have: arch/s390/include/uapi/asm/kvm_virtio.h:#define KVM_S390_VIRTIO_NOTIFY 0 arch/s390/include/uapi/asm/kvm_virtio.h:#define KVM_S390_VIRTIO_RESET 1 arch/s390/include/uapi/asm/kvm_virtio.h:#define KVM_S390_VIRTIO_SET_STATUS 2 KVM_S390_VIRTIO_CSS_NOTIFY is missing? Strange. Maybe it should be added in that header and included here? Do some guests use it? Also, do you want to support KVM_S390_VIRTIO_NOTIFY as well? + + idx = srcu_read_lock(vcpu-kvm-srcu); + /* + * The layout is as follows: + * - gpr 2 contains the subchannel id (passed as addr) + * - gpr 3 contains the virtqueue index (passed as datamatch) + */ + ret = kvm_io_bus_write(vcpu-kvm, KVM_CSS_BUS, + vcpu-run-s.regs.gprs[2], + 8, vcpu-run-s.regs.gprs[3]); + srcu_read_unlock(vcpu-kvm-srcu, idx); + /* kvm_io_bus_write returns -EOPNOTSUPP if it found no match. */ + return ret; +} + int kvm_s390_handle_diag(struct kvm_vcpu *vcpu) { int code = (vcpu-arch.sie_block-ipb 0xfff) 16; @@ -118,6 +141,8 @@ int kvm_s390_handle_diag(struct kvm_vcpu *vcpu) return __diag_time_slice_end_directed(vcpu); case 0x308: return __diag_ipl_functions(vcpu); + case 0x500: + return __diag_virtio_hypercall(vcpu); default: return -EOPNOTSUPP; } diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index f822d36..04d2454 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -142,6 +142,7 @@ int kvm_dev_ioctl_check_extension(long ext) case KVM_CAP_ONE_REG: case KVM_CAP_ENABLE_CAP: case KVM_CAP_S390_CSS_SUPPORT: + case KVM_CAP_IOEVENTFD: r = 1; break; case KVM_CAP_NR_VCPUS: -- 1.7.12.4
Re: [Qemu-devel] [RFC PATCH v2 3/4] KVM: ioeventfd for s390 css devices.
On Fri, Feb 22, 2013 at 01:09:48PM +0100, Cornelia Huck wrote: Enhance KVM_IOEVENTFD with a new flag that allows to attach to s390 css devices. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- Documentation/virtual/kvm/api.txt | 7 +++ include/uapi/linux/kvm.h | 2 ++ virt/kvm/eventfd.c| 8 ++-- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index c2534c3..40e799c 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -1468,15 +1468,22 @@ struct kvm_ioeventfd { __u8 pad[36]; }; +For the special case of s390 css devices, the ioevent is matched to a +subchannel/virtqueue tuple instead. + The following flags are defined: #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 kvm_ioeventfd_flag_nr_datamatch) #define KVM_IOEVENTFD_FLAG_PIO (1 kvm_ioeventfd_flag_nr_pio) #define KVM_IOEVENTFD_FLAG_DEASSIGN (1 kvm_ioeventfd_flag_nr_deassign) +#define KVM_IOEVENTFD_FLAG_CSS (1 kvm_ioeventfd_flag_nr_css) If datamatch flag is set, the event will be signaled only if the written value to the registered address is equal to datamatch in struct kvm_ioeventfd. +For css devices, addr contains the subchannel id and datamatch the virtqueue +index. + 4.60 KVM_DIRTY_TLB diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 9a2db57..1df0766 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -448,12 +448,14 @@ enum { kvm_ioeventfd_flag_nr_datamatch, kvm_ioeventfd_flag_nr_pio, kvm_ioeventfd_flag_nr_deassign, + kvm_ioeventfd_flag_nr_css, kvm_ioeventfd_flag_nr_max, }; #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 kvm_ioeventfd_flag_nr_datamatch) #define KVM_IOEVENTFD_FLAG_PIO (1 kvm_ioeventfd_flag_nr_pio) #define KVM_IOEVENTFD_FLAG_DEASSIGN (1 kvm_ioeventfd_flag_nr_deassign) +#define KVM_IOEVENTFD_FLAG_CSS (1 kvm_ioeventfd_flag_nr_css) So let's explicitly name is KVM_IOEVENTFD_S390_VIRTIO_CCW or even KVM_IOEVENTFD_S390_VIRTIO_CCW_NOTIFY? Also, maybe we want to add KVM_IOEVENTFD_S390_VIRTIO_NOTIFY as well? #define KVM_IOEVENTFD_VALID_FLAG_MASK ((1 kvm_ioeventfd_flag_nr_max) - 1) diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c index f0ced1a..7347652 100644 --- a/virt/kvm/eventfd.c +++ b/virt/kvm/eventfd.c @@ -679,11 +679,13 @@ static int kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) { int pio = args-flags KVM_IOEVENTFD_FLAG_PIO; - enum kvm_bus bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS; + int css = args-flags KVM_IOEVENTFD_FLAG_CSS; + enum kvm_bus bus_idx; struct _ioeventfd*p; struct eventfd_ctx *eventfd; int ret; + bus_idx = pio ? KVM_PIO_BUS : css ? KVM_CSS_BUS: KVM_MMIO_BUS; /* must be natural-word sized */ switch (args-len) { case 1: @@ -759,11 +761,13 @@ static int kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) { int pio = args-flags KVM_IOEVENTFD_FLAG_PIO; - enum kvm_bus bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS; + int css = args-flags KVM_IOEVENTFD_FLAG_CSS; + enum kvm_bus bus_idx; struct _ioeventfd*p, *tmp; struct eventfd_ctx *eventfd; int ret = -ENOENT; + bus_idx = pio ? KVM_PIO_BUS : css ? KVM_CSS_BUS: KVM_MMIO_BUS; eventfd = eventfd_ctx_fdget(args-fd); if (IS_ERR(eventfd)) return PTR_ERR(eventfd); -- 1.7.12.4
Re: [Qemu-devel] [RFC PATCH v2 0/4] kvm: Make ioeventfd usable on s390.
On Fri, Feb 22, 2013 at 01:09:45PM +0100, Cornelia Huck wrote: Here's the second attempt at implementing ioeventfd for s390. The patchset looks fine overall. Minor comments and questions below. Rather than the architecture-specific functions used in v1, we now try to integrate with the kvm_io_device infrastructure. Calls to diagnose 500 subcode 3 are now mapped to _write. These devices are created on a new KVM_CSS_BUS when using a new flag KVM_IOEVENTFD_FLAG_CSS. addr and datamatch are (ab)used to contain the subchannel id and the virtqueue. A drawback is that this interface is not easily extendable should we want to attach other hypercalls or carry more payload. Under s390 kvm_hypercallX already uses diagnose 500 so that seems fine. If you want to make it more generic and support more subcodes, I think you'll have to pass an extra u64 field: to bus to both avoid overflowing int value and avoid ugly bus-specific hacks in generic code. Will we ever need that? Code using subcode 3 does not yet seem to be upstream in 3.8 so maybe yes, but you decide. An alternative is to add new bus types when kvm needs to handle new subcodes. So e.g. KVM_BUS_S390_VIRTIO_CCW_NOTIFY and KVM_BUS_S390_VIRTIO_NOTIFY ? You decide, I'm fine with either approach. More minor comments and questions in response to individual patches. Another limitation is the limit of 1000 io devices per bus, which we would hit easily with a few hundred devices, but that should be fixable. v1 - v2: - Move irqfd initialization from a module init function to kvm_init, eliminating the need for a second module for kvm/s390. - Use kvm_io_device for s390 css devices. Cornelia Huck (4): KVM: Initialize irqfd from kvm_init(). KVM: Introduce KVM_CSS_BUS. KVM: ioeventfd for s390 css devices. KVM: s390: Wire up ioeventfd. Documentation/virtual/kvm/api.txt | 7 +++ arch/s390/kvm/Kconfig | 1 + arch/s390/kvm/Makefile| 2 +- arch/s390/kvm/diag.c | 25 + arch/s390/kvm/kvm-s390.c | 1 + include/linux/kvm_host.h | 14 ++ include/uapi/linux/kvm.h | 2 ++ virt/kvm/eventfd.c| 15 --- virt/kvm/kvm_main.c | 6 ++ 9 files changed, 65 insertions(+), 8 deletions(-) -- 1.7.12.4
Re: [Qemu-devel] [RFC PATCH v2 2/4] KVM: Introduce KVM_CSS_BUS.
On Fri, Feb 22, 2013 at 01:09:47PM +0100, Cornelia Huck wrote: Add a new bus type for s390 css kvm io devices. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- include/linux/kvm_host.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 3b768ef..59be516 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -148,6 +148,7 @@ struct kvm_io_bus { enum kvm_bus { KVM_MMIO_BUS, KVM_PIO_BUS, + KVM_CSS_BUS, so maybe KVM_S390_VIRTIO_CCW_NOTIFY_BUS to make the fact it's s390 and virtio specific, explicit? KVM_NR_BUSES }; -- 1.7.12.4
[Qemu-devel] [PATCH] gtk: Release modifier when graphic console loses keyboard focus
From: Jan Kiszka jan.kis...@siemens.com This solves, e.g., sticky ALT when selecting a GTK menu, switching to a different window or selecting a different virtual console. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- ui/gtk.c | 52 ++-- 1 files changed, 50 insertions(+), 2 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index 7115cf6..daffc30 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -74,6 +74,11 @@ #define MAX_VCS 10 +static const int modifier_keycode[] = { +/* shift, control, alt keys, meta keys, both left right */ +0x2a, 0x36, 0x1d, 0x9d, 0x38, 0xb8, 0xdb, 0xdd, +}; + typedef struct VirtualConsole { GtkWidget *menu_item; @@ -133,6 +138,8 @@ typedef struct GtkDisplayState gboolean free_scale; bool external_pause_update; + +bool modifier_pressed[ARRAY_SIZE(modifier_keycode)]; } GtkDisplayState; static GtkDisplayState *global_state; @@ -201,6 +208,26 @@ static void gd_update_caption(GtkDisplayState *s) g_free(title); } +static void gtk_release_modifiers(GtkDisplayState *s) +{ +int i, keycode; + +if (!gd_on_vga(s)) { +return; +} +for (i = 0; i ARRAY_SIZE(modifier_keycode); i++) { +keycode = modifier_keycode[i]; +if (!s-modifier_pressed[i]) { +continue; +} +if (keycode SCANCODE_GREY) { +kbd_put_keycode(SCANCODE_EMUL0); +} +kbd_put_keycode(keycode | SCANCODE_UP); +s-modifier_pressed[i] = false; +} +} + /** DisplayState Callbacks **/ static void gd_update(DisplayState *ds, int x, int y, int w, int h) @@ -328,8 +355,9 @@ static gboolean gd_window_key_event(GtkWidget *widget, GdkEventKey *key, void *o key-state == (GDK_CONTROL_MASK | GDK_MOD1_MASK)) { handled = gtk_window_activate_key(GTK_WINDOW(widget), key); } - -if (!handled) { +if (handled) { +gtk_release_modifiers(s); +} else { handled = gtk_window_propagate_key_event(GTK_WINDOW(widget), key); } @@ -553,8 +581,10 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button, static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque) { +GtkDisplayState *s = opaque; int gdk_keycode; int qemu_keycode; +int i; gdk_keycode = key-hardware_keycode; @@ -576,6 +606,12 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque) gdk_keycode, qemu_keycode, (key-type == GDK_KEY_PRESS) ? down : up); +for (i = 0; i ARRAY_SIZE(modifier_keycode); i++) { +if (qemu_keycode == modifier_keycode[i]) { +s-modifier_pressed[i] = (key-type == GDK_KEY_PRESS); +} +} + if (qemu_keycode SCANCODE_GREY) { kbd_put_keycode(SCANCODE_EMUL0); } @@ -631,6 +667,7 @@ static void gd_menu_switch_vc(GtkMenuItem *item, void *opaque) } else { int i; +gtk_release_modifiers(s); for (i = 0; i s-nb_vcs; i++) { if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s-vc[i].menu_item))) { gtk_notebook_set_current_page(GTK_NOTEBOOK(s-notebook), i + 1); @@ -840,6 +877,15 @@ static gboolean gd_leave_event(GtkWidget *widget, GdkEventCrossing *crossing, gp return TRUE; } +static gboolean gd_focus_out_event(GtkWidget *widget, GdkEventCrossing *crossing, gpointer data) +{ +GtkDisplayState *s = data; + +gtk_release_modifiers(s); + +return TRUE; +} + /** Virtual Console Callbacks **/ static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len) @@ -1018,6 +1064,8 @@ static void gd_connect_signals(GtkDisplayState *s) G_CALLBACK(gd_enter_event), s); g_signal_connect(s-drawing_area, leave-notify-event, G_CALLBACK(gd_leave_event), s); +g_signal_connect(s-drawing_area, focus-out-event, + G_CALLBACK(gd_focus_out_event), s); } static void gd_create_menus(GtkDisplayState *s) -- 1.7.3.4
Re: [Qemu-devel] [PATCH 2/6] cpu: Introduce ENV_OFFSET macros
Am 22.02.2013 19:22, schrieb Peter Maydell: On 22 February 2013 18:20, Andreas Färber afaer...@suse.de wrote: Am 22.02.2013 19:10, schrieb Peter Maydell: From: Andreas Färber afaer...@suse.de index c0f6c6d..252bd14 100644 --- a/target-alpha/cpu-qom.h +++ b/target-alpha/cpu-qom.h @@ -72,5 +72,6 @@ static inline AlphaCPU *alpha_env_get_cpu(CPUAlphaState *env) #define ENV_GET_CPU(e) CPU(alpha_env_get_cpu(e)) +#define ENV_OFFSET offsetof(AlphaCPU, env) #endif [snip] Compared to my rebased copy of this patch on qom-cpu-10 yours is dropping a white line per target. I've intentionally used two lines spacing for grouping and a line between definitions. Matter of taste. Yeah, I deliberately dropped a bunch of what I felt were extraneous blank lines :-) I've applied it to qom-cpu since it conflicts with my additions of vmstate_..._cpu fields and ..._do_interrupt() functions. https://github.com/afaerber/qemu-cpu/commits/qom-cpu But I will remind you of you messing with my coding style next time you ask me to redo any ifs to your liking. May Markus be my witness. Either we respect each other's style in cross-target refactorings or we don't. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
Am 22.02.2013 20:01, schrieb Jens Freimann: From: Christian Borntraeger borntrae...@de.ibm.com This patch fixes a crash when unplugging a virtio-ccw device. We no longer need to do that in virtio-ccw since common code does now proper handling. Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com --- hw/s390x/virtio-ccw.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 231f81e..679afa6 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -865,7 +865,6 @@ static int virtio_ccw_busdev_unplug(DeviceState *dev) css_generate_sch_crws(sch-cssid, sch-ssid, sch-schid, 1, 0); -object_unparent(OBJECT(dev)); qdev_free(dev); return 0; } qdev_free() does exactly object_unparent(), so in light of wanting to get away from qdev I would suggest to rather drop qdev_free() here. Paolo? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 2/6] cpu: Introduce ENV_OFFSET macros
On 24 February 2013 11:27, Andreas Färber afaer...@suse.de wrote: Am 22.02.2013 19:22, schrieb Peter Maydell: Yeah, I deliberately dropped a bunch of what I felt were extraneous blank lines :-) I've applied it to qom-cpu since it conflicts with my additions of vmstate_..._cpu fields and ..._do_interrupt() functions. https://github.com/afaerber/qemu-cpu/commits/qom-cpu But I will remind you of you messing with my coding style next time you ask me to redo any ifs to your liking. May Markus be my witness. Either we respect each other's style in cross-target refactorings or we don't. In this case I actually kind of 50% thought they were the result of conflicts/merge process rather than intentional, which is why I cleaned them up. I don't actually care one way or the other, so you can reinstate them if you prefer. -- PMM
Re: [Qemu-devel] [PATCH 2/3] s390/css: Fix subchannel detection
Am 22.02.2013 20:01, schrieb Jens Freimann: From: Christian Borntraeger borntrae...@de.ibm.com We have to consider the m bit to find the real channel subsystem when determining the last subchannel. If we fail to take this into account, removal of a subchannel in the middle of a big list of devices will stop device detection after a reboot. Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/css.c| 11 +++ target-s390x/cpu.h| 2 +- target-s390x/ioinst.c | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 3244201..8240e48 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -988,15 +988,18 @@ int css_do_rchp(uint8_t cssid, uint8_t chpid) return 0; } -bool css_schid_final(uint8_t cssid, uint8_t ssid, uint16_t schid) +bool css_schid_final(int m, uint8_t cssid, uint8_t ssid, uint16_t schid) { SubchSet *set; +uint8_t real_cssid; -if (cssid MAX_CSSID || ssid MAX_SSID || !channel_subsys-css[cssid] || -!channel_subsys-css[cssid]-sch_set[ssid]) { +real_cssid = (!m (cssid == 0)) ? channel_subsys-default_cssid : cssid; If m is a single bit and only used as boolean here, any reason not to make the argument a bool? Cheers, Andreas +if (real_cssid MAX_CSSID || ssid MAX_SSID || +!channel_subsys-css[real_cssid] || +!channel_subsys-css[real_cssid]-sch_set[ssid]) { return true; } -set = channel_subsys-css[cssid]-sch_set[ssid]; +set = channel_subsys-css[real_cssid]-sch_set[ssid]; return schid find_last_bit(set-schids_used, (MAX_SCHID + 1) / sizeof(unsigned long)); } diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h index 01e59b9..670603a 100644 --- a/target-s390x/cpu.h +++ b/target-s390x/cpu.h @@ -405,7 +405,7 @@ SubchDev *css_find_subch(uint8_t m, uint8_t cssid, uint8_t ssid, bool css_subch_visible(SubchDev *sch); void css_conditional_io_interrupt(SubchDev *sch); int css_do_stsch(SubchDev *sch, SCHIB *schib); -bool css_schid_final(uint8_t cssid, uint8_t ssid, uint16_t schid); +bool css_schid_final(int m, uint8_t cssid, uint8_t ssid, uint16_t schid); int css_do_msch(SubchDev *sch, SCHIB *schib); int css_do_xsch(SubchDev *sch); int css_do_csch(SubchDev *sch); diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c index e3531f3..28c508d 100644 --- a/target-s390x/ioinst.c +++ b/target-s390x/ioinst.c @@ -316,7 +316,7 @@ int ioinst_handle_stsch(CPUS390XState *env, uint64_t reg1, uint32_t ipb) cc = 3; } } else { -if (css_schid_final(cssid, ssid, schid)) { +if (css_schid_final(m, cssid, ssid, schid)) { cc = 3; /* No more subchannels in this css/ss */ } else { /* Store an empty schib. */ -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 2/6] cpu: Introduce ENV_OFFSET macros
Am 24.02.2013 12:31, schrieb Peter Maydell: On 24 February 2013 11:27, Andreas Färber afaer...@suse.de wrote: Am 22.02.2013 19:22, schrieb Peter Maydell: Yeah, I deliberately dropped a bunch of what I felt were extraneous blank lines :-) I've applied it to qom-cpu since it conflicts with my additions of vmstate_..._cpu fields and ..._do_interrupt() functions. https://github.com/afaerber/qemu-cpu/commits/qom-cpu But I will remind you of you messing with my coding style next time you ask me to redo any ifs to your liking. May Markus be my witness. Either we respect each other's style in cross-target refactorings or we don't. In this case I actually kind of 50% thought they were the result of conflicts/merge process rather than intentional, which is why I cleaned them up. I don't actually care one way or the other, so you can reinstate them if you prefer. My reasoning was to differenciate between the header guard and any in-file #ifdef CONFIG_USER_ONLY or TARGET_FOO, which for functions I usually separate by one line. I don't really care too much though, it's just the principle that angers me that you made me go through hoops, propagating adopt-the-author's-style when it comes to target-arm files, while now violating your own paradigm and apparently even finding that funny. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 2/3] s390/css: Fix subchannel detection
On 24/02/13 12:32, Andreas Färber wrote: Am 22.02.2013 20:01, schrieb Jens Freimann: From: Christian Borntraeger borntrae...@de.ibm.com We have to consider the m bit to find the real channel subsystem when determining the last subchannel. If we fail to take this into account, removal of a subchannel in the middle of a big list of devices will stop device detection after a reboot. Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/css.c| 11 +++ target-s390x/cpu.h| 2 +- target-s390x/ioinst.c | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 3244201..8240e48 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -988,15 +988,18 @@ int css_do_rchp(uint8_t cssid, uint8_t chpid) return 0; } -bool css_schid_final(uint8_t cssid, uint8_t ssid, uint16_t schid) +bool css_schid_final(int m, uint8_t cssid, uint8_t ssid, uint16_t schid) { SubchSet *set; +uint8_t real_cssid; -if (cssid MAX_CSSID || ssid MAX_SSID || !channel_subsys-css[cssid] || -!channel_subsys-css[cssid]-sch_set[ssid]) { +real_cssid = (!m (cssid == 0)) ? channel_subsys-default_cssid : cssid; If m is a single bit and only used as boolean here, any reason not to make the argument a bool? Consistency - the calling code also has m defined as int (the whole css and ioinst code) - making it bool wouldnt fit into that code. Furthermore, bool doesnt seem to be the perfect fit for a bit, a bit is 0 or 1 and not true or false. If bool, the variable should be called mbitset or something like that. Christian Christian
[Qemu-devel] Segfault in block driver with qemu-system-ppc and -M mac99
Hi all, Whilst running through some OpenBIOS tests, I came across the following segfault in qemu-system-ppc with -M mac99 on git master. It is consistently reproducible here with my test openSUSE image although strangely all my other images seem to run fine. The host is running amd64 Debian Wheezy. build@kentang:~/rel-qemu-git/bin$ gdb --args ./qemu-system-ppc -cdrom /home/build/src/qemu/image/ppc/openSUSE-11.1-NET-ppc.iso -boot d -g 1024x768x32 -vnc :1 -m 512 -M mac99 GNU gdb (GDB) 7.4.1-debian Copyright (C) 2012 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type show copying and show warranty for details. This GDB was configured as x86_64-linux-gnu. For bug reporting instructions, please see: http://www.gnu.org/software/gdb/bugs/... Reading symbols from /home/build/rel-qemu-git/bin/qemu-system-ppc...done. (gdb) handle SIGUSR1 nostop noprint SignalStop Print Pass to program Description SIGUSR1 NoNo Yes User defined signal 1 (gdb) run Starting program: /home/build/rel-qemu-git/bin/qemu-system-ppc -cdrom /home/build/src/qemu/image/ppc/openSUSE-11.1-NET-ppc.iso -boot d -g 1024x768x32 -vnc :1 -m 512 -M mac99 [Thread debugging using libthread_db enabled] Using host libthread_db library /lib/x86_64-linux-gnu/libthread_db.so.1. [New Thread 0x7fffdf197700 (LWP 17230)] [New Thread 0x7fffde996700 (LWP 17231)] [New Thread 0x7fffbc9e8700 (LWP 17232)] [Thread 0x7fffdf197700 (LWP 17230) exited] Program received signal SIGSEGV, Segmentation fault. 0x555ef458 in bdrv_co_do_readv (bs=0x0, sector_num=169, nb_sectors=0, qiov=0x5661f878, flags=0) at block.c:2240 2240BlockDriver *drv = bs-drv; (gdb) bt #0 0x555ef458 in bdrv_co_do_readv (bs=0x0, sector_num=169, nb_sectors=0, qiov=0x5661f878, flags=0) at block.c:2240 #1 0x555f3a28 in bdrv_co_do_rw (opaque=0x56620030) at block.c:3837 #2 0x55631fa4 in coroutine_trampoline (i0=1448744496, i1=21845) at coroutine-ucontext.c:138 #3 0x7508c020 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #4 0x7fffb100 in ?? () #5 0x in ?? () (gdb) It seems as if the BlockDriver pointer is coming back as NULL - does anyone have any ideas as to what could be causing this? Many thanks, Mark.
Re: [Qemu-devel] Segfault in block driver with qemu-system-ppc and -M mac99
Hi Mark, Am 24.02.2013 13:08, schrieb Mark Cave-Ayland: Whilst running through some OpenBIOS tests, I came across the following segfault in qemu-system-ppc with -M mac99 on git master. It is consistently reproducible here with my test openSUSE image although strangely all my other images seem to run fine. The host is running amd64 Debian Wheezy. build@kentang:~/rel-qemu-git/bin$ gdb --args ./qemu-system-ppc -cdrom /home/build/src/qemu/image/ppc/openSUSE-11.1-NET-ppc.iso -boot d -g 1024x768x32 -vnc :1 -m 512 -M mac99 GNU gdb (GDB) 7.4.1-debian Copyright (C) 2012 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type show copying and show warranty for details. This GDB was configured as x86_64-linux-gnu. For bug reporting instructions, please see: http://www.gnu.org/software/gdb/bugs/... Reading symbols from /home/build/rel-qemu-git/bin/qemu-system-ppc...done. (gdb) handle SIGUSR1 nostop noprint SignalStop Print Pass to program Description SIGUSR1 NoNo Yes User defined signal 1 (gdb) run Starting program: /home/build/rel-qemu-git/bin/qemu-system-ppc -cdrom /home/build/src/qemu/image/ppc/openSUSE-11.1-NET-ppc.iso -boot d -g 1024x768x32 -vnc :1 -m 512 -M mac99 [Thread debugging using libthread_db enabled] Using host libthread_db library /lib/x86_64-linux-gnu/libthread_db.so.1. [New Thread 0x7fffdf197700 (LWP 17230)] [New Thread 0x7fffde996700 (LWP 17231)] [New Thread 0x7fffbc9e8700 (LWP 17232)] [Thread 0x7fffdf197700 (LWP 17230) exited] Program received signal SIGSEGV, Segmentation fault. 0x555ef458 in bdrv_co_do_readv (bs=0x0, sector_num=169, nb_sectors=0, qiov=0x5661f878, flags=0) at block.c:2240 2240BlockDriver *drv = bs-drv; (gdb) bt #0 0x555ef458 in bdrv_co_do_readv (bs=0x0, sector_num=169, nb_sectors=0, qiov=0x5661f878, flags=0) at block.c:2240 #1 0x555f3a28 in bdrv_co_do_rw (opaque=0x56620030) at block.c:3837 #2 0x55631fa4 in coroutine_trampoline (i0=1448744496, i1=21845) at coroutine-ucontext.c:138 #3 0x7508c020 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #4 0x7fffb100 in ?? () #5 0x in ?? () (gdb) It seems as if the BlockDriver pointer is coming back as NULL - does anyone have any ideas as to what could be causing this? Have you tried a revision before my macio refactoring? It changed which IDE code paths are taken. Cheers, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] Segfault in block driver with qemu-system-ppc and -M mac99
On 24/02/13 12:14, Andreas Färber wrote: Have you tried a revision before my macio refactoring? It changed which IDE code paths are taken. Hi Andreas, It's a git pull as of a few hours ago (commit a345481baa2b2fb3d54f8c9ddb58dfcaf75786df) if that helps? Many thanks, Mark.
Re: [Qemu-devel] Segfault in block driver with qemu-system-ppc and -M mac99
Am 24.02.2013 13:40, schrieb Mark Cave-Ayland: On 24/02/13 12:14, Andreas Färber wrote: Have you tried a revision before my macio refactoring? It changed which IDE code paths are taken. Hi Andreas, It's a git pull as of a few hours ago (commit a345481baa2b2fb3d54f8c9ddb58dfcaf75786df) if that helps? The commit you reference does nothing with block or IDE but with mips, so is very unlikely to be the cause. I was asking if you have narrowed down whether this is a regression or not. If so then a git-bisect would hopefully give us some more insight. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] QEMU no longer debugable with recent gdb
Hi, running qemu-system-xxx under recent gdb gives me this instead of starting QEMU: GNU gdb (GDB) 7.5.1 Copyright (C) 2012 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type show copying and show warranty for details. This GDB was configured as x86_64-unknown-linux-gnu. For bug reporting instructions, please see: http://www.gnu.org/software/gdb/bugs/... Reading symbols from qemu-system-arm...done. (gdb) r Starting program: qemu-system-arm Warning: Cannot insert breakpoint -1. Error accessing memory address 0x34e770: Input/output error. Commenting out the JIT debug information or using a much older gdb is currently the only option to debug QEMU here. It's affecting non-TCG operation, too. Any ideas? Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v2 3/4] Add sample moxie system
Signed-off-by: Anthony Green gr...@moxielogic.com --- hw/moxie/Makefile.objs | 6 ++ hw/moxie/moxiesim.c| 175 + include/sysemu/arch_init.h | 1 + 3 files changed, 182 insertions(+) create mode 100644 hw/moxie/Makefile.objs create mode 100644 hw/moxie/moxiesim.c diff --git a/hw/moxie/Makefile.objs b/hw/moxie/Makefile.objs new file mode 100644 index 000..873c680 --- /dev/null +++ b/hw/moxie/Makefile.objs @@ -0,0 +1,6 @@ +# moxie boards +obj-y = serial.o mc146818rtc.o vga.o +obj-$(CONFIG_FDT) += device_tree.o + +obj-y := $(addprefix ../,$(obj-y)) +obj-y += moxiesim.o \ No newline at end of file diff --git a/hw/moxie/moxiesim.c b/hw/moxie/moxiesim.c new file mode 100644 index 000..3b04f5e --- /dev/null +++ b/hw/moxie/moxiesim.c @@ -0,0 +1,175 @@ +/* + * QEMU/moxiesim emulation + * + * Emulates a very simple machine model similiar to the one use by the + * GDB moxie simulator. + * + * Copyright (c) 2008, 2009, 2010, 2013 Anthony Green + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#include hw/sysbus.h +#include hw/hw.h +#include hw/pc.h +#include hw/isa.h +#include net/net.h +#include sysemu/sysemu.h +#include hw/boards.h +#include hw/loader.h +#include hw/serial.h +#include exec/address-spaces.h + +#define PHYS_MEM_BASE 0x8000 + +static struct loaderparams { +uint64_t ram_size; +const char *kernel_filename; +const char *kernel_cmdline; +const char *initrd_filename; +} loaderparams; + +static void load_kernel(MoxieCPU *cpu) +{ +uint64_t entry, kernel_low, kernel_high; +long kernel_size; +long initrd_size; +ram_addr_t initrd_offset; +kernel_size = load_elf(loaderparams.kernel_filename, NULL, NULL, + entry, kernel_low, kernel_high, 1, + ELF_MACHINE, 0); +if (kernel_size = 0) { +cpu-env.pc = (unsigned) entry; +} else { +fprintf(stderr, qemu: could not load kernel '%s'\n, +loaderparams.kernel_filename); +exit(1); +} + +/* load initrd */ +initrd_size = 0; +initrd_offset = 0; +if (loaderparams.initrd_filename) { +initrd_size = get_image_size(loaderparams.initrd_filename); +if (initrd_size 0) { +initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) + TARGET_PAGE_MASK; +if (initrd_offset + initrd_size loaderparams.ram_size) { +fprintf(stderr, +qemu: memory too small for initial ram disk '%s'\n, +loaderparams.initrd_filename); +exit(1); +} +initrd_size = load_image_targphys(loaderparams.initrd_filename, + initrd_offset, + ram_size); +} +if (initrd_size == (target_ulong)-1) { +fprintf(stderr, qemu: could not load initial ram disk '%s'\n, +loaderparams.initrd_filename); +exit(1); +} +} +} + +static void main_cpu_reset(void *opaque) +{ +MoxieCPU *cpu = opaque; + +cpu_reset(CPU(cpu)); +} + +static inline DeviceState * +moxie_intc_create(hwaddr base, qemu_irq irq, int kind_of_intr) +{ +DeviceState *dev; + +dev = qdev_create(NULL, moxie,intc); +qdev_prop_set_uint32(dev, kind-of-intr, kind_of_intr); +qdev_init_nofail(dev); +sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq); +return dev; +} + +static void moxiesim_init(QEMUMachineInitArgs *args) +{ +MoxieCPU *cpu = NULL; +ram_addr_t ram_size = args-ram_size; +const char *cpu_model = args-cpu_model; +const char *kernel_filename = args-kernel_filename; +const char *kernel_cmdline = args-kernel_cmdline; +const char *initrd_filename = args-initrd_filename; +CPUMoxieState *env; +MemoryRegion
[Qemu-devel] [PATCH v2 2/4] Add moxie disassembler
Signed-off-by: Anthony Green gr...@moxielogic.com --- disas.c | 6 + disas/Makefile.objs | 1 + disas/moxie.c | 360 3 files changed, 367 insertions(+) create mode 100644 disas/moxie.c diff --git a/disas.c b/disas.c index a46faee..74d3ba0 100644 --- a/disas.c +++ b/disas.c @@ -256,6 +256,9 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code, #elif defined(TARGET_MICROBLAZE) s.info.mach = bfd_arch_microblaze; print_insn = print_insn_microblaze; +#elif defined(TARGET_MOXIE) +s.info.mach = bfd_arch_moxie; +print_insn = print_insn_moxie; #elif defined(TARGET_LM32) s.info.mach = bfd_mach_lm32; print_insn = print_insn_lm32; @@ -462,6 +465,9 @@ void monitor_disas(Monitor *mon, CPUArchState *env, #elif defined(TARGET_S390X) s.info.mach = bfd_mach_s390_64; print_insn = print_insn_s390; +#elif defined(TARGET_MOXIE) +s.info.mach = bfd_arch_moxie; +print_insn = print_insn_moxie; #elif defined(TARGET_LM32) s.info.mach = bfd_mach_lm32; print_insn = print_insn_lm32; diff --git a/disas/Makefile.objs b/disas/Makefile.objs index ed75f9a..3b1e77a 100644 --- a/disas/Makefile.objs +++ b/disas/Makefile.objs @@ -7,6 +7,7 @@ common-obj-$(CONFIG_IA64_DIS) += ia64.o common-obj-$(CONFIG_M68K_DIS) += m68k.o common-obj-$(CONFIG_MICROBLAZE_DIS) += microblaze.o common-obj-$(CONFIG_MIPS_DIS) += mips.o +common-obj-$(CONFIG_MOXIE_DIS) += moxie.o common-obj-$(CONFIG_PPC_DIS) += ppc.o common-obj-$(CONFIG_S390_DIS) += s390.o common-obj-$(CONFIG_SH4_DIS) += sh4.o diff --git a/disas/moxie.c b/disas/moxie.c new file mode 100644 index 000..4c5f180 --- /dev/null +++ b/disas/moxie.c @@ -0,0 +1,360 @@ +/* Disassemble moxie instructions. + Copyright (c) 2009 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, see http://www.gnu.org/licenses/. */ + +#include stdio.h +#define STATIC_TABLE +#define DEFINE_TABLE + +#include disas/bfd.h + +static void *stream; + +/* Form 1 instructions come in different flavors: + + Some have no arguments (MOXIE_F1_NARG) + Some only use the A operand (MOXIE_F1_A) + Some use A and B registers (MOXIE_F1_AB) + Some use A and consume a 4 byte immediate value (MOXIE_F1_A4) + Some use just a 4 byte immediate value (MOXIE_F1_4) + Some use just a 4 byte memory address (MOXIE_F1_M) + Some use B and an indirect A(MOXIE_F1_AiB) + Some use A and an indirect B(MOXIE_F1_ABi) + Some consume a 4 byte immediate value and use X (MOXIE_F1_4A) + Some use B and an indirect A plus 4 bytes (MOXIE_F1_AiB4) + Some use A and an indirect B plus 4 bytes (MOXIE_F1_ABi4) + + Form 2 instructions also come in different flavors: + + Some have no arguments (MOXIE_F2_NARG) + Some use the A register and an 8-bit value (MOXIE_F2_A8V) + + Form 3 instructions also come in different flavors: + + Some have no arguments (MOXIE_F3_NARG) + Some have a 10-bit PC relative operand (MOXIE_F3_PCREL). */ + +#define MOXIE_F1_NARG 0x100 +#define MOXIE_F1_A0x101 +#define MOXIE_F1_AB 0x102 +/* #define MOXIE_F1_ABC 0x103 */ +#define MOXIE_F1_A4 0x104 +#define MOXIE_F1_40x105 +#define MOXIE_F1_AiB 0x106 +#define MOXIE_F1_ABi 0x107 +#define MOXIE_F1_4A 0x108 +#define MOXIE_F1_AiB4 0x109 +#define MOXIE_F1_ABi4 0x10a +#define MOXIE_F1_M0x10b + +#define MOXIE_F2_NARG 0x200 +#define MOXIE_F2_A8V 0x201 + +#define MOXIE_F3_NARG 0x300 +#define MOXIE_F3_PCREL 0x301 + +typedef struct moxie_opc_info_t { +short opcode; +unsigned itype; +const char * name; +} moxie_opc_info_t; + +extern const moxie_opc_info_t moxie_form1_opc_info[64]; +extern const moxie_opc_info_t moxie_form2_opc_info[4]; +extern const moxie_opc_info_t moxie_form3_opc_info[16]; + +/* The moxie processor's 16-bit instructions come in two forms: + + FORM 1 instructions start with a 0 bit... + + 0ooo + 0 F + + ooo - form 1 opcode number + - operand A + - operand B + + FORM 2 instructions start with bits 10... + + 10oo + 0 F + + oo - form 2 opcode number + - operand A + -
[Qemu-devel] [PATCH v2 4/4] Add top level changes for moxie
Signed-off-by: Anthony Green gr...@moxielogic.com --- MAINTAINERS | 5 + arch_init.c | 2 ++ configure | 12 +++- cpu-exec.c| 2 ++ default-configs/moxie-softmmu.mak | 2 ++ qapi-schema.json | 6 +++--- 6 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 default-configs/moxie-softmmu.mak diff --git a/MAINTAINERS b/MAINTAINERS index 21043e4..b970159 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -91,6 +91,11 @@ M: Aurelien Jarno aurel...@aurel32.net S: Odd Fixes F: target-mips/ +Moxie +M: Anthony Green gr...@moxielogic.com +S: Maintained +F: target-moxie/ + PowerPC M: Alexander Graf ag...@suse.de L: qemu-...@nongnu.org diff --git a/arch_init.c b/arch_init.c index 8da868b..f82ed2d 100644 --- a/arch_init.c +++ b/arch_init.c @@ -85,6 +85,8 @@ int graphic_depth = 15; #define QEMU_ARCH QEMU_ARCH_MICROBLAZE #elif defined(TARGET_MIPS) #define QEMU_ARCH QEMU_ARCH_MIPS +#elif defined(TARGET_MOXIE) +#define QEMU_ARCH QEMU_ARCH_MOXIE #elif defined(TARGET_OPENRISC) #define QEMU_ARCH QEMU_ARCH_OPENRISC #elif defined(TARGET_PPC) diff --git a/configure b/configure index dcaa67c..02d75db 100755 --- a/configure +++ b/configure @@ -955,6 +955,7 @@ mips-softmmu \ mipsel-softmmu \ mips64-softmmu \ mips64el-softmmu \ +moxie-softmmu \ or32-softmmu \ ppc-softmmu \ ppcemb-softmmu \ @@ -3898,7 +3899,7 @@ target_arch2=`echo $target | cut -d '-' -f 1` target_bigendian=no case $target_arch2 in - armeb|lm32|m68k|microblaze|mips|mipsn32|mips64|or32|ppc|ppcemb|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb) + armeb|lm32|m68k|microblaze|mips|mipsn32|mips64|moxie|or32|ppc|ppcemb|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb) target_bigendian=yes ;; esac @@ -4003,6 +4004,11 @@ case $target_arch2 in echo TARGET_ABI_MIPSN64=y $config_target_mak target_long_alignment=8 ;; + moxie) +TARGET_ARCH=moxie +bflt=yes +target_phys_bits=32 + ;; or32) TARGET_ARCH=openrisc TARGET_BASE_ARCH=openrisc @@ -4247,6 +4253,10 @@ for i in $ARCH $TARGET_BASE_ARCH ; do echo CONFIG_MIPS_DIS=y $config_target_mak echo CONFIG_MIPS_DIS=y config-all-disas.mak ;; + moxie*) +echo CONFIG_MOXIE_DIS=y $config_target_mak +echo CONFIG_MOXIE_DIS=y config-all-disas.mak + ;; or32) echo CONFIG_OPENRISC_DIS=y $config_target_mak echo CONFIG_OPENRISC_DIS=y config-all-disas.mak diff --git a/cpu-exec.c b/cpu-exec.c index afbe497..ba7ea41 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -221,6 +221,7 @@ int cpu_exec(CPUArchState *env) #elif defined(TARGET_LM32) #elif defined(TARGET_MICROBLAZE) #elif defined(TARGET_MIPS) +#elif defined(TARGET_MOXIE) #elif defined(TARGET_OPENRISC) #elif defined(TARGET_SH4) #elif defined(TARGET_CRIS) @@ -655,6 +656,7 @@ int cpu_exec(CPUArchState *env) | env-cc_dest | (env-cc_x 4); #elif defined(TARGET_MICROBLAZE) #elif defined(TARGET_MIPS) +#elif defined(TARGET_MOXIE) #elif defined(TARGET_OPENRISC) #elif defined(TARGET_SH4) #elif defined(TARGET_ALPHA) diff --git a/default-configs/moxie-softmmu.mak b/default-configs/moxie-softmmu.mak new file mode 100644 index 000..d378363 --- /dev/null +++ b/default-configs/moxie-softmmu.mak @@ -0,0 +1,2 @@ +# Default configuration for moxie-softmmu + diff --git a/qapi-schema.json b/qapi-schema.json index cd7ea25..cf5cd60 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2944,9 +2944,9 @@ ## { 'enum': 'TargetType', 'data': [ 'alpha', 'arm', 'cris', 'i386', 'lm32', 'm68k', 'microblazeel', -'microblaze', 'mips64el', 'mips64', 'mipsel', 'mips', 'or32', -'ppc64', 'ppcemb', 'ppc', 's390x', 'sh4eb', 'sh4', 'sparc64', -'sparc', 'unicore32', 'x86_64', 'xtensaeb', 'xtensa' ] } +'microblaze', 'mips64el', 'mips64', 'mipsel', 'mips', 'moxie', +'or32', 'ppc64', 'ppcemb', 'ppc', 's390x', 'sh4eb', 'sh4', +'sparc64', 'sparc', 'unicore32', 'x86_64', 'xtensaeb', 'xtensa' ] } ## # @TargetInfo: -- 1.8.1.2
[Qemu-devel] [PATCH v2 0/4] Moxie CPU target port
This is my resubmission of the moxie port. Thank you for all of the feedback on my first attempt. I believe that these patches address all of the reviewer comments, with the exception of some of rth's code refactoring suggestions around cmp and branch instructions. I'll work on that after approval. Also note that I relicensed the disassembler code, since I am the sole author, but it still doesn't pass checkpatch.pl cleanly because it is based on binutils formatting conventions. Thank you! AG Anthony Green (4): Add moxie target code Add moxie disassembler Add sample moxie system Add top level changes for moxie MAINTAINERS |5 + arch_init.c |2 + configure | 12 +- cpu-exec.c|2 + default-configs/moxie-softmmu.mak |2 + disas.c |6 + disas/Makefile.objs |1 + disas/moxie.c | 360 hw/moxie/Makefile.objs|6 + hw/moxie/moxiesim.c | 175 ++ include/sysemu/arch_init.h|1 + qapi-schema.json |6 +- target-moxie/Makefile.objs|2 + target-moxie/cpu.c| 177 ++ target-moxie/cpu.h| 222 target-moxie/helper.c | 102 target-moxie/helper.h |6 + target-moxie/machine.c| 11 + target-moxie/mmu.c| 37 ++ target-moxie/mmu.h| 19 + target-moxie/op_helper.c | 80 +++ target-moxie/translate.c | 1084 + 22 files changed, 2314 insertions(+), 4 deletions(-) create mode 100644 default-configs/moxie-softmmu.mak create mode 100644 disas/moxie.c create mode 100644 hw/moxie/Makefile.objs create mode 100644 hw/moxie/moxiesim.c create mode 100644 target-moxie/Makefile.objs create mode 100644 target-moxie/cpu.c create mode 100644 target-moxie/cpu.h create mode 100644 target-moxie/helper.c create mode 100644 target-moxie/helper.h create mode 100644 target-moxie/machine.c create mode 100644 target-moxie/mmu.c create mode 100644 target-moxie/mmu.h create mode 100644 target-moxie/op_helper.c create mode 100644 target-moxie/translate.c -- 1.8.1.2
Re: [Qemu-devel] [PATCH 2/6] cpu: Introduce ENV_OFFSET macros
On 24 February 2013 11:42, Andreas Färber afaer...@suse.de wrote: Am 24.02.2013 12:31, schrieb Peter Maydell: In this case I actually kind of 50% thought they were the result of conflicts/merge process rather than intentional, which is why I cleaned them up. I don't actually care one way or the other, so you can reinstate them if you prefer. My reasoning was to differenciate between the header guard and any in-file #ifdef CONFIG_USER_ONLY or TARGET_FOO, which for functions I usually separate by one line. I don't really care too much though, it's just the principle that angers me that you made me go through hoops, propagating adopt-the-author's-style when it comes to target-arm files, while now violating your own paradigm and apparently even finding that funny. I'm sorry; that smiley was perhaps misplaced. I dropped the blank lines because I didn't understand their purpose and part of my process before sending patches out is read patch and fix anything I would comment on if it were code review of somebody else's patch. You've explained the rationale for them, so I will reinstate them. V2 coming up later today or tomorrow. -- PMM
Re: [Qemu-devel] [PATCH 2/6] cpu: Introduce ENV_OFFSET macros
Am 24.02.2013 15:07, schrieb Peter Maydell: On 24 February 2013 11:42, Andreas Färber afaer...@suse.de wrote: Am 24.02.2013 12:31, schrieb Peter Maydell: In this case I actually kind of 50% thought they were the result of conflicts/merge process rather than intentional, which is why I cleaned them up. I don't actually care one way or the other, so you can reinstate them if you prefer. My reasoning was to differenciate between the header guard and any in-file #ifdef CONFIG_USER_ONLY or TARGET_FOO, which for functions I usually separate by one line. I don't really care too much though, it's just the principle that angers me that you made me go through hoops, propagating adopt-the-author's-style when it comes to target-arm files, while now violating your own paradigm and apparently even finding that funny. I'm sorry; that smiley was perhaps misplaced. I dropped the blank lines because I didn't understand their purpose and part of my process before sending patches out is read patch and fix anything I would comment on if it were code review of somebody else's patch. You've explained the rationale for them, so I will reinstate them. V2 coming up later today or tomorrow. Please don't. It's on qom-cpu as is, and I've had to rebase four follow-up branches that all add something to the bottom of those files, so having to go through that again is no improvement. Rather try to be more relaxed when I add something to my cpu.c or cpu-qom.h files next time, the audio code is already big enough an exception-from-the-rule for cross-target changes. ;) Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
On 24/02/13 12:30, Andreas Färber wrote: Am 22.02.2013 20:01, schrieb Jens Freimann: From: Christian Borntraeger borntrae...@de.ibm.com This patch fixes a crash when unplugging a virtio-ccw device. We no longer need to do that in virtio-ccw since common code does now proper handling. Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com --- hw/s390x/virtio-ccw.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 231f81e..679afa6 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -865,7 +865,6 @@ static int virtio_ccw_busdev_unplug(DeviceState *dev) css_generate_sch_crws(sch-cssid, sch-ssid, sch-schid, 1, 0); -object_unparent(OBJECT(dev)); qdev_free(dev); return 0; } qdev_free() does exactly object_unparent(), so in light of wanting to get away from qdev I would suggest to rather drop qdev_free() here. Paolo? Agreed, this is identical from a functional perspective and I was asking that myself, but it seems that qdev_free is an interface. used by many busses: $ git grep qdev_free hw/acpi_piix4.c:qdev_free(qdev); hw/pci/pci-hotplug.c:qdev_free(dev-qdev); hw/pci/pci_bridge.c:/* qbus_free() is called automatically by qdev_free() */ hw/pci/pcie.c:qdev_free(pci_dev-qdev); hw/pci/shpc.c:qdev_free(affected_dev-qdev); hw/qdev-core.h:void qdev_free(DeviceState *dev); hw/qdev-monitor.c:qdev_free(qdev); hw/qdev.c:qdev_free(dev); hw/qdev.c:qdev_free(dev); hw/qdev.c:void qdev_free(DeviceState *dev) hw/qdev.c:qdev_free(dev); hw/s390x/virtio-ccw.c:qdev_free(dev); hw/scsi-bus.c:qdev_free(d-qdev); hw/scsi-bus.c:qdev_free(dev); hw/usb/bus.c:qdev_free(port-dev-qdev); hw/usb/bus.c:qdev_free(dev-qdev); hw/usb/dev-storage.c:qdev_free(dev-qdev); hw/usb/host-linux.c:qdev_free(dev-qdev); hw/virtio-bus.c:qdev_free(qdev); hw/xen_platform.c:qdev_free(d-qdev); ...while object_unparent is still somewhat internal. $ git grep object_unparent hw/qdev.c:object_unparent(OBJECT(dev)); hw/qdev.c:object_unparent(OBJECT(bus)); include/qom/object.h:void object_unparent(Object *obj); qom/object.c:void object_unparent(Object *obj) Another thing is, that qdev_free looks now different, some days ago it also did an unref. As far as I can see the object_unparent in virtio-ccw was always the wrong thing to do. So for a potential backport this version of the patch might be better. Paolo, do you have any guidance? Christian
Re: [Qemu-devel] [PATCH v6 7/9] hw/kvm/arm_gic: Implement support for KVM in-kernel ARM GIC
On 23 February 2013 15:29, Andreas Färber afaer...@suse.de wrote: +static int kvm_arm_gic_init(SysBusDevice *dev) Please make this a realize function ... Will do. As you've probably guessed, this was written some time ago and I never updated it to match our current recommendations. +KVMARMGICClass *kgc = KVM_ARM_GIC_GET_CLASS(s); + +kgc-parent_init(dev); ... and call sbc-init(dev) here as long as the base class is not updated. Maybe I should just update the base class? +static void kvm_arm_gic_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass); +ARMGICCommonClass *agcc = ARM_GIC_COMMON_CLASS(klass); +KVMARMGICClass *kgc = KVM_ARM_GIC_CLASS(klass); Please separate variable definitions and rest of block by a white line, here and elsewhere. OK. -- PMM
[Qemu-devel] Moving BIOS tables from SeaBIOS to QEMU
On Sat, Feb 23, 2013 at 04:47:26PM +, David Woodhouse wrote: On Sat, 2013-02-23 at 11:38 -0500, Kevin O'Connor wrote: IMO, we need to move the ACPI table creation (and PIR/MPTABLE/SMBIOS) to QEMU and just have QEMU pass the tables to SeaBIOS for it to copy into memory like it does on CSM, coreboot, and Xen. I believe it's on Laszlo's TODO list. Laszlo, what is your plan for doing this? I did a review of the SeaBIOS code to see what information is currently used to generate the ACPI, SMBIOS, MPTABLE, and PIR bios tables. Here's what I came up with: - hardcoded information: Most of the tables are simply hardcoded with various values. This should not be a problem to move to QEMU - information passed in from QEMU: RamSize, RamSizeOver4G, fw_cfg (irq0-override, system suspend states, numa memory, additional acpi tables, smbios overrides). These should also be possible to obtain directly within QEMU (though I'm unsure how qemu exposes this information internally). - CPU information: Number of CPUs, the apic id of the CPUs, which CPUs are active, and the cpuid information from the first CPU. Again this should be available in QEMU, but I'm not sure what the internal interfaces look like for obtaining it. - Various hardware probes: The ioapic version, whether or not hpet is present, running on piix4 or ich9, whether or not acpi should be used. Again should be possible to obtain from QEMU with sufficient interfaces. - PCI device info: The list of PCI devices, PCI buses, pin assignments, irq assignments, if hotplug supported, and memory regions. This should mostly be available in QEMU - order of initializing would be important so that the tables were initialized after all PCI devices. Of these, the only thing I see that could be problematic is the PCI irq assignments (used in mptable) and the PCI region space (used in ACPI DSDT _SB.PCI.CRS). These are slightly problematic as they currently rely somewhat on the current SeaBIOS pciinit.c bridge/device setup. However, the mptable irqs is a simple algorithm that could be replicated in QEMU, and it looks to be of dubious value anyway (so could possibly be dropped from the mptable). Also, the PCI region space does not need to be exact, so a heuristic that just ensured it was large enough should suffice. Given this, one possible way to migrate the ACPI tables from SeaBIOS would be to: 1 - replace the BDAT PCI range interface in SeaBIOS with a SSDT based template system similar to the way software suspend states are handled in SeaBIOS today. This would eliminate the only runtime references to SeaBIOS memory from ACPI. 2 - relicense the SeaBIOS' acpi.c, mptable.c, pirtable.c, smbios.c code to GPLv2 (from LGPLv3) and copy into QEMU. Only I've claimed a copyright since Fabrice's work (LGPLv2) and I'm willing to relicense. There have been a handful of contributors to these files, but they all look to be regular QEMU contributors so I don't think there would be any objections. Along with the code, the IASL parsing code and associated build python scripts would also need to be copied into QEMU. 3 - update the code to use the internal QEMU interfaces instead of the SeaBIOS interfaces to obtain the information outlined above. 4 - pass the tables from QEMU to SeaBIOS via the fw_cfg interface. The PIR, MPTABLE, and SMBIOS are easy to copy into memory from fw_cfg. The ACPI does have a few tables that are special (RSDP, RSDT, FADT, DSDT, FACS), but it should be easy to detect these and update the pointers in SeaBIOS during the copy to memory. Thoughts? -Kevin
[Qemu-devel] 'commit' error for 'all': No medium found
Hi, I'm seeing this with git head and 1.4. Apparently, commit on a non-populated medium now generates this error instead of ignoring it like in the past. As we stop iterating over the block devices while doing all, this may leaving uncommitted data behind. Didn't test, but I suspect 58513bde83 has something to do with it. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Segfault in block driver with qemu-system-ppc and -M mac99
On 24/02/13 12:44, Andreas Färber wrote: The commit you reference does nothing with block or IDE but with mips, so is very unlikely to be the cause. I was asking if you have narrowed down whether this is a regression or not. If so then a git-bisect would hopefully give us some more insight. (goes and has a look) Okay it's definitely a regression - git bisect points to the following commit: commit 07a7484e5d713f1eb7c1c37b18a8ab0d56d88875 Author: Andreas Färber afaer...@suse.de Date: Wed Jan 23 23:04:01 2013 + ide/macio: QOM'ify MacIO IDE It was not qdev'ified before. Turn it into a SysBusDevice. Embed them into the MacIO devices. Signed-off-by: Andreas Färber afaer...@suse.de Signed-off-by: Alexander Graf ag...@suse.de :04 04 699140bdb5dd61a682d5c4f4731d47cf7c1ed0aa 794283b7d17694b498691c728315cbeb63078154 M hw If you try booting the openSUSE ISO mentioned in my previous email then the crash occurs at some point during the Starting udev... prompt. HTH, Mark.
[Qemu-devel] [Fwd: qemu tx stop in cloonix]
Hello, I use qemu inside a gplv3 software called cloonix, I have patched qemu to have unix sockets instead of inet ones but the bug I have with unix sockets may also happen with inet ones. The bug can be reproduced in cloonix context by using iperf, it occurs randomly in a virtual cloonix network but occurs within seconds using iperf in nested virtualisation (cloonix inside cloonix), the problem begins when a lot of packets must be transmitted and the socket (inet in the classical qemu, unix in cloonix) gets full and qemu_net_queue_append_iov is called, then tx never restarts. See under in the patch attached, the way I avoided queuing anything, it works, even if it is not a correction to the bug... The patch is for version 21.3 of cloonix which uses qemu-1.4.0-rc1, but I now use qemu-1.4.0 and the bug is still there. Regards Vincent Perrier Original Message Subject: qemu tx stop in cloonix From:clow...@clownix.net Date:Sun, February 24, 2013 9:14 am To: list cloonix-l...@clownix.net -- There is a bug visible more particularly when doing nested cloonix and iperf inside the second level nested machines. The ethernet interface emitting a big load stops working, this has been corrected in my version but I will not deliver the correction outside the regular deliveries (every 2 or 3 months). If you have a stopping of your ethernet access after a burst of traffic, here is the cause: From the kernel virtio_net driver inside the guest, piles of messages are sent into a virtio queue to the qemu user process. The qemu user process does what it can to give the messages to a unix socket (to cloonix). When too much traffic arrives, the unix socket writes 0 bytes as it gets full. Then qemu, instead of droping packet (too much is too much, no need to try harder) qemu does not want to drop, it tries to enqueue packets until the unix socket clears... The mechanics of this unusual case management is too complex, I did not get into it to repair it, I just dropped the packets, simplest solution, and the above layers know low level packets can just disapear... Here is the solution: in sources/Cloonix-Net-Lab/qemu, in file cmd, add the following qemu_drop_burst.patch line after having put the qemu_drop_burst.patch in the qemu directory. patch -p1 ../cloonix_qemu.patch patch -p1 ../qemu_drop_burst.patch The qemu_drop_burst.patch should be with this mail...diff -Naur qemu-1.4.0-rc1/net/net.c new_qemu-1.4.0-rc1/net/net.c --- qemu-1.4.0-rc1/net/net.c 2013-02-07 01:40:56.0 +0100 +++ new_qemu-1.4.0-rc1/net/net.c 2013-02-24 16:03:45.139853349 +0100 @@ -388,10 +388,14 @@ } if (sender-peer-receive_disabled) { -return 0; +//cloonix DROP +//return 0; +return 1; } else if (sender-peer-info-can_receive !sender-peer-info-can_receive(sender-peer)) { -return 0; +//cloonix DROP +//return 0; +return 1; } return 1; }
Re: [Qemu-devel] [Qemu-ppc] Segfault in block driver with qemu-system-ppc and -M mac99
On 24/02/13 19:29, Mark Cave-Ayland wrote: If you try booting the openSUSE ISO mentioned in my previous email then the crash occurs at some point during the Starting udev... prompt. With a manual review of that particular commit, I managed to spot that it was just a typing error. Patch to follow shortly. ATB, Mark.
[Qemu-devel] [PATCH] ide/macio: Fix macio DMA initialisation.
Commit 07a7484e5d713f1eb7c1c37b18a8ab0d56d88875 accidentally introduced a bug in the initialisation of the second macio DMA device which could cause some DMA operations to segfault QEMU. CC: Andreas Färber afaer...@suse.de Signed-off-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk --- hw/macio.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/macio.c b/hw/macio.c index 74bdcd1..0c6a6b8 100644 --- a/hw/macio.c +++ b/hw/macio.c @@ -188,7 +188,7 @@ static int macio_newworld_initfn(PCIDevice *d) sysbus_dev = SYS_BUS_DEVICE(ns-ide[1]); sysbus_connect_irq(sysbus_dev, 0, ns-irqs[3]); sysbus_connect_irq(sysbus_dev, 1, ns-irqs[4]); -macio_ide_register_dma(ns-ide[0], s-dbdma, 0x1a); +macio_ide_register_dma(ns-ide[1], s-dbdma, 0x1a); ret = qdev_init(DEVICE(ns-ide[1])); if (ret 0) { return ret; -- 1.7.10.4
Re: [Qemu-devel] [Qemu-ppc] Segfault in block driver with qemu-system-ppc and -M mac99
Am 24.02.2013 21:40, schrieb Mark Cave-Ayland: On 24/02/13 19:29, Mark Cave-Ayland wrote: If you try booting the openSUSE ISO mentioned in my previous email then the crash occurs at some point during the Starting udev... prompt. With a manual review of that particular commit, I managed to spot that it was just a typing error. Patch to follow shortly. Ugh, you mean this? +macio_ide_register_dma(ns-ide[0], s-dbdma, 0x1a); +ret = qdev_init(DEVICE(ns-ide[1])); Guess it should be 1 in both cases? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] ide/macio: Fix macio DMA initialisation.
Am 24.02.2013 21:46, schrieb Mark Cave-Ayland: Commit 07a7484e5d713f1eb7c1c37b18a8ab0d56d88875 accidentally introduced a bug in the initialisation of the second macio DMA device which could cause some DMA operations to segfault QEMU. CC: Andreas Färber afaer...@suse.de Signed-off-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk Acked-by: Andreas Färber afaer...@suse.de Thanks for catching this, Andreas --- hw/macio.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/macio.c b/hw/macio.c index 74bdcd1..0c6a6b8 100644 --- a/hw/macio.c +++ b/hw/macio.c @@ -188,7 +188,7 @@ static int macio_newworld_initfn(PCIDevice *d) sysbus_dev = SYS_BUS_DEVICE(ns-ide[1]); sysbus_connect_irq(sysbus_dev, 0, ns-irqs[3]); sysbus_connect_irq(sysbus_dev, 1, ns-irqs[4]); -macio_ide_register_dma(ns-ide[0], s-dbdma, 0x1a); +macio_ide_register_dma(ns-ide[1], s-dbdma, 0x1a); ret = qdev_init(DEVICE(ns-ide[1])); if (ret 0) { return ret; -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v2] Fix guest OS hang when 64bit PCI bar present
This patch addresses the issue fully described here: http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg01804.html Linux kernels prior to 2.6.36 do not disable the PCI device during enumeration process. Since lower and higher parts of a 64bit BAR are programmed separately this leads to qemu receiving a request to occupy a completely wrong address region for a short period of time. We have found that the boot process screws up completely if kvm-apic range is overlapped even for a short period of time (it is fine for other regions though). So the issue it that we hide the MSI target region from the device models for a short periods of time? How short is this? It would be good to fully understand what goes completely wrong to avoid that we miss to fix something else (IO-APIC, HPET) or even paper over a problem of the memory subsystem. As far as I can understand the region corruption and recovery happens within one call of pci_read_bases of linux kernel enumeration: (http://lxr.linux.no/#linux+v2.6.21/drivers/pci/probe.c#L173) Unfortunately I cannot even guess what is happening with kvm-apic-msi, or why it does not tolerate overlaps. This is actually a still open qestion. This patch raises the priority of the kvm-apic memory region, so it is never pushed out by PCI devices. The patch is quite safe as it does not touch memory manager. I have fixed styling issues. Please use this version. Signed-off-by: Alexey Korolev akoro...@gmail.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/sysbus.c | 26 ++ hw/sysbus.h |2 ++ target-i386/cpu.c |3 ++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/hw/sysbus.c b/hw/sysbus.c index 6d9d1df..242eb1e 100644 --- a/hw/sysbus.c +++ b/hw/sysbus.c @@ -48,7 +48,8 @@ void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq) } } -void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr) +static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr, + bool may_overlap, unsigned priority) { assert(n = 0 n dev-num_mmio); @@ -61,11 +62,28 @@ void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr) memory_region_del_subregion(get_system_memory(), dev-mmio[n].memory); } dev-mmio[n].addr = addr; -memory_region_add_subregion(get_system_memory(), -addr, -dev-mmio[n].memory); +if (may_overlap) { +memory_region_add_subregion_overlap(get_system_memory(), +addr, +dev-mmio[n].memory, +priority); +} else { +memory_region_add_subregion(get_system_memory(), +addr, +dev-mmio[n].memory); +} +} + +void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr) +{ +sysbus_mmio_map_common(dev, n, addr, false, 0); } +void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr addr, + unsigned priority) +{ +sysbus_mmio_map_common(dev, n, addr, true, priority); +} /* Request an IRQ source. The actual IRQ object may be populated later. */ void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p) diff --git a/hw/sysbus.h b/hw/sysbus.h index a7fcded..2100bd7 100644 --- a/hw/sysbus.h +++ b/hw/sysbus.h @@ -56,6 +56,8 @@ void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size); void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq); void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr); +void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr addr, + unsigned priority); void sysbus_add_memory(SysBusDevice *dev, hwaddr addr, MemoryRegion *mem); void sysbus_add_memory_overlap(SysBusDevice *dev, hwaddr addr, diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 5582e5f..4b72094 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2088,7 +2088,8 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) /* NOTE: the APIC is directly connected to the CPU - it is not on the global memory bus. */ /* XXX: what if the base changes? */ -sysbus_mmio_map(SYS_BUS_DEVICE(env-apic_state), 0, MSI_ADDR_BASE); +sysbus_mmio_map_overlap(SYS_BUS_DEVICE(env-apic_state), 0, +MSI_ADDR_BASE, 1000); apic_mapped = 1; } } -- 1.7.9.5
[Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState
- since hyperv_* helper functions are used only in target-i386/kvm.c move them there as static helpers Signed-off-by: Igor Mammedov imamm...@redhat.com Requestd-By: Eduardo Habkost ehabk...@redhat.com --- target-i386/Makefile.objs |2 +- target-i386/cpu.c | 16 +++--- target-i386/cpu.h |7 + target-i386/hyperv.c | 64 - target-i386/hyperv.h | 45 --- target-i386/kvm.c | 36 ++--- 6 files changed, 45 insertions(+), 125 deletions(-) delete mode 100644 target-i386/hyperv.c delete mode 100644 target-i386/hyperv.h diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs index c1d4f05..887dca7 100644 --- a/target-i386/Makefile.objs +++ b/target-i386/Makefile.objs @@ -2,7 +2,7 @@ obj-y += translate.o helper.o cpu.o obj-y += excp_helper.o fpu_helper.o cc_helper.o int_helper.o svm_helper.o obj-y += smm_helper.o misc_helper.o mem_helper.o seg_helper.o obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o -obj-$(CONFIG_KVM) += kvm.o hyperv.o +obj-$(CONFIG_KVM) += kvm.o obj-$(CONFIG_NO_KVM) += kvm-stub.o obj-$(CONFIG_LINUX_USER) += ioport-user.o obj-$(CONFIG_BSD_USER) += ioport-user.o diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 5626931..35fc303 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -33,8 +33,6 @@ #include qapi/visitor.h #include sysemu/arch_init.h -#include hyperv.h - #include hw/hw.h #if defined(CONFIG_KVM) #include linux/kvm_para.h @@ -1422,12 +1420,19 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp) object_property_parse(OBJECT(cpu), num, tsc-frequency, errp); } else if (!strcmp(featurestr, hv-spinlocks)) { char *err; +const int min = 0xFFF; numvalue = strtoul(val, err, 0); if (!*val || *err) { error_setg(errp, bad numerical value %s, val); goto out; } -hyperv_set_spinlock_retries(numvalue); +if (numvalue min) { +fprintf(stderr, hv-spinlocks value shall always be = 0x%x +, fixup will be removed in future versions\n, +min); +numvalue = min; +} +env-hyperv_spinlock_attempts = numvalue; } else { error_setg(errp, unrecognized feature %s, featurestr); goto out; @@ -1437,9 +1442,9 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp) } else if (!strcmp(featurestr, enforce)) { check_cpuid = enforce_cpuid = 1; } else if (!strcmp(featurestr, hv_relaxed)) { -hyperv_enable_relaxed_timing(true); +env-hyperv_relaxed_timing = true; } else if (!strcmp(featurestr, hv_vapic)) { -hyperv_enable_vapic_recommended(true); +env-hyperv_vapic = true; } else { error_setg(errp, feature string `%s' not in format (+feature| -feature|feature=xyz), featurestr); @@ -1592,6 +1597,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp) def-kvm_features |= kvm_default_features; } def-ext_features |= CPUID_EXT_HYPERVISOR; +env-hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY; object_property_set_str(OBJECT(cpu), def-vendor, vendor, errp); object_property_set_int(OBJECT(cpu), def-level, level, errp); diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 7577e4f..e3723c2 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -549,6 +549,10 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; #define CPUID_MWAIT_IBE (1 1) /* Interrupts can exit capability */ #define CPUID_MWAIT_EMX (1 0) /* enumeration supported */ +#ifndef HYPERV_SPINLOCK_NEVER_RETRY +#define HYPERV_SPINLOCK_NEVER_RETRY 0x +#endif + #define EXCP00_DIVZ0 #define EXCP01_DB 1 #define EXCP02_NMI 2 @@ -840,6 +844,9 @@ typedef struct CPUX86State { uint32_t cpuid_ext4_features; /* Flags from CPUID[EAX=7,ECX=0].EBX */ uint32_t cpuid_7_0_ebx_features; +bool hyperv_vapic; +bool hyperv_relaxed_timing; +int hyperv_spinlock_attempts; /* MTRRs */ uint64_t mtrr_fixed[11]; diff --git a/target-i386/hyperv.c b/target-i386/hyperv.c deleted file mode 100644 index f284e99..000 --- a/target-i386/hyperv.c +++ /dev/null @@ -1,64 +0,0 @@ -/* - * QEMU Hyper-V support - * - * Copyright Red Hat, Inc. 2011 - * - * Author: Vadim Rozenfeld vroze...@redhat.com - * - * This work is licensed under the terms of the GNU GPL, version 2 or later. - * See the COPYING file in the top-level directory. - * - */ - -#include hyperv.h - -static bool hyperv_vapic; -static bool
[Qemu-devel] [PATCH 07/10] target-i386: convert 'check' and 'enforce' to static properties
* additionally convert check_cpuid enforce_cpuid to bool and make them members of CPUX86State * make 'enforce' feature independent from 'check' Signed-off-by: Igor Mammedov imamm...@redhat.com --- v2: - make check_cpuid enforce_cpuid members of CPUX86State and use DEFINE_PROP_BOOL instead of custom property setters --- target-i386/cpu.c | 13 ++--- target-i386/cpu.h |2 ++ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 8f7132a..6826224 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -216,9 +216,6 @@ typedef struct model_features_t { FeatureWord feat_word; } model_features_t; -int check_cpuid = 0; -int enforce_cpuid = 0; - static uint32_t kvm_default_features = (1 KVM_FEATURE_CLOCKSOURCE) | (1 KVM_FEATURE_NOP_IO_DELAY) | (1 KVM_FEATURE_CLOCKSOURCE2) | @@ -1351,6 +1348,8 @@ static Property cpu_x86_properties[] = { DEFINE_PROP_HV_SPINLOCKS(hv-spinlocks, HYPERV_SPINLOCK_NEVER_RETRY), DEFINE_PROP_BOOL(hv-relaxed, X86CPU, env.hyperv_relaxed_timing, false), DEFINE_PROP_BOOL(hv-vapic, X86CPU, env.hyperv_vapic, false), +DEFINE_PROP_BOOL(check, X86CPU, env.check_cpuid, false), +DEFINE_PROP_BOOL(enforce, X86CPU, env.enforce_cpuid, false), DEFINE_PROP_END_OF_LIST(), }; @@ -1487,9 +1486,9 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp) goto out; } } else if (!strcmp(featurestr, check)) { -check_cpuid = 1; +object_property_parse(OBJECT(cpu), on, featurestr, errp); } else if (!strcmp(featurestr, enforce)) { -check_cpuid = enforce_cpuid = 1; +object_property_parse(OBJECT(cpu), on, featurestr, errp); } else if (!strcmp(featurestr, hv_relaxed)) { object_property_parse(OBJECT(cpu), on, hv-relaxed, errp); } else if (!strcmp(featurestr, hv_vapic)) { @@ -2236,8 +2235,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) #ifdef CONFIG_KVM filter_features_for_kvm(cpu); #endif -if (check_cpuid kvm_check_features_against_host(cpu) - enforce_cpuid) { +if ((env-check_cpuid || env-enforce_cpuid) + kvm_check_features_against_host(cpu) env-enforce_cpuid) { error_setg(errp, Host's CPU doesn't support requested features); return; } diff --git a/target-i386/cpu.h b/target-i386/cpu.h index e3723c2..cdec0da 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -847,6 +847,8 @@ typedef struct CPUX86State { bool hyperv_vapic; bool hyperv_relaxed_timing; int hyperv_spinlock_attempts; +bool check_cpuid; +bool enforce_cpuid; /* MTRRs */ uint64_t mtrr_fixed[11]; -- 1.7.1
[Qemu-devel] [PATCH 06/10] target-i386: convert 'hv_vapic' to static property
Signed-off-by: Igor Mammedov imamm...@redhat.com --- target-i386/cpu.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index e38c369..8f7132a 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1350,6 +1350,7 @@ static Property cpu_x86_properties[] = { DEFINE_PROP_TSC_FREQ(tsc-frequency), DEFINE_PROP_HV_SPINLOCKS(hv-spinlocks, HYPERV_SPINLOCK_NEVER_RETRY), DEFINE_PROP_BOOL(hv-relaxed, X86CPU, env.hyperv_relaxed_timing, false), +DEFINE_PROP_BOOL(hv-vapic, X86CPU, env.hyperv_vapic, false), DEFINE_PROP_END_OF_LIST(), }; @@ -1492,7 +1493,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp) } else if (!strcmp(featurestr, hv_relaxed)) { object_property_parse(OBJECT(cpu), on, hv-relaxed, errp); } else if (!strcmp(featurestr, hv_vapic)) { -env-hyperv_vapic = true; +object_property_parse(OBJECT(cpu), on, hv-vapic, errp); } else { error_setg(errp, feature string `%s' not in format (+feature| -feature|feature=xyz), featurestr); -- 1.7.1
[Qemu-devel] [PATCH 01/10] qdev: add qdev property for bool type
Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/qdev-properties.c | 33 + hw/qdev-properties.h | 10 ++ 2 files changed, 43 insertions(+), 0 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index a8a31f5..16ac814 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -106,6 +106,39 @@ PropertyInfo qdev_prop_bit = { .set = set_bit, }; +/* --- bool --- */ + +static void get_bool(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +DeviceState *dev = DEVICE(obj); +Property *prop = opaque; +bool *ptr = qdev_get_prop_ptr(dev, prop); + +visit_type_bool(v, ptr, name, errp); +} + +static void set_bool(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +DeviceState *dev = DEVICE(obj); +Property *prop = opaque; +bool *ptr = qdev_get_prop_ptr(dev, prop); + +if (dev-realized) { +error_setg(errp, Insufficient permission to perform this operation); +return; +} + +visit_type_bool(v, ptr, name, errp); +} + +PropertyInfo qdev_prop_bool = { +.name = boolean, +.get = get_bool, +.set = set_bool, +}; + /* --- 8bit integer --- */ static void get_uint8(Object *obj, Visitor *v, void *opaque, diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h index 20c67f3..3915f7c 100644 --- a/hw/qdev-properties.h +++ b/hw/qdev-properties.h @@ -6,6 +6,7 @@ /*** qdev-properties.c ***/ extern PropertyInfo qdev_prop_bit; +extern PropertyInfo qdev_prop_bool; extern PropertyInfo qdev_prop_uint8; extern PropertyInfo qdev_prop_uint16; extern PropertyInfo qdev_prop_uint32; @@ -51,6 +52,15 @@ extern PropertyInfo qdev_prop_pci_host_devaddr; .defval= (bool)_defval, \ } +#define DEFINE_PROP_BOOL(_name, _state, _field, _defval) { \ +.name = (_name),\ +.info = (qdev_prop_bool), \ +.offset= offsetof(_state, _field)\ ++ type_check(bool, typeof_field(_state, _field)),\ +.qtype = QTYPE_QBOOL,\ +.defval= (bool)_defval, \ +} + #define DEFINE_PROP_UINT8(_n, _s, _f, _d) \ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t) #define DEFINE_PROP_UINT16(_n, _s, _f, _d) \ -- 1.7.1
[Qemu-devel] [PATCH 10/10] target-i386: set [+-]feature using static properties
* Define static properties for cpuid feature bits * property names of CPUID features are changed to have f- prefix, so that it would be easy to distinguish them from other properties. * Convert [+-]cpuid_features to a set(QDict) of key, value pairs, where +feat = (f-feat, on) -feat = (f-feat, off) [+-] unknown feat = (feat, on/off) - it's expected to be rejected by property setter later QDict is used as convinience sturcture to keep -foo semantic. Once all +-foo are parsed, collected features are applied to CPU instance. * Cleanup unused anymore: add_flagname_to_bitmaps(), lookup_feature(), altcmp(), sstrcmp(), * when parsing 'kvmclock' feature, set additional f-kvmclock2 feature in cpu_x86_parse_featurestr() to mantain legacy kvmclock feature behavior Signed-off-by: Igor Mammedov imamm...@redhat.com --- v5: - instead of introducing composit f-kvmclock property to maintain legace behavior, set additional f-kvmclock2 in cpu_x86_parse_featurestr() when kvmclock is parsed. v4: - major patch reordering, rebasing on current qom-cpu-next - merged patches: define static properties... and set [+-]feature... - merge in make 'f-kvmclock' compatible... to aviod behavior breakage - use register name in feature macros, so that if we rename cpuid_* fields, it wouldn't involve mass rename in features array. - when converting feature names into property names, replace '_' with '-', Requested-By: Don Slutz d...@cloudswitch.com, mgs-id: 5085d4aa.1060...@cloudswitch.com v3: - new static properties after rebase: - add features f-rdseed f-adx - add features added by c8acc380 - add features f-kvm_steal_tm and f-kvmclock_stable - ext4 features f-xstore, f-xstore-en, f-xcrypt, f-xcrypt-en, f-ace2, f-ace2-en, f-phe, f-phe-en, f-pmm, f-pmm-en - f-hypervisor set default to false as for the rest of features - define helper FEAT for declaring CPUID feature properties to make shorter lines (80 characters) v2: - use X86CPU as a type to count of offset correctly, because env field isn't starting at CPUstate beginning, but located after it. --- target-i386/cpu.c | 323 +++- 1 files changed, 217 insertions(+), 106 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 6a0d5d3..453d179 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -263,85 +263,6 @@ void host_cpuid(uint32_t function, uint32_t count, #endif } -#define iswhite(c) ((c) ((c) = ' ' || '~' (c))) - -/* general substring compare of *[s1..e1) and *[s2..e2). sx is start of - * a substring. ex if !NULL points to the first char after a substring, - * otherwise the string is assumed to sized by a terminating nul. - * Return lexical ordering of *s1:*s2. - */ -static int sstrcmp(const char *s1, const char *e1, const char *s2, -const char *e2) -{ -for (;;) { -if (!*s1 || !*s2 || *s1 != *s2) -return (*s1 - *s2); -++s1, ++s2; -if (s1 == e1 s2 == e2) -return (0); -else if (s1 == e1) -return (*s2); -else if (s2 == e2) -return (*s1); -} -} - -/* compare *[s..e) to *altstr. *altstr may be a simple string or multiple - * '|' delimited (possibly empty) strings in which case search for a match - * within the alternatives proceeds left to right. Return 0 for success, - * non-zero otherwise. - */ -static int altcmp(const char *s, const char *e, const char *altstr) -{ -const char *p, *q; - -for (q = p = altstr; ; ) { -while (*p *p != '|') -++p; -if ((q == p !*s) || (q != p !sstrcmp(s, e, q, p))) -return (0); -if (!*p) -return (1); -else -q = ++p; -} -} - -/* search featureset for flag *[s..e), if found set corresponding bit in - * *pval and return true, otherwise return false - */ -static bool lookup_feature(uint32_t *pval, const char *s, const char *e, - const char **featureset) -{ -uint32_t mask; -const char **ppc; -bool found = false; - -for (mask = 1, ppc = featureset; mask; mask = 1, ++ppc) { -if (*ppc !altcmp(s, e, *ppc)) { -*pval |= mask; -found = true; -} -} -return found; -} - -static void add_flagname_to_bitmaps(const char *flagname, -FeatureWordArray words) -{ -FeatureWord w; -for (w = 0; w FEATURE_WORDS; w++) { -FeatureWordInfo *wi = feature_word_info[w]; -if (wi-feat_names -lookup_feature(words[w], flagname, NULL, wi-feat_names)) { -break; -} -} -if (w == FEATURE_WORDS) { -fprintf(stderr, CPU feature %s not found\n, flagname); -} -} - typedef struct x86_def_t { const char *name; uint32_t level; @@
[Qemu-devel] [PATCH 08/10] target-i386: cleanup 'foo' feature handling'
features check, enforce, hv_relaxed and hv_vapic are treated as boolean set to 'on' when passed from command line, so it's not neccessary to handle each of them separetly. Collapse them to one catch-all branch which will treat any feature in format 'foo' as boolean set to 'on'. PS: Any unknown feature will be rejected by CPU property setter so there is no need to check for unknown feature in cpu_x86_parse_featurestr(), therefore it's replaced by above mentioned catch-all handler. Signed-off-by: Igor Mammedov imamm...@redhat.com Reviewed-by: Eduardo Habkost ehabk...@redhat.com --- v2: * use feat2prop() to perform name convertion for hv_vapic and hv_relaxed --- target-i386/cpu.c | 13 ++--- 1 files changed, 2 insertions(+), 11 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 6826224..1073bdc 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1485,18 +1485,9 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp) error_setg(errp, unrecognized feature %s, featurestr); goto out; } -} else if (!strcmp(featurestr, check)) { -object_property_parse(OBJECT(cpu), on, featurestr, errp); -} else if (!strcmp(featurestr, enforce)) { -object_property_parse(OBJECT(cpu), on, featurestr, errp); -} else if (!strcmp(featurestr, hv_relaxed)) { -object_property_parse(OBJECT(cpu), on, hv-relaxed, errp); -} else if (!strcmp(featurestr, hv_vapic)) { -object_property_parse(OBJECT(cpu), on, hv-vapic, errp); } else { -error_setg(errp, feature string `%s' not in format (+feature| - -feature|feature=xyz), featurestr); -goto out; +feat2prop(featurestr); +object_property_parse(OBJECT(cpu), on, featurestr, errp); } if (error_is_set(errp)) { goto out; -- 1.7.1
Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/3] powerpc iommu: multiple TCE requests enabled
On Fri, Feb 22, 2013 at 12:03:49PM +1100, Alexey Kardashevskiy wrote: On 22/02/13 09:52, David Gibson wrote: On Tue, Feb 19, 2013 at 06:43:35PM +1100, Alexey Kardashevskiy wrote: [snip] You've implemented the multitce hypercalls in qemu, but because of the kvm capability check, you'll never advertise them in full emu. Instead you should always advertise them as available, and the kvm capability will just be a question of whether they go fast (through kvm) or slow (through qemu). So we do not need the KVM_CAP_PPC_MULTITCE capability at all as we are not going to support real mode without multi-tce support in the host kernel, is that correct? Um.. is CAP_PPC_MULTITCE covering the version of the hypercalls for emulated devices, for vfio devices or both. For emulated devices I don't think we strictly need itm because we can fall back to qemu fine. It might still be an idea to have the capability, so qemu or the user know in advance if things are going to be slow or fast. For now I think it's simplest for qemu to just do the fallback, but it's possible we might want to advertise different things to the guest based on whether they'll be fast or not. For vfio.. there I don't think we need a capability, since multitce hypercall support should be there from day one. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: Digital signature
[Qemu-devel] [PATCH 04/10] target-i386: convert 'hv_spinlocks' to static property
Signed-off-by: Igor Mammedov imamm...@redhat.com --- target-i386/cpu.c | 50 -- 1 files changed, 48 insertions(+), 2 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 35fc303..443c15e 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1295,6 +1295,50 @@ static PropertyInfo qdev_prop_tsc_freq = { #define DEFINE_PROP_TSC_FREQ(_n) \ DEFINE_PROP(_n, X86CPU, env.tsc_khz, qdev_prop_tsc_freq, int32_t) +static void x86_get_hv_spinlocks(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +X86CPU *cpu = X86_CPU(obj); +int64_t value = cpu-env.hyperv_spinlock_attempts; + +visit_type_int(v, value, name, errp); +} + +static void x86_set_hv_spinlocks(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +const int64_t min = 0xFFF; +const int64_t max = UINT_MAX; +X86CPU *cpu = X86_CPU(obj); +int64_t value; + +visit_type_int(v, value, name, errp); +if (error_is_set(errp)) { +return; +} + +if (value min || value max) { +error_setg(errp, Property %s.%s doesn't take value % PRId64 + (minimum: % PRId64 , maximum: % PRId64 ), + object_get_typename(obj), name ? name : null, + value, min, max); +return; +} +cpu-env.hyperv_spinlock_attempts = value; +} + +static PropertyInfo qdev_prop_spinlocks = { +.name = int, +.get = x86_get_hv_spinlocks, +.set = x86_set_hv_spinlocks, +}; +#define DEFINE_PROP_HV_SPINLOCKS(_n, _defval) { \ +.name = _n, \ +.info = qdev_prop_spinlocks, \ +.qtype = QTYPE_QINT, \ +.defval = _defval \ +} + static Property cpu_x86_properties[] = { DEFINE_PROP_FAMILY(family), DEFINE_PROP_MODEL(model), @@ -1304,6 +1348,7 @@ static Property cpu_x86_properties[] = { DEFINE_PROP_VENDOR(vendor), DEFINE_PROP_MODEL_ID(model-id), DEFINE_PROP_TSC_FREQ(tsc-frequency), +DEFINE_PROP_HV_SPINLOCKS(hv-spinlocks, HYPERV_SPINLOCK_NEVER_RETRY), DEFINE_PROP_END_OF_LIST(), }; @@ -1421,6 +1466,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp) } else if (!strcmp(featurestr, hv-spinlocks)) { char *err; const int min = 0xFFF; +char num[32]; numvalue = strtoul(val, err, 0); if (!*val || *err) { error_setg(errp, bad numerical value %s, val); @@ -1432,7 +1478,8 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp) min); numvalue = min; } -env-hyperv_spinlock_attempts = numvalue; +snprintf(num, sizeof(num), % PRId32, numvalue); +object_property_parse(OBJECT(cpu), num, featurestr, errp); } else { error_setg(errp, unrecognized feature %s, featurestr); goto out; @@ -1597,7 +1644,6 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp) def-kvm_features |= kvm_default_features; } def-ext_features |= CPUID_EXT_HYPERVISOR; -env-hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY; object_property_set_str(OBJECT(cpu), def-vendor, vendor, errp); object_property_set_int(OBJECT(cpu), def-level, level, errp); -- 1.7.1
[Qemu-devel] [PATCH 02/10] target-i386: cpu: convert existing dynamic properties into static properties
Following properties are converted: * vendor * xlevel * custom setter/getter replaced by qdev's DEFINE_PROP_UINT32 * level * custom setter/getter replaced by qdev's DEFINE_PROP_UINT32 * tsc-frequency * stepping * model * family * model-id * check if (model_id == NULL) looks unnecessary now, since all builtin model-ids are not NULL and user shouldn't be able to set it NULL (cpumodel string parsing code takes care of it, if feature is specified as model-id= on command line, its parsing will result in an empty string as value). Common changes to all properties: * string properties: changed function signature to conform to form used in qdev-properties.c * extra change is addition of feat2prop() helper to deal with properties naming using '-' instead of '_'. Used in this patch for 'model_id' name conversion and converting along the way 'hv-spinlocks', but will be reused in following patches for hv_* and +-foo conversions as well. Signed-off-by: Igor Mammedov imamm...@redhat.com v2: - removed s/error_set/error_setg/;s/QERR*/with similar message/ - made PropertyInfo static - Left setters/getters atwher they have been, only signature change + usin visitors where needed - use g_malloc0() instead of g_malloc() in x86_cpuid_get_model_id() --- target-i386/cpu.c | 174 - 1 files changed, 105 insertions(+), 69 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index dfcf86e..5626931 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -41,6 +41,7 @@ #endif #include sysemu/sysemu.h +#include hw/qdev-properties.h #ifndef CONFIG_USER_ONLY #include hw/xen.h #include hw/sysbus.h @@ -1055,6 +1056,14 @@ static void x86_cpuid_version_set_family(Object *obj, Visitor *v, void *opaque, } } +static PropertyInfo qdev_prop_family = { +.name = uint32, +.get = x86_cpuid_version_get_family, +.set = x86_cpuid_version_set_family, +}; +#define DEFINE_PROP_FAMILY(_n) \ +DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_family, uint32_t) + static void x86_cpuid_version_get_model(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { @@ -1090,6 +1099,14 @@ static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void *opaque, env-cpuid_version |= ((value 0xf) 4) | ((value 4) 16); } +static PropertyInfo qdev_prop_model = { +.name = uint32, +.get = x86_cpuid_version_get_model, +.set = x86_cpuid_version_set_model, +}; +#define DEFINE_PROP_MODEL(_n) \ +DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_model, uint32_t) + static void x86_cpuid_version_get_stepping(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) @@ -1126,57 +1143,41 @@ static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v, env-cpuid_version |= value 0xf; } -static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque, -const char *name, Error **errp) -{ -X86CPU *cpu = X86_CPU(obj); - -visit_type_uint32(v, cpu-env.cpuid_level, name, errp); -} - -static void x86_cpuid_set_level(Object *obj, Visitor *v, void *opaque, -const char *name, Error **errp) -{ -X86CPU *cpu = X86_CPU(obj); - -visit_type_uint32(v, cpu-env.cpuid_level, name, errp); -} - -static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) -{ -X86CPU *cpu = X86_CPU(obj); - -visit_type_uint32(v, cpu-env.cpuid_xlevel, name, errp); -} +static PropertyInfo qdev_prop_stepping = { +.name = uint32, +.get = x86_cpuid_version_get_stepping, +.set = x86_cpuid_version_set_stepping, +}; +#define DEFINE_PROP_STEPPING(_n) \ +DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_stepping, uint32_t) -static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, void *opaque, +static void x86_cpuid_get_vendor(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { X86CPU *cpu = X86_CPU(obj); - -visit_type_uint32(v, cpu-env.cpuid_xlevel, name, errp); -} - -static char *x86_cpuid_get_vendor(Object *obj, Error **errp) -{ -X86CPU *cpu = X86_CPU(obj); CPUX86State *env = cpu-env; char *value; value = (char *)g_malloc(CPUID_VENDOR_SZ + 1); x86_cpu_vendor_words2str(value, env-cpuid_vendor1, env-cpuid_vendor2, env-cpuid_vendor3); -return value; +visit_type_str(v, value, name, errp); +g_free(value); } -static void
[Qemu-devel] [PATCH qom-cpu-next 00/10 v7] target-i386: convert CPU features into properties
It's a simplified rewrite of previous series, since then cleanups from it were applied to master and I left out kvm_check_features_against_host() and listflags() patches as not directly related to make series simpler. They could follow as separate cleanups later. Also setting defaults with static properties left to post CPU subclasses conversion when it could be done in a more straightforward way and only once. v6-v7: * convert globals check_cpuid, enforce_cpuid and hyperv_* to fields of CPUState * Make PropertyInfo-s static * maintain legacy kvmclock semantic in cpu_x86_parse_featurestr() * existing properties code are not moved around, just fixed signatures where it's needed and used visitors. v5-v6: * when converting feature names to property names, replace '_' with '-' * separate patches converting existing dynamic properties into one, were squashed into one [1/9] and change tested with virt-test(next). * patches that were touching +-foo features are squashed into one [9/9], to avoid behavior change between them(f-kvmclock property). * the rest of conversions were basicaly rebased on top of current qom-cpu-next tree, with small corrections git for testing: https://github.com/imammedo/qemu/tree/x86-cpu-properties.v7 Igor Mammedov (10): qdev: add qdev property for bool type target-i386: cpu: convert existing dynamic properties into static properties target-i386: move hyperv_* static globals to CPUState target-i386: convert 'hv_spinlocks' to static property target-i386: convert 'hv_relaxed' to static property target-i386: convert 'hv_vapic' to static property target-i386: convert 'check' and 'enforce' to static properties target-i386: cleanup 'foo' feature handling' target-i386: cleanup 'foo=val' feature handling target-i386: set [+-]feature using static properties hw/qdev-properties.c | 33 +++ hw/qdev-properties.h | 10 + target-i386/Makefile.objs |2 +- target-i386/cpu.c | 588 + target-i386/cpu.h |9 + target-i386/hyperv.c | 64 - target-i386/hyperv.h | 45 target-i386/kvm.c | 36 ++- 8 files changed, 462 insertions(+), 325 deletions(-) delete mode 100644 target-i386/hyperv.c delete mode 100644 target-i386/hyperv.h
[Qemu-devel] [PATCH 09/10] target-i386: cleanup 'foo=val' feature handling
features family, model, stepping, level, hv_spinlocks are treated similarly when passed from command line, so it's not necessary to handle each of them individually. Collapse them to one catch-all branch which will treat any not explicitly handled feature in format 'foo=val'. PS: Any unknown feature will be rejected by property setter so there is no need to check for unknown feature in cpu_x86_parse_featurestr(), therefore it's replaced by above mentioned catch-all handler. Signed-off-by: Igor Mammedov imamm...@redhat.com --- v2: - style fixes --- target-i386/cpu.c | 17 ++--- 1 files changed, 2 insertions(+), 15 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 1073bdc..6a0d5d3 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1423,15 +1423,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp) } else if ((val = strchr(featurestr, '='))) { feat2prop(featurestr); *val = 0; val++; -if (!strcmp(featurestr, family)) { -object_property_parse(OBJECT(cpu), val, featurestr, errp); -} else if (!strcmp(featurestr, model)) { -object_property_parse(OBJECT(cpu), val, featurestr, errp); -} else if (!strcmp(featurestr, stepping)) { -object_property_parse(OBJECT(cpu), val, featurestr, errp); -} else if (!strcmp(featurestr, level)) { -object_property_parse(OBJECT(cpu), val, featurestr, errp); -} else if (!strcmp(featurestr, xlevel)) { +if (!strcmp(featurestr, xlevel)) { char *err; char num[32]; @@ -1447,10 +1439,6 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp) } snprintf(num, sizeof(num), % PRIu32, numvalue); object_property_parse(OBJECT(cpu), num, featurestr, errp); -} else if (!strcmp(featurestr, vendor)) { -object_property_parse(OBJECT(cpu), val, featurestr, errp); -} else if (!strcmp(featurestr, model-id)) { -object_property_parse(OBJECT(cpu), val, featurestr, errp); } else if (!strcmp(featurestr, tsc-freq)) { int64_t tsc_freq; char *err; @@ -1482,8 +1470,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp) snprintf(num, sizeof(num), % PRId32, numvalue); object_property_parse(OBJECT(cpu), num, featurestr, errp); } else { -error_setg(errp, unrecognized feature %s, featurestr); -goto out; +object_property_parse(OBJECT(cpu), val, featurestr, errp); } } else { feat2prop(featurestr); -- 1.7.1
[Qemu-devel] [PATCH 0/2] bridge helper: includedir conf arg
The goal is to support an 'includedir' to include all files within a directory specified in the bridge.conf file. The rationale is to allow libvirt to be able to configure interfaces to for use by unprivileged users by just simply generating a new configuration file to the directory. CC: Anthony Liguori aligu...@us.ibm.com CC: Richa Marwaha rmar...@linux.vnet.ibm.com CC: Corey Bryant cor...@linux.vnet.ibm.com TO: qemu-devel@nongnu.org Doug Goldstein (2): bridge helper: support conf dirs bridge helper: unified error cleanup for parse_acl_file qemu-bridge-helper.c | 73 +--- 1 file changed, 69 insertions(+), 4 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH 1/2] bridge helper: support conf dirs
Allow the bridge helper to take a config directory rather than having to specify every file in the directory manually via an include statement. Signed-off-by: Doug Goldstein car...@cardoe.com CC: Anthony Liguori aligu...@us.ibm.com CC: Richa Marwaha rmar...@linux.vnet.ibm.com CC: Corey Bryant cor...@linux.vnet.ibm.com TO: qemu-devel@nongnu.org --- qemu-bridge-helper.c | 55 1 file changed, 55 insertions(+) diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c index 287bfd5..b8771a3 100644 --- a/qemu-bridge-helper.c +++ b/qemu-bridge-helper.c @@ -16,6 +16,7 @@ #include config-host.h #include stdio.h +#include dirent.h #include errno.h #include fcntl.h #include unistd.h @@ -70,6 +71,26 @@ static void usage(void) Usage: qemu-bridge-helper [--use-vnet] --br=bridge --fd=unixfd\n); } +static int filter_bridge_conf_dir(const struct dirent *entry) +{ +ssize_t name_pos; + +/* We want to check the last 5 bytes for '.conf' */ +name_pos = strlen(entry-d_name) - 6; + +/* We need the file to at least be called 'a.conf' to make + * sense of this. + */ +if (name_pos 1) +return 0; + +/* If the file didn't end in '.conf', skip it */ +if (strcmp(.conf, entry-d_name + name_pos)) +return 0; + +return 1; +} + static int parse_acl_file(const char *filename, ACLList *acl_list) { FILE *f; @@ -84,6 +105,9 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) while (fgets(line, sizeof(line), f) != NULL) { char *ptr = line; char *cmd, *arg, *argend; +struct dirent **include_list = NULL; +int i, include_count; +char *conf_file; while (isspace(*ptr)) { ptr++; @@ -137,6 +161,37 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) snprintf(acl_rule-iface, IFNAMSIZ, %s, arg); } QSIMPLEQ_INSERT_TAIL(acl_list, acl_rule, entry); +} else if (strcmp(cmd, includedir) == 0) { +include_count = scandir(arg, include_list, +filter_bridge_conf_dir, NULL); +if (include_count 0) { +fprintf(stderr, Unable to retrieve conf files from '%s': %s\n, +arg, strerror(errno)); +fclose(f); +return -1; +} + +for (i = 0; i include_count; i++) { +if (asprintf(conf_file, %s/%s, arg, + include_list[i]-d_name) 0) { +fprintf(stderr, Failed to allocate memory for +file path: %s/%s\n, +arg, include_list[i]-d_name); +fclose(f); +errno = ENOMEM; +return -1; +} + +parse_acl_file(conf_file, acl_list); + +free(conf_file); +free(include_list[i]); +include_list[i] = NULL; +} +free(include_list); +include_list = NULL; +include_count = 0; + } else if (strcmp(cmd, include) == 0) { /* ignore errors */ parse_acl_file(arg, acl_list); -- 1.7.12.4
[Qemu-devel] [PATCH 2/2] bridge helper: unified error cleanup for parse_acl_file
Handle errors and cleanup from the error in a unified place for parse_acl_file(). Signed-off-by: Doug Goldstein car...@cardoe.com CC: Anthony Liguori aligu...@us.ibm.com CC: Richa Marwaha rmar...@linux.vnet.ibm.com CC: Corey Bryant cor...@linux.vnet.ibm.com TO: qemu-devel@nongnu.org --- qemu-bridge-helper.c | 32 +--- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c index b8771a3..d95e760 100644 --- a/qemu-bridge-helper.c +++ b/qemu-bridge-helper.c @@ -96,6 +96,9 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) FILE *f; char line[4096]; ACLRule *acl_rule; +struct dirent **include_list = NULL; +int i, include_count = 0; +char *conf_file = NULL; f = fopen(filename, r); if (f == NULL) { @@ -105,9 +108,6 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) while (fgets(line, sizeof(line), f) != NULL) { char *ptr = line; char *cmd, *arg, *argend; -struct dirent **include_list = NULL; -int i, include_count; -char *conf_file; while (isspace(*ptr)) { ptr++; @@ -126,9 +126,8 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) if (arg == NULL) { fprintf(stderr, Invalid config line:\n %s\n, line); -fclose(f); errno = EINVAL; -return -1; +goto cleanup; } *arg = 0; @@ -167,8 +166,7 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) if (include_count 0) { fprintf(stderr, Unable to retrieve conf files from '%s': %s\n, arg, strerror(errno)); -fclose(f); -return -1; +goto cleanup; } for (i = 0; i include_count; i++) { @@ -177,9 +175,8 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) fprintf(stderr, Failed to allocate memory for file path: %s/%s\n, arg, include_list[i]-d_name); -fclose(f); errno = ENOMEM; -return -1; +goto cleanup; } parse_acl_file(conf_file, acl_list); @@ -197,15 +194,28 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) parse_acl_file(arg, acl_list); } else { fprintf(stderr, Unknown command `%s'\n, cmd); -fclose(f); errno = EINVAL; -return -1; +goto cleanup; } } fclose(f); return 0; + +cleanup: + +fclose(f); + +if (include_list) { +for (i = 0; i include_count; i++) { +if (include_list[i]) +free(include_list[i]); +} +free(include_list); +} + +return -1; } static bool has_vnet_hdr(int fd) -- 1.7.12.4
[Qemu-devel] [PATCH 05/10] target-i386: convert 'hv_relaxed' to static property
Signed-off-by: Igor Mammedov imamm...@redhat.com --- target-i386/cpu.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 443c15e..e38c369 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1349,6 +1349,7 @@ static Property cpu_x86_properties[] = { DEFINE_PROP_MODEL_ID(model-id), DEFINE_PROP_TSC_FREQ(tsc-frequency), DEFINE_PROP_HV_SPINLOCKS(hv-spinlocks, HYPERV_SPINLOCK_NEVER_RETRY), +DEFINE_PROP_BOOL(hv-relaxed, X86CPU, env.hyperv_relaxed_timing, false), DEFINE_PROP_END_OF_LIST(), }; @@ -1489,7 +1490,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp) } else if (!strcmp(featurestr, enforce)) { check_cpuid = enforce_cpuid = 1; } else if (!strcmp(featurestr, hv_relaxed)) { -env-hyperv_relaxed_timing = true; +object_property_parse(OBJECT(cpu), on, hv-relaxed, errp); } else if (!strcmp(featurestr, hv_vapic)) { env-hyperv_vapic = true; } else { -- 1.7.1
[Qemu-devel] [PATCH] show --disable-gtk and --enable-gtk in the help message
Signed-off-by: Hu Tao hu...@cn.fujitsu.com --- configure | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index dcaa67c..46b29dc 100755 --- a/configure +++ b/configure @@ -1052,6 +1052,8 @@ echo --disable-strip disable stripping binaries echo --disable-werror disable compilation abort on warning echo --disable-sdldisable SDL echo --enable-sdl enable SDL +echo --disable-gtkdisable gtk UI +echo --enable-gtk enable gtk UI echo --disable-virtfs disable VirtFS echo --enable-virtfs enable VirtFS echo --disable-vncdisable VNC -- 1.8.0.1.240.ge8a1f5a
Re: [Qemu-devel] 'info help'
Quoting mdroth (mdr...@linux.vnet.ibm.com): On Fri, Feb 22, 2013 at 05:10:29PM +, Serge E. Hallyn wrote: Hi, up to and including 1.3.0, monitor.c:do_info(), if it got no arg or an unknown arg, would do help_cmd(mon, info); That behavior is gone in 1.4.0, so that 'info', 'info help', and 'info whatever' just say unknown argument. Was that intended, or would re-introducing the old behavior be acceptable? Seems intended: 84c44613f9ad8d13e0d2dbee767051527072dc12 introduced the change as a result of refactoring, and the change in behavior was documented (and the original behavior was never really 'official': 'help info' was always the documented help text). Thanks guys, just wanted to make sure. -serge
[Qemu-devel] [PATCH v5] Enable and Handle in-kernel watchdog emulation
This Patch enables and handle the in-kernel watchdog emulation if KVM supports. Initially there were 3 patches, 2 of them are already applied so sending the 3rd patch now. v5: - using store_booke_tcr/tsr in reset handler rather than using SREGS interface. - removed unused SREGS get/set timer register interface. v4: Using one_reg interface to or/clear timer registers Bharat Bhushan (1): Enable kvm emulated watchdog hw/ppc_booke.c | 46 +++--- target-ppc/kvm.c | 74 ++ target-ppc/kvm_ppc.h | 23 +++ 3 files changed, 138 insertions(+), 5 deletions(-)
[Qemu-devel] [PATCH v5] Enable kvm emulated watchdog
Enable the KVM emulated watchdog if KVM supports (use the capability enablement in watchdog handler). Also watchdog exit (KVM_EXIT_WATCHDOG) handling is added. Watchdog state machine is cleared whenever VM state changes to running. This is to handle the cases like return from debug halt etc. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- hw/ppc_booke.c | 46 +++--- target-ppc/kvm.c | 74 ++ target-ppc/kvm_ppc.h | 23 +++ 3 files changed, 138 insertions(+), 5 deletions(-) diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c index 25a4e91..897d528 100644 --- a/hw/ppc_booke.c +++ b/hw/ppc_booke.c @@ -28,7 +28,7 @@ #include nvram.h #include qemu/log.h #include loader.h - +#include kvm_ppc.h /* Timer Control Register */ @@ -211,6 +211,7 @@ void store_booke_tsr(CPUPPCState *env, target_ulong val) PowerPCCPU *cpu = ppc_env_get_cpu(env); env-spr[SPR_BOOKE_TSR] = ~val; +kvmppc_clear_tsr_bits(cpu, val); booke_update_irq(cpu); } @@ -222,6 +223,7 @@ void store_booke_tcr(CPUPPCState *env, target_ulong val) tb_env = env-tb_env; env-spr[SPR_BOOKE_TCR] = val; +kvmppc_set_tcr(cpu); booke_update_irq(cpu); @@ -234,7 +236,6 @@ void store_booke_tcr(CPUPPCState *env, target_ulong val) booke_get_wdt_target(env, tb_env), booke_timer-wdt_next, booke_timer-wdt_timer); - } static void ppc_booke_timer_reset_handle(void *opaque) @@ -242,16 +243,39 @@ static void ppc_booke_timer_reset_handle(void *opaque) PowerPCCPU *cpu = opaque; CPUPPCState *env = cpu-env; -env-spr[SPR_BOOKE_TSR] = 0; -env-spr[SPR_BOOKE_TCR] = 0; +store_booke_tcr(env, 0); +store_booke_tsr(env, -1); +} -booke_update_irq(cpu); +/* + * This function will be called whenever the CPU state changes. + * CPU states are defined typedef enum RunState. + * Regarding timer, When CPU state changes to running after debug halt + * or similar cases which takes time then in between final watchdog + * expiry happenes. This will cause exit to QEMU and configured watchdog + * action will be taken. To avoid this we always clear the watchdog state when + * state changes to running. + */ +static void cpu_state_change_handler(void *opaque, int running, RunState state) +{ +PowerPCCPU *cpu = opaque; +CPUPPCState *env = cpu-env; + +if (!running) { +return; +} + +/* + * Clear watchdog interrupt condition by clearing TSR. + */ +store_booke_tsr(env, TSR_ENW | TSR_WIS | TSR_WRS_MASK); } void ppc_booke_timers_init(PowerPCCPU *cpu, uint32_t freq, uint32_t flags) { ppc_tb_t *tb_env; booke_timer_t *booke_timer; +int ret = 0; tb_env = g_malloc0(sizeof(ppc_tb_t)); booke_timer = g_malloc0(sizeof(booke_timer_t)); @@ -269,5 +293,17 @@ void ppc_booke_timers_init(PowerPCCPU *cpu, uint32_t freq, uint32_t flags) booke_timer-wdt_timer = qemu_new_timer_ns(vm_clock, booke_wdt_cb, cpu); +ret = kvmppc_booke_watchdog_enable(cpu); + +if (ret) { +/* TODO: Start the QEMU emulated watchdog if not running on KVM. + * Also start the QEMU emulated watchdog if KVM does not support + * emulated watchdog or somehow it is not enabled (supported but + * not enabled is though some bug and requires debugging :)). + */ +} + +qemu_add_vm_change_state_handler(cpu_state_change_handler, cpu); + qemu_register_reset(ppc_booke_timer_reset_handle, cpu); } diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index 2c64c63..bf893a1 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -36,6 +36,7 @@ #include hw/sysbus.h #include hw/spapr.h #include hw/spapr_vio.h +#include hw/watchdog.h //#define DEBUG_KVM @@ -61,6 +62,7 @@ static int cap_ppc_smt; static int cap_ppc_rma; static int cap_spapr_tce; static int cap_hior; +static int cap_ppc_watchdog; /* XXX We have a race condition where we actually have a level triggered * interrupt, but the infrastructure can't expose that yet, so the guest @@ -90,6 +92,7 @@ int kvm_arch_init(KVMState *s) cap_ppc_rma = kvm_check_extension(s, KVM_CAP_PPC_RMA); cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE); cap_hior = kvm_check_extension(s, KVM_CAP_PPC_HIOR); +cap_ppc_watchdog = kvm_check_extension(s, KVM_CAP_PPC_BOOKE_WATCHDOG); if (!cap_interrupt_level) { fprintf(stderr, KVM: Couldn't find level irq capability. Expect the @@ -856,6 +859,12 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) run-epr.epr = ldl_phys(env-mpic_iack); ret = 0; break; +case KVM_EXIT_WATCHDOG: +dprintf(handle watchdog expiry\n); +watchdog_perform_action(); +ret = 0; +break; + default: fprintf(stderr, KVM: unknown exit reason %d\n,
Re: [Qemu-devel] scp during migration with vhost fails
On 02/24/2013 05:54 AM, Michael S. Tsirkin wrote: On Sat, Feb 23, 2013 at 10:49:29PM +0200, Michael S. Tsirkin wrote: On Fri, Feb 22, 2013 at 11:33:53PM +0800, Jason Wang wrote: On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote: On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote: On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote: On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote: On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote: Hello all: During testing, I find doing scp during migration with vhost fails with warnings in guest like: Corrupted MAC on input. Disconnecting: Packet corrupt. lost connection Here's the bisect result: Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to MemoryListener API is the last commit that works well. With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to MemoryListener API, guest network is unusable with warning of bad gso type With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix incorrect userspace address, guest network is available, but scp during migration may fail. Looks like the issue is related to memory api, any thoughts? Thanks Tried to reproduce this for a while without success. Which command line was used? -- MST Could be we are not syncing all that we should? Does the following hack make the problem go away? diff --git a/hw/vhost.c b/hw/vhost.c index 8d41fdb..a7a0412 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev, hwaddr end_addr) { int i; +start_addr = 0x0; +end_addr = ~0x0ull; if (!dev-log_enabled || !dev-started) { return 0; Still can reproduce with this. From the bisect result, the vhost dirty bitmap sync itself looks ok but something wrong when converting to memory listener. Reading the code carefully, I found two bugs introduced during this conversion. Patch below, could you please try? vhost: memory sync fixes This fixes two bugs related to memory sync during migration: - ram address calculation was missing the chunk address, so the wrong page was dirtied - one after last was used instead of the end address of a region, which might overflow to 0 and cause us to skip the region when the region ends at ~0x0ull. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- diff --git a/hw/vhost.c b/hw/vhost.c index 8d41fdb..dbf6b46 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, ffsll(log) : ffs(log))) { ram_addr_t ram_addr; bit -= 1; -ram_addr = section-offset_within_region + bit * VHOST_LOG_PAGE; +ram_addr = section-offset_within_region + addr + bit * VHOST_LOG_PAGE; memory_region_set_dirty(section-mr, ram_addr, VHOST_LOG_PAGE); log = ~(0x1ull bit); } @@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener, struct vhost_dev *dev = container_of(listener, struct vhost_dev, memory_listener); hwaddr start_addr = section-offset_within_address_space; -hwaddr end_addr = start_addr + section-size; +hwaddr end_addr = start_addr + section-size - 1; vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr); } I can still reproduce the issue with this patch. Yes it's still wrong. We need the following on top. Could you try please? diff --git a/hw/vhost.c b/hw/vhost.c index dbf6b46..c324903 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -29,7 +29,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, uint64_t end = MIN(mlast, rlast); vhost_log_chunk_t *from = dev-log + start / VHOST_LOG_CHUNK; vhost_log_chunk_t *to = dev-log + end / VHOST_LOG_CHUNK + 1; -uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK; +uint64_t addr = 0; if (end start) { return; Sorry, scratch that last one, sorry. This should be the right thing, I think: on top of 'vhost: memory sync fixes'. diff --git a/hw/vhost.c b/hw/vhost.c index dbf6b46..72c0095 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -53,9 +53,10 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, log = __sync_fetch_and_and(from, 0); while ((bit = sizeof(log) sizeof(int) ? ffsll(log) : ffs(log))) { -ram_addr_t ram_addr; +hwaddr ram_addr; bit -= 1; -ram_addr = section-offset_within_region + addr + bit * VHOST_LOG_PAGE; +ram_addr = addr + bit * VHOST_LOG_PAGE - +section-mr-offset_within_address_space; should be section-offset_within_address_space
Re: [Qemu-devel] scp during migration with vhost fails
On 02/25/2013 01:57 PM, Jason Wang wrote: On 02/24/2013 05:54 AM, Michael S. Tsirkin wrote: On Sat, Feb 23, 2013 at 10:49:29PM +0200, Michael S. Tsirkin wrote: On Fri, Feb 22, 2013 at 11:33:53PM +0800, Jason Wang wrote: On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote: On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote: On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote: On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote: On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote: Hello all: During testing, I find doing scp during migration with vhost fails with warnings in guest like: Corrupted MAC on input. Disconnecting: Packet corrupt. lost connection Here's the bisect result: Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to MemoryListener API is the last commit that works well. With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to MemoryListener API, guest network is unusable with warning of bad gso type With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix incorrect userspace address, guest network is available, but scp during migration may fail. Looks like the issue is related to memory api, any thoughts? Thanks Tried to reproduce this for a while without success. Which command line was used? -- MST Could be we are not syncing all that we should? Does the following hack make the problem go away? diff --git a/hw/vhost.c b/hw/vhost.c index 8d41fdb..a7a0412 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev, hwaddr end_addr) { int i; +start_addr = 0x0; +end_addr = ~0x0ull; if (!dev-log_enabled || !dev-started) { return 0; Still can reproduce with this. From the bisect result, the vhost dirty bitmap sync itself looks ok but something wrong when converting to memory listener. Reading the code carefully, I found two bugs introduced during this conversion. Patch below, could you please try? vhost: memory sync fixes This fixes two bugs related to memory sync during migration: - ram address calculation was missing the chunk address, so the wrong page was dirtied - one after last was used instead of the end address of a region, which might overflow to 0 and cause us to skip the region when the region ends at ~0x0ull. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- diff --git a/hw/vhost.c b/hw/vhost.c index 8d41fdb..dbf6b46 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, ffsll(log) : ffs(log))) { ram_addr_t ram_addr; bit -= 1; -ram_addr = section-offset_within_region + bit * VHOST_LOG_PAGE; +ram_addr = section-offset_within_region + addr + bit * VHOST_LOG_PAGE; memory_region_set_dirty(section-mr, ram_addr, VHOST_LOG_PAGE); log = ~(0x1ull bit); } @@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener, struct vhost_dev *dev = container_of(listener, struct vhost_dev, memory_listener); hwaddr start_addr = section-offset_within_address_space; -hwaddr end_addr = start_addr + section-size; +hwaddr end_addr = start_addr + section-size - 1; vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr); } I can still reproduce the issue with this patch. Yes it's still wrong. We need the following on top. Could you try please? diff --git a/hw/vhost.c b/hw/vhost.c index dbf6b46..c324903 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -29,7 +29,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, uint64_t end = MIN(mlast, rlast); vhost_log_chunk_t *from = dev-log + start / VHOST_LOG_CHUNK; vhost_log_chunk_t *to = dev-log + end / VHOST_LOG_CHUNK + 1; -uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK; +uint64_t addr = 0; if (end start) { return; Sorry, scratch that last one, sorry. This should be the right thing, I think: on top of 'vhost: memory sync fixes'. diff --git a/hw/vhost.c b/hw/vhost.c index dbf6b46..72c0095 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -53,9 +53,10 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, log = __sync_fetch_and_and(from, 0); while ((bit = sizeof(log) sizeof(int) ? ffsll(log) : ffs(log))) { -ram_addr_t ram_addr; +hwaddr ram_addr; bit -= 1; -ram_addr = section-offset_within_region + addr + bit * VHOST_LOG_PAGE; +ram_addr = addr + bit * VHOST_LOG_PAGE - +section-mr-offset_within_address_space; should be
Re: [Qemu-devel] [PATCH v5 4/6] introduce new vma archive format
+if (full_read(vmar-fd, vmar-head_data, sizeof(VmaHeader)) != +sizeof(VmaHeader)) { +error_setg(errp, can't read vma header - %s, + errno ? strerror(errno) : got EOF); You're not the first user, but strerror() isn't thread-safe. strerror_r is not necessarily portable (glibc vs. POSIX), and strerror_l isn't yet widely implemented. Should qemu be providing a better interface qemu_strerror() that guarantees thread-safety when converting errno to a string? I thought I can use g_strerror?
Re: [Qemu-devel] [PATCH v5 4/6] introduce new vma archive format
+return -1; +} + +VmaHeader *h = (VmaHeader *)vmar-head_data; + +if (h-magic != VMA_MAGIC) { +error_setg(errp, not a vma file - wrong magic number); +return -1; +} Doesn't seem like this is endian-safe. h-magic is host-endian, but VMA_MAGIC is big-endian. h-magic is read from the file, so it should be also big-endian. + +/* File Format Definitions */ + +#define VMA_MAGIC (GUINT32_TO_BE(('V'24)|('M'16)|('A'8)|0x00)) +#define VMA_EXTENT_MAGIC +(GUINT32_TO_BE(('V'24)|('M'16)|('A'8)|'E')) Do we care about EBCDIC, in which case you should be using raw hex values instead of relying on ASCII conversion of your magic number? That is, I'd much rather see: #define VMA_MAGIC 0x564D4100 /* ascii VMA\0 */ The other code in qemu (for example block/qcow2.h) also use this style: #define QCOW_MAGIC (('Q' 24) | ('F' 16) | ('I' 8) | 0xfb) + +typedef struct VmaDeviceInfoHeader { +uint32_t devname_ptr; /* offset into blob_buffer table */ +uint32_t reserved0; +uint64_t size; /* device size in bytes */ +uint64_t reserved1; +uint64_t reserved2; +} VmaDeviceInfoHeader; + +typedef struct VmaHeader { +uint32_t magic; +uint32_t version; +unsigned char uuid[16]; +int64_t ctime; +unsigned char md5sum[16]; + +uint32_t blob_buffer_offset; +uint32_t blob_buffer_size; +uint32_t header_size; + +unsigned char reserved[1984]; + +uint32_t config_names[VMA_MAX_CONFIGS]; /* offset into blob_buffer table */ +uint32_t config_data[VMA_MAX_CONFIGS]; /* offset into + blob_buffer table */ + +VmaDeviceInfoHeader dev_info[256]; } VmaHeader; Is it worth a compile-time assertion that this header is a fixed size, to make sure that future edits evenly reduce the size of reserved when carving out new fields? I tried to do that, but it seems that gcc does not support sizeof in preprocessor conditionals, so I added a runtime check in vma_writer_create(). Do you have an example how I can add a compile-time assertion that this header is a fixed size?
[Qemu-devel] [PATCH v2] piix: define a TOM register to report the base of PCI
v2: * Use piix: in the subject rather than qemu: * Define TOM register as one byte * Define default TOM value instead of hardcode 0xe000 in more that one place * Use API pci_set_byte for pci config access * Use dev-config instead of the indirect d-dev.config Define a TOM(top of memory) register to report the base of PCI memory, update memory region dynamically. TOM register are defined to one byte in PCI configure space, because that only upper 4 bit of PCI memory takes effect for Xen, so it requires bios set TOM with 16M-aligned. Signed-off-by: Xudong Hao xudong@intel.com Signed-off-by: Xiantao Zhang xiantao.zh...@intel.com --- hw/pc.h | 7 +++--- hw/pc_piix.c | 12 +++--- hw/piix_pci.c | 73 +++ 3 files changed, 59 insertions(+), 33 deletions(-) diff --git a/hw/pc.h b/hw/pc.h index fbcf43d..62adcc5 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -129,15 +129,14 @@ extern int no_hpet; struct PCII440FXState; typedef struct PCII440FXState PCII440FXState; +#define I440FX_TOM 0xe000 +#define I440FX_XEN_TOM 0xf000 + PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn, ISABus **isa_bus, qemu_irq *pic, MemoryRegion *address_space_mem, MemoryRegion *address_space_io, ram_addr_t ram_size, -hwaddr pci_hole_start, -hwaddr pci_hole_size, -hwaddr pci_hole64_start, -hwaddr pci_hole64_size, MemoryRegion *pci_memory, MemoryRegion *ram_memory); diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 0a6923d..2eef510 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -93,9 +93,9 @@ static void pc_init1(MemoryRegion *system_memory, kvmclock_create(); } -if (ram_size = 0xe000 ) { -above_4g_mem_size = ram_size - 0xe000; -below_4g_mem_size = 0xe000; +if (ram_size = I440FX_TOM) { +above_4g_mem_size = ram_size - I440FX_TOM; +below_4g_mem_size = I440FX_TOM; } else { above_4g_mem_size = 0; below_4g_mem_size = ram_size; @@ -130,12 +130,6 @@ static void pc_init1(MemoryRegion *system_memory, if (pci_enabled) { pci_bus = i440fx_init(i440fx_state, piix3_devfn, isa_bus, gsi, system_memory, system_io, ram_size, - below_4g_mem_size, - 0x1ULL - below_4g_mem_size, - 0x1ULL + above_4g_mem_size, - (sizeof(hwaddr) == 4 - ? 0 - : ((uint64_t)1 62)), pci_memory, ram_memory); } else { pci_bus = NULL; diff --git a/hw/piix_pci.c b/hw/piix_pci.c index 3d79c73..3e5a25c 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -86,6 +86,14 @@ struct PCII440FXState { #define I440FX_PAM_SIZE 7 #define I440FX_SMRAM0x72 +/* The maximum vaule of TOM(top of memory) register in I440FX + * is 1G, so it doesn't meet any popular virutal machines, so + * define another register to report the base of PCI memory. + * Use one byte 0xb0 for the upper 8 bit, they are originally + * resevered for host bridge. + * */ +#define I440FX_PCI_HOLE_BASE 0xb0 + static void piix3_set_irq(void *opaque, int pirq, int level); static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pci_intx); static void piix3_write_config_xen(PCIDevice *dev, @@ -101,6 +109,43 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx) return (pci_intx + slot_addend) 3; } + +static void i440fx_update_pci_mem_hole(PCII440FXState *f, bool del) +{ +ram_addr_t above_4g_mem_size; +hwaddr pci_hole_start, pci_hole_size, pci_hole64_start, pci_hole64_size; + +pci_hole_start = pci_default_read_config(f-dev, I440FX_PCI_HOLE_BASE, 1) 24; +pci_hole_size = 0x1ULL - pci_hole_start; + +if (ram_size = pci_hole_start) { +above_4g_mem_size = ram_size - pci_hole_start; +} else { +above_4g_mem_size = 0; +} +pci_hole64_start = 0x1ULL + above_4g_mem_size; +pci_hole64_size = sizeof(hwaddr) == 4 ? 0 : ((uint64_t)1 62); + +if (del) { +memory_region_del_subregion(f-system_memory, f-pci_hole); +if (pci_hole64_size) { +memory_region_del_subregion(f-system_memory, f-pci_hole_64bit); +} +} + +memory_region_init_alias(f-pci_hole, pci-hole, f-pci_address_space, + pci_hole_start, pci_hole_size); +memory_region_add_subregion(f-system_memory, pci_hole_start, f-pci_hole); +memory_region_init_alias(f-pci_hole_64bit, pci-hole64, + f-pci_address_space, + pci_hole64_start, pci_hole64_size); +if (pci_hole64_size) { +
Re: [Qemu-devel] [PATCH v4 2/6] add basic backup support to block driver
+for (; start end; start++) { +if (block_job_is_cancelled(job-common)) { +ret = -1; +break; +} + +/* we need to yield so that qemu_aio_flush() returns. + * (without, VM does not reboot) +* Note: use 1000 instead of 0 (0 prioritize this task too + much) indentation What does 0 prioritize this task too much mean? If no rate limit has been set the job should run at full speed. We should not hardcode arbitrary delays like 1000. The VM itself gets somehow slower during backup - do not know why. As workaround sleep 1000 works. Please find out why, it's a bug that an arbitrary sleep hides but doesn't fix (plus the sleep makes backup less efficient). If the VM becomes slow this loop is probably spinning without doing blocking I/O and only doing sleep 0. I guess that can happen when you loop over blocks that have already been backed up (bit has been set)? Well, 'slow' is the wrong term. The VM just gets a bit unresponsive - its just a feeling. I think this is because the backup job runs at same priority as normal guest IO. We previously used LVM and run backup with 'idle' IO priority (CFQ) to avoid such behavior. But qemu does not provide an IO queue where we can set scheduling priorities?
Re: [Qemu-devel] [PATCH 20/38] target-arm: Implement adc_cc inline
Hi All, This patch breaks ARM TCG. Fix comming shortly. On Wed, Feb 20, 2013 at 5:52 PM, Richard Henderson r...@twiddle.net wrote: Use add2 if available, otherwise use 64-bit arithmetic. Cc: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Richard Henderson r...@twiddle.net --- target-arm/helper.h| 1 - target-arm/op_helper.c | 15 --- target-arm/translate.c | 39 ++- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/target-arm/helper.h b/target-arm/helper.h index bca5a5b..507bb9c 100644 --- a/target-arm/helper.h +++ b/target-arm/helper.h @@ -140,7 +140,6 @@ DEF_HELPER_2(recpe_u32, i32, i32, env) DEF_HELPER_2(rsqrte_u32, i32, i32, env) DEF_HELPER_5(neon_tbl, i32, env, i32, i32, i32, i32) -DEF_HELPER_3(adc_cc, i32, env, i32, i32) DEF_HELPER_3(sbc_cc, i32, env, i32, i32) DEF_HELPER_3(shl_cc, i32, env, i32, i32) diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index 99610d7..49fc036 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -315,21 +315,6 @@ uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip) The only way to do that in TCG is a conditional branch, which clobbers all our temporaries. For now implement these as helper functions. */ -uint32_t HELPER(adc_cc)(CPUARMState *env, uint32_t a, uint32_t b) -{ -uint32_t result; -if (!env-CF) { -result = a + b; -env-CF = result a; -} else { -result = a + b + 1; -env-CF = result = a; -} -env-VF = (a ^ b ^ -1) (a ^ result); -env-NF = env-ZF = result; -return result; -} - uint32_t HELPER(sbc_cc)(CPUARMState *env, uint32_t a, uint32_t b) { uint32_t result; diff --git a/target-arm/translate.c b/target-arm/translate.c index ca6f0af..493448a 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -421,6 +421,34 @@ static void gen_add_CC(TCGv dest, TCGv t0, TCGv t1) tcg_gen_mov_i32(dest, cpu_NF); } +/* dest = T0 + T1 + CF. Compute C, N, V and Z flags */ +static void gen_adc_CC(TCGv dest, TCGv t0, TCGv t1) +{ +TCGv tmp = tcg_temp_new_i32(); +if (TCG_TARGET_HAS_add2_i32) { +tcg_gen_movi_i32(tmp, 0); +tcg_gen_add2_i32(cpu_NF, cpu_CF, t0, tmp, cpu_CF, tmp); You discard the intermediary add result stored in NF here and add A to B. You have effectively discarded incoming carry here. Regards, Peter +tcg_gen_add2_i32(cpu_NF, cpu_CF, t0, cpu_CF, t1, tmp); +} else { +TCGv_i64 q0 = tcg_temp_new_i64(); +TCGv_i64 q1 = tcg_temp_new_i64(); +tcg_gen_extu_i32_i64(q0, t0); +tcg_gen_extu_i32_i64(q1, t1); +tcg_gen_add_i64(q0, q0, q1); +tcg_gen_extu_i32_i64(q1, cpu_CF); +tcg_gen_add_i64(q0, q0, q1); +tcg_gen_extr_i64_i32(cpu_NF, cpu_CF, q0); +tcg_temp_free_i64(q0); +tcg_temp_free_i64(q1); +} +tcg_gen_mov_i32(cpu_ZF, cpu_NF); +tcg_gen_xor_i32(cpu_VF, cpu_NF, t0); +tcg_gen_xor_i32(tmp, t0, t1); +tcg_gen_andc_i32(cpu_VF, cpu_VF, tmp); +tcg_temp_free_i32(tmp); +tcg_gen_mov_i32(dest, cpu_NF); +} + /* dest = T0 - T1. Compute C, N, V and Z flags */ static void gen_sub_CC(TCGv dest, TCGv t0, TCGv t1) { @@ -7073,7 +7101,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) break; case 0x05: if (set_cc) { -gen_helper_adc_cc(tmp, cpu_env, tmp, tmp2); +gen_adc_CC(tmp, tmp, tmp2); } else { gen_add_carry(tmp, tmp, tmp2); } @@ -7914,7 +7942,7 @@ gen_thumb2_data_op(DisasContext *s, int op, int conds, uint32_t shifter_out, TCG break; case 10: /* adc */ if (conds) -gen_helper_adc_cc(t0, cpu_env, t0, t1); +gen_adc_CC(t0, t0, t1); else gen_adc(t0, t1); break; @@ -9232,10 +9260,11 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s) } break; case 0x5: /* adc */ -if (s-condexec_mask) +if (s-condexec_mask) { gen_adc(tmp, tmp2); -else -gen_helper_adc_cc(tmp, cpu_env, tmp, tmp2); +} else { +gen_adc_CC(tmp, tmp, tmp2); +} break; case 0x6: /* sbc */ if (s-condexec_mask) -- 1.8.1.2
Re: [Qemu-devel] [PATCH v4 0/6] Efficient VM backup for qemu
-Original Message- From: Paolo Bonzini [mailto:pbonz...@redhat.com] Sent: Freitag, 22. Februar 2013 19:07 To: Dietmar Maurer Cc: Stefan Hajnoczi; Kevin Wolf; qemu-devel@nongnu.org; Wenchao Xia Subject: Re: [PATCH v4 0/6] Efficient VM backup for qemu Il 22/02/2013 18:57, Dietmar Maurer ha scritto: If we use nbd, how can we pass additional information to the other side, for example information about unallocated regions? You can either send trim commands, or just skip those regions and let the other side figure it out. But we are lost if we want to transfer something else? I am currently not sure what we want to transfer, but I am sure we will find something in future. Or can we simply add a header to each write with additional information? Let's discuss it once we have a clearer picture of what that additional info would be. Paolo
Re: [Qemu-devel] [PATCH 21/38] target-arm: Implement sbc_cc inline
Hi All, Same problem as commented on patch 20. On Wed, Feb 20, 2013 at 5:52 PM, Richard Henderson r...@twiddle.net wrote: Use sub2 if available, otherwise use 64-bit arithmetic. Cc: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Richard Henderson r...@twiddle.net --- target-arm/helper.h| 2 -- target-arm/op_helper.c | 15 --- target-arm/translate.c | 47 +++ 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/target-arm/helper.h b/target-arm/helper.h index 507bb9c..63ae13a 100644 --- a/target-arm/helper.h +++ b/target-arm/helper.h @@ -140,8 +140,6 @@ DEF_HELPER_2(recpe_u32, i32, i32, env) DEF_HELPER_2(rsqrte_u32, i32, i32, env) DEF_HELPER_5(neon_tbl, i32, env, i32, i32, i32, i32) -DEF_HELPER_3(sbc_cc, i32, env, i32, i32) - DEF_HELPER_3(shl_cc, i32, env, i32, i32) DEF_HELPER_3(shr_cc, i32, env, i32, i32) DEF_HELPER_3(sar_cc, i32, env, i32, i32) diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index 49fc036..a522313 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -315,21 +315,6 @@ uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip) The only way to do that in TCG is a conditional branch, which clobbers all our temporaries. For now implement these as helper functions. */ -uint32_t HELPER(sbc_cc)(CPUARMState *env, uint32_t a, uint32_t b) -{ -uint32_t result; -if (!env-CF) { -result = a - b - 1; -env-CF = a b; -} else { -result = a - b; -env-CF = a = b; -} -env-VF = (a ^ b) (a ^ result); -env-NF = env-ZF = result; -return result; -} - /* Similarly for variable shift instructions. */ uint32_t HELPER(shl_cc)(CPUARMState *env, uint32_t x, uint32_t i) diff --git a/target-arm/translate.c b/target-arm/translate.c index 493448a..9993aea 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -464,6 +464,35 @@ static void gen_sub_CC(TCGv dest, TCGv t0, TCGv t1) tcg_gen_mov_i32(dest, cpu_NF); } +/* dest = T0 + ~T1 + CF = T0 - T1 + CF - 1. Compute C, N, V and Z flags */ +static void gen_sbc_CC(TCGv dest, TCGv t0, TCGv t1) +{ +TCGv tmp = tcg_temp_new_i32(); +tcg_gen_subi_i32(cpu_CF, cpu_CF, 1); +if (TCG_TARGET_HAS_add2_i32) { +tcg_gen_movi_i32(tmp, 0); +tcg_gen_add2_i32(cpu_NF, cpu_CF, t0, tmp, cpu_CF, tmp); Discards NF intermediary result and re-uses operand t0. Regard, Peter +tcg_gen_sub2_i32(cpu_NF, cpu_CF, t0, cpu_CF, t1, tmp); +} else { +TCGv_i64 q0 = tcg_temp_new_i64(); +TCGv_i64 q1 = tcg_temp_new_i64(); +tcg_gen_extu_i32_i64(q0, t0); +tcg_gen_extu_i32_i64(q1, t1); +tcg_gen_sub_i64(q0, q0, q1); +tcg_gen_extu_i32_i64(q1, cpu_CF); +tcg_gen_add_i64(q0, q0, q1); +tcg_gen_extr_i64_i32(cpu_NF, cpu_CF, q0); +tcg_temp_free_i64(q0); +tcg_temp_free_i64(q1); +} +tcg_gen_mov_i32(cpu_ZF, cpu_NF); +tcg_gen_xor_i32(cpu_VF, cpu_NF, t0); +tcg_gen_xor_i32(tmp, t0, t1); +tcg_gen_and_i32(cpu_VF, cpu_VF, tmp); +tcg_temp_free_i32(tmp); +tcg_gen_mov_i32(dest, cpu_NF); +} + #define GEN_SHIFT(name) \ static void gen_##name(TCGv dest, TCGv t0, TCGv t1) \ { \ @@ -7109,7 +7138,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) break; case 0x06: if (set_cc) { -gen_helper_sbc_cc(tmp, cpu_env, tmp, tmp2); +gen_sbc_CC(tmp, tmp, tmp2); } else { gen_sub_carry(tmp, tmp, tmp2); } @@ -7117,7 +7146,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) break; case 0x07: if (set_cc) { -gen_helper_sbc_cc(tmp, cpu_env, tmp2, tmp); +gen_sbc_CC(tmp, tmp2, tmp); } else { gen_sub_carry(tmp, tmp2, tmp); } @@ -7947,10 +7976,11 @@ gen_thumb2_data_op(DisasContext *s, int op, int conds, uint32_t shifter_out, TCG gen_adc(t0, t1); break; case 11: /* sbc */ -if (conds) -gen_helper_sbc_cc(t0, cpu_env, t0, t1); -else +if (conds) { +gen_sbc_CC(t0, t0, t1); +} else { gen_sub_carry(t0, t0, t1); +} break; case 13: /* sub */ if (conds) @@ -9267,10 +9297,11 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s) } break; case 0x6: /* sbc */ -if (s-condexec_mask) +if (s-condexec_mask) { gen_sub_carry(tmp, tmp, tmp2); -else -gen_helper_sbc_cc(tmp, cpu_env,
Re: [Qemu-devel] [PATCH v2 2/2] Add AT24Cxx I2C EEPROM device model
+#include hw.h +#include i2c.h Please use hw/hw.h and hw/i2c.h since Paolo is planning to move I2C devices into hw/i2c/. No, I2C _masters_ move into hw/i2c. This would probably move into hw/nvram. Paolo +#include sysemu/blockdev.h +#include hw/block-common.h + +#define AT24_BASE_ADDRESS 0x50 +#define AT24_MAX_PAGE_LEN 256 + +typedef enum AT24TransferState { +AT24_IDLE, +AT24_RD_ADDR, +AT24_WR_ADDR_HI, +AT24_WR_ADDR_LO, +AT24_RW_DATA0, +AT24_RD_DATA, +AT24_WR_DATA, +} AT24TransferState; + +typedef struct AT24State { +I2CSlave i2c; parent_obj and separate from remaining fields please. All uses of this field except in VMState will be wrong. http://wiki.qemu.org/QOMConventions +BlockConf block_conf; +BlockDriverState *bs; +uint32_t size; +bool wp; +unsigned int addr_mask; +unsigned int page_mask; +bool addr16; +unsigned int hi_addr_mask; +uint8_t device; +AT24TransferState transfer_state; +uint8_t sector_buffer[BDRV_SECTOR_SIZE]; +int cached_sector; +bool cache_dirty; +uint32_t transfer_addr; +} AT24State; + +static void at24_flush_transfer_buffer(AT24State *s) +{ +if (s-cached_sector 0 || !s-cache_dirty) { +return; +} +bdrv_write(s-bs, s-cached_sector, s-sector_buffer, 1); +s-cache_dirty = false; +} + +static void at24_event(I2CSlave *i2c, enum i2c_event event, uint8_t param) +{ +AT24State *s = DO_UPCAST(AT24State, i2c, i2c); Please don't use DO_UPCAST(), add a QOM cast macro AT24() instead. + +switch (event) { +case I2C_START_SEND: +switch (s-transfer_state) { +case AT24_IDLE: +if (s-addr16) { +s-transfer_addr = (param s-hi_addr_mask) 16; +s-transfer_state = AT24_WR_ADDR_HI; +} else { +s-transfer_addr = (param s-hi_addr_mask) 8; +s-transfer_state = AT24_WR_ADDR_LO; +} +break; +default: +s-transfer_state = AT24_IDLE; +break; +} +break; +case I2C_START_RECV: +switch (s-transfer_state) { +case AT24_IDLE: +s-transfer_state = AT24_RD_ADDR; +break; +case AT24_RW_DATA0: +s-transfer_state = AT24_RD_DATA; +break; +default: +s-transfer_state = AT24_IDLE; +break; +} +break; +case I2C_FINISH: +switch (s-transfer_state) { +case AT24_WR_DATA: +at24_flush_transfer_buffer(s); +/* fall through */ +default: +s-transfer_state = AT24_IDLE; +break; +} +break; +default: +s-transfer_state = AT24_IDLE; +break; +} +} + +static int at24_cache_sector(AT24State *s, int sector) +{ +int ret; + +if (s-cached_sector == sector) { +return 0; +} +ret = bdrv_read(s-bs, sector, s-sector_buffer, 1); +if (ret 0) { +s-cached_sector = -1; +return ret; +} +s-cached_sector = sector; +s-cache_dirty = false; +return 0; +} + +static int at24_tx(I2CSlave *i2c, uint8_t data) +{ +AT24State *s = DO_UPCAST(AT24State, i2c, i2c); + +switch (s-transfer_state) { +case AT24_WR_ADDR_HI: +s-transfer_addr |= (data 8) s-addr_mask; +s-transfer_state = AT24_WR_ADDR_LO; +break; +case AT24_WR_ADDR_LO: +s-transfer_addr |= data s-addr_mask; +s-transfer_state = AT24_RW_DATA0; +break; +case AT24_RW_DATA0: +s-transfer_state = AT24_WR_DATA; +if (at24_cache_sector(s, s-transfer_addr BDRV_SECTOR_BITS) 0) { +s-transfer_state = AT24_IDLE; +return -1; +} +/* fall through */ +case AT24_WR_DATA: +if (!s-wp) { +s-sector_buffer[s-transfer_addr ~BDRV_SECTOR_MASK] = data; +s-cache_dirty = true; +} +s-transfer_addr = (s-transfer_addr s-page_mask) | +((s-transfer_addr + 1) ~s-page_mask); +break; +default: +s-transfer_state = AT24_IDLE; +return -1; +} +return 0; +} + +static int at24_rx(I2CSlave *i2c) +{ +AT24State *s = DO_UPCAST(AT24State, i2c, i2c); +unsigned int sector, offset; +int result; + +switch (s-transfer_state) { +case AT24_RD_ADDR: +s-transfer_state = AT24_IDLE; +result = s-transfer_addr; +break; +case AT24_RD_DATA: +sector = s-transfer_addr BDRV_SECTOR_BITS; +offset = s-transfer_addr ~BDRV_SECTOR_MASK; +s-transfer_addr =
Re: [Qemu-devel] [PATCH v4 0/6] Efficient VM backup for qemu
Sure. nbd+unix:///exportname?socket=path is the new URI syntax, I honestly forgot the old one. SCM_CREDENTIALS checks (qemu-nbd --pid or something like that) is not supported, but patches would be very welcome. Yes, this is better than my tcp suggestion. So if I want to use the code from nbd.c, I need to write a specialized BlockDriver for the vma format (to pass that to nbd_export_new())? Yes. But I believe that would be a good thing to do anyway. For one thing, it gives you automatic coverage via qemu-iotests. Paolo
Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
Another thing is, that qdev_free looks now different, some days ago it also did an unref. As far as I can see the object_unparent in virtio-ccw was always the wrong thing to do. object_unparent is almost idempotent, i.e. idempotent as long as it does not cause the last reference to go away. So, doing an object_unparent before qdev_free was not wrong when qdev_free did an object_unref. I think qdev_free is better, unless we want to change all of them at the same time. Paolo So for a potential backport this version of the patch might be better. Paolo, do you have any guidance? Christian