[Qemu-devel] [PATCH v6 1/3] Revert block: Fix unaligned zero write
This reverts commit fc3959e4669a1c2149b91ccb05101cfc7ae1fc05. The core write code already handles the case, so remove this duplication. Because commit 61007b316 moved the touched code from block.c to block/io.c, the change is manually reverted. Signed-off-by: Fam Zheng f...@redhat.com Reviewed-by: Stefan Hajnoczi stefa...@redhat.com --- block/io.c | 45 ++--- 1 file changed, 6 insertions(+), 39 deletions(-) diff --git a/block/io.c b/block/io.c index 1ce62c4..4e5a92e 100644 --- a/block/io.c +++ b/block/io.c @@ -929,19 +929,6 @@ out: return ret; } -static inline uint64_t bdrv_get_align(BlockDriverState *bs) -{ -/* TODO Lift BDRV_SECTOR_SIZE restriction in BlockDriver interface */ -return MAX(BDRV_SECTOR_SIZE, bs-request_alignment); -} - -static inline bool bdrv_req_is_aligned(BlockDriverState *bs, - int64_t offset, size_t bytes) -{ -int64_t align = bdrv_get_align(bs); -return !(offset (align - 1) || (bytes (align - 1))); -} - /* * Handle a read request in coroutine context */ @@ -952,7 +939,8 @@ static int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs, BlockDriver *drv = bs-drv; BdrvTrackedRequest req; -uint64_t align = bdrv_get_align(bs); +/* TODO Lift BDRV_SECTOR_SIZE restriction in BlockDriver interface */ +uint64_t align = MAX(BDRV_SECTOR_SIZE, bs-request_alignment); uint8_t *head_buf = NULL; uint8_t *tail_buf = NULL; QEMUIOVector local_qiov; @@ -1194,7 +1182,8 @@ static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs, BdrvRequestFlags flags) { BdrvTrackedRequest req; -uint64_t align = bdrv_get_align(bs); +/* TODO Lift BDRV_SECTOR_SIZE restriction in BlockDriver interface */ +uint64_t align = MAX(BDRV_SECTOR_SIZE, bs-request_alignment); uint8_t *head_buf = NULL; uint8_t *tail_buf = NULL; QEMUIOVector local_qiov; @@ -1293,10 +1282,6 @@ static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs, bytes = ROUND_UP(bytes, align); } -if (use_local_qiov) { -/* Local buffer may have non-zero data. */ -flags = ~BDRV_REQ_ZERO_WRITE; -} ret = bdrv_aligned_pwritev(bs, req, offset, bytes, use_local_qiov ? local_qiov : qiov, flags); @@ -1337,32 +1322,14 @@ int coroutine_fn bdrv_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, int nb_sectors, BdrvRequestFlags flags) { -int ret; - trace_bdrv_co_write_zeroes(bs, sector_num, nb_sectors, flags); if (!(bs-open_flags BDRV_O_UNMAP)) { flags = ~BDRV_REQ_MAY_UNMAP; } -if (bdrv_req_is_aligned(bs, sector_num BDRV_SECTOR_BITS, -nb_sectors BDRV_SECTOR_BITS)) { -ret = bdrv_co_do_writev(bs, sector_num, nb_sectors, NULL, -BDRV_REQ_ZERO_WRITE | flags); -} else { -uint8_t *buf; -QEMUIOVector local_qiov; -size_t bytes = nb_sectors BDRV_SECTOR_BITS; -buf = qemu_memalign(bdrv_opt_mem_align(bs), bytes); -memset(buf, 0, bytes); -qemu_iovec_init(local_qiov, 1); -qemu_iovec_add(local_qiov, buf, bytes); - -ret = bdrv_co_do_writev(bs, sector_num, nb_sectors, local_qiov, -BDRV_REQ_ZERO_WRITE | flags); -qemu_vfree(buf); -} -return ret; +return bdrv_co_do_writev(bs, sector_num, nb_sectors, NULL, + BDRV_REQ_ZERO_WRITE | flags); } int bdrv_flush_all(void) -- 2.1.0
[Qemu-devel] [PATCH v6 0/3] block: Fix unaligned bdrv_aio_write_zeroes
An unaligned zero write causes NULL deferencing in bdrv_co_do_pwritev. That path is reachable from bdrv_co_write_zeroes and bdrv_aio_write_zeroes. You can easily trigger through the former with qemu-io, as the test case added by 61815d6e0aa. For bdrv_aio_write_zeroes, in common cases there's always a format driver (which uses 512 alignment), so it would be much rarer to have unaligned requests (only concerning top level here, when the request goes down to bs-file, where for example the alignment is 4k, it would then be calling bdrv_co_write_zeroes because it's in a coroutine). fc3959e4669a1c fixed bdrv_co_write_zeroes but not bdrv_aio_write_zeroes. The lattern is the actually used one by device model. Revert the previous fix, do it in bdrv_co_do_pwritev, to cover both paths. v6: Split the function to make it easier to reason. (Stefan) Add rev-by in 1, 3. (Stefan) v5: Rebase on to the block/io.c split. (Kevin) v4: if (!qiov) - if (!qiov bytes = align). (Paolo) v3: Fix the case where the unaligned request is contained within the first block. (Paolo) Also update iotests 033 to cover the code path with qemu-io. v2: Split to three aligned pwritev. Fam Zheng (3): Revert block: Fix unaligned zero write block: Fix NULL deference for unaligned write if qiov is NULL qemu-iotests: Test unaligned sub-block zero write block/io.c | 140 - tests/qemu-iotests/033 | 13 + tests/qemu-iotests/033.out | 30 ++ 3 files changed, 144 insertions(+), 39 deletions(-) -- 2.1.0
[Qemu-devel] [PATCH v6 2/3] block: Fix NULL deference for unaligned write if qiov is NULL
For zero write, callers pass in NULL qiov (qemu-io write -z or scsi-disk write same). Commit fc3959e466 fixed bdrv_co_write_zeroes which is the common case for this bug, but it still exists in bdrv_aio_write_zeroes. A simpler fix would be in bdrv_co_do_pwritev which is the NULL dereference point and covers both cases. So don't access it in bdrv_co_do_pwritev in this case, use three aligned writes. Signed-off-by: Fam Zheng f...@redhat.com --- block/io.c | 95 ++ 1 file changed, 95 insertions(+) diff --git a/block/io.c b/block/io.c index 4e5a92e..d766220 100644 --- a/block/io.c +++ b/block/io.c @@ -1174,6 +1174,97 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs, return ret; } +static int coroutine_fn bdrv_co_do_zero_pwritev(BlockDriverState *bs, +int64_t offset, +unsigned int bytes, +BdrvRequestFlags flags) +{ +BdrvTrackedRequest req; +uint8_t *buf = NULL; +QEMUIOVector local_qiov; +struct iovec iov; +uint64_t align = MAX(BDRV_SECTOR_SIZE, bs-request_alignment); +unsigned int head_padding_bytes, tail_padding_bytes; +int ret; + +head_padding_bytes = offset (align - 1); +tail_padding_bytes = align - ((offset + bytes) (align - 1)); +tracked_request_begin(req, bs, offset, bytes, true); + +mark_request_serialising(req, align); +wait_serialising_requests(req); + +assert(flags BDRV_REQ_ZERO_WRITE); +if (head_padding_bytes || tail_padding_bytes) { +buf = qemu_blockalign(bs, align); +iov = (struct iovec) { +.iov_base = buf, +.iov_len= align, +}; +qemu_iovec_init_external(local_qiov, iov, 1); +} +if (head_padding_bytes) { +uint64_t zero_bytes = MIN(bytes, align - head_padding_bytes); + +/* RMW the unaligned part before head. */ +BLKDBG_EVENT(bs, BLKDBG_PWRITEV_RMW_HEAD); +ret = bdrv_aligned_preadv(bs, req, offset ~(align - 1), align, + align, local_qiov, 0); +if (ret 0) { +goto fail; +} +BLKDBG_EVENT(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD); + +memset(buf + head_padding_bytes, 0, zero_bytes); +ret = bdrv_aligned_pwritev(bs, req, offset ~(align - 1), align, + local_qiov, + flags ~BDRV_REQ_ZERO_WRITE); +if (ret 0) { +goto fail; +} +offset += zero_bytes; +bytes -= zero_bytes; +} + +assert((offset (align - 1)) == 0); +if (bytes = align) { +/* Write the aligned part in the middle. */ +uint64_t aligned_bytes = bytes ~(align - 1); +ret = bdrv_aligned_pwritev(bs, req, offset, aligned_bytes, + NULL, flags); +if (ret 0) { +goto fail; +} +bytes -= aligned_bytes; +offset += aligned_bytes; +} + +assert((offset (align - 1)) == 0); +if (bytes) { +assert(align == tail_padding_bytes + bytes); +/* RMW the unaligned part after tail. */ +BLKDBG_EVENT(bs, BLKDBG_PWRITEV_RMW_TAIL); +ret = bdrv_aligned_preadv(bs, req, offset, align, + align, local_qiov, 0); +if (ret 0) { +goto fail; +} +BLKDBG_EVENT(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL); + +memset(buf, 0, bytes); +printf(tail part %ld %d\n, offset, bytes); +ret = bdrv_aligned_pwritev(bs, req, offset, align, + local_qiov, flags ~BDRV_REQ_ZERO_WRITE); +} +fail: +tracked_request_end(req); +if (buf) { +qemu_vfree(buf); +} +return ret; + +} + /* * Handle a write request in coroutine context */ @@ -1207,6 +1298,10 @@ static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs, bdrv_io_limits_intercept(bs, bytes, true); } +if (!qiov) { +return bdrv_co_do_zero_pwritev(bs, offset, bytes, flags); +} + /* * Align write if necessary by performing a read-modify-write cycle. * Pad qiov with the read parts and be sure to have a tracked request not -- 2.1.0
[Qemu-devel] [PATCH v6 3/3] qemu-iotests: Test unaligned sub-block zero write
Test zero write in byte range 512~1024 for 4k alignment. Signed-off-by: Fam Zheng f...@redhat.com Reviewed-by: Stefan Hajnoczi stefa...@redhat.com --- tests/qemu-iotests/033 | 13 + tests/qemu-iotests/033.out | 30 ++ 2 files changed, 43 insertions(+) diff --git a/tests/qemu-iotests/033 b/tests/qemu-iotests/033 index 4008f10..a61d8ce 100755 --- a/tests/qemu-iotests/033 +++ b/tests/qemu-iotests/033 @@ -78,6 +78,19 @@ for align in 512 4k; do echo echo == verifying patterns (2) == do_test $align read -P 0x0 0x400 0x2 $TEST_IMG | _filter_qemu_io + + echo + echo == rewriting unaligned zeroes == + do_test $align write -P 0xb 0x0 0x1000 $TEST_IMG | _filter_qemu_io + do_test $align write -z 0x200 0x200 $TEST_IMG | _filter_qemu_io + + echo + echo == verifying patterns (3) == + do_test $align read -P 0xb 0x0 0x200 $TEST_IMG | _filter_qemu_io + do_test $align read -P 0x0 0x200 0x200 $TEST_IMG | _filter_qemu_io + do_test $align read -P 0xb 0x400 0xc00 $TEST_IMG | _filter_qemu_io + + echo done # success, all done diff --git a/tests/qemu-iotests/033.out b/tests/qemu-iotests/033.out index 305949f..c3d18aa 100644 --- a/tests/qemu-iotests/033.out +++ b/tests/qemu-iotests/033.out @@ -27,6 +27,21 @@ wrote 65536/65536 bytes at offset 65536 read 131072/131072 bytes at offset 1024 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +== rewriting unaligned zeroes == +wrote 4096/4096 bytes at offset 0 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 512/512 bytes at offset 512 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== verifying patterns (3) == +read 512/512 bytes at offset 0 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 512/512 bytes at offset 512 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 3072/3072 bytes at offset 1024 +3 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + + == preparing image == wrote 1024/1024 bytes at offset 512 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) @@ -52,4 +67,19 @@ wrote 65536/65536 bytes at offset 65536 == verifying patterns (2) == read 131072/131072 bytes at offset 1024 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== rewriting unaligned zeroes == +wrote 4096/4096 bytes at offset 0 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 512/512 bytes at offset 512 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== verifying patterns (3) == +read 512/512 bytes at offset 0 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 512/512 bytes at offset 512 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 3072/3072 bytes at offset 1024 +3 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + *** done -- 2.1.0
Re: [Qemu-devel] [PATCH v2 2/6] block: Fix dirty bitmap in bdrv_co_discard
On Mon, 05/11 15:22, John Snow wrote: On 05/06/2015 12:52 AM, Fam Zheng wrote: Unsetting dirty globally with discard is not very correct. The discard may zero out sectors (depending on can_write_zeroes_with_unmap), we should replicate this change to destinition side to make sure that the guest sees the same data. Calling bdrv_reset_dirty also troubles mirror job because the hbitmap iterator doesn't expect unsetting of bits after current position. So let's do it the opposite way which fixes both problems: set the dirty bits if we are to discard it. Reported-by: wangxiaol...@ucloud.cn Signed-off-by: Fam Zheng f...@redhat.com --- block/io.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/io.c b/block/io.c index 1ce62c4..809688b 100644 --- a/block/io.c +++ b/block/io.c @@ -2343,8 +2343,6 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, return -EROFS; } -bdrv_reset_dirty(bs, sector_num, nb_sectors); - /* Do nothing if disabled. */ if (!(bs-open_flags BDRV_O_UNMAP)) { return 0; @@ -2354,6 +2352,8 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, return 0; } +bdrv_set_dirty(bs, sector_num, nb_sectors); + max_discard = MIN_NON_ZERO(bs-bl.max_discard, BDRV_REQUEST_MAX_SECTORS); while (nb_sectors 0) { int ret; For the clueless: will discard *always* change the data, or is it possible that some implementations might do nothing? It could be nop, partial discard, or an effective write same, depending on the protocol. For raw, it depends on the host file system. Is it possible to just omit a set/reset from this function altogether and let whatever function that is called later (e.g. a write_zeroes call) worry about setting the dirty bits? What I wonder about: Is it possible that we are needlessly marking data as dirty when it has not changed? No. Because write zeroes path is not involved at all, it's this function that changes image data. We have to set the dirty bitmap here. Fam
[Qemu-devel] [PATCHv3 0/2] parallel: Allow to disable CONFIG_PARALLEL
From: Miroslav Rezanina mreza...@redhat.com There's few uncoditional calls to functions in hw/char/parallel.c so disabling CONFIG_PARALLEL can cause build failure. This series allow building with CONFIG_PARALLEL disabled. In addition, it changes behavior so the run of qemu -parallel will abort in case target expects parallel device to exists. Affected targets are: - i386 (pc.c) - mips (mips_fulong2e.c, mips_malta.c, mips_jazz.c) - sparc64 (sun4u.c) Signed-off-by: Miroslav Rezanina mreza...@redhat.com Miroslav Rezanina (2): Move parallel_hds_isa_init to hw/isa/isa-bus.c stubs: Provide parallel_mm_init stub version hw/char/parallel.c | 25 - hw/isa/isa-bus.c| 29 + stubs/Makefile.objs | 1 + stubs/parallel.c| 8 4 files changed, 38 insertions(+), 25 deletions(-) create mode 100644 stubs/parallel.c -- 2.1.0
[Qemu-devel] [PATCHv3 1/2] Move parallel_hds_isa_init to hw/isa/isa-bus.c
From: Miroslav Rezanina mreza...@redhat.com Disabling CONFIG_PARALLEL cause removing parallel_hds_isa_init defined in parallel.c. This function is called during initialization of some boards so disabling CONFIG_PARALLEL cause build failure. This patch moves parallel_hds_isa_init to hw/isa/isa-bus.c so it is included in case of disabled CONFIG_PARALLEL. Build is successful but qemu will abort with Unknown device error when function is called. Signed-off-by: Miroslav Rezanina mreza...@redhat.com --- hw/char/parallel.c | 25 - hw/isa/isa-bus.c | 29 + 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/hw/char/parallel.c b/hw/char/parallel.c index 4079554..c2b553f 100644 --- a/hw/char/parallel.c +++ b/hw/char/parallel.c @@ -641,28 +641,3 @@ static void parallel_register_types(void) } type_init(parallel_register_types) - -static void parallel_init(ISABus *bus, int index, CharDriverState *chr) -{ -DeviceState *dev; -ISADevice *isadev; - -isadev = isa_create(bus, isa-parallel); -dev = DEVICE(isadev); -qdev_prop_set_uint32(dev, index, index); -qdev_prop_set_chr(dev, chardev, chr); -qdev_init_nofail(dev); -} - -void parallel_hds_isa_init(ISABus *bus, int n) -{ -int i; - -assert(n = MAX_PARALLEL_PORTS); - -for (i = 0; i n; i++) { -if (parallel_hds[i]) { -parallel_init(bus, i, parallel_hds[i]); -} -} -} diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c index 825aa62..94f645c 100644 --- a/hw/isa/isa-bus.c +++ b/hw/isa/isa-bus.c @@ -21,6 +21,7 @@ #include hw/sysbus.h #include sysemu/sysemu.h #include hw/isa/isa.h +#include hw/i386/pc.h static ISABus *isabus; @@ -267,3 +268,31 @@ MemoryRegion *isa_address_space_io(ISADevice *dev) } type_init(isabus_register_types) + +static void parallel_init(ISABus *bus, int index, CharDriverState *chr) +{ +DeviceState *dev; +ISADevice *isadev; + +isadev = isa_try_create(bus, isa-parallel); +if (!isadev) { + return; +} +dev = DEVICE(isadev); +qdev_prop_set_uint32(dev, index, index); +qdev_prop_set_chr(dev, chardev, chr); +qdev_init_nofail(dev); +} + +void parallel_hds_isa_init(ISABus *bus, int n) +{ +int i; + +assert(n = MAX_PARALLEL_PORTS); + +for (i = 0; i n; i++) { +if (parallel_hds[i]) { +parallel_init(bus, i, parallel_hds[i]); +} +} +} -- 2.1.0
[Qemu-devel] [PATCHv3 2/2] stubs: Provide parallel_mm_init stub version
From: Miroslav Rezanina mreza...@redhat.com mips build fail with link error in case PARALLEL_CONFIG is disabled as hw/mips/mips_jazz.c calls parallel_mm_init. Due to dependecies to content of parallel.c we can't simply move it to hw/isa/isa-devices.c. This patch adds stubs/parallel.c file that contains stub version of parallel_mm_init. This ensure successful build with PARALLEL_CONFIG disabled. Signed-off-by: Miroslav Rezanina mreza...@redhat.com --- stubs/Makefile.objs | 1 + stubs/parallel.c| 8 2 files changed, 9 insertions(+) create mode 100644 stubs/parallel.c diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 8beff4c..ad4e110 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -24,6 +24,7 @@ stub-obj-y += mon-printf.o stub-obj-y += mon-set-error.o stub-obj-y += monitor-init.o stub-obj-y += notify-event.o +stub-obj-y += parallel.o stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o stub-obj-y += qtest.o stub-obj-y += reset.o diff --git a/stubs/parallel.c b/stubs/parallel.c new file mode 100644 index 000..8293d52 --- /dev/null +++ b/stubs/parallel.c @@ -0,0 +1,8 @@ +#include hw/i386/pc.h + +bool parallel_mm_init(MemoryRegion *address_space, + hwaddr base, int it_shift, qemu_irq irq, + CharDriverState *chr) +{ +return false; +} -- 2.1.0
Re: [Qemu-devel] Bug report - Windows XP guest failure
12.05.2015 04:05, Peter Crosthwaite wrote: On Thu, May 7, 2015 at 2:34 AM, Michael Tokarev m...@tls.msk.ru wrote: ... Ok, I can reproduce this, winXP BSODs on boot in tcg mode. Git bisect points to this: commit 23820dbfc79d1c9dce090b4c555994f2bb6a69b3 Author: Peter Crosthwaite peter.crosthwa...@xilinx.com Date: Mon Mar 16 22:35:54 2015 -0700 exec: Respect as_translate_internal length clamp This winXP BSOD happens on x86_64 target too. Reverting the above commit from git master fixes the BSOD. Any useful info about IO addresses on that BSOD? The last issue with this patch was IOPort code relying on the bug that this patch fixed. This could be similar and if we can track the failure to a particular address we can fix properly rather than another revert of that patch. Oh. I didn't know this patch has been reverted before. Anyway, I disabled auto-reboot on BSOD on my winXP (what a useful feature!) and here's what I see. IRQ_NOT_LESS_OR_EQUAL STOP: 0x0A (0x16, 0x02, 0x00, 0x80500EFC) (with some amount of leading zeros stripped). When this happens, win does something for quite some time, the BSOD comes after quite significant delay. Is there anything else I can look at, maybe some crash dump or something? I haven't done any windows debugging before. Thanks, /mjt
Re: [Qemu-devel] [PULL 00/28] pc, virtio enhancements
On 11 May 2015 at 13:46, Michael S. Tsirkin m...@redhat.com wrote: The following changes since commit 0d81cdddaa40a1988b24657aeac19959cfad0fde: Merge remote-tracking branch 'remotes/qmp-unstable/tags/for-upstream' into staging (2015-04-27 17:28:41 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream for you to fetch changes up to bc1f7c4c915a7c727741c4d27a2795e1039eacd3: acpi: update expected files for memory unplug (2015-05-11 09:21:37 +0200) pc, virtio enhancements Memory hot-unplug support for pc, MSI-X mapping update speedup for virtio-pci, misc refactorings and bugfixes. Signed-off-by: Michael S. Tsirkin m...@redhat.com Applied, thanks (with a conflict resolution in s390 virtio). -- PMM
Re: [Qemu-devel] [PATCH 1/6] hw/s390x/s390-virtio-bus: Remove meaningless blank Property
On 2015/5/12 15:52, Peter Maydell wrote: On 12 May 2015 at 03:25, shannon.z...@linaro.org wrote: From: Shannon Zhao shannon.z...@linaro.org Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/s390x/s390-virtio-bus.c | 15 --- 1 file changed, 15 deletions(-) diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index 1a72b56..273bd9c 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -536,17 +536,12 @@ static unsigned virtio_s390_get_features(DeviceState *d) Net and scsi are already fixed in master (I had to do that as part of the conflict resolution between your virtio patches and Cornelia's). Ok, so need to respin? -- Shannon
Re: [Qemu-devel] [PULL 00/19] target-arm queue
On 11 May 2015 at 14:40, Peter Maydell peter.mayd...@linaro.org wrote: This is mostly the GIC TZ changes, with a couple of other minor bugfixes. -- PMM The following changes since commit b951cda21d6b232f138ccf008e12bce8ddc95465: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-05-11 12:01:09 +0100) are available in the git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20150511 for you to fetch changes up to 49855cdaed78f66f501df6e18b8b3b7012cea2eb: hw/arm/highbank.c: Wire FIQ between CPU GIC (2015-05-11 14:28:54 +0100) Oops: hw/intc/arm_gic_kvm.c: In function ‘kvm_arm_gic_put’: hw/intc/arm_gic_kvm.c:357:12: error: ‘GICState’ has no member named ‘enabled’ hw/intc/arm_gic_kvm.c: In function ‘kvm_arm_gic_get’: hw/intc/arm_gic_kvm.c:458:6: error: ‘GICState’ has no member named ‘enabled’ I could have sworn I'd tested that. Will fix respin... -- PMM
[Qemu-devel] [PATCH v3 2/3] virtio-mmio: introduce set_guest_notifiers
From: Ying-Shiuan Pan yingshiuan@gmail.com Same as host notifier of virtio-mmio, most of codes came from virtio-pci. The kvm-arm does not yet support irqfd, need to fix the hard-coded part after kvm-arm gets irqfd support. Signed-off-by: Ying-Shiuan Pan yingshiuan@gmail.com Signed-off-by: Pavel Fedin p.fe...@samsung.com --- hw/virtio/virtio-mmio.c | 61 + 1 file changed, 61 insertions(+) diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index da56b9c..a4c4d89 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -395,6 +395,66 @@ static void virtio_mmio_reset(DeviceState *d) proxy-guest_page_shift = 0; } +static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool assign, + bool with_irqfd) +{ +VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d); +VirtIODevice *vdev = virtio_bus_get_device(proxy-bus); +VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev); +VirtQueue *vq = virtio_get_queue(vdev, n); +EventNotifier *notifier = virtio_queue_get_guest_notifier(vq); + +if (assign) { +int r = event_notifier_init(notifier, 0); +if (r 0) { +return r; +} +virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd); +} else { +virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd); +event_notifier_cleanup(notifier); +} + +if (vdc-guest_notifier_mask) { +vdc-guest_notifier_mask(vdev, n, !assign); +} + +return 0; +} + +static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs, + bool assign) +{ +VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d); +VirtIODevice *vdev = virtio_bus_get_device(proxy-bus); +/* TODO: need to check if kvm-arm supports irqfd */ +bool with_irqfd = false; +int r, n; + +nvqs = MIN(nvqs, VIRTIO_PCI_QUEUE_MAX); + +for (n = 0; n nvqs; n++) { +if (!virtio_queue_get_num(vdev, n)) { +break; +} + +r = virtio_mmio_set_guest_notifier(d, n, assign, with_irqfd); +if (r 0) { +goto assign_error; +} +} + +return 0; + +assign_error: +/* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */ +assert(assign); +while (--n = 0) { +virtio_mmio_set_guest_notifier(d, n, !assign, false); +} +return r; +} + static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, bool assign) { @@ -472,6 +532,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data) k-save_config = virtio_mmio_save_config; k-load_config = virtio_mmio_load_config; k-set_host_notifier = virtio_mmio_set_host_notifier; +k-set_guest_notifiers = virtio_mmio_set_guest_notifiers; k-get_features = virtio_mmio_get_features; k-device_plugged = virtio_mmio_device_plugged; k-has_variable_vring_alignment = true; -- 1.9.5.msysgit.0
[Qemu-devel] [PATCH v3 3/3] virtio-mmio: start ioeventfd when status gets DRIVER_OK
From: Ying-Shiuan Pan yingshiuan@gmail.com Signed-off-by: Ying-Shiuan Pan yingshiuan@gmail.com Signed-off-by: Pavel Fedin p.fe...@samsung.com --- hw/virtio/virtio-mmio.c | 47 +++ 1 file changed, 47 insertions(+) diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index a4c4d89..c12d3fc 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -22,6 +22,7 @@ #include hw/sysbus.h #include hw/virtio/virtio.h #include qemu/host-utils.h +#include sysemu/kvm.h #include hw/virtio/virtio-bus.h #include qemu/error-report.h @@ -120,6 +121,43 @@ static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy, return r; } +static void virtio_mmio_start_ioeventfd(VirtIOMMIOProxy *proxy) +{ +VirtIODevice *vdev = virtio_bus_get_device(proxy-bus); +int n, r; + +if (!kvm_eventfds_enabled() || +proxy-ioeventfd_disabled || +proxy-ioeventfd_started) { +return; +} + +for (n = 0; n VIRTIO_PCI_QUEUE_MAX; n++) { +if (!virtio_queue_get_num(vdev, n)) { +continue; +} + +r = virtio_mmio_set_host_notifier_internal(proxy, n, true, true); +if (r 0) { +goto assign_error; +} +} +proxy-ioeventfd_started = true; +return; + +assign_error: +while (--n = 0) { +if (!virtio_queue_get_num(vdev, n)) { +continue; +} + +r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false); +assert(r = 0); +} +proxy-ioeventfd_started = false; +error_report(%s: failed. Fallback to a userspace (slower)., __func__); +} + static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy) { int r; @@ -318,7 +356,16 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value, virtio_update_irq(vdev); break; case VIRTIO_MMIO_STATUS: +if (!(value VIRTIO_CONFIG_S_DRIVER_OK)) { +virtio_mmio_stop_ioeventfd(proxy); +} + virtio_set_status(vdev, value 0xff); + +if (value VIRTIO_CONFIG_S_DRIVER_OK) { +virtio_mmio_start_ioeventfd(proxy); +} + if (vdev-status == 0) { virtio_reset(vdev); } -- 1.9.5.msysgit.0
Re: [Qemu-devel] [PATCH v2] Fixes key mapping so all keys work
On 12 May 2015 at 05:11, Programmingkid programmingk...@gmail.com wrote: On May 10, 2015, at 2:45 PM, Peter Maydell wrote: This is two conceptually separate fixes: (1) better handle key input to the ADB keyboard (2) output the right keycodes on the OSX cocoa UI which should be in separate patches. We should also be doing the support for keypad-= by converting both the cocoa UI and the ADB keyboard device to the QKeyCode APIs, which can cleanly handle these key without inventing fake PC keycode numbers, as suggested by Gerd: https://lists.gnu.org/archive/html/qemu-devel/2015-02/msg01322.html So what you want is to eliminate the ps/xt key mappings found in the cocoa.m and use the QKeyCode API? I really didn't like how in the cocoa.m file, a key press is first translated to a ps/xt key value, then translated to a Macintosh ADB keycode. Basically this: Macintosh host key code = PS/XT key code = ADB key code I do admit I'm not fully sure how the new API is suppose to work. For a Macintosh host and guest it probably should work like this: Macintosh host key code = sent directly to guest OS For a Macintosh host and a PC guest, it probably should work like this: Macintosh host key code = PS/2 key code (not PS/XT) For a PC host and a PC guest, it probably should work like this: PC host key code = sent directly to guest OS For a PC host and a Macintosh guest, it might work like this: PC host key code (PS/2) = ADB key code No. In the new scheme it would go: Mac host keycode = QKeyCode = [whatever keycode guest h/w uses] It's similar to the PS/XT scheme, except that the QKeyCodes genuinely cover all possible keys, not just the ones that happen to be present for the PC. (Instead of converting to a scancode in ui/cocoa.m, we would convert to a QKeyCode, then call qemu_input_event_send_key_qcode rather than ...send_key_number.) Doing direct conversion between host and guest is a bad idea, because then every UI frontend needs to know about every possible scancode scheme used by every keyboard device, and to know which device is currently being used. (And adding a new keyboard device would be massively painful.) Going via QKeyCode means that UI frontends only need to know about the keycode the host GUI uses and QEMU's common scheme; similarly keyboard device models only need to know about QEMU's common scheme and the codes they need to send to the guest. (The bug you're trying to fix here is actually a result of the remains of a 'direct conversion' API from when the only guests we cared about were PC/x86 -- that's why the input API is using a PC scancode, and that's why it doesn't work with keys that aren't on a PC. Going via a universal intermediate encoding fixes this problem.) Host key code = QKeyCode = guest key code I would like to avoid this translation because it would be relatively slow. It would be as inefficient as what we have now. At the rate at which keys are pressed (which is governed by how fast a human can type, which is massively slow compared to how fast computers can process) the speed of doing a pair of array lookups to do keycode conversion is absolutely lost in the noise. Maintainability and flexibility of the code is a more important consideration in my opinion. thanks -- PMM
Re: [Qemu-devel] [PATCH 18/31] q35: fix ESMRAMC default
On Mo, 2015-05-11 at 15:49 +0200, Paolo Bonzini wrote: From: Gerd Hoffmann kra...@redhat.com [ more verbose commit message for squashing in ] The cache bits in ESMRAMC are hardcoded to 1 (=disabled) according to the q35 mch specs. Add and use a define with this default. While being at it also update the SMRAM default to use the name (no code change, just makes things a bit more readable). Signed-off-by: Gerd Hoffmann kra...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/pci-host/q35.c | 1 + include/hw/pci-host/q35.h | 7 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 35b89da..8471d7a 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -351,6 +351,7 @@ static void mch_reset(DeviceState *qdev) MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT); d-config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT; +d-config[MCH_HOST_BRIDGE_ESMRAMC] = MCH_HOST_BRIDGE_ESMRAMC_DEFAULT; mch_update(mch); } diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h index 0fff6a2..d3c7bbb 100644 --- a/include/hw/pci-host/q35.h +++ b/include/hw/pci-host/q35.h @@ -128,7 +128,6 @@ typedef struct Q35PCIHost { #define MCH_HOST_BRIDGE_SMRAM 0x9d #define MCH_HOST_BRIDGE_SMRAM_SIZE 2 -#define MCH_HOST_BRIDGE_SMRAM_DEFAULT ((uint8_t)0x2) #define MCH_HOST_BRIDGE_SMRAM_D_OPEN ((uint8_t)(1 6)) #define MCH_HOST_BRIDGE_SMRAM_D_CLS((uint8_t)(1 5)) #define MCH_HOST_BRIDGE_SMRAM_D_LCK((uint8_t)(1 4)) @@ -139,6 +138,8 @@ typedef struct Q35PCIHost { #define MCH_HOST_BRIDGE_SMRAM_C_END0xc #define MCH_HOST_BRIDGE_SMRAM_C_SIZE 0x2 #define MCH_HOST_BRIDGE_UPPER_SYSTEM_BIOS_END 0x10 +#define MCH_HOST_BRIDGE_SMRAM_DEFAULT \ +MCH_HOST_BRIDGE_SMRAM_C_BASE_SEG #define MCH_HOST_BRIDGE_ESMRAMC0x9e #define MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME ((uint8_t)(1 7)) @@ -151,6 +152,10 @@ typedef struct Q35PCIHost { #define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_2MB((uint8_t)(0x1 1)) #define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_8MB((uint8_t)(0x2 1)) #define MCH_HOST_BRIDGE_ESMRAMC_T_EN ((uint8_t)1) +#define MCH_HOST_BRIDGE_ESMRAMC_DEFAULT \ +(MCH_HOST_BRIDGE_ESMRAMC_SM_CACHE | \ + MCH_HOST_BRIDGE_ESMRAMC_SM_L1 |\ + MCH_HOST_BRIDGE_ESMRAMC_SM_L2) /* D1:F0 PCIE* port*/ #define MCH_PCIE_DEV 1
Re: [Qemu-devel] [PATCH 23/31] ich9: implement SMI_LOCK
On Mo, 2015-05-11 at 15:49 +0200, Paolo Bonzini wrote: From: Gerd Hoffmann kra...@redhat.com [ more verbose commit message for squashing in ] Add write mask for the smi enable register, so we can disable write access to certain bits. Open all bits on reset. Disable write access to GBL_SMI_EN when SMI_LOCK (in ich9 lpc pci config space) is set. Write access to SMI_LOCK itself is disabled too. Signed-off-by: Gerd Hoffmann kra...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/acpi/ich9.c | 4 +++- hw/isa/lpc_ich9.c | 19 +++ include/hw/acpi/ich9.h | 1 + include/hw/i386/ich9.h | 6 ++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 5352e19..310aa64 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -94,7 +94,8 @@ static void ich9_smi_writel(void *opaque, hwaddr addr, uint64_t val, ICH9LPCPMRegs *pm = opaque; switch (addr) { case 0: -pm-smi_en = val; +pm-smi_en = ~pm-smi_en_wmask; +pm-smi_en |= (val pm-smi_en_wmask); break; } } @@ -198,6 +199,7 @@ static void pm_reset(void *opaque) * support SMM mode. */ pm-smi_en |= ICH9_PMIO_SMI_EN_APMC_EN; } +pm-smi_en_wmask = ~0; acpi_update_sci(pm-acpi_regs, pm-irq); } diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index dba7585..0269cfe 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -410,12 +410,28 @@ static void ich9_lpc_rcba_update(ICH9LPCState *lpc, uint32_t rbca_old) } } +/* config:GEN_PMCON* */ +static void +ich9_lpc_pmcon_update(ICH9LPCState *lpc) +{ +uint16_t gen_pmcon_1 = pci_get_word(lpc-d.config + ICH9_LPC_GEN_PMCON_1); +uint16_t wmask; + +if (gen_pmcon_1 ICH9_LPC_GEN_PMCON_1_SMI_LOCK) { +wmask = pci_get_word(lpc-d.wmask + ICH9_LPC_GEN_PMCON_1); +wmask = ~ICH9_LPC_GEN_PMCON_1_SMI_LOCK; +pci_set_word(lpc-d.wmask + ICH9_LPC_GEN_PMCON_1, wmask); +lpc-pm.smi_en_wmask = ~1; +} +} + static int ich9_lpc_post_load(void *opaque, int version_id) { ICH9LPCState *lpc = opaque; ich9_lpc_pmbase_update(lpc); ich9_lpc_rcba_update(lpc, 0 /* disabled ICH9_LPC_RBCA_EN */); +ich9_lpc_pmcon_update(lpc); return 0; } @@ -438,6 +454,9 @@ static void ich9_lpc_config_write(PCIDevice *d, if (ranges_overlap(addr, len, ICH9_LPC_PIRQE_ROUT, 4)) { pci_bus_fire_intx_routing_notifier(lpc-d.bus); } +if (ranges_overlap(addr, len, ICH9_LPC_GEN_PMCON_1, 8)) { +ich9_lpc_pmcon_update(lpc); +} } static void ich9_lpc_reset(DeviceState *qdev) diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h index c2d3dba..77cc65c 100644 --- a/include/hw/acpi/ich9.h +++ b/include/hw/acpi/ich9.h @@ -39,6 +39,7 @@ typedef struct ICH9LPCPMRegs { MemoryRegion io_smi; uint32_t smi_en; +uint32_t smi_en_wmask; uint32_t smi_sts; qemu_irq irq; /* SCI */ diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h index f4e522c..a2cc15c 100644 --- a/include/hw/i386/ich9.h +++ b/include/hw/i386/ich9.h @@ -152,6 +152,12 @@ Object *ich9_lpc_find(void); #define ICH9_LPC_PIRQ_ROUT_MASK Q35_MASK(8, 3, 0) #define ICH9_LPC_PIRQ_ROUT_DEFAULT 0x80 +#define ICH9_LPC_GEN_PMCON_10xa0 +#define ICH9_LPC_GEN_PMCON_1_SMI_LOCK (1 4) +#define ICH9_LPC_GEN_PMCON_20xa2 +#define ICH9_LPC_GEN_PMCON_30xa4 +#define ICH9_LPC_GEN_PMCON_LOCK 0xa6 + #define ICH9_LPC_RCBA 0xf0 #define ICH9_LPC_RCBA_BA_MASK Q35_MASK(32, 31, 14) #define ICH9_LPC_RCBA_EN0x1
Re: [Qemu-devel] [PATCH 0/6] ui/cocoa: Fix OSX 10.10 warnings (and drop 10.4 support)
On 12 May 2015 at 00:45, Programmingkid programmingk...@gmail.com wrote: On May 10, 2015, at 6:19 PM, Peter Maydell wrote: I've chosen to implement some of them by simply dropping the backward-compatibility support for OSX 10.4. This is basically a pragmatic decision since I don't think we can support ancient versions forever, especially when I don't actually have a system to test compiling them on. (Last time I tried building QEMU on 10.4 it was an insane pain because you had to start by building all the dependencies and a new compiler too.) I would not be terribly surprised somebody told me we'd already accidentally broken 10.4 compilation, in fact. 10.5 is the last PPC OSX release so it seems like a reasonable minimum-version requirement (though I don't have a 10.5 setup either, so am reliant on people telling me if it breaks.) I hate seeing support for older operating system end. The only thing you appear to be seeing are warnings. Are they really so bad? It might be simpler to just disable warnings. In the end I guess there isn't someone out there who is using QEMU on Mac OS 10.4 or earlier. Well, we've already in some sense dropped 10.4 support, because 10.4 doesn't have rez/setfile, so it won't build with the icon patch applied. I wouldn't mind continuing to support 10.4 if we had at least one person who had 10.4 and had an actual use for 10.4 support and regularly built on 10.4 and told us if we broke things. But we don't, so all that we're really doing with these ifdefs is making our code more complicated and harder to maintain. 10.5 came out in 2007... -- PMM
Re: [Qemu-devel] [PATCH 19/31] q35: add config space wmask for SMRAM and ESMRAMC
On Mo, 2015-05-11 at 15:49 +0200, Paolo Bonzini wrote: From: Gerd Hoffmann kra...@redhat.com [ more verbose commit message for squashing in ] Not all bits in SMRAM and ESMRAMC can be changed by the guest. Add wmask defines accordingly and set them in mch_reset(). Signed-off-by: Gerd Hoffmann kra...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/pci-host/q35.c | 2 ++ include/hw/pci-host/q35.h | 9 + 2 files changed, 11 insertions(+) diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 8471d7a..df0032e 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -352,6 +352,8 @@ static void mch_reset(DeviceState *qdev) d-config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT; d-config[MCH_HOST_BRIDGE_ESMRAMC] = MCH_HOST_BRIDGE_ESMRAMC_DEFAULT; +d-wmask[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_WMASK; +d-wmask[MCH_HOST_BRIDGE_ESMRAMC] = MCH_HOST_BRIDGE_ESMRAMC_WMASK; mch_update(mch); } diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h index d3c7bbb..01b8492 100644 --- a/include/hw/pci-host/q35.h +++ b/include/hw/pci-host/q35.h @@ -140,6 +140,11 @@ typedef struct Q35PCIHost { #define MCH_HOST_BRIDGE_UPPER_SYSTEM_BIOS_END 0x10 #define MCH_HOST_BRIDGE_SMRAM_DEFAULT \ MCH_HOST_BRIDGE_SMRAM_C_BASE_SEG +#define MCH_HOST_BRIDGE_SMRAM_WMASK \ +(MCH_HOST_BRIDGE_SMRAM_D_OPEN | \ + MCH_HOST_BRIDGE_SMRAM_D_CLS | \ + MCH_HOST_BRIDGE_SMRAM_D_LCK | \ + MCH_HOST_BRIDGE_SMRAM_G_SMRAME) #define MCH_HOST_BRIDGE_ESMRAMC0x9e #define MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME ((uint8_t)(1 7)) @@ -156,6 +161,10 @@ typedef struct Q35PCIHost { (MCH_HOST_BRIDGE_ESMRAMC_SM_CACHE | \ MCH_HOST_BRIDGE_ESMRAMC_SM_L1 |\ MCH_HOST_BRIDGE_ESMRAMC_SM_L2) +#define MCH_HOST_BRIDGE_ESMRAMC_WMASK \ +(MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME | \ + MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_MASK | \ + MCH_HOST_BRIDGE_ESMRAMC_T_EN) /* D1:F0 PCIE* port*/ #define MCH_PCIE_DEV 1
Re: [Qemu-devel] [PATCH 6/6] Add comma after DEFINE_PROP_END_OF_LIST()
On 2015/5/12 14:59, Peter Maydell wrote: On 12 May 2015 at 03:25, shannon.z...@linaro.org wrote: From: Shannon Zhao shannon.z...@linaro.org Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c index 433463e..d2b3f37 100644 --- a/hw/audio/intel-hda.c +++ b/hw/audio/intel-hda.c @@ -31,7 +31,7 @@ static Property hda_props[] = { DEFINE_PROP_UINT32(cad, HDACodecDevice, cad, -1), -DEFINE_PROP_END_OF_LIST() +DEFINE_PROP_END_OF_LIST(), }; Why do we need to do this? The usual reason for having a comma after the last item in an array is so that if we add another item after it later we don't have to edit the line that used to be last. But with these lists the DEFINE_PROP_END_OF_LIST() line remains last by definition -- new entries will always go above it, and anything below it would be a bug. So there's no point in having a comma after it. Here I just want to make the code style consistent. If this is not necessary, we can drop this one. -- Shannon
Re: [Qemu-devel] [PATCH 20/31] q35: implement SMRAM.D_LCK
On Mo, 2015-05-11 at 15:49 +0200, Paolo Bonzini wrote: From: Gerd Hoffmann kra...@redhat.com [ more verbose commit message for squashing in ] Once the SMRAM.D_LCK bit has been set by the guest several bits in SMRAM and ESMRAMC become readonly until the next machine reset. Implement this by updating the wmask accordingly when the guest sets the lock bit. As the lock it itself is locked down too we don't need to worry about the guest clearing the lock bit. Signed-off-by: Gerd Hoffmann kra...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/pci-host/q35.c | 8 +++- include/hw/pci-host/q35.h | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index df0032e..972d31e 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -268,6 +268,13 @@ static void mch_update_smram(MCHPCIState *mch) PCIDevice *pd = PCI_DEVICE(mch); bool h_smrame = (pd-config[MCH_HOST_BRIDGE_ESMRAMC] MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME); +/* implement SMRAM.D_LCK */ +if (pd-config[MCH_HOST_BRIDGE_SMRAM] MCH_HOST_BRIDGE_SMRAM_D_LCK) { +pd-config[MCH_HOST_BRIDGE_SMRAM] = ~MCH_HOST_BRIDGE_SMRAM_D_OPEN; +pd-wmask[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_WMASK_LCK; +pd-wmask[MCH_HOST_BRIDGE_ESMRAMC] = MCH_HOST_BRIDGE_ESMRAMC_WMASK_LCK; +} + memory_region_transaction_begin(); if (pd-config[MCH_HOST_BRIDGE_SMRAM] SMRAM_D_OPEN) { @@ -297,7 +304,6 @@ static void mch_write_config(PCIDevice *d, { MCHPCIState *mch = MCH_PCI_DEVICE(d); -/* XXX: implement SMRAM.D_LOCK */ pci_default_write_config(d, address, val, len); if (ranges_overlap(address, len, MCH_HOST_BRIDGE_PAM0, diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h index 01b8492..113cbe8 100644 --- a/include/hw/pci-host/q35.h +++ b/include/hw/pci-host/q35.h @@ -145,6 +145,8 @@ typedef struct Q35PCIHost { MCH_HOST_BRIDGE_SMRAM_D_CLS | \ MCH_HOST_BRIDGE_SMRAM_D_LCK | \ MCH_HOST_BRIDGE_SMRAM_G_SMRAME) +#define MCH_HOST_BRIDGE_SMRAM_WMASK_LCK \ +MCH_HOST_BRIDGE_SMRAM_D_CLS #define MCH_HOST_BRIDGE_ESMRAMC0x9e #define MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME ((uint8_t)(1 7)) @@ -165,6 +167,7 @@ typedef struct Q35PCIHost { (MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME | \ MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_MASK | \ MCH_HOST_BRIDGE_ESMRAMC_T_EN) +#define MCH_HOST_BRIDGE_ESMRAMC_WMASK_LCK 0 /* D1:F0 PCIE* port*/ #define MCH_PCIE_DEV 1
Re: [Qemu-devel] [PATCH 6/6] Add comma after DEFINE_PROP_END_OF_LIST()
On 12 May 2015 at 03:25, shannon.z...@linaro.org wrote: From: Shannon Zhao shannon.z...@linaro.org Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c index 433463e..d2b3f37 100644 --- a/hw/audio/intel-hda.c +++ b/hw/audio/intel-hda.c @@ -31,7 +31,7 @@ static Property hda_props[] = { DEFINE_PROP_UINT32(cad, HDACodecDevice, cad, -1), -DEFINE_PROP_END_OF_LIST() +DEFINE_PROP_END_OF_LIST(), }; Why do we need to do this? The usual reason for having a comma after the last item in an array is so that if we add another item after it later we don't have to edit the line that used to be last. But with these lists the DEFINE_PROP_END_OF_LIST() line remains last by definition -- new entries will always go above it, and anything below it would be a bug. So there's no point in having a comma after it. -- PMM
Re: [Qemu-devel] [RFC PATCH 02/34] tcg+qom: QOMify core CPU defintions
On Mon, May 11, 2015 at 1:18 PM, Richard Henderson r...@twiddle.net wrote: On 05/11/2015 03:24 AM, Paolo Bonzini wrote: On 11/05/2015 12:18, Andreas Färber wrote: +int (*cpu_mmu_index)(CPUState *cpu); +void (*cpu_get_tb_cpu_state)(CPUState *cpu, + void *pc, /* target_long * */ + void *cs_base, /* target_long */ + int *flags); +void (*gen_intermediate_code)(void *env, struct TranslationBlock *tb); +void (*gen_intermediate_code_pc)(void *env, struct TranslationBlock *tb); +void (*restore_state_to_opc)(void *env, struct TranslationBlock *tb, + int pc_pos); +void (*tlb_fill)(CPUState *cs, uint64_t addr, int is_write, int mmu_idx, + uintptr_t retaddr); } CPUClass; #ifdef HOST_WORDS_BIGENDIAN [snip] Paolo had objected to this when I tried it. The counter-suggestion was something about reworking how the cputlb code is built per target - please check the archives. Right. My point was that these functions are not polymorphic. Each call to these should know exactly which function to call. That's some major surgery you have planned there. Just tried this. It's ... rather hard. Especially the path via the qemu_ld/st helpers, where function to call is currently hard-coded into the tcg backend. Yes. I actually seemed to make decent progress on the multi-compile approach until I hit this brick wall: tcg/i386/tcg-target.c:1131:17: error: ‘helper_ret_ldub_mmu’ undeclared here (not in a function) [MO_UB] = helper_ret_ldub_mmu, In my multi-compile approach helper_*[ld|st]* needs to be renamed per-arch for the multiple compiled cputlb.o. Hence I have no symbol with the unqualified name. But even if I do solve my namespacing problem, I still have an ambiguity of which cputlb.o provided helper_*[ld|st]* to use from the TCG backend. This would mean all those APIs would have to virtualised. The big question for Paolo, is what complete set of APIs defines the common-code/non-common-code boundary? tlb_fill does seem to do the job nicely and looking at the architecture implementations it's not a super fast path (falling back to a page table faulter). Somewhere along the call path from the qemu_st_helpers uses (tcg/i386/tcg-target.c) through to tlb_fill there has to be a virtualised function unless I am missing something? I think that this is a decent step forward, modulo the conditionals along the use paths. I think we ought to clean up all of the translators to the new QOM hooks. So the conditional can be ditched by having the CPU base class defaulting the hook to the globally defined function. Then arches can be brought online one-by-one. I can't imagine that most of these hooks are called frequently enough that the indirect call really matters. Certainly gen_intermediate_code need not use the hook when initializing the mmu_idx in the DisasContext. Ok so the solution to this is to opt-out of the hook via a re-#define when we have a target-specific cpu.h handy. This will actually mean no change to single-arch builds but multi-arch will use the hook from core code only. That said, I'd approve of a goal to arrange for the correct qemu_ld/st helpers to be called, and a direct call to the proper tlb_fill. But, one step at a time... I don't know what this means exactly. tlb_fill is called by functions that are linked to common code (TCG backends) so I don't see a non virtualized solution. Is this refactoring to move tlb_fill? Regards, Peter r~
[Qemu-devel] [PATCH v3 1/3] virtio-mmio: introduce set_host_notifier()
From: Ying-Shiuan Pan yingshiuan@gmail.com set_host_notifier() is introduced into virtio-mmio now. Most of codes came from virtio-pci. Signed-off-by: Ying-Shiuan Pan yingshiuan@gmail.com Signed-off-by: Pavel Fedin p.fe...@samsung.com --- hw/virtio/virtio-mmio.c | 73 + 1 file changed, 73 insertions(+) diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index 10123f3..da56b9c 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -23,6 +23,7 @@ #include hw/virtio/virtio.h #include qemu/host-utils.h #include hw/virtio/virtio-bus.h +#include qemu/error-report.h /* #define DEBUG_VIRTIO_MMIO */ @@ -87,8 +88,59 @@ typedef struct { uint32_t guest_page_shift; /* virtio-bus */ VirtioBusState bus; +bool ioeventfd_disabled; +bool ioeventfd_started; } VirtIOMMIOProxy; +static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy, + int n, bool assign, + bool set_handler) +{ +VirtIODevice *vdev = virtio_bus_get_device(proxy-bus); +VirtQueue *vq = virtio_get_queue(vdev, n); +EventNotifier *notifier = virtio_queue_get_host_notifier(vq); +int r = 0; + +if (assign) { +r = event_notifier_init(notifier, 1); +if (r 0) { +error_report(%s: unable to init event notifier: %d, + __func__, r); +return r; +} +virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler); +memory_region_add_eventfd(proxy-iomem, VIRTIO_MMIO_QUEUENOTIFY, 4, + true, n, notifier); +} else { +memory_region_del_eventfd(proxy-iomem, VIRTIO_MMIO_QUEUENOTIFY, 4, + true, n, notifier); +virtio_queue_set_host_notifier_fd_handler(vq, false, false); +event_notifier_cleanup(notifier); +} +return r; +} + +static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy) +{ +int r; +int n; +VirtIODevice *vdev = virtio_bus_get_device(proxy-bus); + +if (!proxy-ioeventfd_started) { +return; +} + +for (n = 0; n VIRTIO_PCI_QUEUE_MAX; n++) { +if (!virtio_queue_get_num(vdev, n)) { +continue; +} + +r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false); +assert(r = 0); +} +proxy-ioeventfd_started = false; +} + static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size) { VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; @@ -336,12 +388,32 @@ static void virtio_mmio_reset(DeviceState *d) { VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d); +virtio_mmio_stop_ioeventfd(proxy); virtio_bus_reset(proxy-bus); proxy-host_features_sel = 0; proxy-guest_features_sel = 0; proxy-guest_page_shift = 0; } +static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, + bool assign) +{ +VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); + +/* Stop using ioeventfd for virtqueue kick if the device starts using host + * notifiers. This makes it easy to avoid stepping on each others' toes. + */ +proxy-ioeventfd_disabled = assign; +if (assign) { +virtio_mmio_stop_ioeventfd(proxy); +} +/* We don't need to start here: it's not needed because backend + * currently only stops on status change away from ok, + * reset, vmstop and such. If we do add code to start here, + * need to check vmstate, device state etc. */ +return virtio_mmio_set_host_notifier_internal(proxy, n, assign, false); +} + /* virtio-mmio device */ /* This is called by virtio-bus just after the device is plugged. */ @@ -399,6 +471,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data) k-notify = virtio_mmio_update_irq; k-save_config = virtio_mmio_save_config; k-load_config = virtio_mmio_load_config; +k-set_host_notifier = virtio_mmio_set_host_notifier; k-get_features = virtio_mmio_get_features; k-device_plugged = virtio_mmio_device_plugged; k-has_variable_vring_alignment = true; -- 1.9.5.msysgit.0
Re: [Qemu-devel] [PULL 3/6] console-gl: add opengl rendering helper functions
On Mo, 2015-05-11 at 22:32 +0200, Alexander Graf wrote: But I see the same error on openSUSE 12.3 x86_64 now: Ah, cool, I have an iso for that laying around. /me goes install. thanks, Gerd
[Qemu-devel] [PATCH v3 0/3] virtio-mmio: introduce eventfd support
This patch set introduces eventfd support for virio-mmio. It was originally published by Ying-Shiuan Pan but never got it to upstream: https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg00715.html I have updated and successfully tested it with vhost-net. I confirm that this solution significantly improves the network performance even without irqfd. I would like to upstream it, since virtio-mmio is still there. I know that some of you consider it deprecated, however i believe this is not entirely true. Because you can add it to machine models which are not supposed to have PCI (like vexpress). An old patch set relied on additional eventfd option in order to disable the support if not implemented in kernel. My version simply checks kvm_eventfds_enabled() for this purpose, so backwards compatibility is much better. I decided to leave this set in three parts because ioeventfd support should be enabled only when both host and guest notifiers are in place. I believe it will not work with partial implementation, at least because vhost-net requires both sets of eventfds. In this version i added correct reset handling. Ying-Shiuan Pan (3): virtio-mmio: introduce set_host_notifier() virtio-mmio: introduce set_guest_notifiers virtio-mmio: start ioeventfd when status gets DRIVER_OK hw/virtio/virtio-mmio.c | 181 1 file changed, 181 insertions(+) -- 1.9.5.msysgit.0
Re: [Qemu-devel] [PATCH 1/6] hw/s390x/s390-virtio-bus: Remove meaningless blank Property
On Tue, 12 May 2015 10:25:16 +0800 shannon.z...@linaro.org wrote: From: Shannon Zhao shannon.z...@linaro.org Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/s390x/s390-virtio-bus.c | 15 --- 1 file changed, 15 deletions(-) Acked-by: Cornelia Huck cornelia.h...@de.ibm.com
Re: [Qemu-devel] [RFC PATCH v5 00/14] data-driven device registers
Ping! On Mon, Apr 27, 2015 at 2:57 PM, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: Hi All. This is a new scheme I've come up with handling device registers in a data driven way. My motivation for this is to factor out a lot of the access checking that seems to be replicated in every device. See P1 commit message for further discussion. P1 is the main patch, adds the register definition functionality P2-3,6 add helpers that glue the register API to the Memory API P4 Defines a set of macros that minimise register and field definitions P5 is QOMfication P7 is a trivial P10-13 Work up to GPIO support P8,9,14 add new devices (the Xilinx Zynq devcfg ZynqMP SLCR) that use this scheme. This Zynq devcfg device was particularly finnicky with per-bit restrictions. I'm also looking for a higher-than-usual modelling fidelity on the register space, with semantics defined for random reserved bits in-between otherwise consistent fields. Here's an example of the qemu_log output for the devcfg device. This is produced by now generic sharable code: /machine/unattached/device[44]:Addr 0x08:CFG: write of value 0508 /machine/unattached/device[44]:Addr 0x80:MCTRL: write of value 00800010 /machine/unattached/device[44]:Addr 0x10:INT_MASK: write of value /machine/unattached/device[44]:Addr :CTRL: write of value 0c00607f And an example of a rogue guest banging on a bad bit: /machine/unattached/device[44]:Addr 0x14:STATUS bits 0x01 may not be \ written to 1 One thing that was suggested early on was built in GPIO support. This is now implemented (as of V5 of the series). To give an example of this, P14 is a (super minimal) implementation of the system controller for the new ZynqMP SoC. The first iteration of the machine model is currently in review stages this this dev cannot be added to a machine yet. Remaking as RFC, as I think it should be added in two stages, one containing the base support then follow up for GPIOs. RFCing the whole thing here to demonstrate the GPIOs. A future feature I am interested in is implementing TCG optimisation of side-effectless registers. The register API allows clear definition of what registers have txn side effects and which ones don't. You could even go a step further and translate such side-effectless accesses based on the data pointer for the register. Changed from v4: Rebased Added QOMification Added GPIO support Refactored Devcfg device to use FIELD/REG/EX macros. Update style of devcfg device Added init_block help. Changed from v3: Rebased Added reserved bits. Cleaner separation of decode and access components (Patch 3) Changed from v2: Fixed for hw/ re-orginisation (Paolo review) Simplified and optimized (PMM and Gerd review) Changed from v1: Added ONES macro patch Dropped bogus former patch 1 (PMM review) Addressed Blue, Gerd and MST comments. Simplified to be more Memory API compatible. Added Memory API helpers. Please see discussion already on list and commit msgs for more detail. Peter Crosthwaite (14): register: Add Register API register: Add Memory API glue register: Add support for decoding information register: Define REG and FIELD macros register: QOMify register: Add block initialise helper bitops: Add ONES macro dma: Add Xilinx Zynq devcfg device model xilinx_zynq: add devcfg to machine model qdev: Define qdev_get_gpio_out qdev: Add qdev_pass_all_gpios API irq: Add opaque setter routine register: Add GPIO API misc: Introduce ZynqMP IOU SLCR default-configs/arm-softmmu.mak| 1 + hw/arm/xilinx_zynq.c | 8 + hw/core/Makefile.objs | 1 + hw/core/irq.c | 5 + hw/core/qdev.c | 21 ++ hw/core/register.c | 390 +++ hw/dma/Makefile.objs | 1 + hw/dma/xlnx-zynq-devcfg.c | 406 + hw/misc/Makefile.objs | 1 + hw/misc/xlnx-zynqmp-iou-slcr.c | 113 + include/hw/dma/xlnx-zynq-devcfg.h | 62 + include/hw/irq.h | 2 + include/hw/misc/xlnx-zynqmp-iou-slcr.h | 47 include/hw/qdev-core.h | 3 + include/hw/register.h | 276 ++ include/qemu/bitops.h | 2 + 16 files changed, 1339 insertions(+) create mode 100644 hw/core/register.c create mode 100644 hw/dma/xlnx-zynq-devcfg.c create mode 100644 hw/misc/xlnx-zynqmp-iou-slcr.c create mode 100644 include/hw/dma/xlnx-zynq-devcfg.h create mode 100644 include/hw/misc/xlnx-zynqmp-iou-slcr.h create mode 100644 include/hw/register.h -- 2.3.6.3.g2cc70ee
Re: [Qemu-devel] [RFC PATCH 02/34] tcg+qom: QOMify core CPU defintions
On Mon, May 11, 2015 at 3:39 AM, Paolo Bonzini pbonz...@redhat.com wrote: On 11/05/2015 12:36, Andreas Färber wrote: Right. My point was that these functions are not polymorphic. Each call to these should know exactly which function to call. cputlb.c, cpu-exec.c and parts of translate-all.c should be the moral equivalent of C++ templates. I wouldn't mind switching to C++, but if we want to make them polymorphic we should do it at compile time through multiple compilation and/or inclusion from target-*. I think we got more concrete than that, possibly on IRC only? I believe #include'ing cputlb.c from some target-* file was one of your proposed solutions? Yes (but Peter is making more functions virtual, so he'd have to do the same also for cpu-exec.c and parts of translate-all.c). And splitting some of the inline functions that I was getting rid of into some new (or existing?) file? If I remember correctly, another solution could be to keep the virtual functions, but ensure that all the hot paths were devirtualizing them and calling the CPU-specific function directly. What does this mean exactly? Is there still a common code for cputlb.c and friends? Regards, Peter Paolo
Re: [Qemu-devel] [PATCH 1/6] hw/s390x/s390-virtio-bus: Remove meaningless blank Property
On 12 May 2015 at 03:25, shannon.z...@linaro.org wrote: From: Shannon Zhao shannon.z...@linaro.org Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/s390x/s390-virtio-bus.c | 15 --- 1 file changed, 15 deletions(-) diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index 1a72b56..273bd9c 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -536,17 +536,12 @@ static unsigned virtio_s390_get_features(DeviceState *d) Net and scsi are already fixed in master (I had to do that as part of the conflict resolution between your virtio patches and Cornelia's). -- PMM
Re: [Qemu-devel] [PULL 00/19] target-arm queue
On Tue, May 12, 2015 at 1:01 AM, Peter Maydell peter.mayd...@linaro.org wrote: On 11 May 2015 at 14:40, Peter Maydell peter.mayd...@linaro.org wrote: This is mostly the GIC TZ changes, with a couple of other minor bugfixes. -- PMM The following changes since commit b951cda21d6b232f138ccf008e12bce8ddc95465: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-05-11 12:01:09 +0100) are available in the git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20150511 for you to fetch changes up to 49855cdaed78f66f501df6e18b8b3b7012cea2eb: hw/arm/highbank.c: Wire FIQ between CPU GIC (2015-05-11 14:28:54 +0100) Oops: hw/intc/arm_gic_kvm.c: In function ‘kvm_arm_gic_put’: hw/intc/arm_gic_kvm.c:357:12: error: ‘GICState’ has no member named ‘enabled’ hw/intc/arm_gic_kvm.c: In function ‘kvm_arm_gic_get’: hw/intc/arm_gic_kvm.c:458:6: error: ‘GICState’ has no member named ‘enabled’ I could have sworn I'd tested that. Will fix respin... Feel like grabbing the new Zynq series (v9) with it? :) Regards, Peter -- PMM
Re: [Qemu-devel] [PULL 00/19] target-arm queue
On 12 May 2015 at 09:10, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: On Tue, May 12, 2015 at 1:01 AM, Peter Maydell peter.mayd...@linaro.org wrote: I could have sworn I'd tested that. Will fix respin... Feel like grabbing the new Zynq series (v9) with it? :) Maybe. I was going to except for that last minute issue in v8. No real issue with doing two pullreqs, though; in some ways that's better than one large one. -- PMM
Re: [Qemu-devel] [PATCH RFC 4/7] vhost: set vring endianness for legacy virtio
On Tue, May 12, 2015 at 06:25:32PM +0200, Cornelia Huck wrote: On Tue, 12 May 2015 17:15:53 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, May 12, 2015 at 03:25:30PM +0200, Cornelia Huck wrote: On Wed, 06 May 2015 14:08:02 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: Legacy virtio is native endian: if the guest and host endianness differ, we have to tell vhost so it can swap bytes where appropriate. This is done through a vhost ring ioctl. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- hw/virtio/vhost.c | 50 +- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 54851b7..1d7b939 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c (...) @@ -677,6 +700,16 @@ static int vhost_virtqueue_start(struct vhost_dev *dev, return -errno; } +if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1) I think this should either go in after the virtio-1 base support (more feature bits etc.) or get a big fat comment and be touched up later. I'd prefer the first solution so it does not get forgotten, but I'm not sure when Michael plans to proceed with the virtio-1 patches (I think they're mostly fine already). There are three main issues with virtio 1 patches that I am aware of. One issue with virtio 1 patches as they are is with how features are handled ATM. There are 3 types of features a. virtio 1 only features b. virtio 0 only features c. shared features and 3 types of devices a. legacy device: has b+c features b. modern device: has a+c features c. transitional device: has a+c features but exposes only c through the legacy interface Wouldn't a transitional device be able to expose b as well? No because the virtio 1 spec says it shouldn't. So I think a callback that gets features depending on guest version isn't a good way to model it because fundamentally device has one set of features. A better way to model this is really just a single host_features bitmask, and for transitional devices, a mask hiding a features - which are so far all bits 31, so maybe for now we can just have a global mask. How would this work for transitional presenting a modern device - would you have a superset of bits and masks for legacy and modern? Basically we expose through modern interface a superset of bits exposed through legacy. F_BAD for pci is probably the only exception. We need to validate features at initialization time and make sure they make sense, fail if not (sometimes we need to mask features if they don't make sense - this is unfortunate but might be needed for compatibility). Moving host_features to virtio core would make all of the above easier. I have started hacking up code that moves host_features, but I'm quite lost with all the different virtio versions floating around. Currently trying against master, but that of course ignores the virtio-1 issues. Yes, I think we should focus on infrastructure cleanups in master first. Second issue is migration, some of it is with migrating the new features, so that's tied to the first one. There's also the used and avail addresses, but that kind of follows from virtio-1 support. Third issue is fixing devices so they don't try to access guest memory until DRIVER_OK is set. This is surprisingly hard to do generally given need to support old drivers which don't set DRIVER_OK or set it very late, and the fact that we tied work-arounds for even older drivers which dont' set pci bus master to the DRIVER_OK bit. I tried, and I'm close to giving up and just checking guest ack for virtio 1, and ignoring DRIVER_OK requirement if not there. If legacy survived like it is until now, it might be best to focus on modern devices for this. I'm kind of unhappy that it's up to guest though as that controls whether we run in modern mode. But yea. -- MST
Re: [Qemu-devel] [PATCH v3 3/7] qom: create objects in two phases
On Fri, May 08, 2015 at 04:40:51PM +0200, Paolo Bonzini wrote: On 08/05/2015 16:37, Andreas Färber wrote: Hi, Can we *please* find a better subject for this? To me, creating QOM objects in two phases is about instance_init vs. realize, and thus I was pretty upset that Paolo dared to apply this without asking me first. Oops, sorry. I very much understand where you came from, now. Ok, I'll change this to say something like create most objects before creating chardev backends to better describe what its trying to achieve. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 2/5] util: move read_password method out of qemu-img into osdep/oslib
On 05/12/2015 10:09 AM, Daniel P. Berrange wrote: The qemu-img.c file has a read_password() method impl that is used to prompt for passwords on the console, with impls for POSIX and Windows. This will be needed by qemu-io.c too, so move it into the QEMU osdep/oslib files where it can be shared without code duplication Signed-off-by: Daniel P. Berrange berra...@redhat.com --- include/qemu/osdep.h | 2 ++ qemu-img.c | 93 +--- util/oslib-posix.c | 66 + util/oslib-win32.c | 24 ++ 4 files changed, 93 insertions(+), 92 deletions(-) diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index b3300cc..3247364 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -259,4 +259,6 @@ void qemu_set_tty_echo(int fd, bool echo); void os_mem_prealloc(int fd, char *area, size_t sz); +int qemu_read_password(char *buf, int buf_size); Should we fix it to use size_t buf_size while at it? (or as a followup, to keep this one limited to code motion) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH 5/5] tests: add test case for encrypted qcow2 read/write
Add a simple test case for qemu-iotests that covers read/write with encrypted qcow2 files. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tests/qemu-iotests/131 | 69 ++ tests/qemu-iotests/131.out | 46 +++ tests/qemu-iotests/group | 1 + 3 files changed, 116 insertions(+) create mode 100755 tests/qemu-iotests/131 create mode 100644 tests/qemu-iotests/131.out diff --git a/tests/qemu-iotests/131 b/tests/qemu-iotests/131 new file mode 100755 index 000..f44b0a0 --- /dev/null +++ b/tests/qemu-iotests/131 @@ -0,0 +1,69 @@ +#!/bin/bash +# +# Test encrypted read/write using plain bdrv_read/bdrv_write +# +# Copyright (C) 2009 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +# + +# creator +owner=berra...@redhat.com + +seq=`basename $0` +echo QA output created by $seq + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap _cleanup; exit \$status 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt qcow2 +_supported_proto generic +_supported_os Linux + + +size=128M +IMGOPTS=encryption=on _make_test_img $size + +echo +echo == reading whole image == +echo astrochicken | $QEMU_IO -c read 0 $size $TEST_IMG | _filter_qemu_io + +echo +echo == rewriting whole image == +echo astrochicken | $QEMU_IO -c write -P 0xa 0 $size $TEST_IMG | _filter_qemu_io + +echo +echo == verify pattern == +echo astrochicken | $QEMU_IO -c read -P 0xa 0 $size $TEST_IMG | _filter_qemu_io + +echo +echo == verify pattern failure with wrong password == +echo platypus | $QEMU_IO -c read -P 0xa 0 $size $TEST_IMG | _filter_qemu_io + + +# success, all done +echo *** done +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/131.out b/tests/qemu-iotests/131.out new file mode 100644 index 000..4eedb35 --- /dev/null +++ b/tests/qemu-iotests/131.out @@ -0,0 +1,46 @@ +QA output created by 131 +qemu-img: Encrypted images are deprecated +Support for them will be removed in a future release. +You can use 'qemu-img convert' to convert your image to an unencrypted one. +qemu-img: Encrypted images are deprecated +Support for them will be removed in a future release. +You can use 'qemu-img convert' to convert your image to an unencrypted one. +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on + +== reading whole image == +Encrypted images are deprecated +Support for them will be removed in a future release. +You can use 'qemu-img convert' to convert your image to an unencrypted one. +Disk image '/home/berrange/src/virt/qemu/tests/qemu-iotests/scratch/t.qcow2' is encrypted. +password: +read 134217728/134217728 bytes at offset 0 +128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== rewriting whole image == +Encrypted images are deprecated +Support for them will be removed in a future release. +You can use 'qemu-img convert' to convert your image to an unencrypted one. +Disk image '/home/berrange/src/virt/qemu/tests/qemu-iotests/scratch/t.qcow2' is encrypted. +password: +wrote 134217728/134217728 bytes at offset 0 +128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== verify pattern == +Encrypted images are deprecated +Support for them will be removed in a future release. +You can use 'qemu-img convert' to convert your image to an unencrypted one. +Disk image '/home/berrange/src/virt/qemu/tests/qemu-iotests/scratch/t.qcow2' is encrypted. +password: +read 134217728/134217728 bytes at offset 0 +128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== verify pattern failure with wrong password == +Encrypted images are deprecated +Support for them will be removed in a future release. +You can use 'qemu-img convert' to convert your image to an unencrypted one. +Disk image '/home/berrange/src/virt/qemu/tests/qemu-iotests/scratch/t.qcow2' is encrypted. +password: +Pattern verification failed at offset 0, 134217728 bytes +read 134217728/134217728 bytes at offset 0 +128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 6ca3466..34b16cb 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -128,3 +128,4 @@ 128 rw auto quick 129 rw auto quick 130 rw auto quick +131 rw auto quick -- 2.1.0
[Qemu-devel] [PATCH 0/5] Misc fixes and testing of qcow[2] encryption
I realize that qcow[2] encryption is a feature we have deprecated and will remove support for running it with the QEMU system emulators in this cycle. We do still need to make sure it continues to work for the sake of letting people run qemu-img convert to retrieve their data though. Some of the other patches I'm working on which introduce a cypto cipher API touch this qcow2 code, thus I wanted to be able to test that it doesn't break anything. I found that qemu-iotests didn't have any coverage of the qcow2 encryption code. For added fun, I then discovered that qemu-io doesn't check if an encryption key is required, so ends up writing plain text to the files instead of cipher, and returning cipher text for reads, instead of plain text. IOW qemu-io will corrupt encrypted qcow2 files on write. This series adds some asserts that will protect against this kind of mistake, adds support for getting passwords to qemu-io (in the same manner that qemu-img supports), and finally adds a test case for reading/writing encrypted qcow2. Daniel P. Berrange (5): qcow2/qcow: protect against uninitialized encryption key util: move read_password method out of qemu-img into osdep/oslib util: allow \n to terminate password input qemu-io: prompt for encryption keys when required tests: add test case for encrypted qcow2 read/write block/qcow.c | 10 +++-- block/qcow2-cluster.c | 3 +- block/qcow2.c | 18 ++--- include/qemu/osdep.h | 2 + qemu-img.c | 93 +- qemu-io.c | 21 +++ tests/qemu-iotests/131 | 69 ++ tests/qemu-iotests/131.out | 46 +++ tests/qemu-iotests/group | 1 + util/oslib-posix.c | 67 + util/oslib-win32.c | 24 11 files changed, 252 insertions(+), 102 deletions(-) create mode 100755 tests/qemu-iotests/131 create mode 100644 tests/qemu-iotests/131.out -- 2.1.0
[Qemu-devel] [PATCH 4/5] qemu-io: prompt for encryption keys when required
The qemu-io tool does not check if the image is encrypted so historically would silently corrupt the sectors by writing plain text data into them instead of cipher text. The earlier commit turns this mistake into a fatal abort, so check for encryption and prompt for key when required. This enables us to add unit tests to ensure we don't break the ability of qemu-img to convert existing encrypted qcow2 files into a non-encrypted format. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- qemu-io.c | 21 + 1 file changed, 21 insertions(+) diff --git a/qemu-io.c b/qemu-io.c index 8e41080..34ae933 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -52,6 +52,7 @@ static const cmdinfo_t close_cmd = { static int openfile(char *name, int flags, QDict *opts) { Error *local_err = NULL; +BlockDriverState *bs; if (qemuio_blk) { fprintf(stderr, file open already, try 'help close'\n); @@ -68,7 +69,27 @@ static int openfile(char *name, int flags, QDict *opts) return 1; } +bs = blk_bs(qemuio_blk); +if (bdrv_is_encrypted(bs)) { +char password[256]; +printf(Disk image '%s' is encrypted.\n, name); +if (qemu_read_password(password, sizeof(password)) 0) { +error_report(No password given); +goto error; +} +if (bdrv_set_key(bs, password) 0) { +error_report(invalid password); +goto error; +} +} + + return 0; + + error: +blk_unref(qemuio_blk); +qemuio_blk = NULL; +return 1; } static void open_help(void) -- 2.1.0
[Qemu-devel] [PATCH 1/5] qcow2/qcow: protect against uninitialized encryption key
When a qcow[2] file is opened, if the header reports an encryption method, this is used to set the 'crypt_method_header' field on the BDRVQcow[2]State struct, and the 'encrypted' flag in the BDRVState struct. When doing I/O operations, the 'crypt_method' field on the BDRVQcow[2]State struct is checked to determine if encryption needs to be applied. The crypt_method_header value is copied into crypt_method when the bdrv_set_key() method is called. The QEMU code which opens a block device is expected to always do a check if (bdrv_is_encrypted(bs)) { bdrv_set_key(bs, key...); } If code forgets todo this, then 'crypt_method' is never set and so when I/O is performed, QEMU writes plain text data into a sector which is expected to contain cipher text, or when reading, will return cipher text instead of plain text. Change the qcow[2] code to consult bs-encrypted when deciding whether encryption is required, and assert(s-crypt_method) to protect against cases where the caller forgets to set the encryption key. Also put an assert in the set_key methods to protect against the case where the caller sets an encryption key on a block device that does not have encryption Signed-off-by: Daniel P. Berrange berra...@redhat.com --- block/qcow.c | 10 +++--- block/qcow2-cluster.c | 3 ++- block/qcow2.c | 18 -- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index ab89328..911e59f 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -269,6 +269,7 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) for(i = 0;i len;i++) { keybuf[i] = key[i]; } +assert(bs-encrypted); s-crypt_method = s-crypt_method_header; if (AES_set_encrypt_key(keybuf, 128, s-aes_encrypt_key) != 0) @@ -411,9 +412,10 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, bdrv_truncate(bs-file, cluster_offset + s-cluster_size); /* if encrypted, we must initialize the cluster content which won't be written */ -if (s-crypt_method +if (bs-encrypted (n_end - n_start) s-cluster_sectors) { uint64_t start_sect; +assert(s-crypt_method); start_sect = (offset ~(s-cluster_size - 1)) 9; memset(s-cluster_data + 512, 0x00, 512); for(i = 0; i s-cluster_sectors; i++) { @@ -590,7 +592,8 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, if (ret 0) { break; } -if (s-crypt_method) { +if (bs-encrypted) { +assert(s-crypt_method); encrypt_sectors(s, sector_num, buf, buf, n, 0, s-aes_decrypt_key); @@ -661,7 +664,8 @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, ret = -EIO; break; } -if (s-crypt_method) { +if (bs-encrypted) { +assert(s-crypt_method); if (!cluster_data) { cluster_data = g_malloc0(s-cluster_size); } diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index ed2b44d..2dd 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -403,7 +403,8 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs, goto out; } -if (s-crypt_method) { +if (bs-encrypted) { +assert(s-crypt_method); qcow2_encrypt_sectors(s, start_sect + n_start, iov.iov_base, iov.iov_base, n, 1, s-aes_encrypt_key); diff --git a/block/qcow2.c b/block/qcow2.c index b9a72e3..f7b4cc6 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1037,6 +1037,7 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key) for(i = 0;i len;i++) { keybuf[i] = key[i]; } +assert(bs-encrypted); s-crypt_method = s-crypt_method_header; if (AES_set_encrypt_key(keybuf, 128, s-aes_encrypt_key) != 0) @@ -1224,7 +1225,9 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, goto fail; } -if (s-crypt_method) { +if (bs-encrypted) { +assert(s-crypt_method); + /* * For encrypted images, read everything into a temporary * contiguous buffer on which the AES functions can work. @@ -1255,7 +1258,8 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, if (ret 0) { goto fail; } -if (s-crypt_method) { +if (bs-encrypted) { +assert(s-crypt_method); qcow2_encrypt_sectors(s, sector_num, cluster_data,
Re: [Qemu-devel] [PULL 00/16] KVM, QOM, NBD, build fixes for 2015-05-08
On Fri, May 08, 2015 at 02:29:01PM +0200, Andreas Färber wrote: Am 08.05.2015 um 14:07 schrieb Paolo Bonzini: The following changes since commit 498147529d1f8e902e6528a0115143b53475791e: Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20150430' into staging (2015-04-30 14:15:56 +0100) are available in the git repository at: git://github.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to d51026b22b97332a95d91acfb6c23cd9b087955c: qemu-nbd: only send a limited number of errno codes on the wire (2015-05-08 13:14:54 +0200) - Daniel's QOM improvements Once again, objection. Paolo, I'll re-send a v4 of these QOM related improvements that incorporate Andreas' feedback. - build bugfix from Fam and new configure check from Emilio - two improvements to info mtere from Gerd - KVM support for memory transaction attributes - one more small step towards unlocked MMIO dispatch - one piece of the qemu-nbd errno fixes - trivial-ish patches from Denis and Thomas Daniel P. Berrange (7): qom: fix typename of 'policy' enum property in hostmem obj qom: document user creatable object types in help text qom: create objects in two phases qom: add object_new_propv / object_new_proplist constructors qom: make enum string tables const-correct qom: add a object_property_add_enum helper method qom: don't pass string table to object_get_enum method Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PATCH v2 11/17] target-alpha: Fix integer overflow checking insns
We need to write the result to the destination register before raising any exception. Thus inline the code for each insn, and check for any exception after we're done. Reported-by: Al Viro v...@zeniv.linux.org.uk Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/helper.h | 7 +- target-alpha/int_helper.c | 59 ++ target-alpha/translate.c | 60 +-- 3 files changed, 56 insertions(+), 70 deletions(-) diff --git a/target-alpha/helper.h b/target-alpha/helper.h index 9e7b771..5b1a5d9 100644 --- a/target-alpha/helper.h +++ b/target-alpha/helper.h @@ -1,12 +1,7 @@ DEF_HELPER_3(excp, noreturn, env, int, int) DEF_HELPER_FLAGS_1(load_pcc, TCG_CALL_NO_RWG_SE, i64, env) -DEF_HELPER_FLAGS_3(addqv, TCG_CALL_NO_WG, i64, env, i64, i64) -DEF_HELPER_FLAGS_3(addlv, TCG_CALL_NO_WG, i64, env, i64, i64) -DEF_HELPER_FLAGS_3(subqv, TCG_CALL_NO_WG, i64, env, i64, i64) -DEF_HELPER_FLAGS_3(sublv, TCG_CALL_NO_WG, i64, env, i64, i64) -DEF_HELPER_FLAGS_3(mullv, TCG_CALL_NO_WG, i64, env, i64, i64) -DEF_HELPER_FLAGS_3(mulqv, TCG_CALL_NO_WG, i64, env, i64, i64) +DEF_HELPER_FLAGS_3(check_overflow, TCG_CALL_NO_WG, void, env, i64, i64) DEF_HELPER_FLAGS_1(ctpop, TCG_CALL_NO_RWG_SE, i64, i64) DEF_HELPER_FLAGS_1(ctlz, TCG_CALL_NO_RWG_SE, i64, i64) diff --git a/target-alpha/int_helper.c b/target-alpha/int_helper.c index 7a205eb..8e4537f 100644 --- a/target-alpha/int_helper.c +++ b/target-alpha/int_helper.c @@ -249,64 +249,9 @@ uint64_t helper_unpkbw(uint64_t op1) | ((op1 0xff00) 24)); } -uint64_t helper_addqv(CPUAlphaState *env, uint64_t op1, uint64_t op2) +void helper_check_overflow(CPUAlphaState *env, uint64_t op1, uint64_t op2) { -uint64_t tmp = op1; -op1 += op2; -if (unlikely((tmp ^ op2 ^ (-1ULL)) (tmp ^ op1) (1ULL 63))) { +if (unlikely(op1 != op2)) { arith_excp(env, GETPC(), EXC_M_IOV, 0); } -return op1; -} - -uint64_t helper_addlv(CPUAlphaState *env, uint64_t op1, uint64_t op2) -{ -uint64_t tmp = op1; -op1 = (uint32_t)(op1 + op2); -if (unlikely((tmp ^ op2 ^ (-1UL)) (tmp ^ op1) (1UL 31))) { -arith_excp(env, GETPC(), EXC_M_IOV, 0); -} -return op1; -} - -uint64_t helper_subqv(CPUAlphaState *env, uint64_t op1, uint64_t op2) -{ -uint64_t res; -res = op1 - op2; -if (unlikely((op1 ^ op2) (res ^ op1) (1ULL 63))) { -arith_excp(env, GETPC(), EXC_M_IOV, 0); -} -return res; -} - -uint64_t helper_sublv(CPUAlphaState *env, uint64_t op1, uint64_t op2) -{ -uint32_t res; -res = op1 - op2; -if (unlikely((op1 ^ op2) (res ^ op1) (1UL 31))) { -arith_excp(env, GETPC(), EXC_M_IOV, 0); -} -return res; -} - -uint64_t helper_mullv(CPUAlphaState *env, uint64_t op1, uint64_t op2) -{ -int64_t res = (int64_t)op1 * (int64_t)op2; - -if (unlikely((int32_t)res != res)) { -arith_excp(env, GETPC(), EXC_M_IOV, 0); -} -return (int64_t)((int32_t)res); -} - -uint64_t helper_mulqv(CPUAlphaState *env, uint64_t op1, uint64_t op2) -{ -uint64_t tl, th; - -muls64(tl, th, op1, op2); -/* If th != 0 th != -1, then we had an overflow */ -if (unlikely((th + 1) 1)) { -arith_excp(env, GETPC(), EXC_M_IOV, 0); -} -return tl; } diff --git a/target-alpha/translate.c b/target-alpha/translate.c index 7868cc4..74f5d07 100644 --- a/target-alpha/translate.c +++ b/target-alpha/translate.c @@ -1362,7 +1362,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) uint16_t fn11; uint8_t opc, ra, rb, rc, fpfn, fn7, lit; bool islit; -TCGv va, vb, vc, tmp; +TCGv va, vb, vc, tmp, tmp2; TCGv_i32 t32; ExitStatus ret; @@ -1574,11 +1574,23 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) break; case 0x40: /* ADDL/V */ -gen_helper_addlv(vc, cpu_env, va, vb); +tmp = tcg_temp_new(); +tcg_gen_ext32s_i64(tmp, va); +tcg_gen_ext32s_i64(vc, vb); +tcg_gen_add_i64(tmp, tmp, vc); +tcg_gen_ext32s_i64(vc, tmp); +gen_helper_check_overflow(cpu_env, vc, tmp); +tcg_temp_free(tmp); break; case 0x49: /* SUBL/V */ -gen_helper_sublv(vc, cpu_env, va, vb); +tmp = tcg_temp_new(); +tcg_gen_ext32s_i64(tmp, va); +tcg_gen_ext32s_i64(vc, vb); +tcg_gen_sub_i64(tmp, tmp, vc); +tcg_gen_ext32s_i64(vc, tmp); +gen_helper_check_overflow(cpu_env, vc, tmp); +tcg_temp_free(tmp); break; case 0x4D: /* CMPLT */ @@ -1586,11 +1598,33 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) break; case 0x60: /* ADDQ/V */ -gen_helper_addqv(vc, cpu_env, va, vb); +tmp = tcg_temp_new(); +
[Qemu-devel] [PATCH v2 10/17] target-alpha: Fix cvttq vs inf
We should raise INV for infinities as well, not OVR+INE. Reported-by: Al Viro v...@zeniv.linux.org.uk Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/fpu_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-alpha/fpu_helper.c b/target-alpha/fpu_helper.c index 9449c57..db523fb 100644 --- a/target-alpha/fpu_helper.c +++ b/target-alpha/fpu_helper.c @@ -444,7 +444,7 @@ static uint64_t do_cvttq(CPUAlphaState *env, uint64_t a, int roundmode) goto do_underflow; } } else if (exp == 0x7ff) { -exc = (frac ? FPCR_INV : FPCR_IOV | FPCR_INE); +exc = FPCR_INV; } else { /* Restore implicit bit. */ frac |= 0x10ull; -- 2.1.0
Re: [Qemu-devel] when does a target frontend need to use gen_io_start()/gen_io_end() ?
On 12/05/2015 17:32, Peter Maydell wrote: In order for -icount to work, it's important for the target translate.c code to correctly bracket any generated code which can do I/O with gen_io_start()/gen_io_end() calls. But does anybody know exactly what the criteria are here for this? It would be nice if we could document this in a comment in gen_icount.h -- I'm happy to write one up if somebody will just tell me what the right answer is :-) It's any instruction that can cause an icount read, typically through QEMU_CLOCK_VIRTUAL or cpu_get_ticks(). Paolo
[Qemu-devel] [PATCH 3/5] util: allow \n to terminate password input
The qemu_read_password() method looks for \r to terminate the reading of the a password. This is what will be seen when reading the password from a TTY. When scripting though, it is useful to be able to send the password via a pipe, in which case we must look for \n to terminate password input. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- util/oslib-posix.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util/oslib-posix.c b/util/oslib-posix.c index 1c23fd2..3ae4987 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -467,7 +467,8 @@ int qemu_read_password(char *buf, int buf_size) ret = -1; break; } else { -if (ch == '\r') { +if (ch == '\r' || +ch == '\n') { ret = 0; break; } -- 2.1.0
[Qemu-devel] [PATCH 2/5] util: move read_password method out of qemu-img into osdep/oslib
The qemu-img.c file has a read_password() method impl that is used to prompt for passwords on the console, with impls for POSIX and Windows. This will be needed by qemu-io.c too, so move it into the QEMU osdep/oslib files where it can be shared without code duplication Signed-off-by: Daniel P. Berrange berra...@redhat.com --- include/qemu/osdep.h | 2 ++ qemu-img.c | 93 +--- util/oslib-posix.c | 66 + util/oslib-win32.c | 24 ++ 4 files changed, 93 insertions(+), 92 deletions(-) diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index b3300cc..3247364 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -259,4 +259,6 @@ void qemu_set_tty_echo(int fd, bool echo); void os_mem_prealloc(int fd, char *area, size_t sz); +int qemu_read_password(char *buf, int buf_size); + #endif diff --git a/qemu-img.c b/qemu-img.c index 8d30e43..60c820d 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -165,97 +165,6 @@ static int GCC_FMT_ATTR(2, 3) qprintf(bool quiet, const char *fmt, ...) return ret; } -#if defined(WIN32) -/* XXX: put correct support for win32 */ -static int read_password(char *buf, int buf_size) -{ -int c, i; - -printf(Password: ); -fflush(stdout); -i = 0; -for(;;) { -c = getchar(); -if (c 0) { -buf[i] = '\0'; -return -1; -} else if (c == '\n') { -break; -} else if (i (buf_size - 1)) { -buf[i++] = c; -} -} -buf[i] = '\0'; -return 0; -} - -#else - -#include termios.h - -static struct termios oldtty; - -static void term_exit(void) -{ -tcsetattr (0, TCSANOW, oldtty); -} - -static void term_init(void) -{ -struct termios tty; - -tcgetattr (0, tty); -oldtty = tty; - -tty.c_iflag = ~(IGNBRK|BRKINT|PARMRK|ISTRIP - |INLCR|IGNCR|ICRNL|IXON); -tty.c_oflag |= OPOST; -tty.c_lflag = ~(ECHO|ECHONL|ICANON|IEXTEN); -tty.c_cflag = ~(CSIZE|PARENB); -tty.c_cflag |= CS8; -tty.c_cc[VMIN] = 1; -tty.c_cc[VTIME] = 0; - -tcsetattr (0, TCSANOW, tty); - -atexit(term_exit); -} - -static int read_password(char *buf, int buf_size) -{ -uint8_t ch; -int i, ret; - -printf(password: ); -fflush(stdout); -term_init(); -i = 0; -for(;;) { -ret = read(0, ch, 1); -if (ret == -1) { -if (errno == EAGAIN || errno == EINTR) { -continue; -} else { -break; -} -} else if (ret == 0) { -ret = -1; -break; -} else { -if (ch == '\r') { -ret = 0; -break; -} -if (i (buf_size - 1)) -buf[i++] = ch; -} -} -term_exit(); -buf[i] = '\0'; -printf(\n); -return ret; -} -#endif static int print_block_option_help(const char *filename, const char *fmt) { @@ -312,7 +221,7 @@ static BlockBackend *img_open(const char *id, const char *filename, bs = blk_bs(blk); if (bdrv_is_encrypted(bs) require_io) { qprintf(quiet, Disk image '%s' is encrypted.\n, filename); -if (read_password(password, sizeof(password)) 0) { +if (qemu_read_password(password, sizeof(password)) 0) { error_report(No password given); goto fail; } diff --git a/util/oslib-posix.c b/util/oslib-posix.c index 37ffd96..1c23fd2 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -50,6 +50,7 @@ extern int daemon(int, int); #include termios.h #include unistd.h +#include termios.h #include glib/gprintf.h @@ -415,3 +416,68 @@ void os_mem_prealloc(int fd, char *area, size_t memory) pthread_sigmask(SIG_SETMASK, oldset, NULL); } } + + +static struct termios oldtty; + +static void term_exit(void) +{ +tcsetattr(0, TCSANOW, oldtty); +} + +static void term_init(void) +{ +struct termios tty; + +tcgetattr(0, tty); +oldtty = tty; + +tty.c_iflag = ~(IGNBRK|BRKINT|PARMRK|ISTRIP + |INLCR|IGNCR|ICRNL|IXON); +tty.c_oflag |= OPOST; +tty.c_lflag = ~(ECHO|ECHONL|ICANON|IEXTEN); +tty.c_cflag = ~(CSIZE|PARENB); +tty.c_cflag |= CS8; +tty.c_cc[VMIN] = 1; +tty.c_cc[VTIME] = 0; + +tcsetattr(0, TCSANOW, tty); + +atexit(term_exit); +} + +int qemu_read_password(char *buf, int buf_size) +{ +uint8_t ch; +int i, ret; + +printf(password: ); +fflush(stdout); +term_init(); +i = 0; +for (;;) { +ret = read(0, ch, 1); +if (ret == -1) { +if (errno == EAGAIN || errno == EINTR) { +continue; +} else { +break; +} +} else if (ret == 0) { +ret = -1; +break; +} else { +if (ch == '\r') { +ret = 0; +
[Qemu-devel] [PATCH v2 16/17] target-alpha: Raise IOV from CVTQL
Even if an exception isn't taken, the status flags need updating and the result should be written to the destination. Move the body of cvtql out of line, since we now always need a call. Reported-by: Al Viro v...@zeniv.linux.org.uk Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/fpu_helper.c | 8 ++-- target-alpha/helper.h | 3 ++- target-alpha/translate.c | 34 +- 3 files changed, 13 insertions(+), 32 deletions(-) diff --git a/target-alpha/fpu_helper.c b/target-alpha/fpu_helper.c index fa4401d..b091aa8 100644 --- a/target-alpha/fpu_helper.c +++ b/target-alpha/fpu_helper.c @@ -539,9 +539,13 @@ uint64_t helper_cvtqt(CPUAlphaState *env, uint64_t a) return float64_to_t(fr); } -void helper_cvtql_v_input(CPUAlphaState *env, uint64_t val) +uint64_t helper_cvtql(CPUAlphaState *env, uint64_t val) { +uint32_t exc = 0; if (val != (int32_t)val) { -arith_excp(env, GETPC(), EXC_M_IOV, 0); +exc = FPCR_IOV | FPCR_INE; } +env-error_code = exc; + +return ((val 0xc000) 32) | ((val 0x3fff) 29); } diff --git a/target-alpha/helper.h b/target-alpha/helper.h index 780b0dc..d221f0d 100644 --- a/target-alpha/helper.h +++ b/target-alpha/helper.h @@ -79,6 +79,8 @@ DEF_HELPER_FLAGS_2(cvtqg, TCG_CALL_NO_RWG, i64, env, i64) DEF_HELPER_FLAGS_2(cvttq, TCG_CALL_NO_RWG, i64, env, i64) DEF_HELPER_FLAGS_2(cvttq_c, TCG_CALL_NO_RWG, i64, env, i64) +DEF_HELPER_FLAGS_2(cvtql, TCG_CALL_NO_RWG, i64, env, i64) + DEF_HELPER_FLAGS_2(setroundmode, TCG_CALL_NO_RWG, void, env, i32) DEF_HELPER_FLAGS_2(setflushzero, TCG_CALL_NO_RWG, void, env, i32) DEF_HELPER_FLAGS_3(fp_exc_raise, TCG_CALL_NO_WG, void, env, i32, i32) @@ -87,7 +89,6 @@ DEF_HELPER_FLAGS_3(fp_exc_raise_s, TCG_CALL_NO_WG, void, env, i32, i32) DEF_HELPER_FLAGS_2(ieee_input, TCG_CALL_NO_WG, void, env, i64) DEF_HELPER_FLAGS_2(ieee_input_cmp, TCG_CALL_NO_WG, void, env, i64) DEF_HELPER_FLAGS_2(ieee_input_s, TCG_CALL_NO_WG, void, env, i64) -DEF_HELPER_FLAGS_2(cvtql_v_input, TCG_CALL_NO_WG, void, env, i64) #if !defined (CONFIG_USER_ONLY) DEF_HELPER_2(hw_ret, void, env, i64) diff --git a/target-alpha/translate.c b/target-alpha/translate.c index 4c441a9..e9927b5 100644 --- a/target-alpha/translate.c +++ b/target-alpha/translate.c @@ -720,19 +720,6 @@ static void gen_cvtlq(TCGv vc, TCGv vb) tcg_temp_free(tmp); } -static void gen_cvtql(TCGv vc, TCGv vb) -{ -TCGv tmp = tcg_temp_new(); - -tcg_gen_andi_i64(tmp, vb, (int32_t)0xc000); -tcg_gen_andi_i64(vc, vb, 0x3FFF); -tcg_gen_shli_i64(tmp, tmp, 32); -tcg_gen_shli_i64(vc, vc, 29); -tcg_gen_or_i64(vc, vc, tmp); - -tcg_temp_free(tmp); -} - static void gen_ieee_arith2(DisasContext *ctx, void (*helper)(TCGv, TCGv_ptr, TCGv), int rb, int rc, int fn11) @@ -2254,25 +2241,14 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) /* FCMOVGT */ gen_fcmov(ctx, TCG_COND_GT, ra, rb, rc); break; -case 0x030: -/* CVTQL */ -REQUIRE_REG_31(ra); -vc = dest_fpr(ctx, rc); -vb = load_fpr(ctx, rb); -gen_cvtql(vc, vb); -break; -case 0x130: -/* CVTQL/V */ -case 0x530: -/* CVTQL/SV */ +case 0x030: /* CVTQL */ +case 0x130: /* CVTQL/V */ +case 0x530: /* CVTQL/SV */ REQUIRE_REG_31(ra); -/* ??? I'm pretty sure there's nothing that /sv needs to do that - /v doesn't do. The only thing I can think is that /sv is a - valid instruction merely for completeness in the ISA. */ vc = dest_fpr(ctx, rc); vb = load_fpr(ctx, rb); -gen_helper_cvtql_v_input(cpu_env, vb); -gen_cvtql(vc, vb); +gen_helper_cvtql(vc, cpu_env, vb); +gen_fp_exc_raise(rc, fn11); break; default: goto invalid_opc; -- 2.1.0
[Qemu-devel] [PATCH v2 13/17] target-alpha: Disallow literal operand to 1C.30 to 1C.37
Before 64f45e49 we used to have literal checks for 4 of these 8 opcodes. Confirmed that real hardware doesn't allow them. Reported-by: Al Viro v...@zeniv.linux.org.uk Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/translate.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/target-alpha/translate.c b/target-alpha/translate.c index 953d1ef..f0556b0 100644 --- a/target-alpha/translate.c +++ b/target-alpha/translate.c @@ -1342,6 +1342,13 @@ static ExitStatus gen_mtpr(DisasContext *ctx, TCGv vb, int regno) } #endif /* !USER_ONLY*/ +#define REQUIRE_NO_LIT \ +do {\ +if (real_islit) { \ +goto invalid_opc; \ +} \ +} while (0) + #define REQUIRE_TB_FLAG(FLAG) \ do {\ if ((ctx-tb-flags (FLAG)) == 0) { \ @@ -1361,7 +1368,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) int32_t disp21, disp16, disp12 __attribute__((unused)); uint16_t fn11; uint8_t opc, ra, rb, rc, fpfn, fn7, lit; -bool islit; +bool islit, real_islit; TCGv va, vb, vc, tmp, tmp2; TCGv_i32 t32; ExitStatus ret; @@ -1371,7 +1378,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) ra = extract32(insn, 21, 5); rb = extract32(insn, 16, 5); rc = extract32(insn, 0, 5); -islit = extract32(insn, 12, 1); +real_islit = islit = extract32(insn, 12, 1); lit = extract32(insn, 13, 8); disp21 = sextract32(insn, 0, 21); @@ -2466,11 +2473,13 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) /* CTPOP */ REQUIRE_TB_FLAG(TB_FLAGS_AMASK_CIX); REQUIRE_REG_31(ra); +REQUIRE_NO_LIT; gen_helper_ctpop(vc, vb); break; case 0x31: /* PERR */ REQUIRE_TB_FLAG(TB_FLAGS_AMASK_MVI); +REQUIRE_NO_LIT; va = load_gpr(ctx, ra); gen_helper_perr(vc, va, vb); break; @@ -2478,36 +2487,42 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) /* CTLZ */ REQUIRE_TB_FLAG(TB_FLAGS_AMASK_CIX); REQUIRE_REG_31(ra); +REQUIRE_NO_LIT; gen_helper_ctlz(vc, vb); break; case 0x33: /* CTTZ */ REQUIRE_TB_FLAG(TB_FLAGS_AMASK_CIX); REQUIRE_REG_31(ra); +REQUIRE_NO_LIT; gen_helper_cttz(vc, vb); break; case 0x34: /* UNPKBW */ REQUIRE_TB_FLAG(TB_FLAGS_AMASK_MVI); REQUIRE_REG_31(ra); +REQUIRE_NO_LIT; gen_helper_unpkbw(vc, vb); break; case 0x35: /* UNPKBL */ REQUIRE_TB_FLAG(TB_FLAGS_AMASK_MVI); REQUIRE_REG_31(ra); +REQUIRE_NO_LIT; gen_helper_unpkbl(vc, vb); break; case 0x36: /* PKWB */ REQUIRE_TB_FLAG(TB_FLAGS_AMASK_MVI); REQUIRE_REG_31(ra); +REQUIRE_NO_LIT; gen_helper_pkwb(vc, vb); break; case 0x37: /* PKLB */ REQUIRE_TB_FLAG(TB_FLAGS_AMASK_MVI); REQUIRE_REG_31(ra); +REQUIRE_NO_LIT; gen_helper_pklb(vc, vb); break; case 0x38: -- 2.1.0
[Qemu-devel] [PATCH v2 15/17] target-alpha: Suppress underflow from CVTTQ if DNZ
I.e. respect flush_inputs_to_zero. Reported-by: Al Viro v...@zeniv.linux.org.uk Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/fpu_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-alpha/fpu_helper.c b/target-alpha/fpu_helper.c index ea1f2e2..fa4401d 100644 --- a/target-alpha/fpu_helper.c +++ b/target-alpha/fpu_helper.c @@ -452,7 +452,7 @@ static uint64_t do_cvttq(CPUAlphaState *env, uint64_t a, int roundmode) frac = a 0xfull; if (exp == 0) { -if (unlikely(frac != 0)) { +if (unlikely(frac != 0) !env-fp_status.flush_inputs_to_zero) { goto do_underflow; } } else if (exp == 0x7ff) { -- 2.1.0
Re: [Qemu-devel] [PATCH v3 4/7] qom: add object_new_propv / object_new_proplist constructors
On Fri, May 08, 2015 at 07:10:49PM +0200, Andreas Färber wrote: Hi Daniel/Paolo, Am 01.05.2015 um 12:30 schrieb Daniel P. Berrange: It is reasonably common to want to create an object, set a number of properties, register it in the hierarchy and then mark it as complete (if a user creatable type). This requires quite a lot of error prone, verbose, boilerplate code to achieve. The object_new_propv / object_new_proplist constructors will simplify this task by performing all required steps in one go, accepting the property names/values as variadic args. With this I disagree. I can see the virtue of adding properties in one go via some handy varargs function. But, 1) The function does something different from what its name implies to me. It does not create a prop or proplist - instead of adding them it sets existing ones. Suggest object_new_with_props()? Sure, with_props() looks fine. 2) You seem to mix up *v and non-v functions. v is with va_list usually, compare tests/libqtest.h. Ok, I didn't see that qemu had a convention on that, so will change to match. 3) Object construction is a tricky thing to get right. Anthony chose to be stricter than C++ and not let object_new() fail, one of the reasons we have the distinct realize step. Can we keep the two separate? qdev with all its convenience helpers didn't mix those either. I.e., use object_new() without Error** followed by object_set_props() or anything with Error**. That tells you if there's an Error* you need to unref the object. Otherwise it's in an unknown state. I don't really think that forcing the callers to call new + set_props separately is really makng it more reliable - in fact the contrary - it means that the callers have more complex boilerplate code which they all have to tediously duplicate in exactly the same way. With the single object_new_with_props call, you know that if it returns NULL then it failed and you have no cleanup that you need todo which is about as reliable as it gets. That said, I can see the value in having a standalone object_set_props() method as a general feature. So I will add that, and simply make the object_new_with_props method call object_new + object_set_props + 4) What's the use case for this? I'm concerned about encouraging people to hardcode properties like this, when doing it in C can let the compiler detect any mismatches. I use it in the VNC server when I convert it to use generic TLS encryption code over to use the QCryptoTLSCreds object - it reduced a 100+ line method into just two calls to object_new_propv. See vnc_display_create_creds() in this RFC patch: https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg02062.html Then, I've got a bunch of unit tests related to that series which are using it, again to reduce the amount of code it takes to create and set props on this TLS creds object. Usage would be: Error *err = NULL; Object *obj; obj = object_new_propv(TYPE_MEMORY_BACKEND_FILE, /objects, This is not an Object*. ;) I like it better as it's implemented below, but cf. above for mixing this Error**-ing operation with object_new(). Yep, that's a docs mistake. hostmem0, err, share, yes, mem-path, /dev/shm/somefile, prealloc, yes, size, 1048576, NULL); Note all property values are passed in string form and will be parsed into their required data types. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- include/qom/object.h | 67 qom/object.c | 66 tests/.gitignore | 1 + tests/Makefile | 5 +- tests/check-qom-proplist.c | 190 + 5 files changed, 328 insertions(+), 1 deletion(-) create mode 100644 tests/check-qom-proplist.c diff --git a/include/qom/object.h b/include/qom/object.h index d2d7748..15ac314 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -607,6 +607,73 @@ Object *object_new(const char *typename); Object *object_new_with_type(Type type); /** + * object_new_propv: + * @typename: The name of the type of the object to instantiate. + * @parent: the parent object + * @id: The unique ID of the object + * @errp: pointer to error object + * @...: list of property names and values + * + * This function with initialize a new object using heap allocated memory. Grammar. (will?) + * The returned object has a reference count of 1, and will be freed when + * the last reference is dropped. + * + * The @id parameter will be used when registering the object as a + * child of @parent in the objects hierarchy. s/objects hierarchy/composition tree/ + * + * The
[Qemu-devel] [PATCH v2 00/17] target-alpha fpu improvments
This is v2 of the work that Al Viro helped with nearly a year ago. At the time, I obsconded with an unused bit in the softfloat exception flags. Which was a bit of a wart, and rightly pointed out as such by someone at the time. After 11 months on the shelf, I've finally found enough time to work out the bugs in the re-implementation of the fpcr. This time all of the bits are private to target-alpha, so no mucking about with the generic softfloat code. r~ Richard Henderson (17): target-alpha: Move VAX helpers to a new file target-alpha: Rename floating-point subroutines target-alpha: Forget installed round mode after MT_FPCR target-alpha: Set PC correctly for floating-point exceptions target-alpha: Tidy FPCR representation target-alpha: Set fpcr_exc_status even for disabled exceptions target-alpha: Set EXC_M_SWC for exceptions from /S insns target-alpha: Raise IOV from CVTTQ target-alpha: Fix cvttq vs large integers target-alpha: Fix cvttq vs inf target-alpha: Fix integer overflow checking insns target-alpha: Implement WH64EN target-alpha: Disallow literal operand to 1C.30 to 1C.37 target-alpha: Raise EXC_M_INV properly for fp inputs target-alpha: Suppress underflow from CVTTQ if DNZ target-alpha: Raise IOV from CVTQL target-alpha: Rewrite helper_zapnot target-alpha/Makefile.objs | 2 +- target-alpha/cpu.h | 95 target-alpha/fpu_helper.c | 530 +++-- target-alpha/helper.c | 132 ++- target-alpha/helper.h | 14 +- target-alpha/int_helper.c | 89 ++-- target-alpha/mem_helper.c | 9 +- target-alpha/translate.c | 265 --- target-alpha/vax_helper.c | 353 ++ 9 files changed, 715 insertions(+), 774 deletions(-) create mode 100644 target-alpha/vax_helper.c -- 2.1.0
[Qemu-devel] [PATCH v2 04/17] target-alpha: Set PC correctly for floating-point exceptions
PC should be one past the faulting insn. Add better commentary for the machine-check exception path. Reported-by: Al Viro v...@zeniv.linux.org.uk Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/helper.c | 2 ++ target-alpha/mem_helper.c | 9 - 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/target-alpha/helper.c b/target-alpha/helper.c index a8aa782..e202fee 100644 --- a/target-alpha/helper.c +++ b/target-alpha/helper.c @@ -571,6 +571,8 @@ void QEMU_NORETURN dynamic_excp(CPUAlphaState *env, uintptr_t retaddr, env-error_code = error; if (retaddr) { cpu_restore_state(cs, retaddr); +/* Floating-point exceptions (our only users) point to the next PC. */ +env-pc += 4; } cpu_loop_exit(cs); } diff --git a/target-alpha/mem_helper.c b/target-alpha/mem_helper.c index fc4f57a..7b5e30d 100644 --- a/target-alpha/mem_helper.c +++ b/target-alpha/mem_helper.c @@ -128,7 +128,14 @@ void alpha_cpu_unassigned_access(CPUState *cs, hwaddr addr, env-trap_arg0 = addr; env-trap_arg1 = is_write ? 1 : 0; -dynamic_excp(env, 0, EXCP_MCHK, 0); +cs-exception_index = EXCP_MCHK; +env-error_code = 0; + +/* ??? We should cpu_restore_state to the faulting insn, but this hook + does not have access to the retaddr value from the orignal helper. + It's all moot until the QEMU PALcode grows an MCHK handler. */ + +cpu_loop_exit(cs); } /* try to fill the TLB and return an exception if error. If retaddr is -- 2.1.0
[Qemu-devel] [PATCH v2 01/17] target-alpha: Move VAX helpers to a new file
Keep the IEEE and VAX floating point emulation separate. Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/Makefile.objs | 2 +- target-alpha/fpu_helper.c | 328 - target-alpha/vax_helper.c | 353 + 3 files changed, 354 insertions(+), 329 deletions(-) create mode 100644 target-alpha/vax_helper.c diff --git a/target-alpha/Makefile.objs b/target-alpha/Makefile.objs index b96c5da..6366462 100644 --- a/target-alpha/Makefile.objs +++ b/target-alpha/Makefile.objs @@ -1,4 +1,4 @@ obj-$(CONFIG_SOFTMMU) += machine.o obj-y += translate.o helper.o cpu.o -obj-y += int_helper.o fpu_helper.o sys_helper.o mem_helper.o +obj-y += int_helper.o fpu_helper.o vax_helper.o sys_helper.o mem_helper.o obj-y += gdbstub.o diff --git a/target-alpha/fpu_helper.c b/target-alpha/fpu_helper.c index d2d776c..8acd460 100644 --- a/target-alpha/fpu_helper.c +++ b/target-alpha/fpu_helper.c @@ -126,263 +126,6 @@ void helper_ieee_input_cmp(CPUAlphaState *env, uint64_t val) } } -/* F floating (VAX) */ -static uint64_t float32_to_f(float32 fa) -{ -uint64_t r, exp, mant, sig; -CPU_FloatU a; - -a.f = fa; -sig = ((uint64_t)a.l 0x8000) 32; -exp = (a.l 23) 0xff; -mant = ((uint64_t)a.l 0x007f) 29; - -if (exp == 255) { -/* NaN or infinity */ -r = 1; /* VAX dirty zero */ -} else if (exp == 0) { -if (mant == 0) { -/* Zero */ -r = 0; -} else { -/* Denormalized */ -r = sig | ((exp + 1) 52) | mant; -} -} else { -if (exp = 253) { -/* Overflow */ -r = 1; /* VAX dirty zero */ -} else { -r = sig | ((exp + 2) 52); -} -} - -return r; -} - -static float32 f_to_float32(CPUAlphaState *env, uintptr_t retaddr, uint64_t a) -{ -uint32_t exp, mant_sig; -CPU_FloatU r; - -exp = ((a 55) 0x80) | ((a 52) 0x7f); -mant_sig = ((a 32) 0x8000) | ((a 29) 0x007f); - -if (unlikely(!exp mant_sig)) { -/* Reserved operands / Dirty zero */ -dynamic_excp(env, retaddr, EXCP_OPCDEC, 0); -} - -if (exp 3) { -/* Underflow */ -r.l = 0; -} else { -r.l = ((exp - 2) 23) | mant_sig; -} - -return r.f; -} - -uint32_t helper_f_to_memory(uint64_t a) -{ -uint32_t r; -r = (a 0x1fffe000ull) 13; -r |= (a 0x07ffe000ull) 45; -r |= (a 0xc000ull) 48; -return r; -} - -uint64_t helper_memory_to_f(uint32_t a) -{ -uint64_t r; -r = ((uint64_t)(a 0xc000)) 48; -r |= ((uint64_t)(a 0x003f)) 45; -r |= ((uint64_t)(a 0x)) 13; -if (!(a 0x4000)) { -r |= 0x7ll 59; -} -return r; -} - -/* ??? Emulating VAX arithmetic with IEEE arithmetic is wrong. We should - either implement VAX arithmetic properly or just signal invalid opcode. */ - -uint64_t helper_addf(CPUAlphaState *env, uint64_t a, uint64_t b) -{ -float32 fa, fb, fr; - -fa = f_to_float32(env, GETPC(), a); -fb = f_to_float32(env, GETPC(), b); -fr = float32_add(fa, fb, FP_STATUS); -return float32_to_f(fr); -} - -uint64_t helper_subf(CPUAlphaState *env, uint64_t a, uint64_t b) -{ -float32 fa, fb, fr; - -fa = f_to_float32(env, GETPC(), a); -fb = f_to_float32(env, GETPC(), b); -fr = float32_sub(fa, fb, FP_STATUS); -return float32_to_f(fr); -} - -uint64_t helper_mulf(CPUAlphaState *env, uint64_t a, uint64_t b) -{ -float32 fa, fb, fr; - -fa = f_to_float32(env, GETPC(), a); -fb = f_to_float32(env, GETPC(), b); -fr = float32_mul(fa, fb, FP_STATUS); -return float32_to_f(fr); -} - -uint64_t helper_divf(CPUAlphaState *env, uint64_t a, uint64_t b) -{ -float32 fa, fb, fr; - -fa = f_to_float32(env, GETPC(), a); -fb = f_to_float32(env, GETPC(), b); -fr = float32_div(fa, fb, FP_STATUS); -return float32_to_f(fr); -} - -uint64_t helper_sqrtf(CPUAlphaState *env, uint64_t t) -{ -float32 ft, fr; - -ft = f_to_float32(env, GETPC(), t); -fr = float32_sqrt(ft, FP_STATUS); -return float32_to_f(fr); -} - - -/* G floating (VAX) */ -static uint64_t float64_to_g(float64 fa) -{ -uint64_t r, exp, mant, sig; -CPU_DoubleU a; - -a.d = fa; -sig = a.ll 0x8000ull; -exp = (a.ll 52) 0x7ff; -mant = a.ll 0x000full; - -if (exp == 2047) { -/* NaN or infinity */ -r = 1; /* VAX dirty zero */ -} else if (exp == 0) { -if (mant == 0) { -/* Zero */ -r = 0; -} else { -/* Denormalized */ -r = sig | ((exp + 1) 52) | mant; -} -} else { -if (exp = 2045) { -/* Overflow */ -r = 1; /* VAX dirty zero */ -} else { -r = sig | ((exp + 2) 52); -} -} - -return r; -} - -static
[Qemu-devel] [PATCH v2 09/17] target-alpha: Fix cvttq vs large integers
The range +- 2**63 - 2**64 was returning the wrong truncated result. We also incorrectly signaled overflow for -2**63. Reported-by: Al Viro v...@zeniv.linux.org.uk Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/fpu_helper.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/target-alpha/fpu_helper.c b/target-alpha/fpu_helper.c index 132b5a2..9449c57 100644 --- a/target-alpha/fpu_helper.c +++ b/target-alpha/fpu_helper.c @@ -453,12 +453,12 @@ static uint64_t do_cvttq(CPUAlphaState *env, uint64_t a, int roundmode) if (shift = 0) { /* In this case the number is so large that we must shift the fraction left. There is no rounding to do. */ -exc = FPCR_IOV | FPCR_INE; -if (shift 63) { +if (shift 64) { ret = frac shift; -if ((ret shift) == frac) { -exc = 0; -} +} +/* Check for overflow. Note the special case of -0x1p63. */ +if (shift = 11 a != 0xC3E0ull) { +exc = FPCR_IOV | FPCR_INE; } } else { uint64_t round; -- 2.1.0
Re: [Qemu-devel] Bug report - Windows XP guest failure
On 05/12/2015 03:22 AM, Michael Tokarev wrote: 12.05.2015 04:05, Peter Crosthwaite wrote: On Thu, May 7, 2015 at 2:34 AM, Michael Tokarev m...@tls.msk.ru wrote: ... Ok, I can reproduce this, winXP BSODs on boot in tcg mode. Git bisect points to this: commit 23820dbfc79d1c9dce090b4c555994f2bb6a69b3 Author: Peter Crosthwaite peter.crosthwa...@xilinx.com Date: Mon Mar 16 22:35:54 2015 -0700 exec: Respect as_translate_internal length clamp This winXP BSOD happens on x86_64 target too. Reverting the above commit from git master fixes the BSOD. Any useful info about IO addresses on that BSOD? The last issue with this patch was IOPort code relying on the bug that this patch fixed. This could be similar and if we can track the failure to a particular address we can fix properly rather than another revert of that patch. Oh. I didn't know this patch has been reverted before. Anyway, I disabled auto-reboot on BSOD on my winXP (what a useful feature!) and here's what I see. IRQ_NOT_LESS_OR_EQUAL STOP: 0x0A (0x16, 0x02, 0x00, 0x80500EFC) (with some amount of leading zeros stripped). When this happens, win does something for quite some time, the BSOD comes after quite significant delay. Is there anything else I can look at, maybe some crash dump or something? I haven't done any windows debugging before. Thanks, /mjt https://support.microsoft.com/en-us/kb/315263 You can configure the type of dump it saves, then use various MS utilities described here (briefly) to perform some basic analysis on the dumps, which sometimes gives extra goodies. I haven't done too much advanced windows debugging myself, but I do generally try to run the !analyze command on any minidumps I create, at least. --js
Re: [Qemu-devel] [PATCH RFC 1/7] virtio: relax feature check
On Tue, 12 May 2015 17:30:21 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, May 12, 2015 at 04:46:11PM +0200, Cornelia Huck wrote: On Tue, 12 May 2015 15:44:46 +0200 Cornelia Huck cornelia.h...@de.ibm.com wrote: On Tue, 12 May 2015 15:34:47 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, May 12, 2015 at 03:14:53PM +0200, Cornelia Huck wrote: On Wed, 06 May 2015 14:07:37 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: Unlike with add and clear, there is no valid reason to abort when checking for a feature. It makes more sense to return false (i.e. the feature bit isn't set). This is exactly what __virtio_has_feature() does if fbit = 32. This allows to introduce code that is aware about new 64-bit features like VIRTIO_F_VERSION_1, even if they are still not implemented. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/hw/virtio/virtio.h |1 - 1 file changed, 1 deletion(-) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index d95f8b6..6ef70f1 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -233,7 +233,6 @@ static inline void virtio_clear_feature(uint32_t *features, unsigned int fbit) static inline bool __virtio_has_feature(uint32_t features, unsigned int fbit) { -assert(fbit 32); return !!(features (1 fbit)); } I must say I'm not very comfortable with knowingly passing out-of-rage values to this function. Can we perhaps apply at least the feature-bit-size extending patches prior to your patchset, if the remainder of the virtio-1 patchset still takes some time? So the feature-bit-size extending patches currently don't support migration correctly, that's why they are not merged. What I think we need to do for this is move host_features out from transports into core virtio device. Then we can simply check host features 31 and skip migrating low guest features is none set. Thoughts? Any takers? After we move host_features, put them into an optional vmstate subsection? I think with the recent patchsets, most of the interesting stuff is already not handled by the transport anymore. There's only VIRTIO_F_NOTIFY_ON_EMPTY and VIRTIO_F_BAD_FEATURE left (set by pci and ccw). notify on empty is likely safe to set for everyone. bad feature should be pci specific, it's a mistake that we have it in ccw. it's there to detect very old buggy guests. in fact ccw ignores this bit completely. For PCI, I think VIRTIO_F_BAD_FEATURE is never actually set in guest features. If guest attempts to set it, it is immediately cleared. So it can be handled in pci specific code, and won't affect migration. Thinking a bit more, we probably don't need this move of host_features to get migration right (although it might be a nice cleanup later). Could we - keep migration of bits 0..31 as-is - add a vmstate subsection for bits 32..63 only included if one of those bits is set - have a post handler that performs a validation of the full set of bits 0..63 ? We could do a similar exercise with a subsection containing the addresses for avail and used with a post handler overwriting any addresses set by the old style migration code. Does that make sense? I don't see how it does: on the receive side you don't know whether guest acked bits 32..63 so you can't decide whether to parse bits 32..63. But if it wasn't set, it obviously wasn't acked, I'd think? The right thing to do IMHO is to migrate the high guest bits if and only if the *host* bits 32..63 are set. And that needs the host features in core, or at least is easier if they are there. Aren't the host bits a prereq? Confused. I'll think about that tomorrow when it's hopefully a bit cooler around here :)
Re: [Qemu-devel] [PATCH v2] qmp: Add qom_path field to query-cpus command
Eduardo Habkost ehabk...@redhat.com writes: On Tue, May 12, 2015 at 05:38:37PM +0200, Markus Armbruster wrote: [...] @@ -699,8 +701,9 @@ #data is sent to the client, the guest may no longer be halted. ## { 'struct': 'CpuInfo', - 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', '*pc': 'int', - '*nip': 'int', '*npc': 'int', '*PC': 'int', 'thread_id': 'int'} } + 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', qom_path': 'str', Long line. It has exactly 80 characters. Several characters too wide for my taste. I just realized checkpatch is fine with 80. I'll revert my line wrap if you feel strongly about it.
Re: [Qemu-devel] [PATCH v3 6/7] qom: add a object_property_add_enum helper method
On Fri, May 08, 2015 at 07:45:10PM +0200, Andreas Färber wrote: Am 01.05.2015 um 12:30 schrieb Daniel P. Berrange: Looks good in general. Some minor nits below, and one limitation possibly worth mentioning in the second paragraph of the commit message: It assumes a 1:1 mapping. I do guess most ones are, but I remember some CPUID bits having different names for the same values, for instance. Worst case, in that edge case, we can simply not use this stricter enum property type - just carry on with existing custom property diff --git a/include/qom/object.h b/include/qom/object.h index bf76f7a..f6a2a9d 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -1271,6 +1271,25 @@ void object_property_add_bool(Object *obj, const char *name, Error **errp); /** + * object_property_add_enum: + * @obj: the object to add a property to + * @name: the name of the property + * @typename: the name of the enum data type + * @get: the getter or NULL if the property is write-only. %NULL + * @set: the setter or NULL if the property is read-only + * @errp: if an error occurs, a pointer to an area to store the error + * + * Add a enum property using getters/setters. This function will add a + * property of type 'enum'. This is slightly ambiguous, as I understand it the type we're actually using is the one in @typename, not enum? Yeah, you are right - this doc mistake is left over from a previous version before Paolo asked me to add the @typename parameter. diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c index 9f16cdb..de142e3 100644 --- a/tests/check-qom-proplist.c +++ b/tests/check-qom-proplist.c @@ -32,10 +32,28 @@ typedef struct DummyObjectClass DummyObjectClass; #define DUMMY_OBJECT(obj) \ OBJECT_CHECK(DummyObject, (obj), TYPE_DUMMY) +typedef enum DummyAnimal DummyAnimal; + +enum DummyAnimal { +DUMMY_FROG, +DUMMY_ALLIGATOR, +DUMMY_PLATYPUS, + +DUMMY_LAST, +}; + +static const char *const dummyanimalmap[DUMMY_LAST + 1] = { dummy_animal_map would be slightly easier to read. Sure +static void test_dummy_badenum(void) +{ +Error *err = NULL; +Object *parent = container_get(object_get_root(), + /objects); +DummyObject *dobj = DUMMY_OBJECT( +object_new_propv(TYPE_DUMMY, + parent, + dummy0, + err, + bv, yes, + sv, Hiss hiss hiss, + av, yeti, + NULL)); + +g_assert(dobj == NULL); Superfluous. +g_assert(err != NULL); +g_assert(g_str_equal(error_get_pretty(err), + Invalid parameter 'yeti')); Same question as in previous test: alternatives? Yep, will check Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PATCH v2 02/17] target-alpha: Rename floating-point subroutines
... to match the instructions, which have no leading f. Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/fpu_helper.c | 2 +- target-alpha/helper.h | 2 +- target-alpha/translate.c | 68 +++ 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/target-alpha/fpu_helper.c b/target-alpha/fpu_helper.c index 8acd460..119559a 100644 --- a/target-alpha/fpu_helper.c +++ b/target-alpha/fpu_helper.c @@ -493,7 +493,7 @@ uint64_t helper_cvtqt(CPUAlphaState *env, uint64_t a) return float64_to_t(fr); } -void helper_fcvtql_v_input(CPUAlphaState *env, uint64_t val) +void helper_cvtql_v_input(CPUAlphaState *env, uint64_t val) { if (val != (int32_t)val) { arith_excp(env, GETPC(), EXC_M_IOV, 0); diff --git a/target-alpha/helper.h b/target-alpha/helper.h index a451cfe..424ea49 100644 --- a/target-alpha/helper.h +++ b/target-alpha/helper.h @@ -94,7 +94,7 @@ DEF_HELPER_FLAGS_3(fp_exc_raise_s, TCG_CALL_NO_WG, void, env, i32, i32) DEF_HELPER_FLAGS_2(ieee_input, TCG_CALL_NO_WG, void, env, i64) DEF_HELPER_FLAGS_2(ieee_input_cmp, TCG_CALL_NO_WG, void, env, i64) -DEF_HELPER_FLAGS_2(fcvtql_v_input, TCG_CALL_NO_WG, void, env, i64) +DEF_HELPER_FLAGS_2(cvtql_v_input, TCG_CALL_NO_WG, void, env, i64) #if !defined (CONFIG_USER_ONLY) DEF_HELPER_2(hw_ret, void, env, i64) diff --git a/target-alpha/translate.c b/target-alpha/translate.c index efeeb05..b3c5dca 100644 --- a/target-alpha/translate.c +++ b/target-alpha/translate.c @@ -718,7 +718,7 @@ static inline void gen_fp_exc_raise(int rc, int fn11) gen_fp_exc_raise_ignore(rc, fn11, fn11 QUAL_I ? 0 : float_flag_inexact); } -static void gen_fcvtlq(TCGv vc, TCGv vb) +static void gen_cvtlq(TCGv vc, TCGv vb) { TCGv tmp = tcg_temp_new(); @@ -733,7 +733,7 @@ static void gen_fcvtlq(TCGv vc, TCGv vb) tcg_temp_free(tmp); } -static void gen_fcvtql(TCGv vc, TCGv vb) +static void gen_cvtql(TCGv vc, TCGv vb) { TCGv tmp = tcg_temp_new(); @@ -763,8 +763,8 @@ static void gen_ieee_arith2(DisasContext *ctx, } #define IEEE_ARITH2(name) \ -static inline void glue(gen_f, name)(DisasContext *ctx, \ - int rb, int rc, int fn11) \ +static inline void glue(gen_, name)(DisasContext *ctx, \ +int rb, int rc, int fn11) \ { \ gen_ieee_arith2(ctx, gen_helper_##name, rb, rc, fn11); \ } @@ -773,7 +773,7 @@ IEEE_ARITH2(sqrtt) IEEE_ARITH2(cvtst) IEEE_ARITH2(cvtts) -static void gen_fcvttq(DisasContext *ctx, int rb, int rc, int fn11) +static void gen_cvttq(DisasContext *ctx, int rb, int rc, int fn11) { TCGv vb, vc; int ignore = 0; @@ -830,8 +830,8 @@ static void gen_ieee_intcvt(DisasContext *ctx, } #define IEEE_INTCVT(name) \ -static inline void glue(gen_f, name)(DisasContext *ctx, \ - int rb, int rc, int fn11) \ +static inline void glue(gen_, name)(DisasContext *ctx, \ +int rb, int rc, int fn11) \ { \ gen_ieee_intcvt(ctx, gen_helper_##name, rb, rc, fn11); \ } @@ -875,8 +875,8 @@ static void gen_ieee_arith3(DisasContext *ctx, } #define IEEE_ARITH3(name) \ -static inline void glue(gen_f, name)(DisasContext *ctx, \ - int ra, int rb, int rc, int fn11) \ +static inline void glue(gen_, name)(DisasContext *ctx, \ +int ra, int rb, int rc, int fn11) \ { \ gen_ieee_arith3(ctx, gen_helper_##name, ra, rb, rc, fn11); \ } @@ -906,8 +906,8 @@ static void gen_ieee_compare(DisasContext *ctx, } #define IEEE_CMP3(name) \ -static inline void glue(gen_f, name)(DisasContext *ctx, \ - int ra, int rb, int rc, int fn11) \ +static inline void glue(gen_, name)(DisasContext *ctx, \ +int ra, int rb, int rc, int fn11) \ { \ gen_ieee_compare(ctx, gen_helper_##name, ra, rb, rc, fn11); \ } @@ -1958,7 +1958,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) case 0x0B: /* SQRTS */ REQUIRE_REG_31(ra); -gen_fsqrts(ctx, rb, rc, fn11); +gen_sqrts(ctx, rb, rc, fn11); break; case 0x14: /* ITOFF */ @@ -1984,7 +1984,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) case 0x02B:
[Qemu-devel] [PATCH v2 14/17] target-alpha: Raise EXC_M_INV properly for fp inputs
Ignore DNZ if software completion isn't used. Raise INV for denormals in system mode so the OS completion handler sees them. Reported-by: Al Viro v...@zeniv.linux.org.uk Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/fpu_helper.c | 32 ++-- target-alpha/helper.h | 1 + target-alpha/translate.c | 7 +++ 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/target-alpha/fpu_helper.c b/target-alpha/fpu_helper.c index db523fb..ea1f2e2 100644 --- a/target-alpha/fpu_helper.c +++ b/target-alpha/fpu_helper.c @@ -104,16 +104,14 @@ void helper_ieee_input(CPUAlphaState *env, uint64_t val) uint64_t frac = val 0xfull; if (exp == 0) { -/* Denormals without DNZ set raise an exception. */ -if (frac != 0 !env-fp_status.flush_inputs_to_zero) { -arith_excp(env, GETPC(), EXC_M_UNF, 0); +/* Denormals without /S raise an exception. */ +if (frac != 0) { +arith_excp(env, GETPC(), EXC_M_INV, 0); } } else if (exp == 0x7ff) { /* Infinity or NaN. */ -/* ??? I'm not sure these exception bit flags are correct. I do - know that the Linux kernel, at least, doesn't rely on them and - just emulates the insn to figure out what exception to use. */ -arith_excp(env, GETPC(), frac ? EXC_M_INV : EXC_M_FOV, 0); +env-fpcr |= FPCR_INV; +arith_excp(env, GETPC(), EXC_M_INV, 0); } } @@ -124,16 +122,30 @@ void helper_ieee_input_cmp(CPUAlphaState *env, uint64_t val) uint64_t frac = val 0xfull; if (exp == 0) { -/* Denormals without DNZ set raise an exception. */ -if (frac != 0 !env-fp_status.flush_inputs_to_zero) { -arith_excp(env, GETPC(), EXC_M_UNF, 0); +/* Denormals without /S raise an exception. */ +if (frac != 0) { +arith_excp(env, GETPC(), EXC_M_INV, 0); } } else if (exp == 0x7ff frac) { /* NaN. */ +env-fpcr |= FPCR_INV; arith_excp(env, GETPC(), EXC_M_INV, 0); } } +/* Input handing with software completion. Trap for denorms, unless DNZ + is set. If we try to support DNOD (which none of the produced hardware + did, AFAICS), we'll need to suppress the trap when FPCR.DNOD is set; + then the code downstream of that will need to cope with denorms sans + flush_input_to_zero. Most of it should work sanely, but there's + nothing to compare with. */ +void helper_ieee_input_s(CPUAlphaState *env, uint64_t val) +{ +if (unlikely(2 * val - 1 0x1full) + !env-fp_status.flush_inputs_to_zero) { +arith_excp(env, GETPC(), EXC_M_INV | EXC_M_SWC, 0); +} +} /* S floating (single) */ diff --git a/target-alpha/helper.h b/target-alpha/helper.h index 5b1a5d9..780b0dc 100644 --- a/target-alpha/helper.h +++ b/target-alpha/helper.h @@ -86,6 +86,7 @@ DEF_HELPER_FLAGS_3(fp_exc_raise_s, TCG_CALL_NO_WG, void, env, i32, i32) DEF_HELPER_FLAGS_2(ieee_input, TCG_CALL_NO_WG, void, env, i64) DEF_HELPER_FLAGS_2(ieee_input_cmp, TCG_CALL_NO_WG, void, env, i64) +DEF_HELPER_FLAGS_2(ieee_input_s, TCG_CALL_NO_WG, void, env, i64) DEF_HELPER_FLAGS_2(cvtql_v_input, TCG_CALL_NO_WG, void, env, i64) #if !defined (CONFIG_USER_ONLY) diff --git a/target-alpha/translate.c b/target-alpha/translate.c index f0556b0..4c441a9 100644 --- a/target-alpha/translate.c +++ b/target-alpha/translate.c @@ -658,6 +658,13 @@ static TCGv gen_ieee_input(DisasContext *ctx, int reg, int fn11, int is_cmp) } else { gen_helper_ieee_input(cpu_env, val); } +} else { +#ifndef CONFIG_USER_ONLY +/* In system mode, raise exceptions for denormals like real + hardware. In user mode, proceed as if the OS completion + handler is handling the denormal as per spec. */ +gen_helper_ieee_input_s(cpu_env, val); +#endif } } return val; -- 2.1.0
[Qemu-devel] [PATCH v2 03/17] target-alpha: Forget installed round mode after MT_FPCR
When we use QUAL_RM_D, we copy fpcr_dyn_round to float_status. When we install a new FPCR value, we update fpcr_dyn_round. Reset the status of the cache so that we re-copy for the next fp insn that requires dynamic rounding. Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/translate.c | 5 + 1 file changed, 5 insertions(+) diff --git a/target-alpha/translate.c b/target-alpha/translate.c index b3c5dca..94dab26 100644 --- a/target-alpha/translate.c +++ b/target-alpha/translate.c @@ -2199,6 +2199,11 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) /* MT_FPCR */ va = load_fpr(ctx, ra); gen_helper_store_fpcr(cpu_env, va); +if (ctx-tb_rm == QUAL_RM_D) { +/* Re-do the copy of the rounding mode to fp_status + the next time we use dynamic rounding. */ +ctx-tb_rm = -1; +} break; case 0x025: /* MF_FPCR */ -- 2.1.0
Re: [Qemu-devel] [PATCH RFC 4/7] vhost: set vring endianness for legacy virtio
On Tue, 12 May 2015 17:15:53 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, May 12, 2015 at 03:25:30PM +0200, Cornelia Huck wrote: On Wed, 06 May 2015 14:08:02 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: Legacy virtio is native endian: if the guest and host endianness differ, we have to tell vhost so it can swap bytes where appropriate. This is done through a vhost ring ioctl. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- hw/virtio/vhost.c | 50 +- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 54851b7..1d7b939 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c (...) @@ -677,6 +700,16 @@ static int vhost_virtqueue_start(struct vhost_dev *dev, return -errno; } +if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1) I think this should either go in after the virtio-1 base support (more feature bits etc.) or get a big fat comment and be touched up later. I'd prefer the first solution so it does not get forgotten, but I'm not sure when Michael plans to proceed with the virtio-1 patches (I think they're mostly fine already). There are three main issues with virtio 1 patches that I am aware of. One issue with virtio 1 patches as they are is with how features are handled ATM. There are 3 types of features a. virtio 1 only features b. virtio 0 only features c. shared features and 3 types of devices a. legacy device: has b+c features b. modern device: has a+c features c. transitional device: has a+c features but exposes only c through the legacy interface Wouldn't a transitional device be able to expose b as well? So I think a callback that gets features depending on guest version isn't a good way to model it because fundamentally device has one set of features. A better way to model this is really just a single host_features bitmask, and for transitional devices, a mask hiding a features - which are so far all bits 31, so maybe for now we can just have a global mask. How would this work for transitional presenting a modern device - would you have a superset of bits and masks for legacy and modern? We need to validate features at initialization time and make sure they make sense, fail if not (sometimes we need to mask features if they don't make sense - this is unfortunate but might be needed for compatibility). Moving host_features to virtio core would make all of the above easier. I have started hacking up code that moves host_features, but I'm quite lost with all the different virtio versions floating around. Currently trying against master, but that of course ignores the virtio-1 issues. Second issue is migration, some of it is with migrating the new features, so that's tied to the first one. There's also the used and avail addresses, but that kind of follows from virtio-1 support. Third issue is fixing devices so they don't try to access guest memory until DRIVER_OK is set. This is surprisingly hard to do generally given need to support old drivers which don't set DRIVER_OK or set it very late, and the fact that we tied work-arounds for even older drivers which dont' set pci bus master to the DRIVER_OK bit. I tried, and I'm close to giving up and just checking guest ack for virtio 1, and ignoring DRIVER_OK requirement if not there. If legacy survived like it is until now, it might be best to focus on modern devices for this.
Re: [Qemu-devel] [PATCH v3 7/7] qom: don't pass string table to object_get_enum method
On Fri, May 08, 2015 at 07:54:48PM +0200, Andreas Färber wrote: Am 01.05.2015 um 12:30 schrieb Daniel P. Berrange: Now that properties can be explicitly registered as an enum type, there is no need to pass the string table to the object_get_enum method. The object property registration already has a pointer to the string table. In changing this method signature, the hostmem backend object has to be converted to use the new enum property registration code, which simplifies it somewhat. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- backends/hostmem.c | 22 -- include/qom/object.h | 4 ++-- numa.c | 2 +- qom/object.c | 32 tests/check-qom-proplist.c | 46 ++ 5 files changed, 81 insertions(+), 25 deletions(-) diff --git a/qom/object.c b/qom/object.c index ba0e4b8..6d2a2a9 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1026,13 +1026,35 @@ int64_t object_property_get_int(Object *obj, const char *name, return retval; } +typedef struct EnumProperty { +const char * const *strings; +int (*get)(Object *, Error **); +void (*set)(Object *, int, Error **); Since get and set and moved unchanged, I would prefer placing it in the final destination in the original patch to avoid churn. Yep, easy to do. diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c index de142e3..d5cd38b 100644 --- a/tests/check-qom-proplist.c +++ b/tests/check-qom-proplist.c @@ -249,6 +249,51 @@ static void test_dummy_badenum(void) } + +static void test_dummy_getenum(void) +{ +Error *err = NULL; +int val; +Object *parent = container_get(object_get_root(), + /objects); +DummyObject *dobj = DUMMY_OBJECT( +object_new_propv(TYPE_DUMMY, + parent, + dummy0, + err, + av, platypus, + NULL)); + +g_assert(dobj != NULL); +g_assert(err == NULL); +g_assert(dobj-av == DUMMY_PLATYPUS); + +val = object_property_get_enum(OBJECT(dobj), + av, + DummyAnimal, + err); +g_assert(err == NULL); Is there any significant difference between g_assert()'ing on error and passing error_abort? I didn't know about error_abort until now :-) I will use that. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PATCH v2 06/17] target-alpha: Set fpcr_exc_status even for disabled exceptions
The qualifiers can suppress the raising of exceptions, but real hardware still records that the exceptions occurred. Reported-by: Al Viro v...@zeniv.linux.org.uk Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/fpu_helper.c | 35 +-- target-alpha/translate.c | 28 +--- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/target-alpha/fpu_helper.c b/target-alpha/fpu_helper.c index caf8317..6e84fd3 100644 --- a/target-alpha/fpu_helper.c +++ b/target-alpha/fpu_helper.c @@ -57,18 +57,16 @@ static uint32_t soft_to_fpcr_exc(CPUAlphaState *env) static void fp_exc_raise1(CPUAlphaState *env, uintptr_t retaddr, uint32_t exc, uint32_t regno) { -if (exc) { -uint32_t hw_exc = 0; +uint32_t hw_exc = 0; -hw_exc |= CONVERT_BIT(exc, FPCR_INV, EXC_M_INV); -hw_exc |= CONVERT_BIT(exc, FPCR_DZE, EXC_M_DZE); -hw_exc |= CONVERT_BIT(exc, FPCR_OVF, EXC_M_FOV); -hw_exc |= CONVERT_BIT(exc, FPCR_UNF, EXC_M_UNF); -hw_exc |= CONVERT_BIT(exc, FPCR_INE, EXC_M_INE); -hw_exc |= CONVERT_BIT(exc, FPCR_IOV, EXC_M_IOV); +hw_exc |= CONVERT_BIT(exc, FPCR_INV, EXC_M_INV); +hw_exc |= CONVERT_BIT(exc, FPCR_DZE, EXC_M_DZE); +hw_exc |= CONVERT_BIT(exc, FPCR_OVF, EXC_M_FOV); +hw_exc |= CONVERT_BIT(exc, FPCR_UNF, EXC_M_UNF); +hw_exc |= CONVERT_BIT(exc, FPCR_INE, EXC_M_INE); +hw_exc |= CONVERT_BIT(exc, FPCR_IOV, EXC_M_IOV); -arith_excp(env, retaddr, hw_exc, 1ull regno); -} +arith_excp(env, retaddr, hw_exc, 1ull regno); } /* Raise exceptions for ieee fp insns without software completion. @@ -76,8 +74,14 @@ static void fp_exc_raise1(CPUAlphaState *env, uintptr_t retaddr, doesn't apply. */ void helper_fp_exc_raise(CPUAlphaState *env, uint32_t ignore, uint32_t regno) { -uint32_t exc = env-error_code ~ignore; -fp_exc_raise1(env, GETPC(), exc, regno); +uint32_t exc = env-error_code; +if (exc) { +env-fpcr |= exc; +exc = ~ignore; +if (exc) { +fp_exc_raise1(env, GETPC(), exc, regno); +} +} } /* Raise exceptions for ieee fp insns with software completion. */ @@ -86,8 +90,11 @@ void helper_fp_exc_raise_s(CPUAlphaState *env, uint32_t ignore, uint32_t regno) uint32_t exc = env-error_code ~ignore; if (exc) { env-fpcr |= exc; -exc = env-fpcr_exc_enable; -fp_exc_raise1(env, GETPC(), exc, regno); +exc = ~ignore; +if (exc) { +exc = env-fpcr_exc_enable; +fp_exc_raise1(env, GETPC(), exc, regno); +} } } diff --git a/target-alpha/translate.c b/target-alpha/translate.c index 25107f9..f121320 100644 --- a/target-alpha/translate.c +++ b/target-alpha/translate.c @@ -663,15 +663,24 @@ static TCGv gen_ieee_input(DisasContext *ctx, int reg, int fn11, int is_cmp) return val; } -static void gen_fp_exc_raise_ignore(int rc, int fn11, int ignore) +static void gen_fp_exc_raise(int rc, int fn11) { /* ??? We ought to be able to do something with imprecise exceptions. E.g. notice we're still in the trap shadow of something within the TB and do not generate the code to signal the exception; end the TB when an exception is forced to arrive, either by consumption of a register value or TRAPB or EXCB. */ -TCGv_i32 ign = tcg_const_i32(ignore); -TCGv_i32 reg; +TCGv_i32 reg, ign; +uint32_t ignore = 0; + +if (!(fn11 QUAL_U)) { +/* Note that QUAL_U == QUAL_V, so ignore either. */ +ignore |= FPCR_UNF | FPCR_IOV; +} +if (!(fn11 QUAL_I)) { +ignore |= FPCR_INE; +} +ign = tcg_const_i32(ignore); /* ??? Pass in the regno of the destination so that the helper can set EXC_MASK, which contains a bitmask of destination registers @@ -679,7 +688,6 @@ static void gen_fp_exc_raise_ignore(int rc, int fn11, int ignore) does not require this. We do need it for a guest kernel's entArith, or if we were to do something clever with imprecise exceptions. */ reg = tcg_const_i32(rc + 32); - if (fn11 QUAL_S) { gen_helper_fp_exc_raise_s(cpu_env, ign, reg); } else { @@ -690,11 +698,6 @@ static void gen_fp_exc_raise_ignore(int rc, int fn11, int ignore) tcg_temp_free_i32(ign); } -static inline void gen_fp_exc_raise(int rc, int fn11) -{ -gen_fp_exc_raise_ignore(rc, fn11, fn11 QUAL_I ? 0 : FPCR_INE); -} - static void gen_cvtlq(TCGv vc, TCGv vb) { TCGv tmp = tcg_temp_new(); @@ -752,7 +755,6 @@ IEEE_ARITH2(cvtts) static void gen_cvttq(DisasContext *ctx, int rb, int rc, int fn11) { TCGv vb, vc; -int ignore = 0; /* No need to set flushzero, since we have an integer output. */ vb = gen_ieee_input(ctx, rb, fn11, 0); @@ -766,20 +768,16 @@ static void gen_cvttq(DisasContext *ctx, int rb, int rc, int fn11) break; case
[Qemu-devel] [PATCH v2 12/17] target-alpha: Implement WH64EN
Backward compatible cache insn introduced for EV7. Reported-by: Al Viro v...@zeniv.linux.org.uk Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/translate.c | 4 1 file changed, 4 insertions(+) diff --git a/target-alpha/translate.c b/target-alpha/translate.c index 74f5d07..953d1ef 100644 --- a/target-alpha/translate.c +++ b/target-alpha/translate.c @@ -2318,6 +2318,10 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) /* WH64 */ /* No-op */ break; +case 0xFC00: +/* WH64EN */ +/* No-op */ +break; default: goto invalid_opc; } -- 2.1.0
[Qemu-devel] [PATCH v2 05/17] target-alpha: Tidy FPCR representation
Store the fpcr as the hardware represents it. Convert the softfpu representation of exceptions into the fpcr representation. Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/cpu.h| 95 + target-alpha/fpu_helper.c | 130 +- target-alpha/helper.c | 130 -- target-alpha/helper.h | 2 - target-alpha/translate.c | 45 +++- 5 files changed, 159 insertions(+), 243 deletions(-) diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h index 9538f19..2a4d5cb 100644 --- a/target-alpha/cpu.h +++ b/target-alpha/cpu.h @@ -150,54 +150,54 @@ enum { FP_ROUND_DYNAMIC = 0x3, }; -/* FPCR bits */ -#define FPCR_SUM (1ULL 63) -#define FPCR_INED (1ULL 62) -#define FPCR_UNFD (1ULL 61) -#define FPCR_UNDZ (1ULL 60) -#define FPCR_DYN_SHIFT 58 -#define FPCR_DYN_CHOPPED (0ULL FPCR_DYN_SHIFT) -#define FPCR_DYN_MINUS (1ULL FPCR_DYN_SHIFT) -#define FPCR_DYN_NORMAL(2ULL FPCR_DYN_SHIFT) -#define FPCR_DYN_PLUS (3ULL FPCR_DYN_SHIFT) -#define FPCR_DYN_MASK (3ULL FPCR_DYN_SHIFT) -#define FPCR_IOV (1ULL 57) -#define FPCR_INE (1ULL 56) -#define FPCR_UNF (1ULL 55) -#define FPCR_OVF (1ULL 54) -#define FPCR_DZE (1ULL 53) -#define FPCR_INV (1ULL 52) -#define FPCR_OVFD (1ULL 51) -#define FPCR_DZED (1ULL 50) -#define FPCR_INVD (1ULL 49) -#define FPCR_DNZ (1ULL 48) -#define FPCR_DNOD (1ULL 47) -#define FPCR_STATUS_MASK (FPCR_IOV | FPCR_INE | FPCR_UNF \ -| FPCR_OVF | FPCR_DZE | FPCR_INV) +/* FPCR bits -- right-shifted 32 so we can use a uint32_t. */ +#define FPCR_SUM(1U (63 - 32)) +#define FPCR_INED (1U (62 - 32)) +#define FPCR_UNFD (1U (61 - 32)) +#define FPCR_UNDZ (1U (60 - 32)) +#define FPCR_DYN_SHIFT (58 - 32) +#define FPCR_DYN_CHOPPED(0U FPCR_DYN_SHIFT) +#define FPCR_DYN_MINUS (1U FPCR_DYN_SHIFT) +#define FPCR_DYN_NORMAL (2U FPCR_DYN_SHIFT) +#define FPCR_DYN_PLUS (3U FPCR_DYN_SHIFT) +#define FPCR_DYN_MASK (3U FPCR_DYN_SHIFT) +#define FPCR_IOV(1U (57 - 32)) +#define FPCR_INE(1U (56 - 32)) +#define FPCR_UNF(1U (55 - 32)) +#define FPCR_OVF(1U (54 - 32)) +#define FPCR_DZE(1U (53 - 32)) +#define FPCR_INV(1U (52 - 32)) +#define FPCR_OVFD (1U (51 - 32)) +#define FPCR_DZED (1U (50 - 32)) +#define FPCR_INVD (1U (49 - 32)) +#define FPCR_DNZ(1U (48 - 32)) +#define FPCR_DNOD (1U (47 - 32)) +#define FPCR_STATUS_MASK(FPCR_IOV | FPCR_INE | FPCR_UNF \ + | FPCR_OVF | FPCR_DZE | FPCR_INV) /* The silly software trap enables implemented by the kernel emulation. These are more or less architecturally required, since the real hardware has read-as-zero bits in the FPCR when the features aren't implemented. For the purposes of QEMU, we pretend the FPCR can hold everything. */ -#define SWCR_TRAP_ENABLE_INV (1ULL 1) -#define SWCR_TRAP_ENABLE_DZE (1ULL 2) -#define SWCR_TRAP_ENABLE_OVF (1ULL 3) -#define SWCR_TRAP_ENABLE_UNF (1ULL 4) -#define SWCR_TRAP_ENABLE_INE (1ULL 5) -#define SWCR_TRAP_ENABLE_DNO (1ULL 6) -#define SWCR_TRAP_ENABLE_MASK ((1ULL 7) - (1ULL 1)) - -#define SWCR_MAP_DMZ (1ULL 12) -#define SWCR_MAP_UMZ (1ULL 13) -#define SWCR_MAP_MASK (SWCR_MAP_DMZ | SWCR_MAP_UMZ) - -#define SWCR_STATUS_INV(1ULL 17) -#define SWCR_STATUS_DZE(1ULL 18) -#define SWCR_STATUS_OVF(1ULL 19) -#define SWCR_STATUS_UNF(1ULL 20) -#define SWCR_STATUS_INE(1ULL 21) -#define SWCR_STATUS_DNO(1ULL 22) -#define SWCR_STATUS_MASK ((1ULL 23) - (1ULL 17)) +#define SWCR_TRAP_ENABLE_INV(1U 1) +#define SWCR_TRAP_ENABLE_DZE(1U 2) +#define SWCR_TRAP_ENABLE_OVF(1U 3) +#define SWCR_TRAP_ENABLE_UNF(1U 4) +#define SWCR_TRAP_ENABLE_INE(1U 5) +#define SWCR_TRAP_ENABLE_DNO(1U 6) +#define SWCR_TRAP_ENABLE_MASK ((1U 7) - (1U 1)) + +#define SWCR_MAP_DMZ(1U 12) +#define SWCR_MAP_UMZ(1U 13) +#define SWCR_MAP_MASK (SWCR_MAP_DMZ | SWCR_MAP_UMZ) + +#define SWCR_STATUS_INV (1U 17) +#define SWCR_STATUS_DZE (1U 18) +#define SWCR_STATUS_OVF (1U 19) +#define SWCR_STATUS_UNF (1U 20) +#define SWCR_STATUS_INE (1U 21) +#define SWCR_STATUS_DNO (1U 22) +#define SWCR_STATUS_MASK((1U 23) - (1U 17))
[Qemu-devel] [PATCH v2 08/17] target-alpha: Raise IOV from CVTTQ
Floating-point overflow is a different bit from integer overflow. Reported-by: Al Viro v...@zeniv.linux.org.uk Signed-off-by: Richard Henderson r...@twiddle.net --- target-alpha/fpu_helper.c | 25 + target-alpha/helper.h | 1 - target-alpha/translate.c | 17 - 3 files changed, 13 insertions(+), 30 deletions(-) diff --git a/target-alpha/fpu_helper.c b/target-alpha/fpu_helper.c index 914c1d5..132b5a2 100644 --- a/target-alpha/fpu_helper.c +++ b/target-alpha/fpu_helper.c @@ -427,12 +427,9 @@ uint64_t helper_cvtqs(CPUAlphaState *env, uint64_t a) /* Implement float64 to uint64 conversion without saturation -- we must supply the truncated result. This behaviour is used by the compiler - to get unsigned conversion for free with the same instruction. + to get unsigned conversion for free with the same instruction. */ - The VI flag is set when overflow or inexact exceptions should be raised. */ - -static inline uint64_t inline_cvttq(CPUAlphaState *env, uint64_t a, -int roundmode, int VI) +static uint64_t do_cvttq(CPUAlphaState *env, uint64_t a, int roundmode) { uint64_t frac, ret = 0; uint32_t exp, sign, exc = 0; @@ -447,7 +444,7 @@ static inline uint64_t inline_cvttq(CPUAlphaState *env, uint64_t a, goto do_underflow; } } else if (exp == 0x7ff) { -exc = (frac ? FPCR_INV : VI ? FPCR_OVF : 0); +exc = (frac ? FPCR_INV : FPCR_IOV | FPCR_INE); } else { /* Restore implicit bit. */ frac |= 0x10ull; @@ -456,10 +453,11 @@ static inline uint64_t inline_cvttq(CPUAlphaState *env, uint64_t a, if (shift = 0) { /* In this case the number is so large that we must shift the fraction left. There is no rounding to do. */ +exc = FPCR_IOV | FPCR_INE; if (shift 63) { ret = frac shift; -if (VI (ret shift) != frac) { -exc = FPCR_OVF; +if ((ret shift) == frac) { +exc = 0; } } } else { @@ -482,7 +480,7 @@ static inline uint64_t inline_cvttq(CPUAlphaState *env, uint64_t a, } if (round) { -exc = (VI ? FPCR_INE : 0); +exc = FPCR_INE; switch (roundmode) { case float_round_nearest_even: if (round == (1ull 63)) { @@ -514,17 +512,12 @@ static inline uint64_t inline_cvttq(CPUAlphaState *env, uint64_t a, uint64_t helper_cvttq(CPUAlphaState *env, uint64_t a) { -return inline_cvttq(env, a, FP_STATUS.float_rounding_mode, 1); +return do_cvttq(env, a, FP_STATUS.float_rounding_mode); } uint64_t helper_cvttq_c(CPUAlphaState *env, uint64_t a) { -return inline_cvttq(env, a, float_round_to_zero, 0); -} - -uint64_t helper_cvttq_svic(CPUAlphaState *env, uint64_t a) -{ -return inline_cvttq(env, a, float_round_to_zero, 1); +return do_cvttq(env, a, float_round_to_zero); } uint64_t helper_cvtqt(CPUAlphaState *env, uint64_t a) diff --git a/target-alpha/helper.h b/target-alpha/helper.h index 67a6e32..9e7b771 100644 --- a/target-alpha/helper.h +++ b/target-alpha/helper.h @@ -83,7 +83,6 @@ DEF_HELPER_FLAGS_2(cvtqg, TCG_CALL_NO_RWG, i64, env, i64) DEF_HELPER_FLAGS_2(cvttq, TCG_CALL_NO_RWG, i64, env, i64) DEF_HELPER_FLAGS_2(cvttq_c, TCG_CALL_NO_RWG, i64, env, i64) -DEF_HELPER_FLAGS_2(cvttq_svic, TCG_CALL_NO_RWG, i64, env, i64) DEF_HELPER_FLAGS_2(setroundmode, TCG_CALL_NO_RWG, void, env, i32) DEF_HELPER_FLAGS_2(setflushzero, TCG_CALL_NO_RWG, void, env, i32) diff --git a/target-alpha/translate.c b/target-alpha/translate.c index f121320..7868cc4 100644 --- a/target-alpha/translate.c +++ b/target-alpha/translate.c @@ -760,23 +760,14 @@ static void gen_cvttq(DisasContext *ctx, int rb, int rc, int fn11) vb = gen_ieee_input(ctx, rb, fn11, 0); vc = dest_fpr(ctx, rc); -/* Almost all integer conversions use cropped rounding, and most - also do not have integer overflow enabled. Special case that. */ -switch (fn11) { -case QUAL_RM_C: +/* Almost all integer conversions use cropped rounding; + special case that. */ +if ((fn11 QUAL_RM_MASK) == QUAL_RM_C) { gen_helper_cvttq_c(vc, cpu_env, vb); -break; -case QUAL_V | QUAL_RM_C: -case QUAL_S | QUAL_V | QUAL_RM_C: -case QUAL_S | QUAL_V | QUAL_I | QUAL_RM_C: -gen_helper_cvttq_svic(vc, cpu_env, vb); -break; -default: +} else { gen_qual_roundmode(ctx, fn11); gen_helper_cvttq(vc, cpu_env, vb); -break; } - gen_fp_exc_raise(rc, fn11); } -- 2.1.0
Re: [Qemu-devel] [PATCH 1/5] qcow2/qcow: protect against uninitialized encryption key
On 05/12/2015 10:09 AM, Daniel P. Berrange wrote: When a qcow[2] file is opened, if the header reports an encryption method, this is used to set the 'crypt_method_header' field on the BDRVQcow[2]State struct, and the 'encrypted' flag in the BDRVState struct. When doing I/O operations, the 'crypt_method' field on the BDRVQcow[2]State struct is checked to determine if encryption needs to be applied. The crypt_method_header value is copied into crypt_method when the bdrv_set_key() method is called. The QEMU code which opens a block device is expected to always do a check if (bdrv_is_encrypted(bs)) { bdrv_set_key(bs, key...); } If code forgets todo this, then 'crypt_method' is never set s/todo/to do/ and so when I/O is performed, QEMU writes plain text data into a sector which is expected to contain cipher text, or when reading, will return cipher text instead of plain text. Change the qcow[2] code to consult bs-encrypted when deciding whether encryption is required, and assert(s-crypt_method) to protect against cases where the caller forgets to set the encryption key. Also put an assert in the set_key methods to protect against the case where the caller sets an encryption key on a block device that does not have encryption Signed-off-by: Daniel P. Berrange berra...@redhat.com --- block/qcow.c | 10 +++--- block/qcow2-cluster.c | 3 ++- block/qcow2.c | 18 -- 3 files changed, 21 insertions(+), 10 deletions(-) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PULL v2 13/14] libqos/ahci: Swap memread/write with bufread/write
Where it makes sense, use the new faster primitives. For generally small reads/writes such as for the PRDT and FIS packets, stick with the more wasteful but easier to debug memread/memwrite. For ahci-test (before migration tests): With this patch: real0m3.675s user0m2.582s sys 0m1.718s Without any qtest protocol improvements: real0m14.171s user0m12.072s sys 0m12.527s Signed-off-by: John Snow js...@redhat.com Message-id: 1430864578-22072-6-git-send-email-js...@redhat.com --- tests/ahci-test.c | 8 tests/libqos/ahci.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 1a967c3..6e3fa81 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -874,7 +874,7 @@ static void ahci_test_io_rw_simple(AHCIQState *ahci, unsigned bufsize, /* Write some indicative pattern to our buffer. */ generate_pattern(tx, bufsize, AHCI_SECTOR_SIZE); -memwrite(ptr, tx, bufsize); +bufwrite(ptr, tx, bufsize); /* Write this buffer to disk, then read it back to the DMA buffer. */ ahci_guest_io(ahci, port, write_cmd, ptr, bufsize, sector); @@ -882,7 +882,7 @@ static void ahci_test_io_rw_simple(AHCIQState *ahci, unsigned bufsize, ahci_guest_io(ahci, port, read_cmd, ptr, bufsize, sector); /*** Read back the Data ***/ -memread(ptr, rx, bufsize); +bufread(ptr, rx, bufsize); g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0); ahci_free(ahci, ptr); @@ -1018,7 +1018,7 @@ static void test_dma_fragmented(void) /* Create a DMA buffer in guest memory, and write our pattern to it. */ ptr = guest_alloc(ahci-parent-alloc, bufsize); g_assert(ptr); -memwrite(ptr, tx, bufsize); +bufwrite(ptr, tx, bufsize); cmd = ahci_command_create(CMD_WRITE_DMA); ahci_command_adjust(cmd, 0, ptr, bufsize, 32); @@ -1035,7 +1035,7 @@ static void test_dma_fragmented(void) g_free(cmd); /* Read back the guest's receive buffer into local memory */ -memread(ptr, rx, bufsize); +bufread(ptr, rx, bufsize); guest_free(ahci-parent-alloc, ptr); g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0); diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 95bfb3d..7e17bb6 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -653,13 +653,13 @@ void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, qmemset(ptr, 0x00, bufsize); if (props-write) { -memwrite(ptr, buffer, bufsize); +bufwrite(ptr, buffer, bufsize); } ahci_guest_io(ahci, port, ide_cmd, ptr, bufsize, sector); if (props-read) { -memread(ptr, buffer, bufsize); +bufread(ptr, buffer, bufsize); } ahci_free(ahci, ptr); -- 2.1.0
[Qemu-devel] [PULL v2 10/14] qtest: allow arbitrarily long sends
qtest currently has a static buffer of size 1024 that if we overflow, ignores the additional data silently which leads to hangs or stream failures. Use glib's string facilities to allow arbitrarily long data, but split this off into a new function, qtest_sendf. Static data can still be sent using qtest_send, which avoids the malloc/copy overhead. Signed-off-by: John Snow js...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com Message-id: 1430864578-22072-2-git-send-email-js...@redhat.com --- qtest.c | 46 -- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/qtest.c b/qtest.c index 8d1e66c..3738eae 100644 --- a/qtest.c +++ b/qtest.c @@ -182,21 +182,29 @@ static void qtest_send_prefix(CharDriverState *chr) (long) tv.tv_sec, (long) tv.tv_usec); } -static void GCC_FMT_ATTR(2, 3) qtest_send(CharDriverState *chr, - const char *fmt, ...) +static void do_qtest_send(CharDriverState *chr, const char *str, size_t len) +{ +qemu_chr_fe_write_all(chr, (uint8_t *)str, len); +if (qtest_log_fp qtest_opened) { +fprintf(qtest_log_fp, %s, str); +} +} + +static void qtest_send(CharDriverState *chr, const char *str) +{ +do_qtest_send(chr, str, strlen(str)); +} + +static void GCC_FMT_ATTR(2, 3) qtest_sendf(CharDriverState *chr, + const char *fmt, ...) { va_list ap; -char buffer[1024]; -size_t len; +gchar *buffer; va_start(ap, fmt); -len = vsnprintf(buffer, sizeof(buffer), fmt, ap); +buffer = g_strdup_vprintf(fmt, ap); +qtest_send(chr, buffer); va_end(ap); - -qemu_chr_fe_write_all(chr, (uint8_t *)buffer, len); -if (qtest_log_fp qtest_opened) { -fprintf(qtest_log_fp, %s, buffer); -} } static void qtest_irq_handler(void *opaque, int n, int level) @@ -208,8 +216,8 @@ static void qtest_irq_handler(void *opaque, int n, int level) CharDriverState *chr = qtest_chr; irq_levels[n] = level; qtest_send_prefix(chr); -qtest_send(chr, IRQ %s %d\n, - level ? raise : lower, n); +qtest_sendf(chr, IRQ %s %d\n, +level ? raise : lower, n); } } @@ -318,7 +326,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) value = cpu_inl(addr); } qtest_send_prefix(chr); -qtest_send(chr, OK 0x%04x\n, value); +qtest_sendf(chr, OK 0x%04x\n, value); } else if (strcmp(words[0], writeb) == 0 || strcmp(words[0], writew) == 0 || strcmp(words[0], writel) == 0 || @@ -375,7 +383,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) tswap64s(value); } qtest_send_prefix(chr); -qtest_send(chr, OK 0x%016 PRIx64 \n, value); +qtest_sendf(chr, OK 0x%016 PRIx64 \n, value); } else if (strcmp(words[0], read) == 0) { uint64_t addr, len, i; uint8_t *data; @@ -390,7 +398,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) qtest_send_prefix(chr); qtest_send(chr, OK 0x); for (i = 0; i len; i++) { -qtest_send(chr, %02x, data[i]); +qtest_sendf(chr, %02x, data[i]); } qtest_send(chr, \n); @@ -434,7 +442,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) } qtest_clock_warp(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ns); qtest_send_prefix(chr); -qtest_send(chr, OK %PRIi64\n, (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); +qtest_sendf(chr, OK %PRIi64\n, +(int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); } else if (qtest_enabled() strcmp(words[0], clock_set) == 0) { int64_t ns; @@ -442,10 +451,11 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) ns = strtoll(words[1], NULL, 0); qtest_clock_warp(ns); qtest_send_prefix(chr); -qtest_send(chr, OK %PRIi64\n, (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); +qtest_sendf(chr, OK %PRIi64\n, +(int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); } else { qtest_send_prefix(chr); -qtest_send(chr, FAIL Unknown command `%s'\n, words[0]); +qtest_sendf(chr, FAIL Unknown command '%s'\n, words[0]); } } -- 2.1.0
[Qemu-devel] [PULL v2 03/14] libqos: Add migration helpers
libqos.c: -set_context for addressing which commands go where -migrate performs the actual migration malloc.c: - Structure of the allocator is adjusted slightly with a second-tier malloc to make swapping around the allocators easy when we migrate the lists from the source to the destination. Signed-off-by: John Snow js...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com Message-id: 1430417242-11859-4-git-send-email-js...@redhat.com --- tests/libqos/libqos.c | 85 +++ tests/libqos/libqos.h | 2 ++ tests/libqos/malloc.c | 74 +--- tests/libqos/malloc.h | 1 + 4 files changed, 145 insertions(+), 17 deletions(-) diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c index 7e72078..fce625b 100644 --- a/tests/libqos/libqos.c +++ b/tests/libqos/libqos.c @@ -1,5 +1,6 @@ #include stdio.h #include stdlib.h +#include string.h #include glib.h #include unistd.h #include fcntl.h @@ -62,6 +63,90 @@ void qtest_shutdown(QOSState *qs) g_free(qs); } +void set_context(QOSState *s) +{ +global_qtest = s-qts; +} + +static QDict *qmp_execute(const char *command) +{ +char *fmt; +QDict *rsp; + +fmt = g_strdup_printf({ 'execute': '%s' }, command); +rsp = qmp(fmt); +g_free(fmt); + +return rsp; +} + +void migrate(QOSState *from, QOSState *to, const char *uri) +{ +const char *st; +char *s; +QDict *rsp, *sub; +bool running; + +set_context(from); + +/* Is the machine currently running? */ +rsp = qmp_execute(query-status); +g_assert(qdict_haskey(rsp, return)); +sub = qdict_get_qdict(rsp, return); +g_assert(qdict_haskey(sub, running)); +running = qdict_get_bool(sub, running); +QDECREF(rsp); + +/* Issue the migrate command. */ +s = g_strdup_printf({ 'execute': 'migrate', +'arguments': { 'uri': '%s' } }, +uri); +rsp = qmp(s); +g_free(s); +g_assert(qdict_haskey(rsp, return)); +QDECREF(rsp); + +/* Wait for STOP event, but only if we were running: */ +if (running) { +qmp_eventwait(STOP); +} + +/* If we were running, we can wait for an event. */ +if (running) { +migrate_allocator(from-alloc, to-alloc); +set_context(to); +qmp_eventwait(RESUME); +return; +} + +/* Otherwise, we need to wait: poll until migration is completed. */ +while (1) { +rsp = qmp_execute(query-migrate); +g_assert(qdict_haskey(rsp, return)); +sub = qdict_get_qdict(rsp, return); +g_assert(qdict_haskey(sub, status)); +st = qdict_get_str(sub, status); + +/* setup, active, completed, failed, cancelled */ +if (strcmp(st, completed) == 0) { +QDECREF(rsp); +break; +} + +if ((strcmp(st, setup) == 0) || (strcmp(st, active) == 0)) { +QDECREF(rsp); +g_usleep(5000); +continue; +} + +fprintf(stderr, Migration did not complete, status: %s\n, st); +g_assert_not_reached(); +} + +migrate_allocator(from-alloc, to-alloc); +set_context(to); +} + void mkimg(const char *file, const char *fmt, unsigned size_mb) { gchar *cli; diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h index f57362b..e1f14ea 100644 --- a/tests/libqos/libqos.h +++ b/tests/libqos/libqos.h @@ -21,6 +21,8 @@ QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...); void qtest_shutdown(QOSState *qs); void mkimg(const char *file, const char *fmt, unsigned size_mb); void mkqcow2(const char *file, unsigned size_mb); +void set_context(QOSState *s); +void migrate(QOSState *from, QOSState *to, const char *uri); void prepare_blkdebug_script(const char *debug_fn, const char *event); static inline uint64_t qmalloc(QOSState *q, size_t bytes) diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c index 67f3190..8276130 100644 --- a/tests/libqos/malloc.c +++ b/tests/libqos/malloc.c @@ -30,8 +30,8 @@ struct QGuestAllocator { uint64_t end; uint32_t page_size; -MemList used; -MemList free; +MemList *used; +MemList *free; }; #define DEFAULT_PAGE_SIZE 4096 @@ -150,7 +150,7 @@ static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode, addr = freenode-addr; if (freenode-size == size) { /* re-use this freenode as our used node */ -QTAILQ_REMOVE(s-free, freenode, MLIST_ENTNAME); +QTAILQ_REMOVE(s-free, freenode, MLIST_ENTNAME); usednode = freenode; } else { /* adjust the free node and create a new used node */ @@ -159,7 +159,7 @@ static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode, usednode = mlist_new(addr, size); } -mlist_sort_insert(s-used, usednode); +mlist_sort_insert(s-used, usednode); return addr; } @@ -171,7 +171,7 @@ static void
Re: [Qemu-devel] [PULL 00/14] Ide patches
On 05/12/2015 11:25 AM, Peter Maydell wrote: On 12 May 2015 at 16:22, John Snow js...@redhat.com wrote: On 05/12/2015 06:44 AM, Peter Maydell wrote: Doesn't build on 32-bit: /root/qemu/qtest.c: In function ‘qtest_process_command’: /root/qemu/qtest.c:519:28: error: format ‘%zu’ expects argument of type ‘size_t’, but argument 2 has type ‘uint64_t’ [-Werror=format] More motivation for me to try another stab at fixing up the -m32 support, I guess. You could also build for Windows 32 bit, or on OSX (where it's only a warning but does detect this even on 64-bit hosts). -- PMM Limping along using -m32 and some hacks for now. v2 sent, sorry for the noise. --js
[Qemu-devel] [PATCH v6 2/7] vmport_rpc: Add the object vmport_rpc
This is the 1st part of Add limited support of VMware's hyper-call rpc. This patch uses existing infrastructure used by vmmouse.c (provided by vmport.c) to handle the VMware backdoor command 30. One of the better on-line references is: https://sites.google.com/site/chitchatvmback/backdoor More in next patch. Signed-off-by: Don Slutz dsl...@verizon.com --- hw/i386/pc.c | 6 +++ hw/misc/Makefile.objs | 1 + hw/misc/vmport_rpc.c | 126 ++ 3 files changed, 133 insertions(+) create mode 100644 hw/misc/vmport_rpc.c diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 769eb25..4d2e004 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1471,8 +1471,14 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, i8042 = isa_create_simple(isa_bus, i8042); i8042_setup_a20_line(i8042, a20_line[0]); if (!no_vmport) { +ISADevice *vmport_rpc; + vmport_init(isa_bus); vmmouse = isa_try_create(isa_bus, vmmouse); +vmport_rpc = isa_try_create(isa_bus, vmport_rpc); +if (vmport_rpc) { +qdev_init_nofail(DEVICE(vmport_rpc)); +} } else { vmmouse = NULL; } diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs index 4aa76ff..e04c8ac 100644 --- a/hw/misc/Makefile.objs +++ b/hw/misc/Makefile.objs @@ -7,6 +7,7 @@ common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o common-obj-$(CONFIG_PCI_TESTDEV) += pci-testdev.o obj-$(CONFIG_VMPORT) += vmport.o +obj-$(CONFIG_VMPORT) += vmport_rpc.o # ARM devices common-obj-$(CONFIG_PL310) += arm_l2x0.o diff --git a/hw/misc/vmport_rpc.c b/hw/misc/vmport_rpc.c new file mode 100644 index 000..b7cd355 --- /dev/null +++ b/hw/misc/vmport_rpc.c @@ -0,0 +1,126 @@ +/* + * QEMU VMPORT RPC emulation + * + * Copyright (C) 2015 Verizon Corporation + * + * This file is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License Version 2 (GPLv2) + * as published by the Free Software Foundation. + * + * This file is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. http://www.gnu.org/licenses/. + */ + +/* + * One of the better on-line references is: + * + * https://sites.google.com/site/chitchatvmback/backdoor + * + * Which points you to: + * + * http://open-vm-tools.sourceforge.net/ + * + * as a place to get more accurate information by studying. + */ + +#include hw/hw.h +#include hw/i386/pc.h +#include hw/qdev.h +#include trace.h +#include qmp-commands.h +#include qapi/qmp/qerror.h + +/* #define VMPORT_RPC_DEBUG */ + +#define TYPE_VMPORT_RPC vmport_rpc +#define VMPORT_RPC(obj) OBJECT_CHECK(VMPortRpcState, (obj), TYPE_VMPORT_RPC) + +/* VMPORT RPC Command */ +#define VMPORT_RPC_COMMAND 30 + +/* The vmport_rpc object. */ +typedef struct VMPortRpcState { +ISADevice parent_obj; + +/* Properties */ +uint64_t reset_time; +uint64_t build_number_value; +uint64_t build_number_time; + +/* Private data */ +} VMPortRpcState; + +typedef struct { +uint32_t eax; +uint32_t ebx; +uint32_t ecx; +uint32_t edx; +uint32_t esi; +uint32_t edi; +} vregs; + +static uint32_t vmport_rpc_ioport_read(void *opaque, uint32_t addr) +{ +VMPortRpcState *s = opaque; +union { +uint32_t data[6]; +vregs regs; +} ur; + +vmmouse_get_data(ur.data); + +s-build_number_time++; + +vmmouse_set_data(ur.data); +return ur.data[0]; +} + +static void vmport_rpc_reset(DeviceState *d) +{ +VMPortRpcState *s = VMPORT_RPC(d); + +s-reset_time = 14; +s-build_number_value = 0; +s-build_number_time = 0; +} + +static void vmport_rpc_realize(DeviceState *dev, Error **errp) +{ +VMPortRpcState *s = VMPORT_RPC(dev); + +vmport_register(VMPORT_RPC_COMMAND, vmport_rpc_ioport_read, s); +} + +static Property vmport_rpc_properties[] = { +DEFINE_PROP_UINT64(reset-time, VMPortRpcState, reset_time, 14), +DEFINE_PROP_UINT64(build-number-value, VMPortRpcState, + build_number_value, 0), +DEFINE_PROP_UINT64(build-number-time, VMPortRpcState, + build_number_time, 0), +DEFINE_PROP_END_OF_LIST(), +}; + +static void vmport_rpc_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); + +dc-realize = vmport_rpc_realize; +dc-reset = vmport_rpc_reset; +dc-props = vmport_rpc_properties; +} + +static const TypeInfo vmport_rpc_info = { +.name = TYPE_VMPORT_RPC, +.parent= TYPE_ISA_DEVICE, +.instance_size = sizeof(VMPortRpcState), +.class_init= vmport_rpc_class_init, +}; + +static void vmport_rpc_register_types(void) +{ +type_register_static(vmport_rpc_info); +} + +type_init(vmport_rpc_register_types) -- 1.8.4
[Qemu-devel] [PATCH v6 0/7] Add limited support of VMware's hyper-call rpc
Changes v5 to v6: Rebase to master Eric Blake Returning a non-dictionary is not extensible. Added new type VmportGuestInfoValue. s/VmportGuestInfo/VmportGuestInfoKey/ s/type/struct/ Issues with examples Fixed. Changes v4 to v5: Paolo Bonzini What is VMPORT_SHORT about? Dropped this. Why not use a bool in CPUX86State? Took his sugestion and moved to a bool in X86CPU. Changes v3 to v4: Paolo Bonzini on vmort_rpc: Add QMP access to vmport_rpc Does this compile on non-x86 targets? Nope. Fixed. Changes v2 to v3: s/2.3/2.4 Changes v1 to v2: Added live migration code. Adjust data structures for migration. Switch to GHashTable. Eric Blake s/spawened/spawned/ Done s/traceing/tracing/ Done Change error_set(errp, ERROR_CLASS_GENERIC_ERROR, to error_setg(errp, Done Why two commands (inject-vmport-reboot, inject-vmport-halt)? Switched to inject-vmport-action. format=base64 bug statements. Dropped. Much more on format=base64: If there is a bug it is in GLIB. However the Glib reference manual refers to RFC 1421 and RFC 2045 and MIME encoding. Based on all that (which seems to match: http://en.wikipedia.org/wiki/Base64 ) MIME states that all characters outside the (base64) alphabet are to be ignored. Testing shows that g_base64_decode() does this. The confusion is that most non-MIME uses reject a base64 string that contain characters outside the alphabet. I was just following the other uses of base64 in this file. DataFormat refers to RFC 3548, which has the info: Implementations MUST reject the encoding if it contains characters outside the base alphabet when interpreting base encoded data, unless the specification referring to this document explicitly states otherwise. Such specifications may, as MIME does, instead state that characters outside the base encoding alphabet should simply be ignored when interpreting data (be liberal in what you accept). So with GLIB going the MIME way, I do not think this is a QEMU bug (you could consider this a GLIB bug, but the document I found says that GLIB goes the MIME way and so does not reject anything). --- The support included is enough to allow VMware tools to install in a guest and provide guestinfo support. guestinfo support is provided by what is known as VMware RPC support. One of the better on-line references is: https://sites.google.com/site/chitchatvmback/backdoor As a place to get more accurate information by studying: http://open-vm-tools.sourceforge.net/ With vmware tools installed, you can do: --- Last login: Fri Jan 30 16:03:08 2015 [root@C63-min-tools ~]# vmtoolsd --cmd info-get guestinfo.joejoel No value found [root@C63-min-tools ~]# vmtoolsd --cmd info-set guestinfo.joejoel bar [root@C63-min-tools ~]# vmtoolsd --cmd info-get guestinfo.joejoel bar [root@C63-min-tools ~]# --- to access guest info. QMP access is also provided. The live migration code is still in progress. Don Slutz (7): vmport.c: Fix vmport_cmd_ram_size vmport_rpc: Add the object vmport_rpc vmport_rpc: Add limited support of VMware's hyper-call rpc vmport_rpc: Add QMP access to vmport_rpc object. vmport_rpc: Add migration vmport: Add VMware all ring hack MAINTAINERS: add VMware port MAINTAINERS |7 + hw/i386/pc.c | 32 +- hw/i386/pc_piix.c|2 +- hw/i386/pc_q35.c |2 +- hw/misc/Makefile.objs|1 + hw/misc/vmport.c |2 +- hw/misc/vmport_rpc.c | 1441 ++ include/hw/i386/pc.h |6 +- monitor.c| 24 + qapi-schema.json | 101 qmp-commands.hx | 120 target-i386/cpu-qom.h|3 + target-i386/seg_helper.c |9 + trace-events | 24 + 14 files changed, 1769 insertions(+), 5 deletions(-) create mode 100644 hw/misc/vmport_rpc.c -- 1.8.4
[Qemu-devel] [PATCH v6 1/7] vmport.c: Fix vmport_cmd_ram_size
Based on https://sites.google.com/site/chitchatvmback/backdoor and testing on ESXi, this should be in MB not bytes. Signed-off-by: Don Slutz dsl...@verizon.com --- hw/misc/vmport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/misc/vmport.c b/hw/misc/vmport.c index 7fcc00d..6b350ce 100644 --- a/hw/misc/vmport.c +++ b/hw/misc/vmport.c @@ -110,7 +110,7 @@ static uint32_t vmport_cmd_ram_size(void *opaque, uint32_t addr) X86CPU *cpu = X86_CPU(current_cpu); cpu-env.regs[R_EBX] = 0x1177; -return ram_size; +return ram_size 20; /* in MB */ } static uint32_t vmport_cmd_xtest(void *opaque, uint32_t addr) -- 1.8.4
Re: [Qemu-devel] [Qemu-block] [PATCH 05/34] block: Use macro for cache option names
On Fri, May 08, 2015 at 07:21:37PM +0200, Kevin Wolf wrote: Signed-off-by: Kevin Wolf kw...@redhat.com --- blockdev.c| 24 include/block/block.h | 8 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/blockdev.c b/blockdev.c index 5eaf77e..77cbe72 100644 --- a/blockdev.c +++ b/blockdev.c @@ -391,13 +391,13 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, } } -if (qemu_opt_get_bool(opts, cache.writeback, true)) { +if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, true)) { bdrv_flags |= BDRV_O_CACHE_WB; } -if (qemu_opt_get_bool(opts, cache.direct, false)) { +if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_DIRECT, false)) { bdrv_flags |= BDRV_O_NOCACHE; } -if (qemu_opt_get_bool(opts, cache.no-flush, false)) { +if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_NO_FLUSH, false)) { bdrv_flags |= BDRV_O_NO_FLUSH; } @@ -733,16 +733,16 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) } /* Specific options take precedence */ -if (!qemu_opt_get(all_opts, cache.writeback)) { -qemu_opt_set_bool(all_opts, cache.writeback, +if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_WB)) { +qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_WB, !!(flags BDRV_O_CACHE_WB), error_abort); } -if (!qemu_opt_get(all_opts, cache.direct)) { -qemu_opt_set_bool(all_opts, cache.direct, +if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_DIRECT)) { +qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_DIRECT, !!(flags BDRV_O_NOCACHE), error_abort); } -if (!qemu_opt_get(all_opts, cache.no-flush)) { -qemu_opt_set_bool(all_opts, cache.no-flush, +if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_NO_FLUSH)) { +qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_NO_FLUSH, !!(flags BDRV_O_NO_FLUSH), error_abort); } qemu_opt_unset(all_opts, cache); @@ -3106,15 +3106,15 @@ QemuOptsList qemu_common_drive_opts = { .type = QEMU_OPT_STRING, .help = discard operation (ignore/off, unmap/on), },{ -.name = cache.writeback, +.name = BDRV_OPT_CACHE_WB, .type = QEMU_OPT_BOOL, .help = enables writeback mode for any caches, },{ -.name = cache.direct, +.name = BDRV_OPT_CACHE_DIRECT, .type = QEMU_OPT_BOOL, .help = enables use of O_DIRECT (bypass the host page cache), },{ -.name = cache.no-flush, +.name = BDRV_OPT_CACHE_NO_FLUSH, .type = QEMU_OPT_BOOL, .help = ignore any flush requests for the device, },{ diff --git a/include/block/block.h b/include/block/block.h index 7d1a717..86bdac8 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -90,6 +90,14 @@ typedef struct HDGeometry { #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH) + +/* Option names of options parsed by the block layer */ + +#define BDRV_OPT_CACHE_WB cache.writeback +#define BDRV_OPT_CACHE_DIRECT cache.direct +#define BDRV_OPT_CACHE_NO_FLUSH cache.no-flush + + #define BDRV_SECTOR_BITS 9 #define BDRV_SECTOR_SIZE (1ULL BDRV_SECTOR_BITS) #define BDRV_SECTOR_MASK ~(BDRV_SECTOR_SIZE - 1) -- 1.8.3.1 Reviewed-by: Jeff Cody jc...@redhat.com
[Qemu-devel] [PATCH] qapi: add dirty bitmap status
Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches. Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands (e.g. busy in another operation, busy being migrated, etc.) Suggested-by: Eric Blake ebl...@redhat.com Signed-off-by: John Snow js...@redhat.com --- block.c | 13 - include/block/block.h | 1 + qapi/block-core.json | 23 +-- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/block.c b/block.c index 7904098..a8b9f25 100644 --- a/block.c +++ b/block.c @@ -3104,6 +3104,17 @@ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap) return !(bitmap-disabled || bitmap-successor); } +DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap) +{ +if (bdrv_dirty_bitmap_frozen(bitmap)) { +return DIRTY_BITMAP_STATUS_FROZEN; +} else if (!bdrv_dirty_bitmap_enabled(bitmap)) { +return DIRTY_BITMAP_STATUS_DISABLED; +} else { +return DIRTY_BITMAP_STATUS_ACTIVE; +} +} + /** * Create a successor bitmap destined to replace this bitmap after an operation. * Requires that the bitmap is not frozen and has no successor. @@ -3244,7 +3255,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) info-granularity = bdrv_dirty_bitmap_granularity(bm); info-has_name = !!bm-name; info-name = g_strdup(bm-name); -info-frozen = bdrv_dirty_bitmap_frozen(bm); +info-status = bdrv_dirty_bitmap_status(bm); entry-value = info; *plist = entry; plist = entry-next; diff --git a/include/block/block.h b/include/block/block.h index 7d1a717..4d3ad88 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -474,6 +474,7 @@ uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs); uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap); bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap); bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap); +DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap); int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector); void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap, int64_t cur_sector, int nr_sectors); diff --git a/qapi/block-core.json b/qapi/block-core.json index 863ffea..8411d4f 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -326,6 +326,25 @@ 'data': 'bool', '*offset': 'int' } } ## +# @DirtyBitmapStatus: +# +# An enumeration of possible states that a dirty bitmap can report to the user. +# +# @frozen: The bitmap is currently in-use by a backup operation or block job, +# and is immutable. +# +# @disabled: The bitmap is currently in-use by an internal operation and is +#read-only. It can still be deleted. +# +# @active: The bitmap is actively monitoring for new writes, and can be cleared, +# deleted, or used for backup operations. +# +# Since: 2.4 +## +{ 'enum': 'DirtyBitmapStatus', + 'data': ['active', 'disabled', 'frozen'] } + +## # @BlockDirtyInfo: # # Block dirty bitmap information. @@ -336,13 +355,13 @@ # # @granularity: granularity of the dirty bitmap in bytes (since 1.4) # -# @frozen: whether the dirty bitmap is frozen (Since 2.4) +# @status: current status of the dirty bitmap (since 2.4) # # Since: 1.3 ## { 'struct': 'BlockDirtyInfo', 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32', - 'frozen': 'bool'} } + 'status': 'DirtyBitmapStatus'} } ## # @BlockInfo: -- 2.1.0
Re: [Qemu-devel] [Qemu-block] [PATCH 04/34] vmdk: Use bdrv_open_image()
On Fri, May 08, 2015 at 07:21:36PM +0200, Kevin Wolf wrote: Besides standardising on a single interface for opening child nodes, this patch allows the user to specify options to individual extent nodes. Overriding file names isn't possible with this yet, so it's of limited usefulness, but still a step forward. Signed-off-by: Kevin Wolf kw...@redhat.com --- block/vmdk.c | 34 +- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index b66745d..641e026 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -543,7 +543,7 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, } static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, - Error **errp); + QDict *options, Error **errp); static char *vmdk_read_desc(BlockDriverState *file, uint64_t desc_offset, Error **errp) @@ -582,7 +582,7 @@ static char *vmdk_read_desc(BlockDriverState *file, uint64_t desc_offset, static int vmdk_open_vmdk4(BlockDriverState *bs, BlockDriverState *file, - int flags, Error **errp) + int flags, QDict *options, Error **errp) { int ret; uint32_t magic; @@ -606,7 +606,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, if (!buf) { return -EINVAL; } -ret = vmdk_open_desc_file(bs, flags, buf, errp); +ret = vmdk_open_desc_file(bs, flags, buf, options, errp); g_free(buf); return ret; } @@ -763,7 +763,7 @@ static int vmdk_parse_description(const char *desc, const char *opt_name, /* Open an extent file and append to bs array */ static int vmdk_open_sparse(BlockDriverState *bs, BlockDriverState *file, int flags, -char *buf, Error **errp) +char *buf, QDict *options, Error **errp) { uint32_t magic; @@ -773,7 +773,7 @@ static int vmdk_open_sparse(BlockDriverState *bs, return vmdk_open_vmfs_sparse(bs, file, flags, errp); break; case VMDK4_MAGIC: -return vmdk_open_vmdk4(bs, file, flags, errp); +return vmdk_open_vmdk4(bs, file, flags, options, errp); break; default: error_setg(errp, Image not in VMDK format); @@ -783,7 +783,8 @@ static int vmdk_open_sparse(BlockDriverState *bs, } static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, - const char *desc_file_path, Error **errp) + const char *desc_file_path, QDict *options, + Error **errp) { int ret; int matches; @@ -797,6 +798,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, BlockDriverState *extent_file; BDRVVmdkState *s = bs-opaque; VmdkExtent *extent; +char extent_opt_prefix[32]; while (*p) { /* parse extent line in one of below formats: @@ -846,8 +848,13 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, extent_path = g_malloc0(PATH_MAX); path_combine(extent_path, PATH_MAX, desc_file_path, fname); extent_file = NULL; -ret = bdrv_open(extent_file, extent_path, NULL, NULL, -bs-open_flags | BDRV_O_PROTOCOL, NULL, errp); + +ret = snprintf(extent_opt_prefix, 32, extents.%d, s-num_extents); +assert(ret 32); + +ret = bdrv_open_image(extent_file, extent_path, + options, extent_opt_prefix, + bs-open_flags | BDRV_O_PROTOCOL, false, errp); g_free(extent_path); if (ret) { return ret; @@ -870,7 +877,8 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, if (!buf) { ret = -EINVAL; } else { -ret = vmdk_open_sparse(bs, extent_file, bs-open_flags, buf, errp); +ret = vmdk_open_sparse(bs, extent_file, bs-open_flags, buf, + options, errp); } g_free(buf); if (ret) { @@ -898,7 +906,7 @@ next_line: } static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, - Error **errp) + QDict *options, Error **errp) { int ret; char ct[128]; @@ -920,7 +928,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, } s-create_type = g_strdup(ct); s-desc_offset = 0; -ret = vmdk_parse_extents(buf, bs, bs-file-exact_filename, errp); +ret = vmdk_parse_extents(buf, bs,
Re: [Qemu-devel] [PATCH v6 4/7] vmport_rpc: Add QMP access to vmport_rpc object.
On 05/12/2015 01:03 PM, Don Slutz wrote: This adds one new inject command: inject-vmport-action And three guest info commands: vmport-guestinfo-set vmport-guestinfo-get query-vmport-guestinfo More details in qmp-commands.hx Signed-off-by: Don Slutz dsl...@verizon.com --- +/* + * Run func() for every VMPortRpc device, traverse the tree for + * everything else. Note: This routine expects that opaque is a + * VMPortRpcFind pointer and not NULL. + */ +static int find_VMPortRpc_device(Object *obj, void *opaque) +{ +VMPortRpcFind *find = opaque; +Object *dev; +VMPortRpcState *s; + +if (find-found) { Why not assert(find) instead of leaving it to the comment? +/* + * Loop through all dynamically created VMPortRpc devices and call + * func() for each instance. + */ +static int foreach_dynamic_vmport_rpc_device(FindVMPortRpcDeviceFunc *func, + void *arg) +{ +VMPortRpcFind find = { +.func = func, +.arg = arg, Is it worth marking arg const here and in the VMPortRpcFind struct... +void qmp_inject_vmport_action(enum VmportAction action, Error **errp) +{ +int rc; + +switch (action) { +case VMPORT_ACTION_REBOOT: +rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send, + (void *)OS_Reboot); ...so that you don't have to cast away const here? +break; +case VMPORT_ACTION_HALT: +rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send, + (void *)OS_Halt); +break; +case VMPORT_ACTION_MAX: +assert(action != VMPORT_ACTION_MAX); +rc = 0; /* Should be impossible to get here. */ I'd rather abort() if someone compiled with -NDEBUG. +break; +} +convert_local_rc(errp, rc); +} + +typedef struct keyValue { +void *key_data; +void *value_data; +unsigned int key_len; +unsigned int value_len; Should these be size_t? +void qmp_vmport_guestinfo_set(const char *key, const char *value, + bool has_format, enum DataFormat format, + Error **errp) +{ +int rc; +keyValue key_value; + +if (strncmp(key, guestinfo., strlen(guestinfo.)) == 0) { +key_value.key_data = (void *)(key + strlen(guestinfo.)); +key_value.key_len = strlen(key) - strlen(guestinfo.); +} else { +key_value.key_data = (void *)key; Casting to (void*) looks awkward; should key_data should be typed 'const void *' to avoid the need for a cast? For that matter, why is it void*, why not 'const char *'? +++ b/qmp-commands.hx @@ -490,6 +490,126 @@ Note: inject-nmi fails when the guest doesn't support injecting. EQMP +SQMP +vmport-guestinfo-set +-- Still mismatches on line length (several sites). +- { execute: vmport-guestinfo-get, +arguments: { key: guestinfo.foo, + format: utf8 } } +- {return: {value: abcdefgh}} + + +EQMP + +{ +.name = query-vmport-guestinfo, +.args_type = , +.mhandler.cmd_new = qmp_marshal_input_query_vmport_guestinfo, +}, + +SQMP +query-vmport-guestinfo +-- + +Returns information about VMWare Tools guestinfo. The returned value is a json-array +of all keys. + +Example: + +- { execute: query-vmport-guestinfo } +- { + return: [ + { +key: guestinfo.ip + }, So if I'm following this correctly, a user has to call query-vmport-guestinfo to learn the key names, and then once per key call vmport-guetsinfo-get to learn the key values. Why not just return key names and values all at once, to save the multiple call overhead? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PULL v2 00/14] Ide patches
On 12 May 2015 at 19:46, John Snow js...@redhat.com wrote: The following changes since commit 968bb75c348a401b85e08d5eb1887a3e6c3185f5: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20150512' into staging (2015-05-12 12:11:32 +0100) are available in the git repository at: https://github.com/jnsnow/qemu.git tags/ide-pull-request for you to fetch changes up to 6a6b2f8ca4a2bd274328e0f1e2a1ee3fa01e1b42: qtest: pre-buffer hex nibs (2015-05-12 13:47:12 -0400) v2: 32bit build issues: 05: Fix hba_base type issue for 32/64bit* *This needs to be addressed more comprehensively in a later patchset that will likely touch all of libqos, to fix the guest pointer type issues (should be uint64_t instead of void * in most places.) 11: Fix printf format modifier. Fails to build with our current minimum glib requirement: qtest.o: In function `qtest_process_command': qemu-for-merges/qtest.c:515: undefined reference to `g_base64_decode_inplace' (Compare commit 0599e56ed.) That function is in glib 2.20. So you could if you want resolve this by getting the bump minimum to glib 2.22 change into master and then trying again :-) -- PMM
Re: [Qemu-devel] [PATCH] qapi: add dirty bitmap status
On 05/12/2015 01:53 PM, John Snow wrote: Bitmaps can be in a handful of different states with potentially more to come as we tool around with migration and persistence patches. Instead of having a bunch of boolean fields, it was suggested that we just have an enum status field that will help expose the reason to management APIs why certain bitmaps may be unavailable for various commands (e.g. busy in another operation, busy being migrated, etc.) Might be worth mentioning that this is an API change, but safe because the old API is unreleased (and therefore, this patch MUST go in the 2.4 time frame, if at all). Suggested-by: Eric Blake ebl...@redhat.com Signed-off-by: John Snow js...@redhat.com --- block.c | 13 - include/block/block.h | 1 + qapi/block-core.json | 23 +-- 3 files changed, 34 insertions(+), 3 deletions(-) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PULL v2 14/14] qtest: pre-buffer hex nibs
Instead of converting each byte one-at-a-time and then sending each byte over the wire, use sprintf() to pre-compute all of the hex nibs into a single buffer, then send the entire buffer all at once. This gives a moderate speed boost to memread() and memwrite() functions. Signed-off-by: John Snow js...@redhat.com Reviewed-by: Markus Armbruster arm...@redhat.com Message-id: 1431021095-7558-2-git-send-email-js...@redhat.com --- qtest.c | 11 +++ tests/libqtest.c | 8 +--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/qtest.c b/qtest.c index 04412dd..05cefd2 100644 --- a/qtest.c +++ b/qtest.c @@ -414,6 +414,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) } else if (strcmp(words[0], read) == 0) { uint64_t addr, len, i; uint8_t *data; +char *enc; g_assert(words[1] words[2]); addr = strtoull(words[1], NULL, 0); @@ -422,14 +423,16 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) data = g_malloc(len); cpu_physical_memory_read(addr, data, len); -qtest_send_prefix(chr); -qtest_send(chr, OK 0x); +enc = g_malloc(2 * len + 1); for (i = 0; i len; i++) { -qtest_sendf(chr, %02x, data[i]); +sprintf(enc[i * 2], %02x, data[i]); } -qtest_send(chr, \n); + +qtest_send_prefix(chr); +qtest_sendf(chr, OK 0x%s\n, enc); g_free(data); +g_free(enc); } else if (strcmp(words[0], b64read) == 0) { uint64_t addr, len; uint8_t *data; diff --git a/tests/libqtest.c b/tests/libqtest.c index 055aad6..e5188e0 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -730,13 +730,15 @@ void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size) { const uint8_t *ptr = data; size_t i; +char *enc = g_malloc(2 * size + 1); -qtest_sendf(s, write 0x% PRIx64 0x%zx 0x, addr, size); for (i = 0; i size; i++) { -qtest_sendf(s, %02x, ptr[i]); +sprintf(enc[i * 2], %02x, ptr[i]); } -qtest_sendf(s, \n); + +qtest_sendf(s, write 0x% PRIx64 0x%zx 0x%s\n, addr, size, enc); qtest_rsp(s, 0); +g_free(enc); } void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size) -- 2.1.0
[Qemu-devel] [PATCH v6 3/7] vmport_rpc: Add limited support of VMware's hyper-call rpc
The support included is enough to allow VMware tools to install in a guest and provide guestinfo support. guestinfo support is provided by what is known as VMware RPC support. If the guest is running VMware tools, then the build version of the tools is also available via the property build-number-value of the vmport_rpc object. The build-number-time property is changed every time the VMware tools running in the guest sends the build version via the rpc. The property reset-time controls how often to request them in seconds minus 1. The minus 1 is to handle to 0 case. I.E. the fastest that can be selected is every second. The default is 4 times a minute. The VMware RPC support includes the notion of channels that are opened, active and closed. All RPC messages sent via a channel starts with normal ASCII text. The message some times does include binary data. Currently there are 2 protocols defined for VMware RPC. They determine the direction for data flow, guest to tool stack or tool stack to guest. There is no provided interrupt for VMware RPC. Getting VMPORT_RPC_DEBUG will provide a higher level trace calls that are simpler to understand. Signed-off-by: Don Slutz dsl...@verizon.com --- hw/misc/vmport_rpc.c | 799 ++- trace-events | 22 ++ 2 files changed, 820 insertions(+), 1 deletion(-) diff --git a/hw/misc/vmport_rpc.c b/hw/misc/vmport_rpc.c index b7cd355..927c0bf 100644 --- a/hw/misc/vmport_rpc.c +++ b/hw/misc/vmport_rpc.c @@ -40,6 +40,123 @@ /* VMPORT RPC Command */ #define VMPORT_RPC_COMMAND 30 +/* Limits on amount of non guest memory to use */ +#define MAX_KEY_LEN 128 +#define MIN_VAL_LEN 64 +#define MAX_VAL_LEN 8192 +#define MAX_NUM_KEY 256 +#define MAX_BKTS 4 +/* Max number of channels. */ +#define GUESTMSG_MAX_CHANNEL 8 + +/* + * All of VMware's rpc is based on 32bit registers. So this is the + * number of bytes in ebx. + */ +#define CHAR_PER_CALL sizeof(uint32_t) +/* Room for basic commands */ +#define EXTRA_SEND 22 +/* Status code and NULL */ +#define EXTRA_RECV 2 +#define MAX_SEND_BUF DIV_ROUND_UP(EXTRA_SEND + MAX_KEY_LEN + MAX_VAL_LEN, \ + CHAR_PER_CALL) +#define MAX_RECV_BUF DIV_ROUND_UP(EXTRA_RECV + MAX_VAL_LEN, CHAR_PER_CALL) +#define MIN_SEND_BUF DIV_ROUND_UP(EXTRA_SEND + MAX_KEY_LEN + MIN_VAL_LEN, \ + CHAR_PER_CALL) + +/* Reply statuses */ +/* The basic request succeeded */ +#define MESSAGE_STATUS_SUCCESS 0x0001 +/* vmware has a message available for its party */ +#define MESSAGE_STATUS_DORECV 0x0002 +/* The channel has been closed */ +#define MESSAGE_STATUS_CLOSED 0x0004 +/* vmware removed the message before the party fetched it */ +#define MESSAGE_STATUS_UNSENT 0x0008 +/* A checkpoint occurred */ +#define MESSAGE_STATUS_CPT 0x0010 +/* An underlying device is powering off */ +#define MESSAGE_STATUS_POWEROFF 0x0020 +/* vmware has detected a timeout on the channel */ +#define MESSAGE_STATUS_TIMEOUT 0x0040 +/* vmware supports high-bandwidth for sending and receiving the payload */ +#define MESSAGE_STATUS_HB 0x0080 + +/* Max number of channels. */ +#define GUESTMSG_MAX_CHANNEL 8 + +/* Flags to open a channel. */ +#define GUESTMSG_FLAG_COOKIE 0x8000 + +/* Data to guest */ +#define VMWARE_PROTO_TO_GUEST 0x4f4c4354 +/* Data from guest */ +#define VMWARE_PROTO_FROM_GUEST 0x49435052 + +/* + * Error return values used only in this file. The routine + * convert_local_rc() is used to convert these to an Error + * object. + */ +#define VMPORT_DEVICE_NOT_FOUND -1 +#define SEND_NOT_OPEN -2 +#define SEND_SKIPPED-3 +#define SEND_TRUCATED -4 +#define SEND_NO_MEMORY -5 +#define GUESTINFO_NOTFOUND -6 +#define GUESTINFO_VALTOOLONG-7 +#define GUESTINFO_KEYTOOLONG-8 +#define GUESTINFO_TOOMANYKEYS -9 +#define GUESTINFO_NO_MEMORY -10 + + +/* The VMware RPC guest info storage . */ +typedef struct { +char *val_data; +uint16_t val_len; +uint16_t val_max; +} guestinfo_t; + +/* The VMware RPC bucket control. */ +typedef struct { +uint16_t recv_len; +uint16_t recv_idx; +uint16_t recv_buf_max; +} bucket_control_t; + +/* The VMware RPC bucket info. */ +typedef struct { +union { +uint32_t *words; +char *bytes; +} recv; +bucket_control_t ctl; +} bucket_t; + + +/* The VMware RPC channel control. */ +typedef struct { +uint64_t active_time; +uint32_t chan_id; +uint32_t cookie; +uint32_t proto_num; +uint16_t send_len; +uint16_t send_idx; +uint16_t send_buf_max; +uint8_t recv_read; +uint8_t recv_write; +} channel_control_t; + +/* The VMware RPC channel info. */ +typedef struct { +union { +uint32_t *words; +char *bytes; +} send; +channel_control_t ctl; +bucket_t recv_bkt[MAX_BKTS]; +} channel_t; + /* The vmport_rpc
[Qemu-devel] [PATCH v6 4/7] vmport_rpc: Add QMP access to vmport_rpc object.
This adds one new inject command: inject-vmport-action And three guest info commands: vmport-guestinfo-set vmport-guestinfo-get query-vmport-guestinfo More details in qmp-commands.hx Signed-off-by: Don Slutz dsl...@verizon.com --- hw/misc/vmport_rpc.c | 268 +++ monitor.c| 24 + qapi-schema.json | 101 +++ qmp-commands.hx | 120 +++ 4 files changed, 513 insertions(+) diff --git a/hw/misc/vmport_rpc.c b/hw/misc/vmport_rpc.c index 927c0bf..9a32c6f 100644 --- a/hw/misc/vmport_rpc.c +++ b/hw/misc/vmport_rpc.c @@ -215,6 +215,56 @@ typedef struct { uint32_t edi; } vregs; +/* + * Run func() for every VMPortRpc device, traverse the tree for + * everything else. Note: This routine expects that opaque is a + * VMPortRpcFind pointer and not NULL. + */ +static int find_VMPortRpc_device(Object *obj, void *opaque) +{ +VMPortRpcFind *find = opaque; +Object *dev; +VMPortRpcState *s; + +if (find-found) { +return 0; +} +dev = object_dynamic_cast(obj, TYPE_VMPORT_RPC); +s = (VMPortRpcState *)dev; + +if (!s) { +/* Container, traverse it for children */ +return object_child_foreach(obj, find_VMPortRpc_device, opaque); +} + +find-found++; +find-rc = find-func(s, find-arg); + +return 0; +} + +/* + * Loop through all dynamically created VMPortRpc devices and call + * func() for each instance. + */ +static int foreach_dynamic_vmport_rpc_device(FindVMPortRpcDeviceFunc *func, + void *arg) +{ +VMPortRpcFind find = { +.func = func, +.arg = arg, +}; + +/* Loop through all VMPortRpc devices that were spawned outside + * the machine */ +find_VMPortRpc_device(qdev_get_machine(), find); +if (find.found) { +return find.rc; +} else { +return VMPORT_DEVICE_NOT_FOUND; +} +} + #ifdef VMPORT_RPC_DEBUG /* * Add helper function for tracing. This routine will convert @@ -464,6 +514,23 @@ static int get_guestinfo(VMPortRpcState *s, return GUESTINFO_NOTFOUND; } +static int get_qmp_guestinfo(VMPortRpcState *s, + unsigned int a_key_len, char *a_info_key, + unsigned int *a_value_len, void **a_value_data) +{ +gpointer key = g_strndup(a_info_key, a_key_len); +guestinfo_t *gi = (guestinfo_t *)g_hash_table_lookup(s-guestinfo, key); + +g_free(key); +if (gi) { +*a_value_len = gi-val_len; +*a_value_data = gi-val_data; +return 0; +} + +return GUESTINFO_NOTFOUND; +} + static int set_guestinfo(VMPortRpcState *s, int a_key_len, unsigned int a_val_len, char *a_info_key, char *val) { @@ -851,6 +918,207 @@ static uint32_t vmport_rpc_ioport_read(void *opaque, uint32_t addr) return ur.data[0]; } +static int vmport_rpc_find_send(VMPortRpcState *s, void *arg) +{ +return vmport_rpc_ctrl_send(s, arg); +} + +static void convert_local_rc(Error **errp, int rc) +{ +switch (rc) { +case 0: +break; +case VMPORT_DEVICE_NOT_FOUND: +error_set(errp, QERR_DEVICE_NOT_FOUND, TYPE_VMPORT_RPC); +break; +case SEND_NOT_OPEN: +error_setg(errp, VMWare rpc not open); +break; +case SEND_SKIPPED: +error_setg(errp, VMWare rpc send skipped); +break; +case SEND_TRUCATED: +error_setg(errp, VMWare rpc send trucated); +break; +case SEND_NO_MEMORY: +error_setg(errp, VMWare rpc send out of memory); +break; +case GUESTINFO_NOTFOUND: +error_setg(errp, VMWare guestinfo not found); +break; +case GUESTINFO_VALTOOLONG: +error_setg(errp, VMWare guestinfo value too long); +break; +case GUESTINFO_KEYTOOLONG: +error_setg(errp, VMWare guestinfo key too long); +break; +case GUESTINFO_TOOMANYKEYS: +error_setg(errp, VMWare guestinfo too many keys); +break; +case GUESTINFO_NO_MEMORY: +error_setg(errp, VMWare guestinfo out of memory); +break; +default: +error_setg(errp, VMWare rpc send rc=%d unknown, rc); +break; +} +} + +void qmp_inject_vmport_action(enum VmportAction action, Error **errp) +{ +int rc; + +switch (action) { +case VMPORT_ACTION_REBOOT: +rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send, + (void *)OS_Reboot); +break; +case VMPORT_ACTION_HALT: +rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send, + (void *)OS_Halt); +break; +case VMPORT_ACTION_MAX: +assert(action != VMPORT_ACTION_MAX); +rc = 0; /* Should be impossible to get here. */ +break; +} +convert_local_rc(errp, rc); +} + +typedef struct keyValue { +void *key_data;
Re: [Qemu-devel] [Qemu-block] [PATCH 5/5] tests: add test case for encrypted qcow2 read/write
On 05/12/2015 02:35 PM, Eric Blake wrote: On 05/12/2015 10:09 AM, Daniel P. Berrange wrote: Add a simple test case for qemu-iotests that covers read/write with encrypted qcow2 files. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tests/qemu-iotests/131 | 69 ++ tests/qemu-iotests/131.out | 46 +++ tests/qemu-iotests/group | 1 + 3 files changed, 116 insertions(+) create mode 100755 tests/qemu-iotests/131 create mode 100644 tests/qemu-iotests/131.out diff --git a/tests/qemu-iotests/131 b/tests/qemu-iotests/131 new file mode 100755 index 000..f44b0a0 --- /dev/null +++ b/tests/qemu-iotests/131 @@ -0,0 +1,69 @@ +#!/bin/bash +# +# Test encrypted read/write using plain bdrv_read/bdrv_write +# +# Copyright (C) 2009 Red Hat, Inc. Copy-and-paste strikes again; welcome to 2015. With that fixed, Reviewed-by: Eric Blake ebl...@redhat.com Fam Zheng already has a patch on-list that uses test 131, and I think his patch was submitted first. (Unless we want to play the Who gets merged first? game.) --js
[Qemu-devel] [PATCH v6 7/7] MAINTAINERS: add VMware port
Signed-off-by: Don Slutz dsl...@verizon.com --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index b3552b2..6945d30d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -766,6 +766,13 @@ M: Jiri Pirko j...@resnulli.us S: Maintained F: hw/net/rocker/ +VMware port +M: Don Slutz dsl...@verizon.com +S: Maintained +F: hw/misc/vmport.c +F: hw/input/vmmouse.c +F: hw/misc/vmport_rpc.c + Subsystems -- Audio -- 1.8.4
Re: [Qemu-devel] when does a target frontend need to use gen_io_start()/gen_io_end() ?
On 12 May 2015 at 19:17, Paolo Bonzini pbonz...@redhat.com wrote: On 12/05/2015 17:32, Peter Maydell wrote: In order for -icount to work, it's important for the target translate.c code to correctly bracket any generated code which can do I/O with gen_io_start()/gen_io_end() calls. But does anybody know exactly what the criteria are here for this? It would be nice if we could document this in a comment in gen_icount.h -- I'm happy to write one up if somebody will just tell me what the right answer is :-) It's any instruction that can cause an icount read, typically through QEMU_CLOCK_VIRTUAL or cpu_get_ticks(). Also anything that can cause a CPU interrupt, since tcg_handle_interrupt() will call cpu_abort() if the CPU gets an interrupt while it's not in a 'can do IO' state. Anything else? [How are -icount and multi-threaded TCG going to interact? Do we just say you get one or the other but not both ?] -- PMM
Re: [Qemu-devel] [PATCH 3/5] util: allow \n to terminate password input
On 05/12/2015 10:09 AM, Daniel P. Berrange wrote: The qemu_read_password() method looks for \r to terminate the reading of the a password. This is what will be seen when reading the password from a TTY. When scripting though, it is useful to be able to send the password via a pipe, in which case we must look for \n to terminate password input. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- util/oslib-posix.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PULL v2 08/14] qtest/ahci: add halted dma test
If we're going to test the migration of halted DMA jobs, we should probably check to make sure we can resume them locally as a first step. Signed-off-by: John Snow js...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com Message-id: 1430417242-11859-9-git-send-email-js...@redhat.com --- tests/ahci-test.c | 60 +++ 1 file changed, 60 insertions(+) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index f6de355..4eb1105 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1155,6 +1155,65 @@ static void test_migrate_dma(void) } /** + * DMA Error Test + * + * Simulate an error on first write, Try to write a pattern, + * Confirm the VM has stopped, resume the VM, verify command + * has completed, then read back the data and verify. + */ +static void test_halted_dma(void) +{ +AHCIQState *ahci; +uint8_t port; +size_t bufsize = 4096; +unsigned char *tx = g_malloc(bufsize); +unsigned char *rx = g_malloc0(bufsize); +unsigned i; +uint64_t ptr; +AHCICommand *cmd; + +prepare_blkdebug_script(debug_path, write_aio); + +ahci = ahci_boot_and_enable(-drive file=blkdebug:%s:%s,if=none,id=drive0, +format=qcow2,cache=writeback, +rerror=stop,werror=stop +-M q35 +-device ide-hd,drive=drive0 , +debug_path, +tmp_path); + +/* Initialize and prepare */ +port = ahci_port_select(ahci); +ahci_port_clear(ahci, port); + +for (i = 0; i bufsize; i++) { +tx[i] = (bufsize - i); +} + +/* create DMA source buffer and write pattern */ +ptr = ahci_alloc(ahci, bufsize); +g_assert(ptr); +memwrite(ptr, tx, bufsize); + +/* Attempt to write (and fail) */ +cmd = ahci_guest_io_halt(ahci, port, CMD_WRITE_DMA, + ptr, bufsize, 0); + +/* Attempt to resume the command */ +ahci_guest_io_resume(ahci, cmd); +ahci_free(ahci, ptr); + +/* Read back and verify */ +ahci_io(ahci, port, CMD_READ_DMA, rx, bufsize, 0); +g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0); + +/* Cleanup and go home */ +ahci_shutdown(ahci); +g_free(rx); +g_free(tx); +} + +/** * Migration test: Try to flush, migrate, then resume. */ static void test_flush_migrate(void) @@ -1455,6 +1514,7 @@ int main(int argc, char **argv) qtest_add_func(/ahci/migrate/sanity, test_migrate_sanity); qtest_add_func(/ahci/migrate/dma, test_migrate_dma); +qtest_add_func(/ahci/io/dma/lba28/retry, test_halted_dma); ret = g_test_run(); -- 2.1.0
[Qemu-devel] [PULL v2 06/14] qtest/ahci: add migrate dma test
Write to one guest, migrate, and then read from the other. adjust ahci_io to clear any buffers it creates, so that we can use ahci_io safely on both guests knowing we are using empty buffers and not accidentally re-using data. Signed-off-by: John Snow js...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com Message-id: 1430417242-11859-7-git-send-email-js...@redhat.com --- tests/ahci-test.c | 45 + tests/libqos/ahci.c | 1 + 2 files changed, 46 insertions(+) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 9fccafc..6513330 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1110,6 +1110,50 @@ static void test_migrate_sanity(void) ahci_shutdown(dst); } +/** + * DMA Migration test: Write a pattern, migrate, then read. + */ +static void test_migrate_dma(void) +{ +AHCIQState *src, *dst; +uint8_t px; +size_t bufsize = 4096; +unsigned char *tx = g_malloc(bufsize); +unsigned char *rx = g_malloc0(bufsize); +unsigned i; +const char *uri = tcp:127.0.0.1:1234; + +src = ahci_boot_and_enable(-m 1024 -M q35 + -hda %s , tmp_path); +dst = ahci_boot(-m 1024 -M q35 +-hda %s +-incoming %s, tmp_path, uri); + +set_context(src-parent); + +/* initialize */ +px = ahci_port_select(src); +ahci_port_clear(src, px); + +/* create pattern */ +for (i = 0; i bufsize; i++) { +tx[i] = (bufsize - i); +} + +/* Write, migrate, then read. */ +ahci_io(src, px, CMD_WRITE_DMA, tx, bufsize, 0); +ahci_migrate(src, dst, uri); +ahci_io(dst, px, CMD_READ_DMA, rx, bufsize, 0); + +/* Verify pattern */ +g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0); + +ahci_shutdown(src); +ahci_shutdown(dst); +g_free(rx); +g_free(tx); +} + /**/ /* AHCI I/O Test Matrix Definitions */ @@ -1360,6 +1404,7 @@ int main(int argc, char **argv) qtest_add_func(/ahci/flush/retry, test_flush_retry); qtest_add_func(/ahci/migrate/sanity, test_migrate_sanity); +qtest_add_func(/ahci/migrate/dma, test_migrate_dma); ret = g_test_run(); diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 8c8fd89..95bfb3d 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -650,6 +650,7 @@ void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, g_assert(props); ptr = ahci_alloc(ahci, bufsize); g_assert(ptr); +qmemset(ptr, 0x00, bufsize); if (props-write) { memwrite(ptr, buffer, bufsize); -- 2.1.0
[Qemu-devel] [PULL v2 12/14] qtest: add memset to qtest protocol
Previously, memset was just a frontend to write() and only stupidly sent the pattern many times across the wire. Let's not discuss who stupidly wrote it like that in the first place. (Hint: It was me.) Signed-off-by: John Snow js...@redhat.com Message-id: 1430864578-22072-4-git-send-email-js...@redhat.com --- qtest.c | 20 tests/libqtest.c | 8 +--- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/qtest.c b/qtest.c index 73b7a0f..04412dd 100644 --- a/qtest.c +++ b/qtest.c @@ -125,6 +125,9 @@ static bool qtest_opened; * b64write ADDR SIZE B64_DATA * OK * + * memset ADDR SIZE VALUE + * OK + * * ADDR, SIZE, VALUE are all integers parsed with strtoul() with a base of 0. * * DATA is an arbitrarily long hex number prefixed with '0x'. If it's smaller @@ -473,6 +476,23 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) qtest_send_prefix(chr); qtest_send(chr, OK\n); +} else if (strcmp(words[0], memset) == 0) { +uint64_t addr, len; +uint8_t *data; +uint8_t pattern; + +g_assert(words[1] words[2] words[3]); +addr = strtoull(words[1], NULL, 0); +len = strtoull(words[2], NULL, 0); +pattern = strtoull(words[3], NULL, 0); + +data = g_malloc(len); +memset(data, pattern, len); +cpu_physical_memory_write(addr, data, len); +g_free(data); + +qtest_send_prefix(chr); +qtest_send(chr, OK\n); } else if (strcmp(words[0], b64write) == 0) { uint64_t addr, len; uint8_t *data; diff --git a/tests/libqtest.c b/tests/libqtest.c index 5f57005..055aad6 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -741,13 +741,7 @@ void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size) void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size) { -size_t i; - -qtest_sendf(s, write 0x% PRIx64 0x%zx 0x, addr, size); -for (i = 0; i size; i++) { -qtest_sendf(s, %02x, pattern); -} -qtest_sendf(s, \n); +qtest_sendf(s, memset 0x% PRIx64 0x%zx 0x%02x\n, addr, size, pattern); qtest_rsp(s, 0); } -- 2.1.0
[Qemu-devel] [PULL v2 00/14] Ide patches
The following changes since commit 968bb75c348a401b85e08d5eb1887a3e6c3185f5: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20150512' into staging (2015-05-12 12:11:32 +0100) are available in the git repository at: https://github.com/jnsnow/qemu.git tags/ide-pull-request for you to fetch changes up to 6a6b2f8ca4a2bd274328e0f1e2a1ee3fa01e1b42: qtest: pre-buffer hex nibs (2015-05-12 13:47:12 -0400) v2: 32bit build issues: 05: Fix hba_base type issue for 32/64bit* *This needs to be addressed more comprehensively in a later patchset that will likely touch all of libqos, to fix the guest pointer type issues (should be uint64_t instead of void * in most places.) 11: Fix printf format modifier. John Snow (14): libqos/ahci: Add halted command helpers libqos/ahci: Fix sector set method libqos: Add migration helpers ich9/ahci: Enable Migration qtest/ahci: Add migration test qtest/ahci: add migrate dma test qtest/ahci: add flush migrate test qtest/ahci: add halted dma test qtest/ahci: add migrate halted dma test qtest: allow arbitrarily long sends qtest: Add base64 encoded read/write qtest: add memset to qtest protocol libqos/ahci: Swap memread/write with bufread/write qtest: pre-buffer hex nibs hw/ide/ahci.c | 1 - hw/ide/ich.c | 1 - qtest.c | 138 ++--- tests/ahci-test.c | 328 +- tests/libqos/ahci.c | 38 +- tests/libqos/ahci.h | 3 + tests/libqos/libqos.c | 85 + tests/libqos/libqos.h | 2 + tests/libqos/malloc.c | 74 +--- tests/libqos/malloc.h | 1 + tests/libqtest.c | 47 ++-- tests/libqtest.h | 49 12 files changed, 711 insertions(+), 56 deletions(-) -- 2.1.0
Re: [Qemu-devel] [PATCH 2/2] s390x: Add laa and laag instructions
On 05/12/2015 05:53 AM, Richard Henderson wrote: On 05/07/2015 06:12 PM, Alexander Graf wrote: +static ExitStatus op_laa(DisasContext *s, DisasOps *o) +{ +TCGv_i64 m2 = tcg_temp_new_i64(); + +/* XXX should be atomic */ +tcg_gen_qemu_ld32s(m2, o-in2, get_mem_index(s)); + +/* Set r1 to the unmodified contents of m2 */ +tcg_gen_mov_i64(o-out, m2); + +/* m2 = r3 + m2 */ +tcg_gen_add_i64(m2, o-in1, m2); +tcg_gen_qemu_st32(m2, o-in2, get_mem_index(s)); + +/* Set in2 to the input operand for cc calculation */ +tcg_gen_mov_i64(o-in2, o-out); + +tcg_temp_free_i64(m2); +return NO_EXIT; +} I don't believe that this computes the right cc. You need to place m2 into in2, not out. That said, maybe tcg_temp_free_i64(o-in2); o-in2 = m2; is the right way to perform that copy. Since we know that in2 was in2_a2, allocating a temp for an address, we also know we can both (1) free it and (2) put another temp in there that will be freed. Well, alternatively I could try to make out be the memory value at the end of the op and invent new load/store helpers for atomic instructions. That way the op bit should shrink significantly and we get the correct operand in s-out automatically. Don't worry about the atomic-ness of the operation until some of the many patch sets going round on that very subject are resolved. This will be good enough for system mode for now. I think maybe the whole target ought to be cleaned up for the new TCGMemOps. For this, you could share the code for LAA and LAAG with the TCGMemOp in insn-data. That's an interesting idea. Let me see what I can pull off. Alex
Re: [Qemu-devel] [Qemu-block] [PATCH 5/5] tests: add test case for encrypted qcow2 read/write
On 05/12/2015 01:06 PM, John Snow wrote: tests/qemu-iotests/131 | 69 ++ tests/qemu-iotests/131.out | 46 +++ Fam Zheng already has a patch on-list that uses test 131, and I think his patch was submitted first. (Unless we want to play the Who gets merged first? game.) That's the sort of conflict that I expect a maintainer can clean up, if there is no other reason for a respin (although it is not always easy to coax git into understanding that a patch would be valid if the file is renamed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PULL v2 01/14] libqos/ahci: Add halted command helpers
Sometimes we want a command to halt the VM instead of complete successfully, so it'd be nice to let the libqos/ahci functions cope with such scenarios. Signed-off-by: John Snow js...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com Message-id: 1430417242-11859-2-git-send-email-js...@redhat.com --- tests/libqos/ahci.c | 27 +++ tests/libqos/ahci.h | 3 +++ 2 files changed, 30 insertions(+) diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 843cf72..229409b 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -566,6 +566,33 @@ inline unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd) return (bytes + bytes_per_prd - 1) / bytes_per_prd; } +/* Issue a command, expecting it to fail and STOP the VM */ +AHCICommand *ahci_guest_io_halt(AHCIQState *ahci, uint8_t port, +uint8_t ide_cmd, uint64_t buffer, +size_t bufsize, uint64_t sector) +{ +AHCICommand *cmd; + +cmd = ahci_command_create(ide_cmd); +ahci_command_adjust(cmd, sector, buffer, bufsize, 0); +ahci_command_commit(ahci, cmd, port); +ahci_command_issue_async(ahci, cmd); +qmp_eventwait(STOP); + +return cmd; +} + +/* Resume a previously failed command and verify/finalize */ +void ahci_guest_io_resume(AHCIQState *ahci, AHCICommand *cmd) +{ +/* Complete the command */ +qmp_async({'execute':'cont' }); +qmp_eventwait(RESUME); +ahci_command_wait(ahci, cmd); +ahci_command_verify(ahci, cmd); +ahci_command_free(cmd); +} + /* Given a guest buffer address, perform an IO operation */ void ahci_guest_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, uint64_t buffer, size_t bufsize, uint64_t sector) diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h index 40e8ca4..779e812 100644 --- a/tests/libqos/ahci.h +++ b/tests/libqos/ahci.h @@ -524,6 +524,9 @@ unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t port); unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd); void ahci_guest_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, uint64_t gbuffer, size_t size, uint64_t sector); +AHCICommand *ahci_guest_io_halt(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, +uint64_t gbuffer, size_t size, uint64_t sector); +void ahci_guest_io_resume(AHCIQState *ahci, AHCICommand *cmd); void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, void *buffer, size_t bufsize, uint64_t sector); -- 2.1.0
[Qemu-devel] [PULL v2 04/14] ich9/ahci: Enable Migration
Lift the flag preventing the migration of the ICH9/AHCI devices. Signed-off-by: John Snow js...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com Message-id: 1430417242-11859-5-git-send-email-js...@redhat.com --- hw/ide/ahci.c | 1 - hw/ide/ich.c | 1 - 2 files changed, 2 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 833fd45..8e36dec 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1461,7 +1461,6 @@ typedef struct SysbusAHCIState { static const VMStateDescription vmstate_sysbus_ahci = { .name = sysbus-ahci, -.unmigratable = 1, /* Still buggy under I/O load */ .fields = (VMStateField[]) { VMSTATE_AHCI(ahci, SysbusAHCIState), VMSTATE_END_OF_LIST() diff --git a/hw/ide/ich.c b/hw/ide/ich.c index b1d8874..350c7f1 100644 --- a/hw/ide/ich.c +++ b/hw/ide/ich.c @@ -82,7 +82,6 @@ static const VMStateDescription vmstate_ich9_ahci = { .name = ich9_ahci, -.unmigratable = 1, /* Still buggy under I/O load */ .version_id = 1, .fields = (VMStateField[]) { VMSTATE_PCI_DEVICE(parent_obj, AHCIPCIState), -- 2.1.0
[Qemu-devel] [PULL v2 05/14] qtest/ahci: Add migration test
Notes: * The migration is performed on QOSState objects. * The migration is performed in such a way that it does not assume consistency between the allocators attached to each. That is to say, you can use each QOSState object completely independently and then at an arbitrary point decide to migrate, and the destination object will now be consistent with the memory within the source guest. The source object that was migrated from will have a completely blank allocator. ahci-test.c: - verify_state is added - ahci_migrate is added as a frontend to migrate - test_migrate_sanity test case is added. Signed-off-by: John Snow js...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com Message-id: 1430417242-11859-6-git-send-email-js...@redhat.com --- tests/ahci-test.c | 90 +++ 1 file changed, 90 insertions(+) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 7c23bb2..9fccafc 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -97,6 +97,72 @@ static void generate_pattern(void *buffer, size_t len, size_t cycle_len) } } +/** + * Verify that the transfer did not corrupt our state at all. + */ +static void verify_state(AHCIQState *ahci) +{ +int i, j; +uint32_t ahci_fingerprint; +uint64_t hba_base; +uint64_t hba_stored; +AHCICommandHeader cmd; + +ahci_fingerprint = qpci_config_readl(ahci-dev, PCI_VENDOR_ID); +g_assert_cmphex(ahci_fingerprint, ==, ahci-fingerprint); + +/* If we haven't initialized, this is as much as can be validated. */ +if (!ahci-hba_base) { +return; +} + +hba_base = (uint64_t)qpci_config_readl(ahci-dev, PCI_BASE_ADDRESS_5); +hba_stored = (uint64_t)(uintptr_t)ahci-hba_base; +g_assert_cmphex(hba_base, ==, hba_stored); + +g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP), ==, ahci-cap); +g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP2), ==, ahci-cap2); + +for (i = 0; i 32; i++) { +g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_FB), ==, +ahci-port[i].fb); +g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_CLB), ==, +ahci-port[i].clb); +for (j = 0; j 32; j++) { +ahci_get_command_header(ahci, i, j, cmd); +g_assert_cmphex(cmd.prdtl, ==, ahci-port[i].prdtl[j]); +g_assert_cmphex(cmd.ctba, ==, ahci-port[i].ctba[j]); +} +} +} + +static void ahci_migrate(AHCIQState *from, AHCIQState *to, const char *uri) +{ +QOSState *tmp = to-parent; +QPCIDevice *dev = to-dev; +if (uri == NULL) { +uri = tcp:127.0.0.1:1234; +} + +/* context will be 'to' after completion. */ +migrate(from-parent, to-parent, uri); + +/* We'd like for the AHCIState objects to still point + * to information specific to its specific parent + * instance, but otherwise just inherit the new data. */ +memcpy(to, from, sizeof(AHCIQState)); +to-parent = tmp; +to-dev = dev; + +tmp = from-parent; +dev = from-dev; +memset(from, 0x00, sizeof(AHCIQState)); +from-parent = tmp; +from-dev = dev; + +verify_state(to); +} + /*** Test Setup Teardown ***/ /** @@ -146,6 +212,8 @@ static AHCIQState *ahci_boot(const char *cli, ...) static void ahci_shutdown(AHCIQState *ahci) { QOSState *qs = ahci-parent; + +set_context(qs); ahci_clean_mem(ahci); free_ahci_device(ahci-dev); g_free(ahci); @@ -1022,6 +1090,26 @@ static void test_flush_retry(void) ahci_shutdown(ahci); } +/** + * Basic sanity test to boot a machine, find an AHCI device, and shutdown. + */ +static void test_migrate_sanity(void) +{ +AHCIQState *src, *dst; +const char *uri = tcp:127.0.0.1:1234; + +src = ahci_boot(-m 1024 -M q35 +-hda %s , tmp_path); +dst = ahci_boot(-m 1024 -M q35 +-hda %s +-incoming %s, tmp_path, uri); + +ahci_migrate(src, dst, uri); + +ahci_shutdown(src); +ahci_shutdown(dst); +} + /**/ /* AHCI I/O Test Matrix Definitions */ @@ -1271,6 +1359,8 @@ int main(int argc, char **argv) qtest_add_func(/ahci/flush/simple, test_flush); qtest_add_func(/ahci/flush/retry, test_flush_retry); +qtest_add_func(/ahci/migrate/sanity, test_migrate_sanity); + ret = g_test_run(); /* Cleanup */ -- 2.1.0
[Qemu-devel] [PULL v2 09/14] qtest/ahci: add migrate halted dma test
Test migrating a halted DMA transaction. Resume, then test data integrity. Signed-off-by: John Snow js...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com Message-id: 1430417242-11859-10-git-send-email-js...@redhat.com --- tests/ahci-test.c | 75 ++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 4eb1105..1a967c3 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1214,6 +1214,78 @@ static void test_halted_dma(void) } /** + * DMA Error Migration Test + * + * Simulate an error on first write, Try to write a pattern, + * Confirm the VM has stopped, migrate, resume the VM, + * verify command has completed, then read back the data and verify. + */ +static void test_migrate_halted_dma(void) +{ +AHCIQState *src, *dst; +uint8_t port; +size_t bufsize = 4096; +unsigned char *tx = g_malloc(bufsize); +unsigned char *rx = g_malloc0(bufsize); +unsigned i; +uint64_t ptr; +AHCICommand *cmd; +const char *uri = tcp:127.0.0.1:1234; + +prepare_blkdebug_script(debug_path, write_aio); + +src = ahci_boot_and_enable(-drive file=blkdebug:%s:%s,if=none,id=drive0, + format=qcow2,cache=writeback, + rerror=stop,werror=stop + -M q35 + -device ide-hd,drive=drive0 , + debug_path, + tmp_path); + +dst = ahci_boot(-drive file=%s,if=none,id=drive0, +format=qcow2,cache=writeback, +rerror=stop,werror=stop +-M q35 +-device ide-hd,drive=drive0 +-incoming %s, +tmp_path, uri); + +set_context(src-parent); + +/* Initialize and prepare */ +port = ahci_port_select(src); +ahci_port_clear(src, port); + +for (i = 0; i bufsize; i++) { +tx[i] = (bufsize - i); +} + +/* create DMA source buffer and write pattern */ +ptr = ahci_alloc(src, bufsize); +g_assert(ptr); +memwrite(ptr, tx, bufsize); + +/* Write, trigger the VM to stop, migrate, then resume. */ +cmd = ahci_guest_io_halt(src, port, CMD_WRITE_DMA, + ptr, bufsize, 0); +ahci_migrate(src, dst, uri); +ahci_guest_io_resume(dst, cmd); +ahci_free(dst, ptr); + +/* Read back */ +ahci_io(dst, port, CMD_READ_DMA, rx, bufsize, 0); + +/* Verify TX and RX are identical */ +g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0); + +/* Cleanup and go home. */ +ahci_shutdown(src); +ahci_shutdown(dst); +g_free(rx); +g_free(tx); +} + +/** * Migration test: Try to flush, migrate, then resume. */ static void test_flush_migrate(void) @@ -1513,8 +1585,9 @@ int main(int argc, char **argv) qtest_add_func(/ahci/flush/migrate, test_flush_migrate); qtest_add_func(/ahci/migrate/sanity, test_migrate_sanity); -qtest_add_func(/ahci/migrate/dma, test_migrate_dma); +qtest_add_func(/ahci/migrate/dma/simple, test_migrate_dma); qtest_add_func(/ahci/io/dma/lba28/retry, test_halted_dma); +qtest_add_func(/ahci/migrate/dma/halted, test_migrate_halted_dma); ret = g_test_run(); -- 2.1.0
Re: [Qemu-devel] Help with deadlock when using sound
On May 12, 2015, at 3:45 AM, Paolo Bonzini wrote: On 12/05/2015 00:43, Programmingkid wrote: On May 10, 2015, at 10:54 AM, Paolo Bonzini wrote: On 06/05/2015 18:40, Programmingkid wrote: When I try to use the pcspk sound hardware, QEMU freezes and uses 100% of the cpu time. This is the command I use: qemu-system-i386 -cdrom anything you wan here -soundhw pcspk This looks like a deadlock situation because some unknown code called qemu_mutex_lock(). Here is the stack trace at the freeze: (gdb) bt #0 0x7fff824e2db6 in semaphore_wait_trap () #1 0x7fff824e8417 in pthread_mutex_lock () #2 0x000100267199 in qemu_mutex_lock (mutex=value temporarily unavailable, due to optimizations) at util/qemu-thread-posix.c:73 #3 0x003c44016e95153b in ?? () My host is Mac OS 10.6.8. My guest isn't really anything. I have used Windows XP before but it isn't necessary to reproduce the problem. The ?? is what appears to be the problem. I can't even print instructions at that address. Any ideas as to what is calling the qemu_mutex_lock() function could help. The unknown code here is probably some place where gdb cannot find the frame pointer. Not a surprise if you are using a 5 year old debugger with (presumably) a newer compiler. Reproduced with a FreeDOS image from QEMU Advent Calendar. It locks up as soon as you type beep. It works with the PulseAudio and ALSA backends, but it doesn't with the SDL backend, even on Linux. Also, it deadlocks even with KVM enabled. Paolo OK, I see a pattern. SDL and CoreAudio both don't support audio input. Both of them have this code: .voice_size_in = 0 Alsa and PulseAudio do support audio input and work. Coincidence? Yes. Locking in SDL is completely broken. sdl_callback runs with the SDL audio lock taken, but then it waits on a semaphore so you cannot call any other SDL audio function from the main thread. As soon as you do that, you get a deadlock. I'm strongly tempted to just remove the driver. This sounds very similar to what happens to CoreAudio. On the other hand, CoreAudio seems to be okay. Can you try thread apply all bt full from gdb? Paolo Here is the output you wanted. Note: used run -soundhw ac97 -cdrom ~/debian.iso Thread 9 (process 44956): #0 0x7fff824e2dda in semaphore_timedwait_signal_trap () No symbol table info available. #1 0x7fff82521772 in _pthread_cond_wait () No symbol table info available. #2 0x7fff8423468c in CAGuard::WaitFor () No symbol table info available. #3 0x7fff84236c1b in CAGuard::WaitUntil () No symbol table info available. #4 0x7fff84234d85 in HP_IOThread::WorkLoop () No symbol table info available. #5 0x7fff84234827 in HP_IOThread::ThreadEntry () No symbol table info available. #6 0x7fff84234755 in CAPThread::Entry () No symbol table info available. #7 0x7fff8251bfd6 in _pthread_start () No symbol table info available. #8 0x7fff8251be89 in thread_start () No symbol table info available. Thread 8 (process 44956): #0 addr_add (env=0x121ff2e78, addr=1, arg=247) at /Users/user/Documents/Development/Projects/Qemu/qemu-git/target-ppc/mem_helper.c:42 No locals. #1 0x000100158f4b in helper_lmw (env=0x101db1220, addr=132087416, reg=30) at /Users/user/Documents/Development/Projects/Qemu/qemu-git/target-ppc/mem_helper.c:61 No locals. #2 0x000116426c97 in ?? () No symbol table info available. Current language: auto; currently c Thread 6 (process 44956): #0 0x7fff8254499e in __sigwait () No symbol table info available. #1 0x7fff82544977 in sigwait () No symbol table info available. #2 0x0001003add68 in sigwait_compat (opaque=0x101eb7350) at util/compatfd.c:36 sig = 0 err = 0 info = (struct sigfd_compat_info *) 0x101eb7350 #3 0x7fff8251bfd6 in _pthread_start () No symbol table info available. #4 0x7fff8251be89 in thread_start () No symbol table info available. Thread 3 (process 44956): #0 0x7fff824fbc0a in kevent () No symbol table info available. #1 0x7fff824fdadd in _dispatch_mgr_invoke () No symbol table info available. #2 0x7fff824fd7b4 in _dispatch_queue_invoke () No symbol table info available. #3 0x7fff824fd2de in _dispatch_worker_thread2 () No symbol table info available. #4 0x7fff824fcc08 in _pthread_wqthread () No symbol table info available. #5 0x7fff824fcaa5 in start_wqthread () No symbol table info available. Thread 2 (process 44956): #0 0x7fff824e2dc2 in semaphore_wait_signal_trap () No symbol table info available. #1 0x7fff824e840d in pthread_mutex_lock () No symbol table info available. #2 0x0001003a98c2 in qemu_mutex_lock (mutex=0x10070e080) at util/qemu-thread-posix.c:73 err = 0 #3 0x00010004da9d in qemu_mutex_lock_iothread () at /Users/user/Documents/Development/Projects/Qemu/qemu-git/cpus.c:1128 No locals. #4 0x0001003be885 in call_rcu_thread (opaque=0x0) at util/rcu.c:241
[Qemu-devel] [PATCH v6 5/7] vmport_rpc: Add migration
Signed-off-by: Don Slutz dsl...@verizon.com --- hw/misc/vmport_rpc.c | 250 +++ trace-events | 8 +- 2 files changed, 255 insertions(+), 3 deletions(-) diff --git a/hw/misc/vmport_rpc.c b/hw/misc/vmport_rpc.c index 9a32c6f..fe9cfc5 100644 --- a/hw/misc/vmport_rpc.c +++ b/hw/misc/vmport_rpc.c @@ -171,6 +171,14 @@ typedef struct VMPortRpcState { uint32_t open_cookie; channel_t chans[GUESTMSG_MAX_CHANNEL]; GHashTable *guestinfo; +/* Temporary cache for migration purposes */ +int32_t mig_chan_num; +int32_t mig_bucket_num; +uint32_t mig_guestinfo_size; +uint32_t mig_guestinfo_off; +uint8_t *mig_guestinfo_buf; +channel_control_t *mig_chans; +bucket_control_t *mig_buckets; #ifdef VMPORT_RPC_DEBUG unsigned int end; unsigned int last; @@ -1167,6 +1175,247 @@ static Property vmport_rpc_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const VMStateDescription vmstate_vmport_rpc_chan = { +.name = vmport_rpc/chan, +.version_id = 1, +.minimum_version_id = 1, +.fields = (VMStateField []) +{ +VMSTATE_UINT64(active_time, channel_control_t), +VMSTATE_UINT32(chan_id, channel_control_t), +VMSTATE_UINT32(cookie, channel_control_t), +VMSTATE_UINT32(proto_num, channel_control_t), +VMSTATE_UINT16(send_len, channel_control_t), +VMSTATE_UINT16(send_idx, channel_control_t), +VMSTATE_UINT16(send_buf_max, channel_control_t), +VMSTATE_UINT8(recv_read, channel_control_t), +VMSTATE_UINT8(recv_write, channel_control_t), +VMSTATE_END_OF_LIST() +}, +}; + +static const VMStateDescription vmstate_vmport_rpc_bucket = { +.name = vmport_rpc/bucket, +.version_id = 1, +.minimum_version_id = 1, +.fields = (VMStateField []) +{ +VMSTATE_UINT16(recv_len, bucket_control_t), +VMSTATE_UINT16(recv_idx, bucket_control_t), +VMSTATE_UINT16(recv_buf_max, bucket_control_t), +VMSTATE_END_OF_LIST() +}, +}; + +static void vmport_rpc_size_mig_guestinfo(gpointer key, gpointer value, + gpointer opaque) +{ +VMPortRpcState *s = opaque; +unsigned int key_len = strlen(key) + 1; +guestinfo_t *gi = value; + +s-mig_guestinfo_size += 1 + key_len + 4 + gi-val_max; +} + +static void vmport_rpc_fill_mig_guestinfo(gpointer key, gpointer value, + gpointer opaque) +{ +VMPortRpcState *s = opaque; +unsigned int key_len = strlen(key) + 1; +guestinfo_t *gi = value; + +assert(gi-val_len = gi-val_max); +trace_vmport_rpc_fill_mig_guestinfo(key_len, key_len, key, gi-val_len, +gi-val_len, gi-val_data); +s-mig_guestinfo_buf[s-mig_guestinfo_off++] = key_len; +memcpy(s-mig_guestinfo_buf + s-mig_guestinfo_off, key, key_len); +s-mig_guestinfo_off += key_len; +s-mig_guestinfo_buf[s-mig_guestinfo_off++] = gi-val_len 8; +s-mig_guestinfo_buf[s-mig_guestinfo_off++] = gi-val_len; +s-mig_guestinfo_buf[s-mig_guestinfo_off++] = gi-val_max 8; +s-mig_guestinfo_buf[s-mig_guestinfo_off++] = gi-val_max; +memcpy(s-mig_guestinfo_buf + s-mig_guestinfo_off, gi-val_data, + gi-val_max); +s-mig_guestinfo_off += gi-val_max; +} + +static int vmport_rpc_pre_load(void *opaque) +{ +VMPortRpcState *s = opaque; + +g_free(s-mig_guestinfo_buf); +s-mig_guestinfo_buf = NULL; +s-mig_guestinfo_size = 0; +s-mig_guestinfo_off = 0; +g_free(s-mig_chans); +s-mig_chans = NULL; +s-mig_chan_num = 0; +g_free(s-mig_buckets); +s-mig_buckets = NULL; +s-mig_bucket_num = 0; + +return 0; +} + +static void vmport_rpc_pre_save(void *opaque) +{ +VMPortRpcState *s = opaque; +unsigned int i; +unsigned int mig_chan_idx = 0; +unsigned int mig_bucket_idx = 0; + +(void)vmport_rpc_pre_load(opaque); +for (i = 0; i GUESTMSG_MAX_CHANNEL; ++i) { +channel_t *c = s-chans[i]; + +if (c-ctl.proto_num) { +unsigned int j; + +s-mig_chan_num++; +for (j = 0; j MAX_BKTS; ++j) { +bucket_t *b = c-recv_bkt[j]; + +s-mig_bucket_num++; +s-mig_guestinfo_size += +(b-ctl.recv_buf_max + 1) * CHAR_PER_CALL; +} +s-mig_guestinfo_size += (c-ctl.send_buf_max + 1) * CHAR_PER_CALL; +} +} +g_hash_table_foreach(s-guestinfo, vmport_rpc_size_mig_guestinfo, s); +s-mig_guestinfo_size++; +s-mig_guestinfo_buf = g_malloc(s-mig_guestinfo_size); +s-mig_chans = g_malloc(s-mig_chan_num * sizeof(channel_control_t)); +s-mig_buckets = g_malloc(s-mig_bucket_num * sizeof(bucket_control_t)); + +for (i = 0; i GUESTMSG_MAX_CHANNEL; ++i) { +channel_t *c = s-chans[i]; + +if (c-ctl.proto_num) { +unsigned int j; +
Re: [Qemu-devel] [Qemu-block] [PATCH 03/34] quorum: Use bdrv_open_image()
On Fri, May 08, 2015 at 07:21:35PM +0200, Kevin Wolf wrote: Besides standardising on a single interface for opening child nodes, this simplifies the .bdrv_open() implementation of the quorum block driver by using block layer functionality for handling BlockdevRefs. Signed-off-by: Kevin Wolf kw...@redhat.com --- block/quorum.c | 51 +++ 1 file changed, 11 insertions(+), 40 deletions(-) diff --git a/block/quorum.c b/block/quorum.c index f91ef75..a33881a 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -866,25 +866,18 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags, Error *local_err = NULL; QemuOpts *opts = NULL; bool *opened; -QDict *sub = NULL; -QList *list = NULL; -const QListEntry *lentry; int i; int ret = 0; qdict_flatten(options); -qdict_extract_subqdict(options, sub, children.); -qdict_array_split(sub, list); -if (qdict_size(sub)) { -error_setg(local_err, Invalid option children.%s, - qdict_first(sub)-key); +/* count how many different children are present */ +s-num_children = qdict_array_entries(options, children.); +if (s-num_children 0) { +error_setg(local_err, Option children is not a valid array); ret = -EINVAL; goto exit; } - -/* count how many different children are present */ -s-num_children = qlist_size(list); if (s-num_children 2) { error_setg(local_err, Number of provided children must be greater than 1); @@ -937,37 +930,17 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags, s-bs = g_new0(BlockDriverState *, s-num_children); opened = g_new0(bool, s-num_children); -for (i = 0, lentry = qlist_first(list); lentry; - lentry = qlist_next(lentry), i++) { -QDict *d; -QString *string; - -switch (qobject_type(lentry-value)) -{ -/* List of options */ -case QTYPE_QDICT: -d = qobject_to_qdict(lentry-value); -QINCREF(d); -ret = bdrv_open(s-bs[i], NULL, NULL, d, flags, NULL, -local_err); -break; - -/* QMP reference */ -case QTYPE_QSTRING: -string = qobject_to_qstring(lentry-value); -ret = bdrv_open(s-bs[i], NULL, qstring_get_str(string), NULL, -flags, NULL, local_err); -break; - -default: -error_setg(local_err, Specification of child block device %i - is invalid, i); -ret = -EINVAL; -} +for (i = 0; i s-num_children; i++) { +char indexstr[32]; +ret = snprintf(indexstr, 32, children.%d, i); +assert(ret 32); +ret = bdrv_open_image(s-bs[i], NULL, options, indexstr, flags, + false, local_err); if (ret 0) { goto close_exit; } + opened[i] = true; } @@ -990,8 +963,6 @@ exit: if (local_err) { error_propagate(errp, local_err); } -QDECREF(list); -QDECREF(sub); return ret; } -- 1.8.3.1 Reviewed-by: Jeff Cody jc...@redhat.com