[Qemu-devel] [PATCH] arm/translate.c: Fix adc_CC/sbc_CC implementation
commits 49b4c31efcce45ab714f286f14fa5d5173f9069d and
2de68a4900ef6eb67380b0c128abfe1976bc66e8 reworked the implementation of adc_CC
and sub_CC. The new implementations (on the TCG_TARGET_HAS_add2_i32 code path)
are incorrect. The new logic is:
CF:NF = 0:A +/- 0:CF
CF:NF = CF:A +/- 0:B
The lower 32 bits of the intermediate result stored in NF needs to be passes
into the second addition in place of A (s/CF:A/CF:NF):
CF:NF = 0:A +/- 0:CF
CF:NF = CF:NF +/- 0:B
This patch fixes the issue.
Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
target-arm/translate.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 9993aea..6d91b70 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -428,7 +428,7 @@ static void gen_adc_CC(TCGv dest, TCGv t0, TCGv t1)
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);
-tcg_gen_add2_i32(cpu_NF, cpu_CF, t0, cpu_CF, t1, tmp);
+tcg_gen_add2_i32(cpu_NF, cpu_CF, cpu_NF, cpu_CF, t1, tmp);
} else {
TCGv_i64 q0 = tcg_temp_new_i64();
TCGv_i64 q1 = tcg_temp_new_i64();
@@ -472,7 +472,7 @@ static void gen_sbc_CC(TCGv dest, TCGv t0, TCGv t1)
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);
-tcg_gen_sub2_i32(cpu_NF, cpu_CF, t0, cpu_CF, t1, tmp);
+tcg_gen_sub2_i32(cpu_NF, cpu_CF, cpu_NF, cpu_CF, t1, tmp);
} else {
TCGv_i64 q0 = tcg_temp_new_i64();
TCGv_i64 q1 = tcg_temp_new_i64();
--
1.7.0.4
Re: [Qemu-devel] [PATCH v2 8/8] qtest: Add boot order test
Andreas Färber afaer...@suse.de writes:
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.
Brain fart, dropping.
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. ;)
Point taken :)
I assume q35 is reusing pc code? Or should it be tested as well?
(Having any of our qtests cover it would be nice.)
They share the RTC CMOS setup.
+qtest_start(extra_args);
+test_cmos(boot1, boot2);
+qtest_qmp(global_qtest, { 'execute': 'system_reset' });
qmp()?
Yes.
+test_cmos(reboot1, reboot2);
+qtest_quit(global_qtest);
+g_free(extra_args);
+}
[snip]
Thanks!
Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
On 25/02/13 08:55, Paolo Bonzini wrote: 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. Hmm, the old sequence was object_unparent(OBJECT(dev)); qdev_free(dev) ---+ | V ... object_unparent(OBJECT(dev)); now the last reference is gone, object is freed object_unref(OBJECT(dev)); now the reference of a deleted object becomes -1 ... Isnt that a problem in itself that we modify a reference counter in an deleted object? I think qdev_free is better, unless we want to change all of them at the same time. Ok, so we leave the patch as is. Christian
[Qemu-devel] [PATCH] xilinx_axienet: pump events as appropriate
When the conditions blocking receiving are cleared, check for buffered rx
packets.
Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
hw/xilinx_axienet.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index e5d9251..9a82b1b 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -515,6 +515,8 @@ static void enet_write(void *opaque, hwaddr addr,
s-rcw[addr 1] = value;
if ((addr 1) value RCW1_RST) {
axienet_rx_reset(s);
+} else {
+qemu_flush_queued_packets(qemu_get_queue(s-nic));
}
break;
--
1.7.0.4
Re: [Qemu-devel] [PATCHv2] tap: set IFF_ONE_QUEUE per default
On 18.02.2013 13:39, Peter Lieven wrote: On 15.02.2013 14:32, Stefan Hajnoczi wrote: On Fri, Feb 15, 2013 at 10:32:31AM +0100, Peter Lieven wrote: historically the kernel queues packets two times. once at the device and second in qdisc. this is believed to cause interface stalls if one of these queues overruns. setting IFF_ONE_QUEUE is the default in kernels = 3.8. the flag is ignored since then. see kernel commit 5d097109257c03a71845729f8db6b5770c4bbedc v2: - do only set the flag on linux as it breaks macvtap - define IFF_ONE_QUEUE in tap-linux.h Signed-off-by: Peter Lieven p...@kamp.de --- net/tap-linux.c |4 net/tap-linux.h |1 + 2 files changed, 5 insertions(+) diff --git a/net/tap-linux.c b/net/tap-linux.c index a953189..d49f2fd 100644 --- a/net/tap-linux.c +++ b/net/tap-linux.c @@ -51,6 +51,10 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, memset(ifr, 0, sizeof(ifr)); ifr.ifr_flags = IFF_TAP | IFF_NO_PI; +#ifdef __linux__ +ifr.ifr_flags |= IFF_ONE_QUEUE; +#endif tap-linux.c --- notice the filename Perhaps the solution is to try with IFF_ONE_QUEUE. If the result is -EINVAL, try without. Is IFF_ONE_QUEUE queryable with ioctl(fd, TUNGETFEATURES, features) ? Peter Anyone? Thanks, Peter
Re: [Qemu-devel] [PATCHv2] tap: set IFF_ONE_QUEUE per default
On 25/02/13 09:13, Peter Lieven wrote: Is IFF_ONE_QUEUE queryable with ioctl(fd, TUNGETFEATURES, features) ? Yes, that should work. The default tun/tap driver returns it, macvtap not.
Re: [Qemu-devel] [PATCH] boot-order-test: Add tests for PowerMacs
This is on top of my [PATCH v2 0/8] -boot and -no-fd-bootchk fixes.
Since I need to respin my series anyway, I can include your patch in the
respin.
Andreas Färber afaer...@suse.de writes:
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.
*/
Comment is no longer correct. Feel free to drop it entirely.
+#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
Looks like fw_cfg.h should be split up. Out of this patch's scope.
+
+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);
+}
+}
More general than necessary (readw() would do), but that's okay.
+
+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');
Doesn't test -boot once. Tolerable, because test_pc_boot_order() tests
it, and the only machine-specific part is the boot set handler.
Hmm, test_powermac_with_args() is prepared for -boot once testing.
Throw in -boot once=d,order=c?
+}
+}
+
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();
}
If you don't want to respin, I can take this patch as is, and fix up the
bogus comment in my tree.
Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
Am 25.02.2013 08:55, schrieb Paolo Bonzini: 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. I did have a patch doing that but revoked it because I thought you said you wanted to do it differently as part of your series... If it stays it needs to be renamed device_* and probably not *_free either since that depends on reference count. 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 v2 0/4] kvm: Make ioeventfd usable on s390.
On Sun, 24 Feb 2013 11:56:39 +0200 Michael S. Tsirkin m...@redhat.com wrote: 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. Cool, thanks for reviewing. 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. Subcode 3 will be in 3.9. 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 ? I think I'll fall back to a new bus type should we ever need a new notification type - the less strange hacks, the better. 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 4/4] KVM: s390: Wire up ioeventfd.
On Sun, 24 Feb 2013 11:45:22 +0200
Michael S. Tsirkin m...@redhat.com wrote:
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?
kvm_virtio.h is for the old-style s390-virtio mechanism. Subcode 3 will
be included in 3.9.
Do some guests use it?
Also, do you want to support KVM_S390_VIRTIO_NOTIFY as well?
We don't want to invest any more effort in the old style s390-virtio
transport (other than keeping it working). Any future work will be for
virtio-ccw.
+
+ 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] [PATCH v4 0/6] Efficient VM backup for qemu
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. But the only thing I can do with vma is sequential writes with 64K block size. So using qemu-iotests will not work anyways. I thought I can simply impl. a callback which gets called instead of using a BlockDriver?
Re: [Qemu-devel] emulate powerpc(mpc8544ds) on x86 architecture
thx, I get it :) 于 2013/02/21 9:12, Scott Wood 写道: On 02/20/2013 07:04:39 PM, Shi Rong wrote: 于 2013/02/21 2:30, Scott Wood 写道: On 02/20/2013 08:46:52 AM, Alexander Graf wrote: Also no need for enable-kvm, as you're running on x86. KVM only helps for compatible CPUs (ppc on ppc for example). you mean that KVM only support the same architecture, and when emulate ppc on x86 platform, qemu uses his tcg to realize it. Yes. KVM involves running code natively. so how could I raise the guest's efficiency if ignore KVM? thx~~~ Improve the tcg code, and/or improve the guest code. :-) -Scott -- -- Best regards! -- Shi Rong Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST) No. 6 Wenzhu Road, Nanjing, 210012, China TEL：+86+25-86630566-8222 MOBILE: 15996209501 EMail：shir.f...@cn.fujitsu.com -- This communication is for use by the intended recipient(s) only and may contain information that is privileged, confidential and exempt from disclosure under applicable law. If you are not an intended recipient of this communication, you are hereby notified that any dissemination, distribution or copying hereof is strictly prohibited. If you have received this communication in error, please notify me by reply e-mail, permanently delete this communication from your system, and destroy any hard copies you may have printed.
Re: [Qemu-devel] Block I/O optimizations
Stefan Hajnoczi stefa...@gmail.com wrote on 21/02/2013 10:11:12 AM: From: Stefan Hajnoczi stefa...@gmail.com To: Loic Dachary l...@dachary.org, Cc: qemu-devel qemu-devel@nongnu.org Date: 21/02/2013 10:11 AM Subject: Re: [Qemu-devel] Block I/O optimizations Sent by: qemu-devel-bounces+abelg=il.ibm@nongnu.org On Mon, Feb 18, 2013 at 7:19 PM, Loic Dachary l...@dachary.org wrote: I recently tried to figure out the best and easiest ways to increase block I/O performances with qemu. Not being a qemu expert, I expected to find a few optimization tricks. Much to my surprise, it appears that there are many significant improvements being worked on. This is excellent news :-) However, I'm not sure I understand how they all fit together. It's probably quite obvious from the developer point of view but I would very much appreciate an overview of how dataplane, vhost-blk, ELVIS etc. should be used or developed to maximize I/O performances. Are there documents I should read ? If not, would someone be willing to share bits of wisdom ? Hi Loic, There will be more information on dataplane shortly. I'll write up a blog post and share the link with you. Hi Stefan, I assume dataplane could provide a significant performance boost and approximate vhost-blk performance. If I understand properly, that's because dataplane finally removes the dependency on the global mutex and uses eventfd to process notifications. However, I am concerned dataplane may not solve the scalability problem because QEMU will be still running 1 thread per VCPU and 1 per virtual device to handle I/O for each VM. Assuming we run N VMs with 1 VCPU and 1 virtual I/O device, we will have 2N threads competing for CPU cycles. In a cloud-like environment running I/O intensive VMs that could be a problem because the I/O threads and VCPU threads may starve each other. Further more, the linux kernel can't make good scheduling decisions (from I/O perspective) because it has no information about the content of the I/O queues. We did some experiments with a modified vhost-blk back-end that uses a single (or few) thread/s to process I/O for many VMs as opposed to 1 thread per VM (I/O device). These thread/s decide for how-long and when to process the request of each VM based on the I/O activity of each queue. We noticed that this model (part of what we call ELVIS) significantly improves the scalability of the system when you run many I/O intensive guests. I was wondering if you have considered this type of threading model for dataplane as well. With vhost-blk (or-net) it's relatively easy to use a kernel thread to process I/O for many VMs (user-space processes). However, with a QEMU back-end (like dataplane/virtio-blk) the shared thread model may be challenging because it requires a shared user-space process (for the I/O thread/s) to handle I/O for many QEMU processes. Any thoughts/opinions on the share-thread direction ?
Re: [Qemu-devel] [PATCH 13/42] migration: add migrate_set_state tracepoint
On 02/22/2013 06:36 PM, Paolo Bonzini wrote:
From: Kazuya Saito saito.kaz...@jp.fujitsu.com
Signed-off-by: Kazuya Saito saito.kaz...@jp.fujitsu.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
migration.c | 9 -
trace-events | 3 +++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/migration.c b/migration.c
index 9a234c7..b091532 100644
--- a/migration.c
+++ b/migration.c
@@ -23,6 +23,7 @@
#include migration/block.h
#include qemu/thread.h
#include qmp-commands.h
+#include trace.h
//#define DEBUG_MIGRATION
@@ -282,6 +283,7 @@ void migrate_fd_error(MigrationState *s)
{
DPRINTF(setting error state\n);
s-state = MIG_STATE_ERROR;
+trace_migrate_set_state(MIG_STATE_ERROR);
migrate_fd_cleanup(s);
}
@@ -289,6 +291,7 @@ static void migrate_fd_completed(MigrationState *s)
{
DPRINTF(setting completed state\n);
s-state = MIG_STATE_COMPLETED;
+trace_migrate_set_state(MIG_STATE_COMPLETED);
migrate_fd_cleanup(s);
}
@@ -319,6 +322,7 @@ static void migrate_fd_cancel(MigrationState *s)
DPRINTF(cancelling migration\n);
s-state = MIG_STATE_CANCELLED;
+trace_migrate_set_state(MIG_STATE_CANCELLED);
migrate_fd_cleanup(s);
}
@@ -377,8 +381,9 @@ static MigrationState *migrate_init(const MigrationParams
*params)
s-bandwidth_limit = bandwidth_limit;
s-state = MIG_STATE_SETUP;
-s-total_time = qemu_get_clock_ms(rt_clock);
+trace_migrate_set_state(MIG_STATE_SETUP);
+s-total_time = qemu_get_clock_ms(rt_clock);
return s;
}
@@ -738,6 +743,8 @@ static const QEMUFileOps buffered_file_ops = {
void migrate_fd_connect(MigrationState *s)
{
s-state = MIG_STATE_ACTIVE;
+trace_migrate_set_state(MIG_STATE_ACTIVE);
+
s-bytes_xfer = 0;
s-buffer = NULL;
s-buffer_size = 0;
diff --git a/trace-events b/trace-events
index a27ae43..2999c0e 100644
--- a/trace-events
+++ b/trace-events
@@ -1090,3 +1090,6 @@ css_io_interrupt(int cssid, int ssid, int schid,
uint32_t intparm, uint8_t isc,
# hw/s390x/virtio-ccw.c
virtio_ccw_interpret_ccw(int cssid, int ssid, int schid, int cmd_code)
VIRTIO-CCW: %x.%x.%04x: interpret command %x
virtio_ccw_new_device(int cssid, int ssid, int schid, int devno, const char
*devno_mode) VIRTIO-CCW: add subchannel %x.%x.%04x, devno %04x (%s)
+
+# migration.c
+migrate_set_state(int new_state) new state %d
Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [RFC PATCH v2 3/4] KVM: ioeventfd for s390 css devices.
On Sun, 24 Feb 2013 11:47:50 +0200
Michael S. Tsirkin m...@redhat.com wrote:
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?
How about KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY?
Also, maybe we want to add KVM_IOEVENTFD_S390_VIRTIO_NOTIFY as well?
No, since s390-virtio is a dead end.
#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 2/4] KVM: Introduce KVM_CSS_BUS.
On Sun, 24 Feb 2013 11:57:25 +0200
Michael S. Tsirkin m...@redhat.com wrote:
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?
Hm, I don't really see a need to point out that this is s390-specific,
so I think I'd prefer KVM_VIRTIO_CCW_NOTIFY_BUS.
KVM_NR_BUSES
};
--
1.7.12.4
[Qemu-devel] [PATCH v2 3/8] xilinx_axienet: Register reset properly
Register the reset function and the Device::reset function rather than
explicitly call it from the sysbus::init.
Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
hw/xilinx_axienet.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index 7b50682..3932846 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -401,8 +401,10 @@ static inline int axienet_newfunc_enabled(XilinxAXIEnet *s)
return !!(s-regs[R_RAF] RAF_NEWFUNC_EN);
}
-static void axienet_reset(XilinxAXIEnet *s)
+static void xilinx_axienet_reset(DeviceState *d)
{
+XilinxAXIEnet *s = XILINX_AXI_ENET(d);
+
axienet_rx_reset(s);
axienet_tx_reset(s);
@@ -868,7 +870,6 @@ static int xilinx_enet_init(SysBusDevice *dev)
s-TEMAC.parent = s;
s-rxmem = g_malloc(s-c_rxmem);
-axienet_reset(s);
return 0;
}
@@ -899,6 +900,7 @@ static void xilinx_enet_class_init(ObjectClass *klass, void
*data)
k-init = xilinx_enet_init;
dc-props = xilinx_enet_properties;
+dc-reset = xilinx_axienet_reset;
ssc-push = axienet_stream_push;
}
--
1.7.0.4
[Qemu-devel] [PATCH v2 4/8] xilinx_axienet: converted init-realize
The prescribed transition from SysBusDevice::init to Device::realize. Im going
with Andreas suggestion to move the sysbus foo to Object::init for early IRQ
visibility.
Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
hw/xilinx_axienet.c | 24 +++-
1 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index 3932846..27d94bd 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -850,18 +850,13 @@ static NetClientInfo net_xilinx_enet_info = {
.cleanup = eth_cleanup,
};
-static int xilinx_enet_init(SysBusDevice *dev)
+static void xilinx_enet_realize(DeviceState *dev, Error **errp)
{
XilinxAXIEnet *s = XILINX_AXI_ENET(dev);
-sysbus_init_irq(dev, s-irq);
-
-memory_region_init_io(s-iomem, enet_ops, s, enet, 0x4);
-sysbus_init_mmio(dev, s-iomem);
-
qemu_macaddr_default_if_unset(s-conf.macaddr);
s-nic = qemu_new_nic(net_xilinx_enet_info, s-conf,
- object_get_typename(OBJECT(dev)), dev-qdev.id, s);
+ object_get_typename(OBJECT(dev)), dev-id, s);
qemu_format_nic_info_str(qemu_get_queue(s-nic), s-conf.macaddr.a);
tdk_init(s-TEMAC.phy);
@@ -870,18 +865,22 @@ static int xilinx_enet_init(SysBusDevice *dev)
s-TEMAC.parent = s;
s-rxmem = g_malloc(s-c_rxmem);
-
-return 0;
}
-static void xilinx_enet_initfn(Object *obj)
+static void xilinx_enet_init(Object *obj)
{
XilinxAXIEnet *s = XILINX_AXI_ENET(obj);
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
Error *errp = NULL;
object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE,
(Object **) s-tx_dev, errp);
assert_no_error(errp);
+
+sysbus_init_irq(sbd, s-irq);
+
+memory_region_init_io(s-iomem, enet_ops, s, enet, 0x4);
+sysbus_init_mmio(sbd, s-iomem);
}
static Property xilinx_enet_properties[] = {
@@ -895,10 +894,9 @@ static Property xilinx_enet_properties[] = {
static void xilinx_enet_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
StreamSlaveClass *ssc = STREAM_SLAVE_CLASS(klass);
-k-init = xilinx_enet_init;
+dc-realize = xilinx_enet_realize;
dc-props = xilinx_enet_properties;
dc-reset = xilinx_axienet_reset;
ssc-push = axienet_stream_push;
@@ -909,7 +907,7 @@ static const TypeInfo xilinx_enet_info = {
.parent= TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(XilinxAXIEnet),
.class_init= xilinx_enet_class_init,
-.instance_init = xilinx_enet_initfn,
+.instance_init = xilinx_enet_init,
.interfaces = (InterfaceInfo[]) {
{ TYPE_STREAM_SLAVE },
{ }
--
1.7.0.4
[Qemu-devel] [PATCH v2 5/8] petalogix_ml605_mmu: Fix machine node attachment
Just attach devices straight to the root machine node, rather than the unattached node Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- Suggested (indirectly) by Andreas if he wants to put his Suggested-by to it. hw/petalogix_ml605_mmu.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c index 82d7183..98e647d 100644 --- a/hw/petalogix_ml605_mmu.c +++ b/hw/petalogix_ml605_mmu.c @@ -134,8 +134,8 @@ petalogix_ml605_init(QEMUMachineInitArgs *args) dma = qdev_create(NULL, xlnx.axi-dma); /* FIXME: attach to the sysbus instead */ -object_property_add_child(container_get(qdev_get_machine(), /unattached), - xilinx-dma, OBJECT(dma), NULL); +object_property_add_child(qdev_get_machine(), xilinx-dma, OBJECT(dma), + NULL); xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(dma), 0x8278, irq[3], 0x1000, 0x1000); -- 1.7.0.4
[Qemu-devel] [PATCH v2 6/8] petalogix_ml605_mmu: Attach ethernet to machine
Explicitily make the ethernet a child of the machine. This is needed to set and use links pre-realize. Also makes the ethernet initialization consistent with its peer DMA. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/petalogix_ml605_mmu.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c index 98e647d..e3528c0 100644 --- a/hw/petalogix_ml605_mmu.c +++ b/hw/petalogix_ml605_mmu.c @@ -134,6 +134,8 @@ petalogix_ml605_init(QEMUMachineInitArgs *args) dma = qdev_create(NULL, xlnx.axi-dma); /* FIXME: attach to the sysbus instead */ +object_property_add_child(qdev_get_machine(), xilinx-eth, OBJECT(eth0), + NULL); object_property_add_child(qdev_get_machine(), xilinx-dma, OBJECT(dma), NULL); -- 1.7.0.4
[Qemu-devel] [PATCH v2 8/8] xilinx_axienet: stub out second stream connection
Example patch adding a second proxy object for the second stream connection of
axienet.
This is a non-functional RFC, please see the cover letter for discussion.
Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
changed from v1:
Rebased on series refactorings
hw/xilinx_axienet.c | 35 +++
1 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index a4ec1d8..e5a99e8 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -33,6 +33,7 @@
#define TYPE_XILINX_AXI_ENET xlnx.axi-ethernet
#define TYPE_XILINX_AXI_ENET_DATA_STREAM xilinx-axienet-data-stream
+#define TYPE_XILINX_AXI_ENET_CONTROL_STREAM xilinx-axienet-control-stream
#define XILINX_AXI_ENET(obj) \
OBJECT_CHECK(XilinxAXIEnet, (obj), TYPE_XILINX_AXI_ENET)
@@ -41,6 +42,10 @@
OBJECT_CHECK(XilinxAXIEnetStreamSlave, (obj),\
TYPE_XILINX_AXI_ENET_DATA_STREAM)
+#define XILINX_AXI_ENET_CONTROL_STREAM(obj) \
+ OBJECT_CHECK(XilinxAXIEnetStreamSlave, (obj),\
+ TYPE_XILINX_AXI_ENET_CONTROL_STREAM)
+
/* Advertisement control register. */
#define ADVERTISE_10HALF0x0020 /* Try for 10mbps half-duplex */
#define ADVERTISE_10FULL0x0040 /* Try for 10mbps full-duplex */
@@ -330,6 +335,7 @@ struct XilinxAXIEnet {
qemu_irq irq;
StreamSlave *tx_dev;
XilinxAXIEnetStreamSlave rx_data_dev;
+XilinxAXIEnetStreamSlave rx_control_dev;
NICState *nic;
NICConf conf;
@@ -813,6 +819,13 @@ static void eth_cleanup(NetClientState *nc)
}
static void
+axienet_control_stream_push(StreamSlave *obj, uint8_t *buf, size_t size,
+uint32_t *hdr)
+{
+/* ... */
+}
+
+static void
axienet_data_stream_push(StreamSlave *obj, uint8_t *buf, size_t size,
uint32_t *hdr)
{
@@ -869,14 +882,19 @@ static void xilinx_enet_realize(DeviceState *dev, Error
**errp)
{
XilinxAXIEnet *s = XILINX_AXI_ENET(dev);
XilinxAXIEnetStreamSlave *ds =
XILINX_AXI_ENET_DATA_STREAM(s-rx_data_dev);
+XilinxAXIEnetStreamSlave *cs = XILINX_AXI_ENET_CONTROL_STREAM(
+
s-rx_control_dev);
Error *local_errp = NULL;
object_property_add_link(OBJECT(ds), enet, xlnx.axi-ethernet,
(Object **) ds-enet, local_errp);
+object_property_add_link(OBJECT(cs), enet, xlnx.axi-ethernet,
+ (Object **) ds-enet, local_errp);
if (local_errp) {
goto xilinx_enet_realize_fail;
}
object_property_set_link(OBJECT(ds), OBJECT(s), enet, local_errp);
+object_property_set_link(OBJECT(cs), OBJECT(s), enet, local_errp);
if (local_errp) {
goto xilinx_enet_realize_fail;
}
@@ -911,9 +929,13 @@ static void xilinx_enet_init(Object *obj)
assert_no_error(errp);
object_initialize(s-rx_data_dev, TYPE_XILINX_AXI_ENET_DATA_STREAM);
+object_initialize(s-rx_control_dev, TYPE_XILINX_AXI_ENET_CONTROL_STREAM);
object_property_add_child(OBJECT(s), axistream-connected-target,
(Object *)s-rx_data_dev, errp);
assert_no_error(errp);
+object_property_add_child(OBJECT(s), axistream-control-connected-target,
+ (Object *)s-rx_control_dev, errp);
+assert_no_error(errp);
sysbus_init_irq(sbd, s-irq);
@@ -965,10 +987,23 @@ static const TypeInfo xilinx_enet_data_stream_info = {
}
};
+static const TypeInfo xilinx_enet_control_stream_info = {
+.name = TYPE_XILINX_AXI_ENET_CONTROL_STREAM,
+.parent= TYPE_OBJECT,
+.instance_size = sizeof(struct XilinxAXIEnetStreamSlave),
+.class_init= xilinx_enet_stream_class_init,
+.class_data= axienet_control_stream_push,
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_STREAM_SLAVE },
+{ }
+}
+};
+
static void xilinx_enet_register_types(void)
{
type_register_static(xilinx_enet_info);
type_register_static(xilinx_enet_data_stream_info);
+type_register_static(xilinx_enet_control_stream_info);
}
type_init(xilinx_enet_register_types)
--
1.7.0.4
[Qemu-devel] [PATCH v2 1/8] xilinx_axienet: typedef XilinxAXIEnet struct
Typedef xilinx_axienets object state struct to shorten the repeated usages of
struct XilinxAXIEnet.
Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
hw/xilinx_axienet.c | 44 +++-
1 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index e5d9251..4042e1a 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -305,6 +305,8 @@ struct TEMAC {
void *parent;
};
+typedef struct XilinxAXIEnet XilinxAXIEnet;
+
struct XilinxAXIEnet {
SysBusDevice busdev;
MemoryRegion iomem;
@@ -364,37 +366,37 @@ struct XilinxAXIEnet {
uint8_t *rxmem;
};
-static void axienet_rx_reset(struct XilinxAXIEnet *s)
+static void axienet_rx_reset(XilinxAXIEnet *s)
{
s-rcw[1] = RCW1_JUM | RCW1_FCS | RCW1_RX | RCW1_VLAN;
}
-static void axienet_tx_reset(struct XilinxAXIEnet *s)
+static void axienet_tx_reset(XilinxAXIEnet *s)
{
s-tc = TC_JUM | TC_TX | TC_VLAN;
}
-static inline int axienet_rx_resetting(struct XilinxAXIEnet *s)
+static inline int axienet_rx_resetting(XilinxAXIEnet *s)
{
return s-rcw[1] RCW1_RST;
}
-static inline int axienet_rx_enabled(struct XilinxAXIEnet *s)
+static inline int axienet_rx_enabled(XilinxAXIEnet *s)
{
return s-rcw[1] RCW1_RX;
}
-static inline int axienet_extmcf_enabled(struct XilinxAXIEnet *s)
+static inline int axienet_extmcf_enabled(XilinxAXIEnet *s)
{
return !!(s-regs[R_RAF] RAF_EMCF_EN);
}
-static inline int axienet_newfunc_enabled(struct XilinxAXIEnet *s)
+static inline int axienet_newfunc_enabled(XilinxAXIEnet *s)
{
return !!(s-regs[R_RAF] RAF_NEWFUNC_EN);
}
-static void axienet_reset(struct XilinxAXIEnet *s)
+static void axienet_reset(XilinxAXIEnet *s)
{
axienet_rx_reset(s);
axienet_tx_reset(s);
@@ -405,7 +407,7 @@ static void axienet_reset(struct XilinxAXIEnet *s)
s-emmc = EMMC_LINKSPEED_100MB;
}
-static void enet_update_irq(struct XilinxAXIEnet *s)
+static void enet_update_irq(XilinxAXIEnet *s)
{
s-regs[R_IP] = s-regs[R_IS] s-regs[R_IE];
qemu_set_irq(s-irq, !!s-regs[R_IP]);
@@ -413,7 +415,7 @@ static void enet_update_irq(struct XilinxAXIEnet *s)
static uint64_t enet_read(void *opaque, hwaddr addr, unsigned size)
{
-struct XilinxAXIEnet *s = opaque;
+XilinxAXIEnet *s = opaque;
uint32_t r = 0;
addr = 2;
@@ -505,7 +507,7 @@ static uint64_t enet_read(void *opaque, hwaddr addr,
unsigned size)
static void enet_write(void *opaque, hwaddr addr,
uint64_t value, unsigned size)
{
-struct XilinxAXIEnet *s = opaque;
+XilinxAXIEnet *s = opaque;
struct TEMAC *t = s-TEMAC;
addr = 2;
@@ -617,7 +619,7 @@ static const MemoryRegionOps enet_ops = {
static int eth_can_rx(NetClientState *nc)
{
-struct XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
+XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
/* RX enabled? */
return !axienet_rx_resetting(s) axienet_rx_enabled(s);
@@ -640,7 +642,7 @@ static int enet_match_addr(const uint8_t *buf, uint32_t f0,
uint32_t f1)
static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
{
-struct XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
+XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
static const unsigned char sa_bcast[6] = {0xff, 0xff, 0xff,
0xff, 0xff, 0xff};
static const unsigned char sa_ipmcast[3] = {0x01, 0x00, 0x52};
@@ -785,7 +787,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t
*buf, size_t size)
static void eth_cleanup(NetClientState *nc)
{
/* FIXME. */
-struct XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
+XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
g_free(s-rxmem);
g_free(s);
}
@@ -793,7 +795,7 @@ static void eth_cleanup(NetClientState *nc)
static void
axienet_stream_push(StreamSlave *obj, uint8_t *buf, size_t size, uint32_t *hdr)
{
-struct XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
+XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
/* TX enable ? */
if (!(s-tc TC_TX)) {
@@ -843,7 +845,7 @@ static NetClientInfo net_xilinx_enet_info = {
static int xilinx_enet_init(SysBusDevice *dev)
{
-struct XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), dev);
+XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), dev);
sysbus_init_irq(dev, s-irq);
@@ -868,7 +870,7 @@ static int xilinx_enet_init(SysBusDevice *dev)
static void xilinx_enet_initfn(Object *obj)
{
-struct XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
+XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
Error *errp = NULL;
object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE,
@@ -877,10 +879,10 @@ static void xilinx_enet_initfn(Object *obj)
}
static Property xilinx_enet_properties[] = {
-DEFINE_PROP_UINT32(phyaddr, struct XilinxAXIEnet, c_phyaddr, 7),
-
[Qemu-devel] [PATCH 7/9] chardev: add stdio support to qapi
This patch adds 'stdio' support to qapi and also switches over the
stdio chardev initialization to the new qapi code path.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
qapi-schema.json | 14 +-
qemu-char.c | 26 --
2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index f873451..b0ca67d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3146,6 +3146,17 @@
{ 'type': 'ChardevMux', 'data': { 'chardev' : 'str' } }
##
+# @ChardevStdio:
+#
+# Configuration info for stdio chardevs.
+#
+# @signal: Allow signals (such as SIGINT triggered by ^C) be delivered to qemu.
+#
+# Since: 1.5
+##
+{ 'type': 'ChardevStdio', 'data': { '*signal' : 'bool' } }
+
+##
# @ChardevBackend:
#
# Configuration info for the new chardev backend.
@@ -3162,7 +3173,8 @@
'null' : 'ChardevDummy',
'mux': 'ChardevMux',
'msmouse': 'ChardevDummy',
- 'braille': 'ChardevDummy' } }
+ 'braille': 'ChardevDummy',
+ 'stdio' : 'ChardevStdio' } }
##
# @ChardevReturn:
diff --git a/qemu-char.c b/qemu-char.c
index 51a43c1..4fdf381 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -753,7 +753,7 @@ static void qemu_chr_close_stdio(struct CharDriverState
*chr)
fd_chr_close(chr);
}
-static CharDriverState *qemu_chr_open_stdio(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
{
CharDriverState *chr;
@@ -776,8 +776,10 @@ static CharDriverState *qemu_chr_open_stdio(QemuOpts *opts)
chr-chr_set_echo = qemu_chr_set_echo_stdio;
qemu_set_fd_handler2(0, stdio_read_poll, stdio_read, NULL, chr);
stdio_nb_clients++;
-stdio_allow_signal = qemu_opt_get_bool(opts, signal,
- display_type != DT_NOGRAPHIC);
+stdio_allow_signal = display_type != DT_NOGRAPHIC;
+if (opts-has_signal) {
+stdio_allow_signal = opts-signal;
+}
qemu_chr_fe_set_echo(chr, false);
return chr;
@@ -1928,7 +1930,7 @@ static void win_stdio_close(CharDriverState *chr)
stdio_nb_clients--;
}
-static CharDriverState *qemu_chr_open_win_stdio(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
{
CharDriverState *chr;
WinStdioCharState *stdio;
@@ -2959,6 +2961,15 @@ static void qemu_chr_parse_file_out(QemuOpts *opts,
ChardevBackend *backend,
backend-file-out = g_strdup(path);
}
+static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend,
+ Error **errp)
+{
+backend-stdio = g_new0(ChardevStdio, 1);
+backend-stdio-has_signal = true;
+backend-stdio-signal =
+qemu_opt_get_bool(opts, signal, display_type != DT_NOGRAPHIC);
+}
+
static const struct {
const char *name;
/* old, pre qapi */
@@ -2975,14 +2986,14 @@ static const struct {
{ .name = memory,.open = qemu_chr_open_ringbuf },
{ .name = file, .kind = CHARDEV_BACKEND_KIND_FILE,
.parse = qemu_chr_parse_file_out },
+{ .name = stdio, .kind = CHARDEV_BACKEND_KIND_STDIO,
+ .parse = qemu_chr_parse_stdio },
#ifdef _WIN32
{ .name = pipe, .open = qemu_chr_open_win_pipe },
{ .name = console, .open = qemu_chr_open_win_con },
{ .name = serial,.open = qemu_chr_open_win },
-{ .name = stdio, .open = qemu_chr_open_win_stdio },
#else
{ .name = pipe, .open = qemu_chr_open_pipe },
-{ .name = stdio, .open = qemu_chr_open_stdio },
#endif
#ifdef CONFIG_BRLAPI
{ .name = braille, .kind = CHARDEV_BACKEND_KIND_BRAILLE },
@@ -3472,6 +3483,9 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
chr = chr_baum_init();
break;
#endif
+case CHARDEV_BACKEND_KIND_STDIO:
+chr = qemu_chr_open_stdio(backend-stdio);
+break;
default:
error_setg(errp, unknown chardev backend (%d), backend-kind);
break;
--
1.7.9.7
[Qemu-devel] [PATCH 5/9] chardev: add braille support to qapi
This patch adds 'braille' support to qapi and also switches over
the braille chardev initialization to the new qapi code path.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/baum.c|2 +-
hw/baum.h|2 +-
qapi-schema.json |3 ++-
qemu-char.c |7 ++-
4 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/hw/baum.c b/hw/baum.c
index 09dcb9c..7f0384c 100644
--- a/hw/baum.c
+++ b/hw/baum.c
@@ -562,7 +562,7 @@ static void baum_close(struct CharDriverState *chr)
g_free(baum);
}
-CharDriverState *chr_baum_init(QemuOpts *opts)
+CharDriverState *chr_baum_init(void)
{
BaumDriverState *baum;
CharDriverState *chr;
diff --git a/hw/baum.h b/hw/baum.h
index 7635884..6fcb627 100644
--- a/hw/baum.h
+++ b/hw/baum.h
@@ -25,6 +25,6 @@
#define HW_BAUM_H 1
/* char device */
-CharDriverState *chr_baum_init(QemuOpts *opts);
+CharDriverState *chr_baum_init(void);
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index cd93707..f873451 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3161,7 +3161,8 @@
'pty': 'ChardevDummy',
'null' : 'ChardevDummy',
'mux': 'ChardevMux',
- 'msmouse': 'ChardevDummy' } }
+ 'msmouse': 'ChardevDummy',
+ 'braille': 'ChardevDummy' } }
##
# @ChardevReturn:
diff --git a/qemu-char.c b/qemu-char.c
index c824ed8..156eade 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2998,7 +2998,7 @@ static const struct {
{ .name = stdio, .open = qemu_chr_open_stdio },
#endif
#ifdef CONFIG_BRLAPI
-{ .name = braille, .open = chr_baum_init },
+{ .name = braille, .kind = CHARDEV_BACKEND_KIND_BRAILLE },
#endif
#ifdef HAVE_CHARDEV_TTY
{ .name = tty, .open = qemu_chr_open_tty },
@@ -3480,6 +3480,11 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_MSMOUSE:
chr = qemu_chr_open_msmouse();
break;
+#ifdef CONFIG_BRLAPI
+case CHARDEV_BACKEND_KIND_BRAILLE:
+chr = chr_baum_init();
+break;
+#endif
default:
error_setg(errp, unknown chardev backend (%d), backend-kind);
break;
--
1.7.9.7
[Qemu-devel] [PATCH 1/2] require gtk 2.20+
The gtk code uses gtk_widget_get_realized which is available in 2.20+ only, so make this the minimum accepted versions. Fixes build failures on RHEL-6 (which ships 2.18) by not building gtk support there. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- configure |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 0dadd31..a6b0c02 100755 --- a/configure +++ b/configure @@ -1644,7 +1644,7 @@ fi # GTK probe if test $gtk != no; then -if $pkg_config gtk+-2.0 --modversion /dev/null 2/dev/null \ +if $pkg_config gtk+-2.0 --atleast-version=2.20 /dev/null 2/dev/null \ $pkg_config vte --modversion /dev/null 2/dev/null; then gtk_cflags=`$pkg_config --cflags gtk+-2.0 2/dev/null` gtk_libs=`$pkg_config --libs gtk+-2.0 2/dev/null` -- 1.7.9.7
[Qemu-devel] [PATCH v2 7/8] xilinx_axienet: Create Proxy object for stream
Create a separate child object to proxy the stream slave connection. This is
setup for future work where a second stream slave connection is needed. The
new child object is created at qdev init time and is linked back to the parent
(the ethernet device itself) automatically.
Stream slave masters differentiate which slave connection they are connected to
by linking to the proxy object rather than the parent.
Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
change from v1:
renamed data-stream proxy link to axistream-connected-target
rebased ontop of realize conversion
reworked error return mechanism (Andreas review)
inlined child device state struct into parents (Andreas review)
replaced object_new() - object_initialize() (Andreas review)
hw/petalogix_ml605_mmu.c |6 -
hw/xilinx_axienet.c | 60 ++---
2 files changed, 61 insertions(+), 5 deletions(-)
diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c
index e3528c0..4223173 100644
--- a/hw/petalogix_ml605_mmu.c
+++ b/hw/petalogix_ml605_mmu.c
@@ -79,6 +79,7 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)
const char *cpu_model = args-cpu_model;
MemoryRegion *address_space_mem = get_system_memory();
DeviceState *dev, *dma, *eth0;
+Object *peer;
MicroBlazeCPU *cpu;
SysBusDevice *busdev;
CPUMBState *env;
@@ -142,7 +143,10 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)
xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(dma),
0x8278, irq[3], 0x1000, 0x1000);
-xilinx_axidma_init(dma, STREAM_SLAVE(eth0), 0x8460, irq[1], irq[0],
+peer = object_property_get_link(OBJECT(eth0), data-stream, NULL);
+assert(peer);
+
+xilinx_axidma_init(dma, STREAM_SLAVE(peer), 0x8460, irq[1], irq[0],
100 * 100);
{
diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index 27d94bd..a4ec1d8 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -32,10 +32,15 @@
#define DPHY(x)
#define TYPE_XILINX_AXI_ENET xlnx.axi-ethernet
+#define TYPE_XILINX_AXI_ENET_DATA_STREAM xilinx-axienet-data-stream
#define XILINX_AXI_ENET(obj) \
OBJECT_CHECK(XilinxAXIEnet, (obj), TYPE_XILINX_AXI_ENET)
+#define XILINX_AXI_ENET_DATA_STREAM(obj) \
+ OBJECT_CHECK(XilinxAXIEnetStreamSlave, (obj),\
+ TYPE_XILINX_AXI_ENET_DATA_STREAM)
+
/* Advertisement control register. */
#define ADVERTISE_10HALF0x0020 /* Try for 10mbps half-duplex */
#define ADVERTISE_10FULL0x0040 /* Try for 10mbps full-duplex */
@@ -310,13 +315,21 @@ struct TEMAC {
void *parent;
};
+typedef struct XilinxAXIEnetStreamSlave XilinxAXIEnetStreamSlave;
typedef struct XilinxAXIEnet XilinxAXIEnet;
+struct XilinxAXIEnetStreamSlave {
+Object parent;
+
+struct XilinxAXIEnet *enet;
+} ;
+
struct XilinxAXIEnet {
SysBusDevice busdev;
MemoryRegion iomem;
qemu_irq irq;
StreamSlave *tx_dev;
+XilinxAXIEnetStreamSlave rx_data_dev;
NICState *nic;
NICConf conf;
@@ -800,9 +813,11 @@ static void eth_cleanup(NetClientState *nc)
}
static void
-axienet_stream_push(StreamSlave *obj, uint8_t *buf, size_t size, uint32_t *hdr)
+axienet_data_stream_push(StreamSlave *obj, uint8_t *buf, size_t size,
+ uint32_t *hdr)
{
-XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
+XilinxAXIEnetStreamSlave *ds = XILINX_AXI_ENET_DATA_STREAM(obj);
+XilinxAXIEnet *s = ds-enet;
/* TX enable ? */
if (!(s-tc TC_TX)) {
@@ -853,6 +868,18 @@ static NetClientInfo net_xilinx_enet_info = {
static void xilinx_enet_realize(DeviceState *dev, Error **errp)
{
XilinxAXIEnet *s = XILINX_AXI_ENET(dev);
+XilinxAXIEnetStreamSlave *ds =
XILINX_AXI_ENET_DATA_STREAM(s-rx_data_dev);
+Error *local_errp = NULL;
+
+object_property_add_link(OBJECT(ds), enet, xlnx.axi-ethernet,
+ (Object **) ds-enet, local_errp);
+if (local_errp) {
+goto xilinx_enet_realize_fail;
+}
+object_property_set_link(OBJECT(ds), OBJECT(s), enet, local_errp);
+if (local_errp) {
+goto xilinx_enet_realize_fail;
+}
qemu_macaddr_default_if_unset(s-conf.macaddr);
s-nic = qemu_new_nic(net_xilinx_enet_info, s-conf,
@@ -865,6 +892,12 @@ static void xilinx_enet_realize(DeviceState *dev, Error
**errp)
s-TEMAC.parent = s;
s-rxmem = g_malloc(s-c_rxmem);
+return;
+
+xilinx_enet_realize_fail:
+if (!*errp) {
+*errp = local_errp;
+}
}
static void xilinx_enet_init(Object *obj)
@@ -877,6 +910,11 @@ static void xilinx_enet_init(Object *obj)
(Object **) s-tx_dev, errp);
assert_no_error(errp);
+object_initialize(s-rx_data_dev, TYPE_XILINX_AXI_ENET_DATA_STREAM);
+object_property_add_child(OBJECT(s), axistream-connected-target,
+ (Object *)s-rx_data_dev,
[Qemu-devel] [PATCHv3] tap: set IFF_ONE_QUEUE per default
historically the kernel queues packets two times. once
at the device and second in qdisc. this is believed to cause
interface stalls if one of these queues overruns.
setting IFF_ONE_QUEUE is the default in kernels = 3.8. the
flag is ignored since then. see kernel commit
5d097109257c03a71845729f8db6b5770c4bbedc
v3:
- probe if IFF_ONE_QUEUE feature is available
v2:
- do only set the flag on linux as it breaks macvtap
- define IFF_ONE_QUEUE in tap-linux.h
Signed-off-by: Peter Lieven p...@kamp.de
---
net/tap-linux.c | 10 ++
net/tap-linux.h |9 +
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/net/tap-linux.c b/net/tap-linux.c
index a953189..36c09e2 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -42,6 +42,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
struct ifreq ifr;
int fd, ret;
int len = sizeof(struct virtio_net_hdr);
+unsigned int features;
TFR(fd = open(PATH_NET_TUN, O_RDWR));
if (fd 0) {
@@ -51,9 +52,12 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
memset(ifr, 0, sizeof(ifr));
ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
-if (*vnet_hdr) {
-unsigned int features;
+if (ioctl(fd, TUNGETFEATURES, features) == 0
+features IFF_ONE_QUEUE) {
+ifr.ifr_flags |= IFF_ONE_QUEUE;
+}
+if (*vnet_hdr) {
if (ioctl(fd, TUNGETFEATURES, features) == 0
features IFF_VNET_HDR) {
*vnet_hdr = 1;
@@ -78,8 +82,6 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
}
if (mq_required) {
-unsigned int features;
-
if ((ioctl(fd, TUNGETFEATURES, features) != 0) ||
!(features IFF_MULTI_QUEUE)) {
error_report(multiqueue required, but no kernel
diff --git a/net/tap-linux.h b/net/tap-linux.h
index 65087e1..5704efb 100644
--- a/net/tap-linux.h
+++ b/net/tap-linux.h
@@ -34,10 +34,11 @@
#endif
/* TUNSETIFF ifr flags */
-#define IFF_TAP0x0002
-#define IFF_NO_PI 0x1000
-#define IFF_VNET_HDR 0x4000
-#define IFF_MULTI_QUEUE 0x0100
+#define IFF_TAP 0x0002
+#define IFF_NO_PI0x1000
+#define IFF_ONE_QUEUE0x2000
+#define IFF_VNET_HDR 0x4000
+#define IFF_MULTI_QUEUE 0x0100
#define IFF_ATTACH_QUEUE 0x0200
#define IFF_DETACH_QUEUE 0x0400
--
1.7.9.5
[Qemu-devel] [PATCH 2/9] chardev: add mux chardev support to qapi
This adds mux chardev support to the qapi and also makes the qapi-based
chardev creation path handle the mux=on option correctly.
---
qapi-schema.json | 14 +-
qemu-char.c | 35 ---
2 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index cd7ea25..d633e1f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3135,6 +3135,17 @@
'*telnet' : 'bool' } }
##
+# @ChardevMux:
+#
+# Configuration info for mux chardevs.
+#
+# @chardev: name of the base chardev.
+#
+# Since: 1.5
+##
+{ 'type': 'ChardevMux', 'data': { 'chardev' : 'str' } }
+
+##
# @ChardevBackend:
#
# Configuration info for the new chardev backend.
@@ -3148,7 +3159,8 @@
'parallel': 'ChardevHostdev',
'socket' : 'ChardevSocket',
'pty': 'ChardevDummy',
- 'null' : 'ChardevDummy' } }
+ 'null' : 'ChardevDummy',
+ 'mux': 'ChardevMux' } }
##
# @ChardevReturn:
diff --git a/qemu-char.c b/qemu-char.c
index cf6b98b..fc908fa 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3049,6 +3049,11 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
ChardevBackend *backend = g_new0(ChardevBackend, 1);
ChardevReturn *ret = NULL;
const char *id = qemu_opts_id(opts);
+const char *bid = NULL;
+
+if (qemu_opt_get_bool(opts, mux, 0)) {
+bid = g_strdup_printf(%s-base, id);
+}
chr = NULL;
backend-kind = backend_table[i].kind;
@@ -3058,10 +3063,24 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
goto qapi_out;
}
}
-ret = qmp_chardev_add(qemu_opts_id(opts), backend, errp);
+ret = qmp_chardev_add(bid ? bid : id, backend, errp);
if (error_is_set(errp)) {
goto qapi_out;
}
+
+if (bid) {
+qapi_free_ChardevBackend(backend);
+qapi_free_ChardevReturn(ret);
+backend = g_new0(ChardevBackend, 1);
+backend-mux = g_new0(ChardevMux, 1);
+backend-kind = CHARDEV_BACKEND_KIND_MUX;
+backend-mux-chardev = g_strdup(bid);
+ret = qmp_chardev_add(id, backend, errp);
+if (error_is_set(errp)) {
+goto qapi_out;
+}
+}
+
chr = qemu_chr_find(id);
qapi_out:
@@ -3411,7 +3430,7 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
Error **errp)
{
ChardevReturn *ret = g_new0(ChardevReturn, 1);
-CharDriverState *chr = NULL;
+CharDriverState *base, *chr = NULL;
chr = qemu_chr_find(id);
if (chr) {
@@ -3449,6 +3468,15 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_NULL:
chr = qemu_chr_open_null(NULL);
break;
+case CHARDEV_BACKEND_KIND_MUX:
+base = qemu_chr_find(backend-mux-chardev);
+if (base == NULL) {
+error_setg(errp, mux: base chardev %s not found,
+ backend-mux-chardev);
+break;
+}
+chr = qemu_chr_open_mux(base);
+break;
default:
error_setg(errp, unknown chardev backend (%d), backend-kind);
break;
@@ -3459,7 +3487,8 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
}
if (chr) {
chr-label = g_strdup(id);
-chr-avail_connections = 1;
+chr-avail_connections =
+(backend-kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
QTAILQ_INSERT_TAIL(chardevs, chr, next);
return ret;
} else {
--
1.7.9.7
[Qemu-devel] [PATCH v2 2/8] xilinx_axienet: Defined and use type cast macro
Standard QOM cast macro. Replaces usages of FROM_SYSBUS
Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
hw/xilinx_axienet.c | 11 ---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index 4042e1a..7b50682 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -31,6 +31,11 @@
#define DPHY(x)
+#define TYPE_XILINX_AXI_ENET xlnx.axi-ethernet
+
+#define XILINX_AXI_ENET(obj) \
+ OBJECT_CHECK(XilinxAXIEnet, (obj), TYPE_XILINX_AXI_ENET)
+
/* Advertisement control register. */
#define ADVERTISE_10HALF0x0020 /* Try for 10mbps half-duplex */
#define ADVERTISE_10FULL0x0040 /* Try for 10mbps full-duplex */
@@ -845,7 +850,7 @@ static NetClientInfo net_xilinx_enet_info = {
static int xilinx_enet_init(SysBusDevice *dev)
{
-XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), dev);
+XilinxAXIEnet *s = XILINX_AXI_ENET(dev);
sysbus_init_irq(dev, s-irq);
@@ -870,7 +875,7 @@ static int xilinx_enet_init(SysBusDevice *dev)
static void xilinx_enet_initfn(Object *obj)
{
-XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
+XilinxAXIEnet *s = XILINX_AXI_ENET(obj);
Error *errp = NULL;
object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE,
@@ -898,7 +903,7 @@ static void xilinx_enet_class_init(ObjectClass *klass, void
*data)
}
static const TypeInfo xilinx_enet_info = {
-.name = xlnx.axi-ethernet,
+.name = TYPE_XILINX_AXI_ENET,
.parent= TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(XilinxAXIEnet),
.class_init= xilinx_enet_class_init,
--
1.7.0.4
[Qemu-devel] [PATCH] qom/object.c: fix a typo in comment
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
qom/object.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qom/object.c b/qom/object.c
index 3d638ff..a90b131 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -189,7 +189,7 @@ static bool type_is_ancestor(TypeImpl *type, TypeImpl
*target_type)
{
assert(target_type);
-/* Check if typename is a direct ancestor of type */
+/* Check if target_type is a direct ancestor of type */
while (type) {
if (type == target_type) {
return true;
--
1.8.1.4
[Qemu-devel] [PATCH 8/9] chardev: switch serial/tty init to qapi
This patch switches over the serial chardev initialization
to the new qapi code path.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
qemu-char.c | 37 +
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 4fdf381..eac4460 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1230,18 +1230,6 @@ static CharDriverState *qemu_chr_open_tty_fd(int fd)
chr-chr_close = qemu_chr_close_tty;
return chr;
}
-
-static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
-{
-const char *filename = qemu_opt_get(opts, path);
-int fd;
-
-TFR(fd = qemu_open(filename, O_RDWR | O_NONBLOCK));
-if (fd 0) {
-return NULL;
-}
-return qemu_chr_open_tty_fd(fd);
-}
#endif /* __linux__ || __sun__ */
#if defined(__linux__)
@@ -1668,11 +1656,6 @@ static CharDriverState *qemu_chr_open_win_path(const
char *filename)
return chr;
}
-static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
-{
-return qemu_chr_open_win_path(qemu_opt_get(opts, path));
-}
-
static int win_chr_pipe_poll(void *opaque)
{
CharDriverState *chr = opaque;
@@ -2970,6 +2953,19 @@ static void qemu_chr_parse_stdio(QemuOpts *opts,
ChardevBackend *backend,
qemu_opt_get_bool(opts, signal, display_type != DT_NOGRAPHIC);
}
+static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
+ Error **errp)
+{
+const char *device = qemu_opt_get(opts, path);
+
+if (path == NULL) {
+error_setg(errp, chardev: serial/tty: no device path given);
+return;
+}
+backend-serial = g_new0(ChardevHostdev, 1);
+backend-serial-device = g_strdup(device);
+}
+
static const struct {
const char *name;
/* old, pre qapi */
@@ -2988,10 +2984,13 @@ static const struct {
.parse = qemu_chr_parse_file_out },
{ .name = stdio, .kind = CHARDEV_BACKEND_KIND_STDIO,
.parse = qemu_chr_parse_stdio },
+{ .name = serial,.kind = CHARDEV_BACKEND_KIND_SERIAL,
+ .parse = qemu_chr_parse_serial },
+{ .name = tty, .kind = CHARDEV_BACKEND_KIND_SERIAL,
+ .parse = qemu_chr_parse_serial },
#ifdef _WIN32
{ .name = pipe, .open = qemu_chr_open_win_pipe },
{ .name = console, .open = qemu_chr_open_win_con },
-{ .name = serial,.open = qemu_chr_open_win },
#else
{ .name = pipe, .open = qemu_chr_open_pipe },
#endif
@@ -2999,8 +2998,6 @@ static const struct {
{ .name = braille, .kind = CHARDEV_BACKEND_KIND_BRAILLE },
#endif
#ifdef HAVE_CHARDEV_TTY
-{ .name = tty, .open = qemu_chr_open_tty },
-{ .name = serial,.open = qemu_chr_open_tty },
{ .name = pty, .open = qemu_chr_open_pty },
#endif
#ifdef HAVE_CHARDEV_PARPORT
--
1.7.9.7
[Qemu-devel] [PATCH 0/9] chardev: qapi conversion continued
Hi, This patch series starts to switch the chardev initialization to qapi, i.e. instead of passing the QemuOpts we'll get directly to a initialization function we'll create a ChardevBackend from the QemuOpts, then go create the chardev via qmp_chardev_add. Once finished all chardev initialization will use the same code paths, no matter whenever they where triggered via cmdline or via qmp. This patch series is a first step only and converts only a part of the backends over to the new api. please review, Gerd Gerd Hoffmann (9): chardev: add support for qapi-based chardev initialization chardev: add mux chardev support to qapi chardev: switch null init to qapi chardev: add msmouse support to qapi chardev: add braille support to qapi chardev: switch file init to qapi chardev: add stdio support to qapi chardev: switch serial/tty init to qapi chardev: switch parallel init to qapi hw/baum.c|2 +- hw/baum.h|2 +- hw/msmouse.c |2 +- hw/msmouse.h |2 +- qapi-schema.json | 28 +++- qemu-char.c | 210 +++--- 6 files changed, 167 insertions(+), 79 deletions(-) -- 1.7.9.7
[Qemu-devel] [PATCH 3/9] chardev: switch null init to qapi
This patch switches over the 'null' chardev initialization
to the new qapi code path.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
qemu-char.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index fc908fa..ec49fb7 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -221,7 +221,7 @@ static int null_chr_write(CharDriverState *chr, const
uint8_t *buf, int len)
return len;
}
-static CharDriverState *qemu_chr_open_null(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_null(void)
{
CharDriverState *chr;
@@ -2980,7 +2980,7 @@ static const struct {
const int kind;
void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
} backend_table[] = {
-{ .name = null, .open = qemu_chr_open_null },
+{ .name = null, .kind = CHARDEV_BACKEND_KIND_NULL },
{ .name = socket,.open = qemu_chr_open_socket },
{ .name = udp, .open = qemu_chr_open_udp },
{ .name = msmouse, .open = qemu_chr_open_msmouse },
@@ -3466,7 +3466,7 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
}
#endif
case CHARDEV_BACKEND_KIND_NULL:
-chr = qemu_chr_open_null(NULL);
+chr = qemu_chr_open_null();
break;
case CHARDEV_BACKEND_KIND_MUX:
base = qemu_chr_find(backend-mux-chardev);
--
1.7.9.7
[Qemu-devel] [PATCH 4/9] chardev: add msmouse support to qapi
This patch adds 'msmouse' support to qapi and also switches over
the msmouse chardev initialization to the new qapi code path.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/msmouse.c |2 +-
hw/msmouse.h |2 +-
qapi-schema.json |3 ++-
qemu-char.c |5 -
4 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/msmouse.c b/hw/msmouse.c
index ef47aed..1e0b84f 100644
--- a/hw/msmouse.c
+++ b/hw/msmouse.c
@@ -64,7 +64,7 @@ static void msmouse_chr_close (struct CharDriverState *chr)
g_free (chr);
}
-CharDriverState *qemu_chr_open_msmouse(QemuOpts *opts)
+CharDriverState *qemu_chr_open_msmouse(void)
{
CharDriverState *chr;
diff --git a/hw/msmouse.h b/hw/msmouse.h
index 8cff3a7..a1f1759 100644
--- a/hw/msmouse.h
+++ b/hw/msmouse.h
@@ -2,6 +2,6 @@
#define HW_MSMOUSE_H 1
/* msmouse.c */
-CharDriverState *qemu_chr_open_msmouse(QemuOpts *opts);
+CharDriverState *qemu_chr_open_msmouse(void);
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index d633e1f..cd93707 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3160,7 +3160,8 @@
'socket' : 'ChardevSocket',
'pty': 'ChardevDummy',
'null' : 'ChardevDummy',
- 'mux': 'ChardevMux' } }
+ 'mux': 'ChardevMux',
+ 'msmouse': 'ChardevDummy' } }
##
# @ChardevReturn:
diff --git a/qemu-char.c b/qemu-char.c
index ec49fb7..c824ed8 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2983,7 +2983,7 @@ static const struct {
{ .name = null, .kind = CHARDEV_BACKEND_KIND_NULL },
{ .name = socket,.open = qemu_chr_open_socket },
{ .name = udp, .open = qemu_chr_open_udp },
-{ .name = msmouse, .open = qemu_chr_open_msmouse },
+{ .name = msmouse, .kind = CHARDEV_BACKEND_KIND_MSMOUSE },
{ .name = vc,.open = vc_init },
{ .name = memory,.open = qemu_chr_open_ringbuf },
#ifdef _WIN32
@@ -3477,6 +3477,9 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
}
chr = qemu_chr_open_mux(base);
break;
+case CHARDEV_BACKEND_KIND_MSMOUSE:
+chr = qemu_chr_open_msmouse();
+break;
default:
error_setg(errp, unknown chardev backend (%d), backend-kind);
break;
--
1.7.9.7
[Qemu-devel] [PATCH 2/2] unbreak hw/usb/redirect.c build
Commit 8550a02d1239415342959f6a32d178bc05c557cc added a streams
parameter to usb_wakeup and didn't update redirect.c. Fix it.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/redirect.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 7078403..c519b9b 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1897,7 +1897,7 @@ static void usbredir_interrupt_packet(void *priv,
uint64_t id,
}
if (QTAILQ_EMPTY(dev-endpoint[EP2I(ep)].bufpq)) {
-usb_wakeup(usb_ep_get(dev-dev, USB_TOKEN_IN, ep 0x0f));
+usb_wakeup(usb_ep_get(dev-dev, USB_TOKEN_IN, ep 0x0f), 0);
}
/* bufp_alloc also adds the packet to the ep queue */
--
1.7.9.7
Re: [Qemu-devel] scp during migration with vhost fails
On Sat, Feb 23, 2013 at 11:54:55PM +0200, 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)))
[Qemu-devel] [PATCH v2 0/8] Stream - Multiple connections for the one device
Hi all. The Xilinx AXIEnet and DMA devices have two AXI stream connections (control and data), only one of which is currently modelled (data). AXI stream is modelled using the stream QOM interface described in stream.h. Unfortunately, interfaces have no nice way of modelling multiple connections of the same type. So to overcome this I created a secondary object which acts as a proxy for the stream connection. Multiple connections can be implemented using multiple proxies and stream masters link to the relevant proxy, rather than the ethernet device itself. This RFC is an example for how to do this for AXIEnet only. If acceptable, this change pattern will also be applied to AXIDMA. Patches 1-6 are cleanup of axienet as per the current QOM styling guidelines and can be cherry-picked off the front. Patch 7 is the proxy patch. Patch 8 is an example stub for how a secondary connection can be created. changed from v1: Removed former P12 (already merged) Address Andreas review Refactor axienet to be more QOM friendly. Peter Crosthwaite (8): xilinx_axienet: typedef XilinxAXIEnet struct xilinx_axienet: Defined and use type cast macro xilinx_axienet: Register reset properly xilinx_axienet: converted init-realize petalogix_ml605_mmu: Fix machine node attachment petalogix_ml605_mmu: Attach ethernet to machine xilinx_axienet: Create Proxy object for stream xilinx_axienet: stub out second stream connection hw/petalogix_ml605_mmu.c | 12 +++- hw/xilinx_axienet.c | 170 +++-- 2 files changed, 141 insertions(+), 41 deletions(-)
[Qemu-devel] [PATCH 6/9] chardev: switch file init to qapi
This patch switches over the 'file' chardev initialization
to the new qapi code path.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
qemu-char.c | 43 +++
1 file changed, 15 insertions(+), 28 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 156eade..51a43c1 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -637,18 +637,6 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int
fd_out)
return chr;
}
-static CharDriverState *qemu_chr_open_file_out(QemuOpts *opts)
-{
-int fd_out;
-
-TFR(fd_out = qemu_open(qemu_opt_get(opts, path),
- O_WRONLY | O_TRUNC | O_CREAT | O_BINARY, 0666));
-if (fd_out 0) {
-return NULL;
-}
-return qemu_chr_open_fd(-1, fd_out);
-}
-
static CharDriverState *qemu_chr_open_pipe(QemuOpts *opts)
{
int fd_in, fd_out;
@@ -1802,20 +1790,6 @@ static CharDriverState *qemu_chr_open_win_con(QemuOpts
*opts)
return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE));
}
-static CharDriverState *qemu_chr_open_win_file_out(QemuOpts *opts)
-{
-const char *file_out = qemu_opt_get(opts, path);
-HANDLE fd_out;
-
-fd_out = CreateFile(file_out, GENERIC_WRITE, FILE_SHARE_READ, NULL,
-OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
-if (fd_out == INVALID_HANDLE_VALUE) {
-return NULL;
-}
-
-return qemu_chr_open_win_file(fd_out);
-}
-
static int win_stdio_write(CharDriverState *chr, const uint8_t *buf, int len)
{
HANDLE hStdOut = GetStdHandle(STD_OUTPUT_HANDLE);
@@ -2972,6 +2946,19 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
#endif
+static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
+Error **errp)
+{
+const char *path = qemu_opt_get(opts, path);
+
+if (path == NULL) {
+error_setg(errp, chardev: file: no filename given);
+return;
+}
+backend-file = g_new0(ChardevFile, 1);
+backend-file-out = g_strdup(path);
+}
+
static const struct {
const char *name;
/* old, pre qapi */
@@ -2986,14 +2973,14 @@ static const struct {
{ .name = msmouse, .kind = CHARDEV_BACKEND_KIND_MSMOUSE },
{ .name = vc,.open = vc_init },
{ .name = memory,.open = qemu_chr_open_ringbuf },
+{ .name = file, .kind = CHARDEV_BACKEND_KIND_FILE,
+ .parse = qemu_chr_parse_file_out },
#ifdef _WIN32
-{ .name = file, .open = qemu_chr_open_win_file_out },
{ .name = pipe, .open = qemu_chr_open_win_pipe },
{ .name = console, .open = qemu_chr_open_win_con },
{ .name = serial,.open = qemu_chr_open_win },
{ .name = stdio, .open = qemu_chr_open_win_stdio },
#else
-{ .name = file, .open = qemu_chr_open_file_out },
{ .name = pipe, .open = qemu_chr_open_pipe },
{ .name = stdio, .open = qemu_chr_open_stdio },
#endif
--
1.7.9.7
[Qemu-devel] [PATCH 9/9] chardev: switch parallel init to qapi
This patch switches over the parallel chardev initialization
to the new qapi code path.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
qemu-char.c | 37 +
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index eac4460..c53607b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2915,22 +2915,6 @@ fail:
return NULL;
}
-#ifdef HAVE_CHARDEV_PARPORT
-
-static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
-{
-const char *filename = qemu_opt_get(opts, path);
-int fd;
-
-fd = qemu_open(filename, O_RDWR);
-if (fd 0) {
-return NULL;
-}
-return qemu_chr_open_pp_fd(fd);
-}
-
-#endif
-
static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
Error **errp)
{
@@ -2966,6 +2950,19 @@ static void qemu_chr_parse_serial(QemuOpts *opts,
ChardevBackend *backend,
backend-serial-device = g_strdup(device);
}
+static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
+Error **errp)
+{
+const char *device = qemu_opt_get(opts, path);
+
+if (path == NULL) {
+error_setg(errp, chardev: parallel: no device path given);
+return;
+}
+backend-parallel = g_new0(ChardevHostdev, 1);
+backend-parallel-device = g_strdup(device);
+}
+
static const struct {
const char *name;
/* old, pre qapi */
@@ -2988,6 +2985,10 @@ static const struct {
.parse = qemu_chr_parse_serial },
{ .name = tty, .kind = CHARDEV_BACKEND_KIND_SERIAL,
.parse = qemu_chr_parse_serial },
+{ .name = parallel, .kind = CHARDEV_BACKEND_KIND_PARALLEL,
+ .parse = qemu_chr_parse_parallel },
+{ .name = parport, .kind = CHARDEV_BACKEND_KIND_PARALLEL,
+ .parse = qemu_chr_parse_parallel },
#ifdef _WIN32
{ .name = pipe, .open = qemu_chr_open_win_pipe },
{ .name = console, .open = qemu_chr_open_win_con },
@@ -3000,10 +3001,6 @@ static const struct {
#ifdef HAVE_CHARDEV_TTY
{ .name = pty, .open = qemu_chr_open_pty },
#endif
-#ifdef HAVE_CHARDEV_PARPORT
-{ .name = parallel, .open = qemu_chr_open_pp },
-{ .name = parport, .open = qemu_chr_open_pp },
-#endif
#ifdef CONFIG_SPICE
{ .name = spicevmc, .open = qemu_chr_open_spice },
#if SPICE_SERVER_VERSION = 0x000c02
--
1.7.9.7
[Qemu-devel] [PATCH 1/9] chardev: add support for qapi-based chardev initialization
This patch add support for a new way to initialize chardev devices.
Instead of calling a initialization function with a QemuOpts we will
now create a (qapi) ChardevBackend, optionally call a function to
fill ChardevBackend from QemuOpts, then go create the chardev using
the new qapi code path which is also used by chardev-add.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
qemu-char.c | 30 ++
1 file changed, 30 insertions(+)
diff --git a/qemu-char.c b/qemu-char.c
index 160decc..cf6b98b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2974,7 +2974,11 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
static const struct {
const char *name;
+/* old, pre qapi */
CharDriverState *(*open)(QemuOpts *opts);
+/* new, qapi-based */
+const int kind;
+void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
} backend_table[] = {
{ .name = null, .open = qemu_chr_open_null },
{ .name = socket,.open = qemu_chr_open_socket },
@@ -3040,6 +3044,32 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
goto err;
}
+if (!backend_table[i].open) {
+/* using new, qapi init */
+ChardevBackend *backend = g_new0(ChardevBackend, 1);
+ChardevReturn *ret = NULL;
+const char *id = qemu_opts_id(opts);
+
+chr = NULL;
+backend-kind = backend_table[i].kind;
+if (backend_table[i].parse) {
+backend_table[i].parse(opts, backend, errp);
+if (error_is_set(errp)) {
+goto qapi_out;
+}
+}
+ret = qmp_chardev_add(qemu_opts_id(opts), backend, errp);
+if (error_is_set(errp)) {
+goto qapi_out;
+}
+chr = qemu_chr_find(id);
+
+qapi_out:
+qapi_free_ChardevBackend(backend);
+qapi_free_ChardevReturn(ret);
+return chr;
+}
+
chr = backend_table[i].open(opts);
if (!chr) {
error_setg(errp, chardev: opening backend \%s\ failed,
--
1.7.9.7
Re: [Qemu-devel] scp during migration with vhost fails
On Mon, Feb 25, 2013 at 02:11:44PM +0800, Jason Wang wrote:
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 =
Re: [Qemu-devel] [PATCH v2 2/2] Add AT24Cxx I2C EEPROM device model
Am 25.02.2013 08:49, schrieb Paolo Bonzini: +#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. OK, but my point of using hw/ is valid whatever the directory name. :) 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] [PATCHv3] tap: set IFF_ONE_QUEUE per default
On Mon, Feb 25, 2013 at 10:17:08AM +0100, Peter Lieven wrote:
historically the kernel queues packets two times. once
at the device and second in qdisc. this is believed to cause
interface stalls if one of these queues overruns.
setting IFF_ONE_QUEUE is the default in kernels = 3.8. the
flag is ignored since then. see kernel commit
5d097109257c03a71845729f8db6b5770c4bbedc
v3:
- probe if IFF_ONE_QUEUE feature is available
v2:
- do only set the flag on linux as it breaks macvtap
- define IFF_ONE_QUEUE in tap-linux.h
Signed-off-by: Peter Lieven p...@kamp.de
Acked-by: Michael S. Tsirkin m...@redhat.com
---
net/tap-linux.c | 10 ++
net/tap-linux.h |9 +
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/net/tap-linux.c b/net/tap-linux.c
index a953189..36c09e2 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -42,6 +42,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
struct ifreq ifr;
int fd, ret;
int len = sizeof(struct virtio_net_hdr);
+unsigned int features;
TFR(fd = open(PATH_NET_TUN, O_RDWR));
if (fd 0) {
@@ -51,9 +52,12 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
memset(ifr, 0, sizeof(ifr));
ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
-if (*vnet_hdr) {
-unsigned int features;
+if (ioctl(fd, TUNGETFEATURES, features) == 0
+features IFF_ONE_QUEUE) {
+ifr.ifr_flags |= IFF_ONE_QUEUE;
+}
+if (*vnet_hdr) {
if (ioctl(fd, TUNGETFEATURES, features) == 0
features IFF_VNET_HDR) {
*vnet_hdr = 1;
@@ -78,8 +82,6 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
}
if (mq_required) {
-unsigned int features;
-
if ((ioctl(fd, TUNGETFEATURES, features) != 0) ||
!(features IFF_MULTI_QUEUE)) {
error_report(multiqueue required, but no kernel
diff --git a/net/tap-linux.h b/net/tap-linux.h
index 65087e1..5704efb 100644
--- a/net/tap-linux.h
+++ b/net/tap-linux.h
@@ -34,10 +34,11 @@
#endif
/* TUNSETIFF ifr flags */
-#define IFF_TAP 0x0002
-#define IFF_NO_PI0x1000
-#define IFF_VNET_HDR 0x4000
-#define IFF_MULTI_QUEUE 0x0100
+#define IFF_TAP 0x0002
+#define IFF_NO_PI0x1000
+#define IFF_ONE_QUEUE0x2000
+#define IFF_VNET_HDR 0x4000
+#define IFF_MULTI_QUEUE 0x0100
#define IFF_ATTACH_QUEUE 0x0200
#define IFF_DETACH_QUEUE 0x0400
--
1.7.9.5
Re: [Qemu-devel] Moving BIOS tables from SeaBIOS to QEMU
On Sun, Feb 24, 2013 at 01:00:28PM -0500, Kevin O'Connor wrote: 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 IIRC SMBIOS has some tables with information about a BIOS. - 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. Again IIRC there are still OSes that uses mptable to obtain irq information. See 928d4dffef5c374. 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 -- Gleb.
Re: [Qemu-devel] [PATCHv3] tap: set IFF_ONE_QUEUE per default
On Mon, Feb 25, 2013 at 10:17:08AM +0100, Peter Lieven wrote: git-am(1) cannot apply this patch. There is whitespace damage, untouched lines have an extra space. I applied it manually, please use git-send-email(1) or fix your email client configuration for future patches. historically the kernel queues packets two times. once at the device and second in qdisc. this is believed to cause interface stalls if one of these queues overruns. setting IFF_ONE_QUEUE is the default in kernels = 3.8. the flag is ignored since then. see kernel commit 5d097109257c03a71845729f8db6b5770c4bbedc v3: - probe if IFF_ONE_QUEUE feature is available v2: - do only set the flag on linux as it breaks macvtap - define IFF_ONE_QUEUE in tap-linux.h In the future, please put patch revision below the '---' in the future so it's not part of the commit description. That way it is not stored in git (the information isn't useful once the patch has been merged). Signed-off-by: Peter Lieven p...@kamp.de --- net/tap-linux.c | 10 ++ net/tap-linux.h |9 + 2 files changed, 11 insertions(+), 8 deletions(-) Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan
Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
Il 25/02/2013 09:09, Christian Borntraeger ha scritto: Hmm, the old sequence was object_unparent(OBJECT(dev)); qdev_free(dev) ---+ | V ... object_unparent(OBJECT(dev)); now the last reference is gone, object is freed object_unref(OBJECT(dev)); now the reference of a deleted object becomes -1 ... Isnt that a problem in itself that we modify a reference counter in an deleted object? The second object_unparent should do nothing. So before you had: object_unparent(OBJECT(dev)); leaves refcount=1 qdev_free(dev) ---+ | V object_unparent(OBJECT(dev)); do nothing object_unref(OBJECT(dev)); refcount=0, object freed After the object_unref was removed you had: object_unparent(OBJECT(dev)); refcount=0, object freed qdev_free(dev) ---+ | V object_unparent(OBJECT(dev)); dangling pointer! Paolo
Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
On 25/02/13 11:44, Paolo Bonzini wrote: Il 25/02/2013 09:09, Christian Borntraeger ha scritto: Hmm, the old sequence was object_unparent(OBJECT(dev)); qdev_free(dev) ---+ | V ... object_unparent(OBJECT(dev)); now the last reference is gone, object is freed object_unref(OBJECT(dev)); now the reference of a deleted object becomes -1 ... Isnt that a problem in itself that we modify a reference counter in an deleted object? The second object_unparent should do nothing. So before you had: object_unparent(OBJECT(dev)); leaves refcount=1 qdev_free(dev) ---+ | V object_unparent(OBJECT(dev)); do nothing object_unref(OBJECT(dev)); refcount=0, object freed After the object_unref was removed you had: object_unparent(OBJECT(dev)); refcount=0, object freed qdev_free(dev) ---+ | V object_unparent(OBJECT(dev)); dangling pointer! Got it. Thanks
Re: [Qemu-devel] [PATCHv4] iscsi: add iscsi_truncate support
Il 18/02/2013 14:50, Peter Lieven ha scritto:
this patch adds iscsi_truncate which effectively allows for
online resizing of iscsi volumes. for this to work you have
to resize the volume on your storage and then call
block_resize command in qemu which will issue a
readcapacity16 to update the capacity.
v4:
- factor out complete readcapacity logic into a separate function
- handle capacity change check condition in readcapacity function
(this happens if the block_resize cmd is the first iscsi task
executed after a resize on the storage)
v3:
- remove switch statement in iscsi_open
- create separate patch for brdv_drain_all() in bdrv_truncate()
v2:
- add a general bdrv_drain_all() before bdrv_truncate() to avoid
in-flight AIOs while the device is truncated
- since no AIOs are in flight we can use a sync libiscsi call
to re-read the capacity
- factor out the readcapacity16 logic as it is redundant
to iscsi_open() and iscsi_truncate().
Signed-off-by: Peter Lieven p...@kamp.de
Applied to scsi-next branch, thanks.
Paolo
---
block/iscsi.c | 133
+
1 file changed, 87 insertions(+), 46 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index deb3b68..1ea290e 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -823,6 +823,70 @@ static void iscsi_nop_timed_event(void *opaque)
}
#endif
+static int iscsi_readcapacity_sync(IscsiLun *iscsilun) {
+struct scsi_task *task = NULL;
+struct scsi_readcapacity10 *rc10 = NULL;
+struct scsi_readcapacity16 *rc16 = NULL;
+int ret = 0;
+
+try_again:
+switch (iscsilun-type) {
+case TYPE_DISK:
+task = iscsi_readcapacity16_sync(iscsilun-iscsi, iscsilun-lun);
+if (task == NULL || task-status != SCSI_STATUS_GOOD) {
+/* Capacity data has changed */
+if (task != NULL task-status ==
SCSI_STATUS_CHECK_CONDITION
+ task-sense.key == SCSI_SENSE_UNIT_ATTENTION
+ task-sense.ascq == 0x2a09) {
+scsi_free_scsi_task(task);
+goto try_again;
+}
+error_report(iSCSI: failed to send readcapacity16 command.);
+ret = -EINVAL;
+goto out;
+}
+rc16 = scsi_datain_unmarshall(task);
+if (rc16 == NULL) {
+error_report(iSCSI: Failed to unmarshall readcapacity16
data.);
+ret = -EINVAL;
+goto out;
+}
+iscsilun-block_size = rc16-block_length;
+iscsilun-num_blocks = rc16-returned_lba + 1;
+break;
+case TYPE_ROM:
+task = iscsi_readcapacity10_sync(iscsilun-iscsi,
iscsilun-lun, 0, 0);
+if (task == NULL || task-status != SCSI_STATUS_GOOD) {
+error_report(iSCSI: failed to send readcapacity10 command.);
+ret = -EINVAL;
+goto out;
+}
+rc10 = scsi_datain_unmarshall(task);
+if (rc10 == NULL) {
+error_report(iSCSI: Failed to unmarshall readcapacity10
data.);
+ret = -EINVAL;
+goto out;
+}
+iscsilun-block_size = rc10-block_size;
+if (rc10-lba == 0) {
+/* blank disk loaded */
+iscsilun-num_blocks = 0;
+} else {
+iscsilun-num_blocks = rc10-lba + 1;
+}
+break;
+default:
+break;
+}
+
+out:
+if (task) {
+scsi_free_scsi_task(task);
+}
+
+return ret;
+}
+
/*
* We support iscsi url's on the form
* iscsi://[username%password@]host[:port]/targetname/lun
@@ -834,8 +898,6 @@ static int iscsi_open(BlockDriverState *bs, const
char *filename, int flags)
struct iscsi_url *iscsi_url = NULL;
struct scsi_task *task = NULL;
struct scsi_inquiry_standard *inq = NULL;
-struct scsi_readcapacity10 *rc10 = NULL;
-struct scsi_readcapacity16 *rc16 = NULL;
char *initiator_name = NULL;
int ret;
@@ -925,48 +987,8 @@ static int iscsi_open(BlockDriverState *bs, const
char *filename, int flags)
iscsilun-type = inq-periperal_device_type;
-scsi_free_scsi_task(task);
-
-switch (iscsilun-type) {
-case TYPE_DISK:
-task = iscsi_readcapacity16_sync(iscsi, iscsilun-lun);
-if (task == NULL || task-status != SCSI_STATUS_GOOD) {
-error_report(iSCSI: failed to send readcapacity16 command.);
-ret = -EINVAL;
-goto out;
-}
-rc16 = scsi_datain_unmarshall(task);
-if (rc16 == NULL) {
-error_report(iSCSI: Failed to unmarshall readcapacity16
data.);
-ret = -EINVAL;
-goto out;
-}
-iscsilun-block_size = rc16-block_length;
-iscsilun-num_blocks = rc16-returned_lba + 1;
-break;
-case TYPE_ROM:
-task = iscsi_readcapacity10_sync(iscsi,
Re: [Qemu-devel] [PATCHv2] iscsi: retry read, write, flush and unmap on unit attention check conditions
Il 21/02/2013 16:15, Peter Lieven ha scritto:
the storage might return a check condition status for various reasons.
(e.g. bus reset, capacity change, thin-provisioning info etc.)
currently all these informative status responses lead to an I/O error
which is populated to the guest. this patch introduces a retry mechanism
to avoid this.
v2:
- fix duplication in iscsi_aio_discard()
- allow any type of unit attention check condition in
iscsi_readcapacity_sync().
Moved the iscsi_readcapacity_sync() hunk to the iscsi_truncate() patch
and applied to scsi-next branch, thanks.
Paolo
Signed-off-by: Peter Lieven p...@kamp.de
---
block/iscsi.c | 303
+++--
1 file changed, 209 insertions(+), 94 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 1ea290e..62ebd79 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -60,8 +60,11 @@ typedef struct IscsiAIOCB {
uint8_t *buf;
int status;
int canceled;
+int retries;
size_t read_size;
size_t read_offset;
+int64_t sector_num;
+int nb_sectors;
#ifdef __linux__
sg_io_hdr_t *ioh;
#endif
@@ -69,6 +72,7 @@ typedef struct IscsiAIOCB {
#define NOP_INTERVAL 5000
#define MAX_NOP_FAILURES 3
+#define ISCSI_CMD_RETRIES 5
static void
iscsi_bh_cb(void *p)
@@ -191,6 +195,8 @@ iscsi_process_write(void *arg)
iscsi_set_events(iscsilun);
}
+static int
+iscsi_aio_writev_acb(IscsiAIOCB *acb);
static void
iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
@@ -208,7 +214,19 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi,
int status,
}
acb-status = 0;
-if (status 0) {
+if (status != 0) {
+if (status == SCSI_STATUS_CHECK_CONDITION
+ acb-task-sense.key == SCSI_SENSE_UNIT_ATTENTION
+ acb-retries-- 0) {
+if (acb-task != NULL) {
+scsi_free_scsi_task(acb-task);
+acb-task = NULL;
+}
+if (iscsi_aio_writev_acb(acb) == 0) {
+iscsi_set_events(acb-iscsilun);
+return;
+}
+}
error_report(Failed to write16 data to iSCSI lun. %s,
iscsi_get_error(iscsi));
acb-status = -EIO;
@@ -222,15 +240,10 @@ static int64_t sector_qemu2lun(int64_t sector,
IscsiLun *iscsilun)
return sector * BDRV_SECTOR_SIZE / iscsilun-block_size;
}
-static BlockDriverAIOCB *
-iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
- QEMUIOVector *qiov, int nb_sectors,
- BlockDriverCompletionFunc *cb,
- void *opaque)
+static int
+iscsi_aio_writev_acb(IscsiAIOCB *acb)
{
-IscsiLun *iscsilun = bs-opaque;
-struct iscsi_context *iscsi = iscsilun-iscsi;
-IscsiAIOCB *acb;
+struct iscsi_context *iscsi = acb-iscsilun-iscsi;
size_t size;
uint32_t num_sectors;
uint64_t lba;
@@ -238,20 +251,14 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t
sector_num,
struct iscsi_data data;
#endif
int ret;
-
-acb = qemu_aio_get(iscsi_aiocb_info, bs, cb, opaque);
-trace_iscsi_aio_writev(iscsi, sector_num, nb_sectors, opaque, acb);
-
-acb-iscsilun = iscsilun;
-acb-qiov = qiov;
-
+
acb-canceled = 0;
acb-bh = NULL;
acb-status = -EINPROGRESS;
acb-buf= NULL;
/* this will allow us to get rid of 'buf' completely */
-size = nb_sectors * BDRV_SECTOR_SIZE;
+size = acb-nb_sectors * BDRV_SECTOR_SIZE;
#if !defined(LIBISCSI_FEATURE_IOVECTOR)
data.size = MIN(size, acb-qiov-size);
@@ -270,48 +277,76 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t
sector_num,
if (acb-task == NULL) {
error_report(iSCSI: Failed to allocate task for scsi WRITE16
command. %s, iscsi_get_error(iscsi));
-qemu_aio_release(acb);
-return NULL;
+return -1;
}
memset(acb-task, 0, sizeof(struct scsi_task));
acb-task-xfer_dir = SCSI_XFER_WRITE;
acb-task-cdb_size = 16;
acb-task-cdb[0] = 0x8a;
-lba = sector_qemu2lun(sector_num, iscsilun);
+lba = sector_qemu2lun(acb-sector_num, acb-iscsilun);
*(uint32_t *)acb-task-cdb[2] = htonl(lba 32);
*(uint32_t *)acb-task-cdb[6] = htonl(lba 0x);
-num_sectors = size / iscsilun-block_size;
+num_sectors = size / acb-iscsilun-block_size;
*(uint32_t *)acb-task-cdb[10] = htonl(num_sectors);
acb-task-expxferlen = size;
#if defined(LIBISCSI_FEATURE_IOVECTOR)
-ret = iscsi_scsi_command_async(iscsi, iscsilun-lun, acb-task,
+ret = iscsi_scsi_command_async(iscsi, acb-iscsilun-lun, acb-task,
iscsi_aio_write16_cb,
NULL,
acb);
#else
-ret =
Re: [Qemu-devel] [PATCH for-next v3 2/5] tmp105: Add debug output
On 16.02.2013, at 12:59, Andreas Färber wrote: Am 15.02.2013 14:14, schrieb Alexander Graf: In parallel to the completely disastrous user experience when using trace points. Debug printfs are easy and understandable. Tracepoints are not. However, how about we take this one gradually? +1, I'm looking for a minimally invasive solution that addresses my compilation-test needs. It doesn't need to be the final bells-and-whistles version. :) If all debug prints in all files do an #ifdef DEBUG static const debug_enabled = 1; #else static const debug_enabled = 0; #endif then Stefan can probably add a -DDEBUG to a specific c file through Makefile magic if he wants to do iPXE-style debugging. And if you're - like me - more happy with local #define DEBUG, then you can do that as well. Could you please clarify: Are you suggesting to consistently use just DEBUG in place of the various FOO_DEBUGs? That would enable all debug output for --enable-debug builds, wouldn't it? (Or am I mixing that up with NDEBUG in the opposite case...?) Ah, DEBUG is already taken. I was thinking of some define that only applies to files you want to debug, which you can then mark the debug_enabled variable to true with. I see 2 options to do this: - Add a DEBUG_THIS_FILE define. This define is only set for files you explicitly mark to debug. I don't know how hard it would be to do this for a Makefile magician - Convert file paths into define compatible strings. hw/ppc/ppc_newworld.c would become HW_PPC_MAC_NEWWORLD_C. In that file, check for DEBUG_HW_PPC_MAC_NEWWORLD_C and set debug to enabled if it's defined. With a small script that does the above conversion you could then maybe do make debug=hw/ppc/mac_newworld.c to easily enable debugging for that whole file. That's iPXE style :). Alex Or just having a static const variable to avoid #ifdef FOO_DEBUG for statements as done in openpic code? Andreas I would definitely oppose moving to tracepoints. Alex -- 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] [Qemu-ppc] [PATCH] prep: Fix software reset
On 16.02.2013, at 16:08, Julio Guerra wrote:
The software reset of a PReP machine should reset the entire system
and not only the processor. It occurs when changing the 7th bit of
port 0092 from 0 to 1.
Adding a new variable in PReP's sysctrl_t to store the soft reset bit
makes possible to be compliant with PReP specification :
* reset the system when changing soft reset bit from 0 to 1.
* the soft reset bit value is 1 after a soft reset.
* Port 0092 is read/write.
qemu_system_reset_request() does the required job (calling the reset
handlers) when the software reset is needed.
reset_irq is no longer needed, the CPU reset (calling ppc_prep_reset)
is called when qemu_system_reset calls every reset handlers.
Signed-off-by: Julio Guerra gu...@julio.in
Andreas, could you take this one through the prep queue please?
Alex
---
hw/ppc/prep.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index e06dded..64dab8b 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -178,12 +178,12 @@ static const MemoryRegionOps PPC_XCSR_ops = {
/* Fake super-io ports for PREP platform (Intel 82378ZB) */
typedef struct sysctrl_t {
-qemu_irq reset_irq;
M48t59State *nvram;
uint8_t state;
uint8_t syscontrol;
int contiguous_map;
int endian;
+uint8_t sreset;
} sysctrl_t;
enum {
@@ -203,9 +203,11 @@ static void PREP_io_800_writeb (void *opaque, uint32_t
addr, uint32_t val)
/* Special port 92 */
/* Check soft reset asked */
if (val 0x01) {
-qemu_irq_raise(sysctrl-reset_irq);
+ if (!sysctrl-sreset)
+qemu_system_reset_request();
+sysctrl-sreset = 1;
} else {
-qemu_irq_lower(sysctrl-reset_irq);
+sysctrl-sreset = 0;
}
/* Check LE mode */
if (val 0x02) {
@@ -267,7 +269,7 @@ static uint32_t PREP_io_800_readb (void *opaque, uint32_t
addr)
switch (addr) {
case 0x0092:
/* Special port 92 */
-retval = 0x00;
+retval = (sysctrl-endian 1) | sysctrl-sreset;
break;
case 0x0800:
/* Motorola CPU configuration register */
@@ -624,7 +626,8 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
}
isa_create_simple(isa_bus, i8042);
-sysctrl-reset_irq = first_cpu-irq_inputs[PPC6xx_INPUT_HRESET];
+sysctrl-sreset = 0;
+sysctrl-endian = 0;
/* System control ports */
register_ioport_read(0x0092, 0x01, 1, PREP_io_800_readb, sysctrl);
register_ioport_write(0x0092, 0x01, 1, PREP_io_800_writeb, sysctrl);
--
1.8.1.2
Re: [Qemu-devel] [PATCH] pseries: Implements h_read hcall
On 18.02.2013, at 16:00, Erlon Cruz wrote:
From: Erlon Cruz erlon.c...@br.flextronics.com
This h_call is useful for DLPAR in future amongst other things. Given an index
it fetches the corresponding PTE stored in the htab.
Signed-off-by: Erlon Cruz erlon.c...@br.flextronics.com
Thanks, applied to ppc-next.
Alex
---
hw/spapr_hcall.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 2889742..56394a1 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -323,6 +323,36 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
return H_SUCCESS;
}
+static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+ target_ulong opcode, target_ulong *args)
+{
+CPUPPCState *env = cpu-env;
+target_ulong flags = args[0];
+target_ulong pte_index = args[1];
+uint8_t *hpte;
+int i, ridx, n_entries = 1;
+
+if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) {
+return H_PARAMETER;
+}
+
+if (flags H_READ_4) {
+/* Clear the two low order bits */
+pte_index = ~(3ULL);
+n_entries = 4;
+}
+
+hpte = env-external_htab + (pte_index * HASH_PTE_SIZE_64);
+
+for (i = 0, ridx = 0; i n_entries; i++) {
+args[ridx++] = ldq_p(hpte);
+args[ridx++] = ldq_p(hpte + (HASH_PTE_SIZE_64/2));
+hpte += HASH_PTE_SIZE_64;
+}
+
+return H_SUCCESS;
+}
+
static target_ulong h_set_dabr(PowerPCCPU *cpu, sPAPREnvironment *spapr,
target_ulong opcode, target_ulong *args)
{
@@ -714,6 +744,7 @@ static void hypercall_register_types(void)
spapr_register_hypercall(H_ENTER, h_enter);
spapr_register_hypercall(H_REMOVE, h_remove);
spapr_register_hypercall(H_PROTECT, h_protect);
+spapr_register_hypercall(H_READ, h_read);
/* hcall-bulk */
spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
--
1.7.9.5
Re: [Qemu-devel] [PATCH v2 2/3] dtc: add submodule
On 19.02.2013, at 05:19, Peter Crosthwaite wrote: Add dtc submodule as a fallback for old distros. Picking version 1.3.0. as this is the most recently tagged stable version. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- .gitmodules |3 +++ dtc |1 + 2 files changed, 4 insertions(+), 0 deletions(-) create mode 16 dtc diff --git a/.gitmodules b/.gitmodules index cfa2af9..b426ea6 100644 --- a/.gitmodules +++ b/.gitmodules @@ -22,3 +22,6 @@ [submodule pixman] path = pixman url = git://anongit.freedesktop.org/pixman +[submodule dtc] + path = dtc + url = git://git.qemu.org/dtc.git diff --git a/dtc b/dtc new file mode 16 index 000..bc895d6 --- /dev/null +++ b/dtc I still don't think that we should have compat submodules in the top level directory. Alex @@ -0,0 +1 @@ +Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde -- 1.7.0.4
[Qemu-devel] KVM call agenda for 2013-02-26
Hi Please send in any agenda topics you are interested in. Later, Juan.
[Qemu-devel] [PATCH] page_cache: Fix memory leak
XBZRLE encoded migration introduced a MRU page cache meachnism.
Unfortunately, cached items where never freed on a collision.
This lead to out of memory conditions during XBZRLE migration
if the page cache was small and there where a lot of collisions.
Signed-off-by: Peter Lieven p...@kamp.de
---
page_cache.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/page_cache.c b/page_cache.c
index ba5640b..a6c3a15 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -152,8 +152,10 @@ void cache_insert(PageCache *cache, uint64_t addr, uint8_t
*pdata)
/* actual update of entry */
it = cache_get_by_addr(cache, addr);
-if (!it-it_data) {
+if (it-it_data == NULL) {
cache-num_items++;
+} else {
+g_free(it-it_data);
}
it-it_data = pdata;
--
1.7.9.5
Re: [Qemu-devel] [PATCH] Save memory allocation in the elf loader
On 19.02.2013, at 15:41, Fabien Chouteau wrote: The current elf loader uses too much memory. For example, I have a executable with a bss section of 400 MB and I set the ram size to 512 MB. Qemu uses about 780MB of RAM (which is fine), but there's a peak at 1.6 GB during initialization (this is not fine). This patch fixes two things: 1) do not allocate each elf program twice. 2) do not allocate memory for areas that are only zeros. For this we need a new field in Rom: datasize which is the size of the allocated data. If datasize is less than romsize, it means that the area from datasize to romsize is filled with zeros. Signed-off-by: Fabien Chouteau chout...@adacore.com In the long run, we should make the elf loader not use the rom framework at all, but instead provide its own reset functions. However, for now I think this is a valuable optimization. Thanks for looking into it :). Applied to ppc-next. Alex
[Qemu-devel] [PATCH] page_cache: dup memory on insert
The page cache frees all data on finish, on resize and
if there is collision on insert. So it should be the caches
responsibility to dup the data that is stored in the cache.
Signed-off-by: Peter Lieven p...@kamp.de
---
arch_init.c|3 +--
include/migration/page_cache.h |3 ++-
page_cache.c |2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 8da868b..97bcc29 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -293,8 +293,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t
*current_data,
if (!cache_is_cached(XBZRLE.cache, current_addr)) {
if (!last_stage) {
-cache_insert(XBZRLE.cache, current_addr,
- g_memdup(current_data, TARGET_PAGE_SIZE));
+cache_insert(XBZRLE.cache, current_addr, current_data);
}
acct_info.xbzrle_cache_miss++;
return -1;
diff --git a/include/migration/page_cache.h b/include/migration/page_cache.h
index 3839ac7..87894fe 100644
--- a/include/migration/page_cache.h
+++ b/include/migration/page_cache.h
@@ -57,7 +57,8 @@ bool cache_is_cached(const PageCache *cache, uint64_t addr);
uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
/**
- * cache_insert: insert the page into the cache. the previous value will be
overwritten
+ * cache_insert: insert the page into the cache. the page cache
+ * will dup the data on insert. the previous value will be overwritten
*
* @cache pointer to the PageCache struct
* @addr: page address
diff --git a/page_cache.c b/page_cache.c
index a6c3a15..e670d91 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -158,7 +158,7 @@ void cache_insert(PageCache *cache, uint64_t addr, uint8_t
*pdata)
g_free(it-it_data);
}
-it-it_data = pdata;
+it-it_data = g_memdup(pdata, cache-page_size);
it-it_age = ++cache-max_item_age;
it-it_addr = addr;
}
--
1.7.9.5
Re: [Qemu-devel] [PATCH] page_cache: Fix memory leak
On 02/25/2013 01:42 PM, Peter Lieven wrote:
XBZRLE encoded migration introduced a MRU page cache meachnism.
Unfortunately, cached items where never freed on a collision.
This lead to out of memory conditions during XBZRLE migration
if the page cache was small and there where a lot of collisions.
Signed-off-by: Peter Lieven p...@kamp.de
---
page_cache.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/page_cache.c b/page_cache.c
index ba5640b..a6c3a15 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -152,8 +152,10 @@ void cache_insert(PageCache *cache, uint64_t addr,
uint8_t *pdata)
/* actual update of entry */
it = cache_get_by_addr(cache, addr);
-if (!it-it_data) {
+if (it-it_data == NULL) {
cache-num_items++;
+} else {
+g_free(it-it_data);
Why? we don't allocate here but just store the pointer.
It is the caller responsibility to allocate/free the data,
for example for migration it is the guest memory page.
Orit
}
it-it_data = pdata;
Re: [Qemu-devel] [PATCH 00/38] Add double-word addition and widening multiply tcg ops
On 20.02.2013, at 08:51, Richard Henderson wrote: ... and use them where appropriate in the targets. As can be seen, most targets can make use of a widening multiply. And if one is sufficiently clever, one can use add2 to significantly improve carry generation for the target. This patch series is based on the target-i386 flags rewrite that I just posted, but except for the two target-i386 patches, isn't really dependant on it. This patch series also cleans up target-ppc more ought to be present here, but I didn't feel up to breaking it out as a prerequisite. PPC bits are: Reviewed-by: Alexander Graf ag...@suse.de Alex
Re: [Qemu-devel] [PATCH] page_cache: Fix memory leak
Am 25.02.2013 um 12:58 schrieb Orit Wasserman owass...@redhat.com:
On 02/25/2013 01:42 PM, Peter Lieven wrote:
XBZRLE encoded migration introduced a MRU page cache meachnism.
Unfortunately, cached items where never freed on a collision.
This lead to out of memory conditions during XBZRLE migration
if the page cache was small and there where a lot of collisions.
Signed-off-by: Peter Lieven p...@kamp.de
---
page_cache.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/page_cache.c b/page_cache.c
index ba5640b..a6c3a15 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -152,8 +152,10 @@ void cache_insert(PageCache *cache, uint64_t addr,
uint8_t *pdata)
/* actual update of entry */
it = cache_get_by_addr(cache, addr);
-if (!it-it_data) {
+if (it-it_data == NULL) {
cache-num_items++;
+} else {
+g_free(it-it_data);
Why? we don't allocate here but just store the pointer.
It is the caller responsibility to allocate/free the data,
for example for migration it is the guest memory page.
It is being dupped when the page is added to the cache.
cache_insert(XBZRLE.cache, current_addr,
g_memdup(current_data, TARGET_PAGE_SIZE));
This is, of course, necessary because the memory might change
and you need the state of the page that has been transferred to the
destination.
It is also obvious that the cache needs to hold his own copy because
it frees the data on finish and on resize.
Peter
Orit
}
it-it_data = pdata;
Re: [Qemu-devel] [PATCH] page_cache: dup memory on insert
On 25 February 2013 11:52, Peter Lieven p...@dlhnet.de wrote: The page cache frees all data on finish, on resize and if there is collision on insert. So it should be the caches responsibility to dup the data that is stored in the cache. diff --git a/page_cache.c b/page_cache.c index a6c3a15..e670d91 100644 --- a/page_cache.c +++ b/page_cache.c @@ -158,7 +158,7 @@ void cache_insert(PageCache *cache, uint64_t addr, uint8_t *pdata) g_free(it-it_data); } -it-it_data = pdata; +it-it_data = g_memdup(pdata, cache-page_size); it-it_age = ++cache-max_item_age; it-it_addr = addr; } Doesn't this introduce a leak on cache resize in the case where the element being moved from the old cache to the new does not collide with any element we've already moved? [ie the code path where we just cache_insert() the old item's data]. -- PMM
Re: [Qemu-devel] [PATCH] page_cache: dup memory on insert
Hi Peter,
Now I get the previous patch, it should have been a patch series :).
The reason we allocate from outside of the page cache is because of cache_resize
that also uses cache_insert but doesn't duplicate the buffer.
There is no memory leak because if the page is cached we don't call cache_insert
but do memcpy instead.
Regards,
Orit
On 02/25/2013 01:52 PM, Peter Lieven wrote:
The page cache frees all data on finish, on resize and
if there is collision on insert. So it should be the caches
responsibility to dup the data that is stored in the cache.
Signed-off-by: Peter Lieven p...@kamp.de
---
arch_init.c|3 +--
include/migration/page_cache.h |3 ++-
page_cache.c |2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 8da868b..97bcc29 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -293,8 +293,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t
*current_data,
if (!cache_is_cached(XBZRLE.cache, current_addr)) {
if (!last_stage) {
-cache_insert(XBZRLE.cache, current_addr,
- g_memdup(current_data, TARGET_PAGE_SIZE));
+cache_insert(XBZRLE.cache, current_addr, current_data);
}
acct_info.xbzrle_cache_miss++;
return -1;
diff --git a/include/migration/page_cache.h b/include/migration/page_cache.h
index 3839ac7..87894fe 100644
--- a/include/migration/page_cache.h
+++ b/include/migration/page_cache.h
@@ -57,7 +57,8 @@ bool cache_is_cached(const PageCache *cache, uint64_t addr);
uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
/**
- * cache_insert: insert the page into the cache. the previous value will be
overwritten
+ * cache_insert: insert the page into the cache. the page cache
+ * will dup the data on insert. the previous value will be overwritten
*
* @cache pointer to the PageCache struct
* @addr: page address
diff --git a/page_cache.c b/page_cache.c
index a6c3a15..e670d91 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -158,7 +158,7 @@ void cache_insert(PageCache *cache, uint64_t addr,
uint8_t *pdata)
g_free(it-it_data);
}
-it-it_data = pdata;
+it-it_data = g_memdup(pdata, cache-page_size);
it-it_age = ++cache-max_item_age;
it-it_addr = addr;
}
Re: [Qemu-devel] [PATCH] page_cache: dup memory on insert
Am 25.02.2013 um 13:15 schrieb Orit Wasserman owass...@redhat.com:
Hi Peter,
Now I get the previous patch, it should have been a patch series :).
The reason we allocate from outside of the page cache is because of
cache_resize
that also uses cache_insert but doesn't duplicate the buffer.
There is no memory leak because if the page is cached we don't call
cache_insert
but do memcpy instead.
Ah ok, haven't noticed the resize case. But there is still a men leak on a
simple collision (first patch) - nothing
to do with resize.
We should discuss if the page cache frees data it should also allocate the
data. Maybe we need to different functions.
Peter
Regards,
Orit
On 02/25/2013 01:52 PM, Peter Lieven wrote:
The page cache frees all data on finish, on resize and
if there is collision on insert. So it should be the caches
responsibility to dup the data that is stored in the cache.
Signed-off-by: Peter Lieven p...@kamp.de
---
arch_init.c|3 +--
include/migration/page_cache.h |3 ++-
page_cache.c |2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 8da868b..97bcc29 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -293,8 +293,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t
*current_data,
if (!cache_is_cached(XBZRLE.cache, current_addr)) {
if (!last_stage) {
-cache_insert(XBZRLE.cache, current_addr,
- g_memdup(current_data, TARGET_PAGE_SIZE));
+cache_insert(XBZRLE.cache, current_addr, current_data);
}
acct_info.xbzrle_cache_miss++;
return -1;
diff --git a/include/migration/page_cache.h b/include/migration/page_cache.h
index 3839ac7..87894fe 100644
--- a/include/migration/page_cache.h
+++ b/include/migration/page_cache.h
@@ -57,7 +57,8 @@ bool cache_is_cached(const PageCache *cache, uint64_t
addr);
uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
/**
- * cache_insert: insert the page into the cache. the previous value will be
overwritten
+ * cache_insert: insert the page into the cache. the page cache
+ * will dup the data on insert. the previous value will be overwritten
*
* @cache pointer to the PageCache struct
* @addr: page address
diff --git a/page_cache.c b/page_cache.c
index a6c3a15..e670d91 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -158,7 +158,7 @@ void cache_insert(PageCache *cache, uint64_t addr,
uint8_t *pdata)
g_free(it-it_data);
}
-it-it_data = pdata;
+it-it_data = g_memdup(pdata, cache-page_size);
it-it_age = ++cache-max_item_age;
it-it_addr = addr;
}
Re: [Qemu-devel] [PATCH] page_cache: dup memory on insert
Am 25.02.2013 um 13:13 schrieb Peter Maydell peter.mayd...@linaro.org: On 25 February 2013 11:52, Peter Lieven p...@dlhnet.de wrote: The page cache frees all data on finish, on resize and if there is collision on insert. So it should be the caches responsibility to dup the data that is stored in the cache. diff --git a/page_cache.c b/page_cache.c index a6c3a15..e670d91 100644 --- a/page_cache.c +++ b/page_cache.c @@ -158,7 +158,7 @@ void cache_insert(PageCache *cache, uint64_t addr, uint8_t *pdata) g_free(it-it_data); } -it-it_data = pdata; +it-it_data = g_memdup(pdata, cache-page_size); it-it_age = ++cache-max_item_age; it-it_addr = addr; } Doesn't this introduce a leak on cache resize in the case where the element being moved from the old cache to the new does not collide with any element we've already moved? [ie the code path where we just cache_insert() the old item's data]. you are right. maybe we need different functions for inserts made during resize and inserts from outside. Peter -- PMM
Re: [Qemu-devel] Moving BIOS tables from SeaBIOS to QEMU
Il 25/02/2013 09:51, Gleb Natapov ha scritto: 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. Again IIRC there are still OSes that uses mptable to obtain irq information. See 928d4dffef5c374. It should work to use a fixed mptable that overrides interrupts 5/9/10/11, like we do in the MADT. It doesn't need to be just the interrupts that are in use. Paolo
Re: [Qemu-devel] [0/3] Use ONE_REG interface to synchronize more state with KVM
On 21.02.2013, at 03:41, David Gibson wrote: At the moment, there is a significant amount of state which both qemu and KVM track, which is not synchronized between the two. In a KVM setup, qemu never updates that information, so we mostly get away with it, but we'll need this data in qemu to implement savevm and migration. This series improves the situation, synchronizing several bits of state that previously weren't. There may some more yet to go, but this at least gets most of it. (Note, this series should be completely independent of my MMU cleanup work, so order shouldn't matter.) Thanks, applied 1/3 and 2/3 to ppc-next. 3/3 has outstanding comments :). Alex
Re: [Qemu-devel] [PATCH] page_cache: Fix memory leak
On 25 February 2013 11:42, Peter Lieven p...@dlhnet.de wrote:
XBZRLE encoded migration introduced a MRU page cache meachnism.
Unfortunately, cached items where never freed on a collision.
This lead to out of memory conditions during XBZRLE migration
if the page cache was small and there where a lot of collisions.
Signed-off-by: Peter Lieven p...@kamp.de
---
page_cache.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/page_cache.c b/page_cache.c
index ba5640b..a6c3a15 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -152,8 +152,10 @@ void cache_insert(PageCache *cache, uint64_t addr,
uint8_t *pdata)
/* actual update of entry */
it = cache_get_by_addr(cache, addr);
-if (!it-it_data) {
+if (it-it_data == NULL) {
Please don't make minor syntactic tweaks in patches like this,
it makes them harder to review.
cache-num_items++;
+} else {
+g_free(it-it_data);
}
g_free(NULL) is OK so you could just make it unconditional.
Looking at the code in general I think it is probably the
right thing to move it to cache owns (ie duplicates
and frees) the data, since the code is already most of
the way there and just has some bits which aren't.
-- PMM
Re: [Qemu-devel] [PATCH v2 2/3] dtc: add submodule
Hi, diff --git a/dtc b/dtc new file mode 16 index 000..bc895d6 --- /dev/null +++ b/dtc I still don't think that we should have compat submodules in the top level directory. Don't think this is a big issue. We have only two now. If the number grows to 4-5 (which I don't expect happening) it's worth moving them into a compat subdir to not clutter the toplevel dir too much. cheers, Gerd
Re: [Qemu-devel] [Qemu-ppc] [PATCH] prep: Fix software reset
Am 25.02.2013 12:20, schrieb Alexander Graf: On 16.02.2013, at 16:08, Julio Guerra wrote: The software reset of a PReP machine should reset the entire system and not only the processor. It occurs when changing the 7th bit of port 0092 from 0 to 1. Adding a new variable in PReP's sysctrl_t to store the soft reset bit makes possible to be compliant with PReP specification : * reset the system when changing soft reset bit from 0 to 1. * the soft reset bit value is 1 after a soft reset. * Port 0092 is read/write. qemu_system_reset_request() does the required job (calling the reset handlers) when the software reset is needed. reset_irq is no longer needed, the CPU reset (calling ppc_prep_reset) is called when qemu_system_reset calls every reset handlers. Signed-off-by: Julio Guerra gu...@julio.in Andreas, could you take this one through the prep queue please? It's PReP only, so I intend to handle it. But apart from checkpatch.pl style problems (that I could fix up myself) this is touching on the same soft reset topic that I am awaiting the outcome for x86. The issue of returning endianness bit for 0x0092 is independent of that and could be split out. I want a qtest case though, therefore my interest in Markus' series. Could become a separate prep-test though. Andreas
Re: [Qemu-devel] [PATCH 3/3] target-ppc: Synchronize VPA state with KVM
On 21.02.2013, at 03:41, David Gibson wrote:
For PAPR guests, KVM tracks the various areas registered with the
H_REGISTER_VPA hypercall. For full emulation, of course, these are tracked
within qemu. At present these values are not synchronized. This is a
problem for reset (qemu's reset of the VPA address is not pushed to KVM)
and will also be a problem for savevm / migration.
The kernel now supports accessing the VPA state via the ONE_REG interface,
this patch adds code to qemu to use that interface to keep the qemu and
KVM ideas of the VPA state synchronized.
Signed-off-by: David Gibson da...@gibson.dropbear.id.au
---
target-ppc/kvm.c | 122 ++
1 file changed, 122 insertions(+)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 5a6f608..ac310c1 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -62,6 +62,7 @@ static int cap_ppc_rma;
static int cap_spapr_tce;
static int cap_hior;
static int cap_one_reg;
+static int cap_papr;
/* 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
@@ -92,6 +93,8 @@ int kvm_arch_init(KVMState *s)
cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
cap_one_reg = kvm_check_extension(s, KVM_CAP_ONE_REG);
cap_hior = kvm_check_extension(s, KVM_CAP_PPC_HIOR);
+/* Note: we don't set cap_papr here, because this capability is
+ * only activated after this by kvmppc_set_papr() */
if (!cap_interrupt_level) {
fprintf(stderr, KVM: Couldn't find level irq capability. Expect the
@@ -647,6 +650,103 @@ static int kvm_get_fp(CPUState *cs)
return 0;
}
+#if defined(TARGET_PPC64)
+static int kvm_get_vpa(CPUState *cs)
+{
+PowerPCCPU *cpu = POWERPC_CPU(cs);
+CPUPPCState *env = cpu-env;
+struct kvm_one_reg reg;
+int ret;
+
+reg.id = KVM_REG_PPC_VPA_ADDR;
+reg.addr = (uintptr_t)env-vpa_addr;
+ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, reg);
+if (ret 0) {
+dprintf(Unable to get VPA address from KVM: %s\n, strerror(errno));
+return ret;
+}
+
+assert((uintptr_t)env-slb_shadow_size
+ == ((uintptr_t)env-slb_shadow_addr + 8));
+reg.id = KVM_REG_PPC_VPA_SLB;
+reg.addr = (uintptr_t)env-slb_shadow_addr;
+ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, reg);
+if (ret 0) {
+dprintf(Unable to get SLB shadow state from KVM: %s\n,
+strerror(errno));
+return ret;
+}
+
+assert((uintptr_t)env-dtl_size == ((uintptr_t)env-dtl_addr + 8));
+reg.id = KVM_REG_PPC_VPA_DTL;
+reg.addr = (uintptr_t)env-dtl_addr;
+ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, reg);
+if (ret 0) {
+dprintf(Unable to get dispatch trace log state from KVM: %s\n,
+strerror(errno));
+return ret;
+}
+
+return 0;
+}
+
+static int kvm_put_vpa(CPUState *cs)
+{
+PowerPCCPU *cpu = POWERPC_CPU(cs);
+CPUPPCState *env = cpu-env;
+struct kvm_one_reg reg;
+int ret;
+
+/* SLB shadow or DTL can't be registered unless a master VPA is
+ * registered. That means when restoring state, if a VPA *is*
+ * registered, we need to set that up first. If not, we need to
+ * deregister the others before deregistering the master VPA */
+assert(env-vpa_addr || !(env-slb_shadow_addr || env-dtl_addr));
+
+if (env-vpa_addr) {
+reg.id = KVM_REG_PPC_VPA_ADDR;
+reg.addr = (uintptr_t)env-vpa_addr;
+ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg);
+if (ret 0) {
+dprintf(Unable to set VPA address to KVM: %s\n,
strerror(errno));
+return ret;
+}
+}
+
+assert((uintptr_t)env-slb_shadow_size
+ == ((uintptr_t)env-slb_shadow_addr + 8));
+reg.id = KVM_REG_PPC_VPA_SLB;
+reg.addr = (uintptr_t)env-slb_shadow_addr;
+ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg);
+if (ret 0) {
+dprintf(Unable to set SLB shadow state to KVM: %s\n,
strerror(errno));
+return ret;
+}
+
+assert((uintptr_t)env-dtl_size == ((uintptr_t)env-dtl_addr + 8));
+reg.id = KVM_REG_PPC_VPA_DTL;
+reg.addr = (uintptr_t)env-dtl_addr;
+ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg);
+if (ret 0) {
+dprintf(Unable to set dispatch trace log state to KVM: %s\n,
+strerror(errno));
+return ret;
+}
+
+if (!env-vpa_addr) {
+reg.id = KVM_REG_PPC_VPA_ADDR;
+reg.addr = (uintptr_t)env-vpa_addr;
+ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg);
+if (ret 0) {
+dprintf(Unable to set VPA address to KVM: %s\n,
strerror(errno));
+return ret;
+}
+}
+
+return 0;
+}
+#endif /* TARGET_PPC64 */
+
int kvm_arch_put_registers(CPUState *cs, int level)
{
Re: [Qemu-devel] [PATCH] page_cache: Fix memory leak
Am 25.02.2013 um 13:21 schrieb Peter Maydell peter.mayd...@linaro.org:
On 25 February 2013 11:42, Peter Lieven p...@dlhnet.de wrote:
XBZRLE encoded migration introduced a MRU page cache meachnism.
Unfortunately, cached items where never freed on a collision.
This lead to out of memory conditions during XBZRLE migration
if the page cache was small and there where a lot of collisions.
Signed-off-by: Peter Lieven p...@kamp.de
---
page_cache.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/page_cache.c b/page_cache.c
index ba5640b..a6c3a15 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -152,8 +152,10 @@ void cache_insert(PageCache *cache, uint64_t addr,
uint8_t *pdata)
/* actual update of entry */
it = cache_get_by_addr(cache, addr);
-if (!it-it_data) {
+if (it-it_data == NULL) {
Please don't make minor syntactic tweaks in patches like this,
it makes them harder to review.
cache-num_items++;
+} else {
+g_free(it-it_data);
}
g_free(NULL) is OK so you could just make it unconditional.
Looking at the code in general I think it is probably the
right thing to move it to cache owns (ie duplicates
and frees) the data, since the code is already most of
the way there and just has some bits which aren't.
ok, i just had a look at the call to cache_insert from cache_resize.
there cache_insert is only called if it_data == NULL so it doesn`t
do any harm.
May intention was to fix the memory leak. I have noticed that XBZRLE
migration was broken somewhen last year and just haven`t had the time
to look into this. This leak is definitely critical as it kills the source VM
easily if there is sufficient activity.
We can postpone the discussion to change the semantics or do not
enter any discussion as week. I don`t mind.
Peter
-- PMM
Re: [Qemu-devel] [PATCH] page_cache: dup memory on insert
On 02/25/2013 02:17 PM, Peter Lieven wrote:
Am 25.02.2013 um 13:15 schrieb Orit Wasserman owass...@redhat.com:
Hi Peter,
Now I get the previous patch, it should have been a patch series :).
The reason we allocate from outside of the page cache is because of
cache_resize
that also uses cache_insert but doesn't duplicate the buffer.
There is no memory leak because if the page is cached we don't call
cache_insert
but do memcpy instead.
Ah ok, haven't noticed the resize case. But there is still a men leak on a
simple collision (first patch) - nothing
to do with resize.
There is no memory leak because the migration code only call the cache_insert
if the page is not cached
(i.e no collision) and the cache_resize calls it when there a collision but
doesn't allocate.
We can split the code to two, on internal insert function that doesn't allocate
for cache_resize to call
and one external that duplicate the buffer (and calls the internal function).
The external function should abort if there is a collision.
Orit
We should discuss if the page cache frees data it should also allocate the
data. Maybe we need to different functions.
Peter
Regards,
Orit
On 02/25/2013 01:52 PM, Peter Lieven wrote:
The page cache frees all data on finish, on resize and
if there is collision on insert. So it should be the caches
responsibility to dup the data that is stored in the cache.
Signed-off-by: Peter Lieven p...@kamp.de
---
arch_init.c|3 +--
include/migration/page_cache.h |3 ++-
page_cache.c |2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 8da868b..97bcc29 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -293,8 +293,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t
*current_data,
if (!cache_is_cached(XBZRLE.cache, current_addr)) {
if (!last_stage) {
-cache_insert(XBZRLE.cache, current_addr,
- g_memdup(current_data, TARGET_PAGE_SIZE));
+cache_insert(XBZRLE.cache, current_addr, current_data);
}
acct_info.xbzrle_cache_miss++;
return -1;
diff --git a/include/migration/page_cache.h b/include/migration/page_cache.h
index 3839ac7..87894fe 100644
--- a/include/migration/page_cache.h
+++ b/include/migration/page_cache.h
@@ -57,7 +57,8 @@ bool cache_is_cached(const PageCache *cache, uint64_t
addr);
uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
/**
- * cache_insert: insert the page into the cache. the previous value will
be overwritten
+ * cache_insert: insert the page into the cache. the page cache
+ * will dup the data on insert. the previous value will be overwritten
*
* @cache pointer to the PageCache struct
* @addr: page address
diff --git a/page_cache.c b/page_cache.c
index a6c3a15..e670d91 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -158,7 +158,7 @@ void cache_insert(PageCache *cache, uint64_t addr,
uint8_t *pdata)
g_free(it-it_data);
}
-it-it_data = pdata;
+it-it_data = g_memdup(pdata, cache-page_size);
it-it_age = ++cache-max_item_age;
it-it_addr = addr;
}
Re: [Qemu-devel] [PATCH v2 2/3] dtc: add submodule
On 25.02.2013, at 13:24, Gerd Hoffmann wrote: Hi, diff --git a/dtc b/dtc new file mode 16 index 000..bc895d6 --- /dev/null +++ b/dtc I still don't think that we should have compat submodules in the top level directory. Don't think this is a big issue. We have only two now. If the number grows to 4-5 (which I don't expect happening) it's worth moving them into a compat subdir to not clutter the toplevel dir too much. Would it be much work to do so now? Alex
Re: [Qemu-devel] [PATCH] page_cache: dup memory on insert
On 25 February 2013 12:17, Peter Lieven p...@dlhnet.de wrote:
Am 25.02.2013 um 13:13 schrieb Peter Maydell peter.mayd...@linaro.org:
Doesn't this introduce a leak on cache resize in the case where
the element being moved from the old cache to the new does not
collide with any element we've already moved? [ie the code
path where we just cache_insert() the old item's data].
you are right. maybe we need different functions for inserts made
during resize and inserts from outside.
Looking a little more closely, I think you need that anyway,
because cache_insert() updates the it_age field, when I would
have expected that in the just moving everything over to the
resized cache case we would want to retain the existing age.
Since cache_resize() already has code in it that does a
direct access and copy of CacheItem* fields [for the copy
old_it over new_it case] it might be cleaner to either
(a) have all the cases open-coded in cache_resize() rather
than calling cache_insert(), or (b) abstract out the
whole of the resize inner loop into something like
cache_insert_item(PageCache *cache, CacheItem *olditem)
{
/* Insert olditem (an item from a different page cache) into this one.
* If there is a collision then we keep the data from whichever of
* olditem and the existing entry is newer. In either case, olditem's
* data pointer is either copied into the new cache or freed, so
* the caller must do nothing further with it.
*/
}
Extra bonus leak: I think cache_resize() is leaking
cache-page_cache.
-- PMM
Re: [Qemu-devel] [PATCH] page_cache: dup memory on insert
Am 25.02.2013 um 13:33 schrieb Orit Wasserman owass...@redhat.com:
On 02/25/2013 02:17 PM, Peter Lieven wrote:
Am 25.02.2013 um 13:15 schrieb Orit Wasserman owass...@redhat.com:
Hi Peter,
Now I get the previous patch, it should have been a patch series :).
The reason we allocate from outside of the page cache is because of
cache_resize
that also uses cache_insert but doesn't duplicate the buffer.
There is no memory leak because if the page is cached we don't call
cache_insert
but do memcpy instead.
Ah ok, haven't noticed the resize case. But there is still a men leak on a
simple collision (first patch) - nothing
to do with resize.
There is no memory leak because the migration code only call the cache_insert
if the page is not cached
(i.e no collision) and the cache_resize calls it when there a collision but
doesn't allocate.
This is not true. cache_insert is actually called in 2 cases:
a) there is no entry in the cache at the position calculated for current_addr
(it_addr = -1)
b) there is an entry, but the address of the cached entry does not match
current_addr (collision)
Peter
We can split the code to two, on internal insert function that doesn't
allocate for cache_resize to call
and one external that duplicate the buffer (and calls the internal function).
The external function should abort if there is a collision.
Orit
We should discuss if the page cache frees data it should also allocate the
data. Maybe we need to different functions.
Peter
Regards,
Orit
On 02/25/2013 01:52 PM, Peter Lieven wrote:
The page cache frees all data on finish, on resize and
if there is collision on insert. So it should be the caches
responsibility to dup the data that is stored in the cache.
Signed-off-by: Peter Lieven p...@kamp.de
---
arch_init.c|3 +--
include/migration/page_cache.h |3 ++-
page_cache.c |2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 8da868b..97bcc29 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -293,8 +293,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t
*current_data,
if (!cache_is_cached(XBZRLE.cache, current_addr)) {
if (!last_stage) {
-cache_insert(XBZRLE.cache, current_addr,
- g_memdup(current_data, TARGET_PAGE_SIZE));
+cache_insert(XBZRLE.cache, current_addr, current_data);
}
acct_info.xbzrle_cache_miss++;
return -1;
diff --git a/include/migration/page_cache.h
b/include/migration/page_cache.h
index 3839ac7..87894fe 100644
--- a/include/migration/page_cache.h
+++ b/include/migration/page_cache.h
@@ -57,7 +57,8 @@ bool cache_is_cached(const PageCache *cache, uint64_t
addr);
uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
/**
- * cache_insert: insert the page into the cache. the previous value will
be overwritten
+ * cache_insert: insert the page into the cache. the page cache
+ * will dup the data on insert. the previous value will be overwritten
*
* @cache pointer to the PageCache struct
* @addr: page address
diff --git a/page_cache.c b/page_cache.c
index a6c3a15..e670d91 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -158,7 +158,7 @@ void cache_insert(PageCache *cache, uint64_t addr,
uint8_t *pdata)
g_free(it-it_data);
}
-it-it_data = pdata;
+it-it_data = g_memdup(pdata, cache-page_size);
it-it_age = ++cache-max_item_age;
it-it_addr = addr;
}
Re: [Qemu-devel] [PATCH v2 4/8] xilinx_axienet: converted init-realize
Am 25.02.2013 09:50, schrieb Peter Crosthwaite: The prescribed transition from SysBusDevice::init to Device::realize. Im going with Andreas suggestion to move the sysbus foo to Object::init for early IRQ visibility. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Andreas Färber afaer...@suse.de Thanks, 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] page_cache: dup memory on insert
On 02/25/2013 02:33 PM, Orit Wasserman wrote:
On 02/25/2013 02:17 PM, Peter Lieven wrote:
Am 25.02.2013 um 13:15 schrieb Orit Wasserman owass...@redhat.com:
Hi Peter,
Now I get the previous patch, it should have been a patch series :).
The reason we allocate from outside of the page cache is because of
cache_resize
that also uses cache_insert but doesn't duplicate the buffer.
There is no memory leak because if the page is cached we don't call
cache_insert
but do memcpy instead.
Ah ok, haven't noticed the resize case. But there is still a men leak on a
simple collision (first patch) - nothing
to do with resize.
There is no memory leak because the migration code only call the cache_insert
if the page is not cached
(i.e no collision) and the cache_resize calls it when there a collision but
doesn't allocate.
We can split the code to two, on internal insert function that doesn't
allocate for cache_resize to call
and one external that duplicate the buffer (and calls the internal function).
The external function should abort if there is a collision.
Orit
Fixing myself :),
There is a leak in case of a collision, so no need to abort in the external
function but free the memory.
Orit
We should discuss if the page cache frees data it should also allocate the
data. Maybe we need to different functions.
Peter
Regards,
Orit
On 02/25/2013 01:52 PM, Peter Lieven wrote:
The page cache frees all data on finish, on resize and
if there is collision on insert. So it should be the caches
responsibility to dup the data that is stored in the cache.
Signed-off-by: Peter Lieven p...@kamp.de
---
arch_init.c|3 +--
include/migration/page_cache.h |3 ++-
page_cache.c |2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 8da868b..97bcc29 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -293,8 +293,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t
*current_data,
if (!cache_is_cached(XBZRLE.cache, current_addr)) {
if (!last_stage) {
-cache_insert(XBZRLE.cache, current_addr,
- g_memdup(current_data, TARGET_PAGE_SIZE));
+cache_insert(XBZRLE.cache, current_addr, current_data);
}
acct_info.xbzrle_cache_miss++;
return -1;
diff --git a/include/migration/page_cache.h
b/include/migration/page_cache.h
index 3839ac7..87894fe 100644
--- a/include/migration/page_cache.h
+++ b/include/migration/page_cache.h
@@ -57,7 +57,8 @@ bool cache_is_cached(const PageCache *cache, uint64_t
addr);
uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
/**
- * cache_insert: insert the page into the cache. the previous value will
be overwritten
+ * cache_insert: insert the page into the cache. the page cache
+ * will dup the data on insert. the previous value will be overwritten
*
* @cache pointer to the PageCache struct
* @addr: page address
diff --git a/page_cache.c b/page_cache.c
index a6c3a15..e670d91 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -158,7 +158,7 @@ void cache_insert(PageCache *cache, uint64_t addr,
uint8_t *pdata)
g_free(it-it_data);
}
-it-it_data = pdata;
+it-it_data = g_memdup(pdata, cache-page_size);
it-it_age = ++cache-max_item_age;
it-it_addr = addr;
}
Re: [Qemu-devel] [PATCH] page_cache: dup memory on insert
On 02/25/2013 02:37 PM, Peter Lieven wrote:
Am 25.02.2013 um 13:33 schrieb Orit Wasserman owass...@redhat.com:
On 02/25/2013 02:17 PM, Peter Lieven wrote:
Am 25.02.2013 um 13:15 schrieb Orit Wasserman owass...@redhat.com:
Hi Peter,
Now I get the previous patch, it should have been a patch series :).
The reason we allocate from outside of the page cache is because of
cache_resize
that also uses cache_insert but doesn't duplicate the buffer.
There is no memory leak because if the page is cached we don't call
cache_insert
but do memcpy instead.
Ah ok, haven't noticed the resize case. But there is still a men leak on a
simple collision (first patch) - nothing
to do with resize.
There is no memory leak because the migration code only call the
cache_insert if the page is not cached
(i.e no collision) and the cache_resize calls it when there a collision but
doesn't allocate.
This is not true. cache_insert is actually called in 2 cases:
a) there is no entry in the cache at the position calculated for current_addr
(it_addr = -1)
b) there is an entry, but the address of the cached entry does not match
current_addr (collision)
Peter
You are right, good catch.
I will send a fixed patch
Orit
We can split the code to two, on internal insert function that doesn't
allocate for cache_resize to call
and one external that duplicate the buffer (and calls the internal function).
The external function should abort if there is a collision.
Orit
We should discuss if the page cache frees data it should also allocate the
data. Maybe we need to different functions.
Peter
Regards,
Orit
On 02/25/2013 01:52 PM, Peter Lieven wrote:
The page cache frees all data on finish, on resize and
if there is collision on insert. So it should be the caches
responsibility to dup the data that is stored in the cache.
Signed-off-by: Peter Lieven p...@kamp.de
---
arch_init.c|3 +--
include/migration/page_cache.h |3 ++-
page_cache.c |2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 8da868b..97bcc29 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -293,8 +293,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t
*current_data,
if (!cache_is_cached(XBZRLE.cache, current_addr)) {
if (!last_stage) {
-cache_insert(XBZRLE.cache, current_addr,
- g_memdup(current_data, TARGET_PAGE_SIZE));
+cache_insert(XBZRLE.cache, current_addr, current_data);
}
acct_info.xbzrle_cache_miss++;
return -1;
diff --git a/include/migration/page_cache.h
b/include/migration/page_cache.h
index 3839ac7..87894fe 100644
--- a/include/migration/page_cache.h
+++ b/include/migration/page_cache.h
@@ -57,7 +57,8 @@ bool cache_is_cached(const PageCache *cache, uint64_t
addr);
uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
/**
- * cache_insert: insert the page into the cache. the previous value will
be overwritten
+ * cache_insert: insert the page into the cache. the page cache
+ * will dup the data on insert. the previous value will be overwritten
*
* @cache pointer to the PageCache struct
* @addr: page address
diff --git a/page_cache.c b/page_cache.c
index a6c3a15..e670d91 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -158,7 +158,7 @@ void cache_insert(PageCache *cache, uint64_t addr,
uint8_t *pdata)
g_free(it-it_data);
}
-it-it_data = pdata;
+it-it_data = g_memdup(pdata, cache-page_size);
it-it_age = ++cache-max_item_age;
it-it_addr = addr;
}
Re: [Qemu-devel] [PATCH v2 1/8] xilinx_axienet: typedef XilinxAXIEnet struct
Am 25.02.2013 09:50, schrieb Peter Crosthwaite: Typedef xilinx_axienets object state struct to shorten the repeated usages of struct XilinxAXIEnet. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Andreas Färber afaer...@suse.de 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 v2 2/8] xilinx_axienet: Defined and use type cast macro
Am 25.02.2013 09:50, schrieb Peter Crosthwaite: Standard QOM cast macro. Replaces usages of FROM_SYSBUS Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Andreas Färber afaer...@suse.de 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] [PATCHv2] page_cache: fix memory leak
XBZRLE encoded migration introduced a MRU page cache
meachnism. Unfortunately, cached items where never freed in
case of a collision in the page cache on cache_insert().
This lead to out of memory conditions during XBZRLE migration
if the page cache was small and there where a lot of collisions
in the cache.
Signed-off-by: Peter Lieven p...@kamp.de
---
v2:
- make g_free unconditional.
page_cache.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/page_cache.c b/page_cache.c
index ba5640b..376f1db 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -152,6 +152,9 @@ void cache_insert(PageCache *cache, uint64_t addr, uint8_t
*pdata)
/* actual update of entry */
it = cache_get_by_addr(cache, addr);
+/* free old cached data if any */
+g_free(it-it_data);
+
if (!it-it_data) {
cache-num_items++;
}
--
1.7.9.5
Re: [Qemu-devel] [PATCH v2 3/8] xilinx_axienet: Register reset properly
Am 25.02.2013 09:50, schrieb Peter Crosthwaite: Register the reset function and the Device::reset function rather than explicitly call it from the sysbus::init. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Andreas Färber afaer...@suse.de Seems like it might also fix genuine reboot issues - I don't see a qemu_register_reset() being removed, so looks like it was only called on initial device init before. 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 v2 5/8] petalogix_ml605_mmu: Fix machine node attachment
Am 25.02.2013 09:50, schrieb Peter Crosthwaite: Just attach devices straight to the root machine node, rather than the unattached node Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- Suggested (indirectly) by Andreas if he wants to put his Suggested-by to it. I don't insist. :) Reviewed-by: Andreas Färber afaer...@suse.de 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 v2 02/15] target-ppc: Move PPC_DUMP_CPU to translate.c
On 21.02.2013, at 05:24, Andreas Färber wrote: There's an opcode handler field dependent on PPC_DUMP_CPU without which the build fails. Signed-off-by: Andreas Färber afaer...@suse.de --- target-ppc/translate.c |1 + target-ppc/translate_init.c |1 - 2 Dateien geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-) diff --git a/target-ppc/translate.c b/target-ppc/translate.c index 2ac5794..2e74e45 100644 --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -33,6 +33,7 @@ /* Include definitions for instructions classes and implementations flags */ //#define PPC_DEBUG_DISAS +#undef PPC_DUMP_CPU #undef? Alex //#define DO_PPC_STATISTICS #ifdef PPC_DEBUG_DISAS diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 181bd99..74d93a4 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -30,7 +30,6 @@ #include sysemu/arch_init.h #include sysemu/cpus.h -//#define PPC_DUMP_CPU //#define PPC_DEBUG_SPR //#define PPC_DUMP_SPR_ACCESSES #if defined(CONFIG_USER_ONLY) -- 1.7.10.4
Re: [Qemu-devel] [PATCH v2 01/15] target-ppc: Fix PPC_DUMP_SPR_ACCESS build
On 21.02.2013, at 05:24, Andreas Färber wrote:
A victim of the d523dd00a7d73b28f2e99acf45a4b3f92e56e40a AREG0
conversion, insert the missing cpu_env arguments.
Cc: Blue Swirl blauwir...@gmail.com
Signed-off-by: Andreas Färber afaer...@suse.de
Thanks, applied to ppc-next.
Alex
---
target-ppc/translate_init.c |4 ++--
1 Datei geändert, 2 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 5df2057..181bd99 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -61,7 +61,7 @@ static void spr_load_dump_spr(int sprn)
{
#ifdef PPC_DUMP_SPR_ACCESSES
TCGv_i32 t0 = tcg_const_i32(sprn);
-gen_helper_load_dump_spr(t0);
+gen_helper_load_dump_spr(cpu_env, t0);
tcg_temp_free_i32(t0);
#endif
}
@@ -76,7 +76,7 @@ static void spr_store_dump_spr(int sprn)
{
#ifdef PPC_DUMP_SPR_ACCESSES
TCGv_i32 t0 = tcg_const_i32(sprn);
-gen_helper_store_dump_spr(t0);
+gen_helper_store_dump_spr(cpu_env, t0);
tcg_temp_free_i32(t0);
#endif
}
--
1.7.10.4
Re: [Qemu-devel] [PATCH] page_cache: dup memory on insert
Am 25.02.2013 um 13:36 schrieb Peter Maydell peter.mayd...@linaro.org:
On 25 February 2013 12:17, Peter Lieven p...@dlhnet.de wrote:
Am 25.02.2013 um 13:13 schrieb Peter Maydell peter.mayd...@linaro.org:
Doesn't this introduce a leak on cache resize in the case where
the element being moved from the old cache to the new does not
collide with any element we've already moved? [ie the code
path where we just cache_insert() the old item's data].
you are right. maybe we need different functions for inserts made
during resize and inserts from outside.
Looking a little more closely, I think you need that anyway,
because cache_insert() updates the it_age field, when I would
have expected that in the just moving everything over to the
resized cache case we would want to retain the existing age.
Since cache_resize() already has code in it that does a
direct access and copy of CacheItem* fields [for the copy
old_it over new_it case] it might be cleaner to either
(a) have all the cases open-coded in cache_resize() rather
than calling cache_insert(), or (b) abstract out the
whole of the resize inner loop into something like
cache_insert_item(PageCache *cache, CacheItem *olditem)
{
/* Insert olditem (an item from a different page cache) into this one.
* If there is a collision then we keep the data from whichever of
* olditem and the existing entry is newer. In either case, olditem's
* data pointer is either copied into the new cache or freed, so
* the caller must do nothing further with it.
*/
}
Extra bonus leak: I think cache_resize() is leaking
cache-page_cache.
sth like this?
diff --git a/page_cache.c b/page_cache.c
index 376f1db..04205ee 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -196,21 +196,19 @@ int64_t cache_resize(PageCache *cache, int64_t
new_num_pages)
/* check for collision, if there is, keep MRU page */
new_it = cache_get_by_addr(new_cache, old_it-it_addr);
if (new_it-it_data) {
-/* keep the MRU page */
if (new_it-it_age = old_it-it_age) {
g_free(old_it-it_data);
-} else {
-g_free(new_it-it_data);
-new_it-it_data = old_it-it_data;
-new_it-it_age = old_it-it_age;
-new_it-it_addr = old_it-it_addr;
+continue;
}
-} else {
-cache_insert(new_cache, old_it-it_addr, old_it-it_data);
}
+g_free(new_it-it_data);
+new_it-it_data = old_it-it_data;
+new_it-it_age = old_it-it_age;
+new_it-it_addr = old_it-it_addr;
}
}
+g_free(cache-page_cache);
cache-page_cache = new_cache-page_cache;
cache-max_num_items = new_cache-max_num_items;
cache-num_items = new_cache-num_items;
-- PMM
Re: [Qemu-devel] [PATCH v2 6/8] petalogix_ml605_mmu: Attach ethernet to machine
Am 25.02.2013 09:50, schrieb Peter Crosthwaite: Explicitily make the ethernet a child of the machine. This is needed to set and use links pre-realize. Also makes the ethernet initialization consistent with its peer DMA. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/petalogix_ml605_mmu.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c index 98e647d..e3528c0 100644 --- a/hw/petalogix_ml605_mmu.c +++ b/hw/petalogix_ml605_mmu.c @@ -134,6 +134,8 @@ petalogix_ml605_init(QEMUMachineInitArgs *args) dma = qdev_create(NULL, xlnx.axi-dma); /* FIXME: attach to the sysbus instead */ +object_property_add_child(qdev_get_machine(), xilinx-eth, OBJECT(eth0), + NULL); object_property_add_child(qdev_get_machine(), xilinx-dma, OBJECT(dma), NULL); Reviewed-by: Andreas Färber afaer...@suse.de BTW concerning the above FIXME, I believe we do have a /machine/sysbus node or so, which contains link properties to its child devices. Adding those devices as child properties before qdev_init*() (realized = true) changes the canonical paths these are displayed as from /machine/unassigned/device[n] or no to /machine/xilinx-eth in this case. 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 v2 12/15] target-ppc: Refactor debug output macros
On 21.02.2013, at 05:25, Andreas Färber wrote:
Make debug output compile-testable even if disabled.
Rename dprintf() in kvm.c to kvm_dprintf() to avoid conflict with glibc.
Inline DEBUG_OP check in excp_helper.c.
Inline LOG_MMU_STATE() in mmu_helper.c.
Inline PPC_{DEBUG_SPR,DUMP_SPR_ACCESSES} checks in translate_init.c.
Signed-off-by: Andreas Färber afaer...@suse.de
I assume you verified that all the bits do get optimized out, right? :)
Reviewed-by: Alexander Graf ag...@suse.de
Alex
Re: [Qemu-devel] [PATCH v2 13/15] target-s390x: Refactor debug output macros
On 21.02.2013, at 05:25, Andreas Färber wrote: Make debug output compile-testable even if disabled. Rename dprintf() in kvm.c to kvm_dprintf() due to a conflict with glibc. Drop unused DEBUG_HELPER and LOG_HELPER() in fpu_helper.c. Drop unused LOG_DISAS() in translate.c and inline S390X_DEBUG_DISAS. Signed-off-by: Andreas Färber afaer...@suse.de Reviewed-by: Alexander Graf ag...@suse.de Alex
Re: [Qemu-devel] [PATCH v4 0/6] Efficient VM backup for qemu
unix sockets works with qemu nbd code? 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. I tried to use named exports, but get the following when I try to connect to the nbd server: nbd.c:nbd_receive_negotiate():L452: Receiving negotiation. name = ide0 nbd.c:nbd_receive_negotiate():L476: Magic is NBDMAGIC nbd.c:nbd_receive_negotiate():L488: Magic is 0x420281861253 nbd.c:nbd_receive_negotiate():L495: Checking magic (opts_magic) nbd.c:nbd_receive_negotiate():L497: Bad magic received Seems the server sends NBD_CLIENT_MAGIC, but the c lient expects NBD_OPTS_MAGIC?
Re: [Qemu-devel] Block I/O optimizations
On Mon, Feb 25, 2013 at 10:48:47AM +0200, Abel Gordon wrote: Stefan Hajnoczi stefa...@gmail.com wrote on 21/02/2013 10:11:12 AM: From: Stefan Hajnoczi stefa...@gmail.com To: Loic Dachary l...@dachary.org, Cc: qemu-devel qemu-devel@nongnu.org Date: 21/02/2013 10:11 AM Subject: Re: [Qemu-devel] Block I/O optimizations Sent by: qemu-devel-bounces+abelg=il.ibm@nongnu.org On Mon, Feb 18, 2013 at 7:19 PM, Loic Dachary l...@dachary.org wrote: I recently tried to figure out the best and easiest ways to increase block I/O performances with qemu. Not being a qemu expert, I expected to find a few optimization tricks. Much to my surprise, it appears that there are many significant improvements being worked on. This is excellent news :-) However, I'm not sure I understand how they all fit together. It's probably quite obvious from the developer point of view but I would very much appreciate an overview of how dataplane, vhost-blk, ELVIS etc. should be used or developed to maximize I/O performances. Are there documents I should read ? If not, would someone be willing to share bits of wisdom ? Hi Loic, There will be more information on dataplane shortly. I'll write up a blog post and share the link with you. Hi Stefan, I assume dataplane could provide a significant performance boost and approximate vhost-blk performance. If I understand properly, that's because dataplane finally removes the dependency on the global mutex and uses eventfd to process notifications. Right, it's the same approach - ioeventfd for kicks and irqfd for notifies. The difference is kernel thread vs userspace thread. However, I am concerned dataplane may not solve the scalability problem because QEMU will be still running 1 thread per VCPU and 1 per virtual device to handle I/O for each VM. Assuming we run N VMs with 1 VCPU and 1 virtual I/O device, we will have 2N threads competing for CPU cycles. In a cloud-like environment running I/O intensive VMs that could be a problem because the I/O threads and VCPU threads may starve each other. Further more, the linux kernel can't make good scheduling decisions (from I/O perspective) because it has no information about the content of the I/O queues. The kernel knows when the dataplane thread is schedulable - when the ioeventfd is signalled. In the worst case the scheduler could allow the vcpu thread to complete an entire time slice before letting the dataplane thread run. So are you saying that the Linux scheduler wouldn't allow the dataplane thread to run on a loaded box? My first thought would be to raise the priority of the dataplane thread so it preempts the vcpu thread upon becoming schedulable. We did some experiments with a modified vhost-blk back-end that uses a single (or few) thread/s to process I/O for many VMs as opposed to 1 thread per VM (I/O device). These thread/s decide for how-long and when to process the request of each VM based on the I/O activity of each queue. We noticed that this model (part of what we call ELVIS) significantly improves the scalability of the system when you run many I/O intensive guests. When you say this model (part of what we call ELVIS) significantly improves the scalability of the system when you run many I/O intensive guests, do you mean exit-less vs exit-based or shared thread vs 1 thread per device (without polling)? I'm not sure if you're advocating exit-less (polling) or shared thread without polling. I was wondering if you have considered this type of threading model for dataplane as well. With vhost-blk (or-net) it's relatively easy to use a kernel thread to process I/O for many VMs (user-space processes). However, with a QEMU back-end (like dataplane/virtio-blk) the shared thread model may be challenging because it requires a shared user-space process (for the I/O thread/s) to handle I/O for many QEMU processes. Any thoughts/opinions on the share-thread direction ? For low latency polling makes sense and a shared thread is an efficient way to implement polling. But it throws away resource control and isolation - now you can no longer use cgroups and other standard resource control mechanisms to manage guests. You also create a privileged thread that has access to all guests on the host - a security bug here compromises all guests. This can be fine for private deployments where guests are trusted. For untrusted guests and public clouds it seems risky. Maybe a hybrid approach is possible where exit-less is possible but I/O emulation still happens in per-guest userspace threads. Not sure how much performance can be retained by doing that - e.g. a kernel driver that allows processes to bind an eventfd to a memory notification area. The kernel driver does polling in a single thread and signals eventfds. Userspace threads do the actual I/O emulation. Stefan
Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] Fix dma interrupt
On 21.02.2013, at 18:34, Amadeusz Sławiński wrote: In openbios (drivers/ide.c) they are set to 000d 0002 000e 0003 000f 0004 (The last one seems to be not implemented in qemu) It follows convention of how they are set on real machines, both ide and dma ones are increased Real machine one: http://web.archive.org/web/20090107151044/http://penguinppc.org/historical/dev-trees-html/g4_agp_500_2.html 0013 0001 000b 0014 0001 000c 0015 0001 000d Signed-off-by: Amadeusz Sławiński am...@asmblr.net Thanks, applied both to ppc-next. Alex --- hw/ppc/mac_newworld.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 065ea87..a08a6b2 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -370,7 +370,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args) qdev_connect_gpio_out(dev, 1, pic[0x0d]); /* IDE */ qdev_connect_gpio_out(dev, 2, pic[0x02]); /* IDE DMA */ qdev_connect_gpio_out(dev, 3, pic[0x0e]); /* IDE */ -qdev_connect_gpio_out(dev, 4, pic[0x02]); /* IDE DMA */ +qdev_connect_gpio_out(dev, 4, pic[0x03]); /* IDE DMA */ macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ -- 1.8.1.4
Re: [Qemu-devel] [PATCH v2 02/15] target-ppc: Move PPC_DUMP_CPU to translate.c
Am 25.02.2013 13:49, schrieb Alexander Graf: On 21.02.2013, at 05:24, Andreas Färber wrote: There's an opcode handler field dependent on PPC_DUMP_CPU without which the build fails. Signed-off-by: Andreas Färber afaer...@suse.de --- target-ppc/translate.c |1 + target-ppc/translate_init.c |1 - 2 Dateien geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-) diff --git a/target-ppc/translate.c b/target-ppc/translate.c index 2ac5794..2e74e45 100644 --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -33,6 +33,7 @@ /* Include definitions for instructions classes and implementations flags */ //#define PPC_DEBUG_DISAS +#undef PPC_DUMP_CPU #undef? // is not permitted. :) Alternative would be /* #define ... */ Just edit the line to your liking. :) 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] 'commit' error for 'all': No medium found
On Sun, Feb 24, 2013 at 07:29:31PM +0100, Jan Kiszka wrote: 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 Hi Jan, That commit just affected the reporting on the error - for the all case, bdrv_commit_all() still returned error on the first failure. When that happened 'commit' may have indicated success rather than error, depending on the error. That would have also left uncommitted data behind, but done so silently and reported success. However, commit e8877497 added error checking to the bdrv_commit() return value in bdrv_commit_all() - before that bdrv_commit_all() ignored all error returns of bdrv_commit(). Do you think there specific error returns that we should ignore then, in bdrv_commit_all(), such as -ENOMEDIUM? Also, could you expand on what you mean by non-populated medium (test case) - is the error being returned No medium found? Jeff
Re: [Qemu-devel] [PATCH v2 00/15] Debug output revamp
On 22.02.2013, at 18:41, Andreas Färber wrote:
Am 22.02.2013 17:54, schrieb Richard Henderson:
On 02/22/2013 08:16 AM, Andreas Färber wrote:
I would be willing to do a macro-based v3 using do { ... } while (0) if
maintainers can reach agreement on that and on how to do the if (0).
FWIW, I'm in favor of the
#ifndef DEBUG
# define DEBUG 0
#endif
#define MACRO_NAME(...) \
do { if (DEBUG) { log(...) } } while (0)
sort of solution. Immediate constant into the IF, which even an -O0
compile will delete as dead.
I remember there being complaints about me changing in-code #ifdefs to
#ifs in v1 as a consequence... some naming convention like #ifdef DEBUG
#define DEBUG_ON 1 #else #define DEBUG_ON 0 or so would address that.
Let's wait for some more feedback.
Yeah, that certainly works for me.
Alex
Re: [Qemu-devel] [PATCH] page_cache: dup memory on insert
On 25 February 2013 12:50, Peter Lieven p...@dlhnet.de wrote:
sth like this?
diff --git a/page_cache.c b/page_cache.c
index 376f1db..04205ee 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -196,21 +196,19 @@ int64_t cache_resize(PageCache *cache, int64_t
new_num_pages)
/* check for collision, if there is, keep MRU page */
new_it = cache_get_by_addr(new_cache, old_it-it_addr);
if (new_it-it_data) {
-/* keep the MRU page */
if (new_it-it_age = old_it-it_age) {
g_free(old_it-it_data);
-} else {
-g_free(new_it-it_data);
-new_it-it_data = old_it-it_data;
-new_it-it_age = old_it-it_age;
-new_it-it_addr = old_it-it_addr;
+continue;
}
-} else {
-cache_insert(new_cache, old_it-it_addr, old_it-it_data);
}
+g_free(new_it-it_data);
+new_it-it_data = old_it-it_data;
+new_it-it_age = old_it-it_age;
+new_it-it_addr = old_it-it_addr;
}
}
+g_free(cache-page_cache);
cache-page_cache = new_cache-page_cache;
cache-max_num_items = new_cache-max_num_items;
cache-num_items = new_cache-num_items;
Yeah, more or less. You could roll the two if () conditions
into if (new_it-it_data new_it-it_age = old_it-it_age)
but that's just a style thing.
-- PMM
Re: [Qemu-devel] [PATCH v2 12/15] target-ppc: Refactor debug output macros
Am 25.02.2013 13:54, schrieb Alexander Graf:
On 21.02.2013, at 05:25, Andreas Färber wrote:
Make debug output compile-testable even if disabled.
Rename dprintf() in kvm.c to kvm_dprintf() to avoid conflict with glibc.
Inline DEBUG_OP check in excp_helper.c.
Inline LOG_MMU_STATE() in mmu_helper.c.
Inline PPC_{DEBUG_SPR,DUMP_SPR_ACCESSES} checks in translate_init.c.
Signed-off-by: Andreas Färber afaer...@suse.de
I assume you verified that all the bits do get optimized out, right? :)
No, I didn't for each, my focus was to make debug code compile and keep
it compiling after my CPU changes. :)
Please read up on the new discussion of rth not liking static const and
proposing to go back to v1, modulo do { ... } while (0).
Like I said there, if finding a solution that pleases everyone fails,
then I will leave it to the maintainers (i.e., you) to choose and apply
a solution or to live with the resulting breakages.
Andreas
Reviewed-by: Alexander Graf ag...@suse.de
Alex
--
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 v4 0/6] Efficient VM backup for qemu
Il 25/02/2013 13:59, Dietmar Maurer ha scritto: unix sockets works with qemu nbd code? 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. I tried to use named exports, but get the following when I try to connect to the nbd server: nbd.c:nbd_receive_negotiate():L452: Receiving negotiation. name = ide0 nbd.c:nbd_receive_negotiate():L476: Magic is NBDMAGIC nbd.c:nbd_receive_negotiate():L488: Magic is 0x420281861253 nbd.c:nbd_receive_negotiate():L495: Checking magic (opts_magic) nbd.c:nbd_receive_negotiate():L497: Bad magic received Seems the server sends NBD_CLIENT_MAGIC, but the c lient expects NBD_OPTS_MAGIC? It means your server is not configured for named exports. See the protocol as described in nbd_send_negotiate: Negotiation header without options: [ 0 .. 7] passwd (NBDMAGIC) [ 8 .. 15] magic(NBD_CLIENT_MAGIC) [16 .. 23] size [24 .. 25] server flags (0) [24 .. 27] export flags [28 .. 151] reserved (0) Negotiation header with options, part 1: [ 0 .. 7] passwd (NBDMAGIC) [ 8 .. 15] magic(NBD_OPTS_MAGIC) [16 .. 17] server flags (0) part 2 (after options are sent): [18 .. 25] size [26 .. 27] export flags [28 .. 151] reserved (0) So you're receiving the first kind instead of the second. You can try it with QEMU's embedded NBD server. (Of course it's possible that there are bugs in the implementation). Paolo
Re: [Qemu-devel] Moving BIOS tables from SeaBIOS to QEMU
On 02/24/13 19:00, Kevin O'Connor wrote: 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? Didn't have much of a plan until now, just look into it. It seems quite a bit of work (I expect many resubmits to qemu-devel) and I think I'd prefer to start working on it no earlier than March 18th. (Of course if anyone else implements it by then I'll be happy :)) 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). In the long term I believe everything should be passed as fw_cfg files, one file per table. I'm not sure about the naming convention, but probably something like acpi/SSDT. This already seems quite messy. For example, acpi-dsdt.aml is built as part of SeaBIOS, then installed on the filesystem with qemu. Qemu can load manually specified ACPI tables from files, with the -acpitable switch: do_acpitable_option() [arch_init.c] acpi_table_add() [hw/acpi.c] If no such option is specified, it auto-loads acpi-dsdt.aml (I'm ignoring q35 for now). Then the loaded tables are all exported under one fw_cfg key: pc_init1() [hw/pc_piix.c] pc_acpi_init() [hw/pc.c] acpi_table_add() [hw/acpi.c] pc_memory_init() [hw/pc.c] bochs_bios_init() fw_cfg_add_bytes(..., FW_CFG_ACPI_TABLES, acpi_tables, ...) SeaBIOS then splits/relabels this single blob into sub-blobs, qemu_cfg_legacy() loop qemu_romfile_add(acpi/table%d, QEMU_CFG_ACPI_TABLES, offset, len) Then eg. the DSDT is installed in qemu_platform_setup() acpi_setup() romfile_findprefix() qemu_cfg_read_file() via funcptr fill_dsdt() So it's a seabios-qemu-seabios ping-pong. At first I would export the ACPI table in qemu (install the fw_cfg file) in the same spot where currently the corresponding base info is prepared for SeaBIOS. If a table in SeaBIOS is currently built from several fw_cfg sources, then I'd probably export the qemu replacement in the latest base info spot, verifying if I can still collect earlier pieces of base info. I think we should move forward table-wise... each could take a separate series. Don't know what to do with the -acpitable switch though. A mixture of loaded and autogenerated tables promises trouble. - 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. I'd just look at what the fw_cfg info is composed from, and re-use 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. Without the CRS stuff efifb wasn't working in OVMF-based guests, so I already had to implement a similar search in OVMF (with ample guidance from Gerd others of course). The series is archived under http://thread.gmane.org/gmane.comp.bios.tianocore.devel/81 The interesting commits are: http://tianocore.git.sourceforge.net/git/gitweb.cgi?p=tianocore/edk2;a=commitdiff;h=57c0beb609a75349c067075b45cdafce1a1b77f8
Re: [Qemu-devel] [Bug 1130533] Re: Documentation cannot be build since commit c70a01e449536c616c85ab820c6fbad7d7e9cf39
On Fri, Feb 22, 2013 at 10:03:50AM -, FredBezies wrote: Patch is ok, with this little warning when applied. Nothing really bad. patching file qemu-options.hx Hunk #1 succeeded at 2097 (offset 2 lines). git head is : http://git.qemu.org/?p=qemu.git;a=commit;h=73d4dc71f3a41131541c73b3ac2a8b160a51842b Besides this, it builds perfectly. Any hope to get it upstream ? Yes, it will get merged.
