Re: [Qemu-devel] [PATCH 3/4] monitor: fix oob command leak
Marc-André Lureau writes: > Hi > > On Thu, Aug 9, 2018 at 1:44 PM, Marc-André Lureau > wrote: >> Spotted by ASAN, during make check... >> >> Direct leak of 40 byte(s) in 1 object(s) allocated from: >> #0 0x7f8e27262c48 in malloc (/lib64/libasan.so.5+0xeec48) >> #1 0x7f8e26a5f3c5 in g_malloc (/lib64/libglib-2.0.so.0+0x523c5) >> #2 0x555ab67078a8 in qstring_from_str >> /home/elmarco/src/qq/qobject/qstring.c:67 >> #3 0x555ab67071e4 in qstring_new >> /home/elmarco/src/qq/qobject/qstring.c:24 >> #4 0x555ab6713fbf in qstring_from_escaped_str >> /home/elmarco/src/qq/qobject/json-parser.c:144 >> #5 0x555ab671738c in parse_literal >> /home/elmarco/src/qq/qobject/json-parser.c:506 >> #6 0x555ab67179c3 in parse_value >> /home/elmarco/src/qq/qobject/json-parser.c:569 >> #7 0x555ab6715123 in parse_pair >> /home/elmarco/src/qq/qobject/json-parser.c:306 >> #8 0x555ab6715483 in parse_object >> /home/elmarco/src/qq/qobject/json-parser.c:357 >> #9 0x555ab671798b in parse_value >> /home/elmarco/src/qq/qobject/json-parser.c:561 >> #10 0x555ab6717a6b in json_parser_parse_err >> /home/elmarco/src/qq/qobject/json-parser.c:592 >> #11 0x555ab4fd4dcf in handle_qmp_command >> /home/elmarco/src/qq/monitor.c:4257 >> #12 0x555ab6712c4d in json_message_process_token >> /home/elmarco/src/qq/qobject/json-streamer.c:105 >> #13 0x555ab67e01e2 in json_lexer_feed_char >> /home/elmarco/src/qq/qobject/json-lexer.c:323 >> #14 0x555ab67e0af6 in json_lexer_feed >> /home/elmarco/src/qq/qobject/json-lexer.c:373 >> #15 0x555ab6713010 in json_message_parser_feed >> /home/elmarco/src/qq/qobject/json-streamer.c:124 >> #16 0x555ab4fd58ec in monitor_qmp_read >> /home/elmarco/src/qq/monitor.c:4337 >> #17 0x555ab6559df2 in qemu_chr_be_write_impl >> /home/elmarco/src/qq/chardev/char.c:175 >> #18 0x555ab6559e95 in qemu_chr_be_write >> /home/elmarco/src/qq/chardev/char.c:187 >> #19 0x555ab6560127 in fd_chr_read >> /home/elmarco/src/qq/chardev/char-fd.c:66 >> #20 0x555ab65d9c73 in qio_channel_fd_source_dispatch >> /home/elmarco/src/qq/io/channel-watch.c:84 >> #21 0x7f8e26a598ac in g_main_context_dispatch >> (/lib64/libglib-2.0.so.0+0x4c8ac) >> >> Signed-off-by: Marc-André Lureau > > It looks like it was introduced with commit > b27314567d4cd8e204a18feba60d3341fb2d1aed "qmp: Simplify code around > monitor_qmp_dispatch_one()" Yes. Before that commit, handle_qmp_command() freed @req and @id as follows. * Early errors: goto err, where we free req, and leak @id (I think). * Out of band: store @req and @id in @req_obj, free it in monitor_qmp_dispatch_one(). * In band: store @req and @id in @req_obj - queue full: free by calling qmp_request_free() - else: enqueue, monitor_qmp_bh_dispatcher() will deqeueue and pass to monitor_qmp_dispatch_one(), which frees. Current head always stores @req and @id in @req_obj. It frees as follows. * Out of band: it doesn't, since monitor_qmp_dispatch(), renamed from monitor_qmp_dispatch_one(), doesn't free anymore. This patch fixes it. * In band: - queue full: free by calling qmp_request_free(). - else: enqueue, monitor_qmp_bh_dispatcher() will deqeueue and pass to monitor_qmp_dispatch(), then free with qmp_request_free(). Looks like the commit replaced one leak by another one. > >> --- >> monitor.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/monitor.c b/monitor.c >> index 77861e96af..a1999e396c 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -4277,6 +4277,8 @@ static void handle_qmp_command(JSONMessageParser >> *parser, GQueue *tokens) >> trace_monitor_qmp_cmd_out_of_band(qobject_get_try_str(id) >>?: ""); >> monitor_qmp_dispatch(mon, req, id); >> +qobject_unref(req); >> +qobject_unref(id); >> return; >> } >> >> -- >> 2.18.0.547.g1d89318c48 >> >> I'm not going to argue for inclusion in 3.0, because OOB is experimental and off by default due to its remaining flaws. Cc: qemu-sta...@nongnu.org Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH] hw/ppc: on 40p machine, change default firmware to OpenBIOS
On Fri, Aug 10, 2018 at 07:37:12AM +0200, Hervé Poussineau wrote:
> OpenBIOS gained 40p support in 5b20e4cacecb62fb2bdc6867c11d44cddd77c4ff
> Use it, instead of relying on an unmaintained and very limited firmware.
>
> Signed-off-by: Hervé Poussineau
Applied to ppc-for-3.1, thanks.
> ---
> hw/ppc/prep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 3401570d98..1558855247 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -736,7 +736,7 @@ static void ibm_40p_init(MachineState *machine)
> /* PCI host */
> dev = qdev_create(NULL, "raven-pcihost");
> if (!bios_name) {
> -bios_name = BIOS_FILENAME;
> +bios_name = "openbios-ppc";
> }
> qdev_prop_set_string(dev, "bios-name", bios_name);
> qdev_prop_set_uint32(dev, "elf-machine", PPC_ELF_MACHINE);
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH v7 9/9] qcow2: Explicit number replaced by a constant
Signed-off-by: Leonid Bloch
Reviewed-by: Alberto Garcia
---
block/qcow2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index b4f291765b..b0e20aeffc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1324,7 +1324,7 @@ static int coroutine_fn qcow2_do_open(BlockDriverState
*bs, QDict *options,
/* 2^(s->refcount_order - 3) is the refcount width in bytes */
s->refcount_block_bits = s->cluster_bits - (s->refcount_order - 3);
s->refcount_block_size = 1 << s->refcount_block_bits;
-bs->total_sectors = header.size / 512;
+bs->total_sectors = header.size / BDRV_SECTOR_SIZE;
s->csize_shift = (62 - (s->cluster_bits - 8));
s->csize_mask = (1 << (s->cluster_bits - 8)) - 1;
s->cluster_offset_mask = (1LL << s->csize_shift) - 1;
@@ -3450,7 +3450,7 @@ static int coroutine_fn
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
goto fail;
}
-old_length = bs->total_sectors * 512;
+old_length = bs->total_sectors * BDRV_SECTOR_SIZE;
new_l1_size = size_to_l1(s, offset);
if (offset < old_length) {
--
2.17.1
[Qemu-devel] [PATCH v7 6/9] qcow2: Increase the default upper limit on the L2 cache size
The upper limit on the L2 cache size is increased from 1 MB to 32 MB. This is done in order to allow default full coverage with the L2 cache for images of up to 256 GB in size (was 8 GB). Note, that only the needed amount to cover the full image is allocated. The value which is changed here is just the upper limit on the L2 cache size, beyond which it will not grow, even if the size of the image will require it to. Signed-off-by: Leonid Bloch --- block/qcow2.h| 2 +- docs/qcow2-cache.txt | 6 +++--- qemu-options.hx | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index d917b5f577..e699a55d02 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -74,7 +74,7 @@ /* Must be at least 4 to cover all cases of refcount table growth */ #define MIN_REFCOUNT_CACHE_SIZE 4 /* clusters */ -#define DEFAULT_L2_CACHE_MAX_SIZE (1 * MiB) +#define DEFAULT_L2_CACHE_MAX_SIZE (32 * MiB) #define DEFAULT_CLUSTER_SIZE (64 * KiB) diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt index 69af306267..6ad1081d1a 100644 --- a/docs/qcow2-cache.txt +++ b/docs/qcow2-cache.txt @@ -125,8 +125,8 @@ There are a few things that need to be taken into account: (or the cache entry size: see "Using smaller cache sizes" below). - The default L2 cache size will cover the entire virtual size of an - image, up to a certain maximum. This maximum is 1 MB by default - (enough for image sizes of up to 8 GB with the default cluster size) + image, up to a certain maximum. This maximum is 32 MB by default + (enough for image sizes of up to 256 GB with the default cluster size) and it can be reduced or enlarged using the "l2-cache-size" option. The minimum is 2 clusters (or 2 cache entries, see below). @@ -186,7 +186,7 @@ Some things to take into account: always uses the cluster size as the entry size. - If the L2 cache is big enough to hold all of the image's L2 tables - (the default behavior for images of up to 8 GB in size) then none + (the default behavior for images of up to 256 GB in size) then none of this is necessary and you can omit the "l2-cache-entry-size" parameter altogether. diff --git a/qemu-options.hx b/qemu-options.hx index 22e8e2d113..4c44cdbc23 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -756,7 +756,7 @@ The maximum total size of the L2 table and refcount block caches in bytes @item l2-cache-size The maximum size of the L2 table cache in bytes -(default: if cache-size is not specified - 1M; otherwise, as large as possible +(default: if cache-size is not specified - 32M; otherwise, as large as possible within the cache-size, while permitting the requested or the minimal refcount cache size) -- 2.17.1
[Qemu-devel] [PATCH v7 5/9] qcow2: Assign the L2 cache relatively to the image size
Sufficient L2 cache can noticeably improve the performance when using
large images with frequent I/O.
Previously, the L2 cache was allocated without considering the image
size, and an option existed to manually determine its size. Thus to
achieve a full coverage of an image by the L2 cache (i.e. use more than
the default value of MAX(1 MB, 8 clusters)), a user needed to calculate
the required size manually, or with a script, and pass this value to the
'l2-cache-size' option.
Now, the L2 cache is assigned taking the virtual image size into account,
and will cover the entire image, unless the size needed for that is
larger than a certain maximum. This maximum is set to 1 MB by default
(enough to cover an 8 GB image with the default cluster size) but can
be increased or decreased using the 'l2-cache-size' option. This option
was previously documented as the *maximum* L2 cache size, and this patch
makes it behave as such, instead of as a constant size. Also, the
existing option 'cache-size' can limit the sum of both L2 and refcount
caches, as previously.
Signed-off-by: Leonid Bloch
---
block/qcow2.c | 22 ++
block/qcow2.h | 4 +---
docs/qcow2-cache.txt | 13 -
qemu-options.hx| 6 +++---
tests/qemu-iotests/137 | 1 -
tests/qemu-iotests/137.out | 1 -
6 files changed, 22 insertions(+), 25 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 053242f94e..434fb89076 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -777,16 +777,19 @@ static void read_cache_sizes(BlockDriverState *bs,
QemuOpts *opts,
uint64_t *refcount_cache_size, Error **errp)
{
BDRVQcow2State *s = bs->opaque;
-uint64_t combined_cache_size;
+uint64_t combined_cache_size, l2_cache_max_setting;
bool l2_cache_size_set, refcount_cache_size_set, combined_cache_size_set;
int min_refcount_cache = MIN_REFCOUNT_CACHE_SIZE * s->cluster_size;
+uint64_t virtual_disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
+uint64_t max_l2_cache = virtual_disk_size / (s->cluster_size / 8);
combined_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_CACHE_SIZE);
l2_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_L2_CACHE_SIZE);
refcount_cache_size_set = qemu_opt_get(opts,
QCOW2_OPT_REFCOUNT_CACHE_SIZE);
combined_cache_size = qemu_opt_get_size(opts, QCOW2_OPT_CACHE_SIZE, 0);
-*l2_cache_size = qemu_opt_get_size(opts, QCOW2_OPT_L2_CACHE_SIZE, 0);
+l2_cache_max_setting = qemu_opt_get_size(opts, QCOW2_OPT_L2_CACHE_SIZE,
+ DEFAULT_L2_CACHE_MAX_SIZE);
*refcount_cache_size = qemu_opt_get_size(opts,
QCOW2_OPT_REFCOUNT_CACHE_SIZE, 0);
@@ -794,13 +797,16 @@ static void read_cache_sizes(BlockDriverState *bs,
QemuOpts *opts,
QCOW2_OPT_L2_CACHE_ENTRY_SIZE,
s->cluster_size);
+*l2_cache_size = MIN(max_l2_cache, l2_cache_max_setting);
+
if (combined_cache_size_set) {
if (l2_cache_size_set && refcount_cache_size_set) {
error_setg(errp, QCOW2_OPT_CACHE_SIZE ", " QCOW2_OPT_L2_CACHE_SIZE
" and " QCOW2_OPT_REFCOUNT_CACHE_SIZE " may not be set "
"at the same time");
return;
-} else if (*l2_cache_size > combined_cache_size) {
+} else if (l2_cache_size_set &&
+ (l2_cache_max_setting > combined_cache_size)) {
error_setg(errp, QCOW2_OPT_L2_CACHE_SIZE " may not exceed "
QCOW2_OPT_CACHE_SIZE);
return;
@@ -815,13 +821,9 @@ static void read_cache_sizes(BlockDriverState *bs,
QemuOpts *opts,
} else if (refcount_cache_size_set) {
*l2_cache_size = combined_cache_size - *refcount_cache_size;
} else {
-uint64_t virtual_disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
-uint64_t max_l2_cache = virtual_disk_size / (s->cluster_size / 8);
-
/* Assign as much memory as possible to the L2 cache, and
* use the remainder for the refcount cache */
-if (combined_cache_size >= max_l2_cache + min_refcount_cache) {
-*l2_cache_size = max_l2_cache;
+if (combined_cache_size >= *l2_cache_size + min_refcount_cache) {
*refcount_cache_size = combined_cache_size - *l2_cache_size;
} else {
*refcount_cache_size = MIN(combined_cache_size,
@@ -829,10 +831,6 @@ static void read_cache_sizes(BlockDriverState *bs,
QemuOpts *opts,
*l2_cache_size = combined_cache_size - *refcount_cache_size;
}
}
-} else if (!l2_cache_size_set) {
-*l2_cache_size = MAX(DEFAULT_L2_CACHE_SIZE,
- (uint64_t)DEFAULT_L2_CACHE_CLUSTERS
- * s->clust
Re: [Qemu-devel] [PATCH 2/4] tests: fix bdrv-drain leak
Marc-André Lureau writes: > Spotted by ASAN: > > = > ==5378==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 65536 byte(s) in 1 object(s) allocated from: > #0 0x7f788f83bc48 in malloc (/lib64/libasan.so.5+0xeec48) > #1 0x7f788c9923c5 in g_malloc (/lib64/libglib-2.0.so.0+0x523c5) > #2 0x5622a1fe37bc in coroutine_trampoline > /home/elmarco/src/qq/util/coroutine-ucontext.c:116 > #3 0x7f788a15d75f in __correctly_grouped_prefixwc > (/lib64/libc.so.6+0x4c75f) > > Signed-off-by: Marc-André Lureau > --- > tests/test-bdrv-drain.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c > index 17bb8508ae..abc8bbe6f0 100644 > --- a/tests/test-bdrv-drain.c > +++ b/tests/test-bdrv-drain.c > @@ -948,6 +948,7 @@ static void coroutine_fn test_co_delete_by_drain(void > *opaque) > } > > dbdd->done = true; > +g_free(buffer); > } > > /** Screwed up in commit 4c8158e359d. Max, could the coroutine's stack accomodate the buffer?
[Qemu-devel] [PATCH v7 4/9] qcow2: Avoid duplication in setting the refcount cache size
The refcount cache size does not need to be set to its minimum value in
read_cache_sizes(), as it is set to at least its minimum value in
qcow2_update_options_prepare().
Signed-off-by: Leonid Bloch
---
block/qcow2.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 7a2d7a1d48..053242f94e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -829,16 +829,13 @@ static void read_cache_sizes(BlockDriverState *bs,
QemuOpts *opts,
*l2_cache_size = combined_cache_size - *refcount_cache_size;
}
}
-} else {
-if (!l2_cache_size_set) {
-*l2_cache_size = MAX(DEFAULT_L2_CACHE_SIZE,
- (uint64_t)DEFAULT_L2_CACHE_CLUSTERS
- * s->cluster_size);
-}
-if (!refcount_cache_size_set) {
-*refcount_cache_size = min_refcount_cache;
-}
+} else if (!l2_cache_size_set) {
+*l2_cache_size = MAX(DEFAULT_L2_CACHE_SIZE,
+ (uint64_t)DEFAULT_L2_CACHE_CLUSTERS
+ * s->cluster_size);
}
+/* If refcount-cache-size is not specified, it will be set to minimum
+ * in qcow2_update_options_prepare(). No need to set it here. */
if (*l2_cache_entry_size < (1 << MIN_CLUSTER_BITS) ||
*l2_cache_entry_size > s->cluster_size ||
--
2.17.1
[Qemu-devel] [PATCH v7 8/9] qcow2: Set the default cache-clean-interval to 10 minutes
The default cache-clean-interval is set to 10 minutes, in order to lower
the overhead of the qcow2 caches (before the default was 0, i.e.
disabled).
Signed-off-by: Leonid Bloch
Reviewed-by: Alberto Garcia
---
block/qcow2.c| 2 +-
block/qcow2.h| 1 +
docs/qcow2-cache.txt | 4 ++--
qapi/block-core.json | 3 ++-
qemu-options.hx | 2 +-
5 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index ba4dfae735..b4f291765b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -944,7 +944,7 @@ static int qcow2_update_options_prepare(BlockDriverState
*bs,
/* New interval for cache cleanup timer */
r->cache_clean_interval =
qemu_opt_get_number(opts, QCOW2_OPT_CACHE_CLEAN_INTERVAL,
-s->cache_clean_interval);
+DEFAULT_CACHE_CLEAN_INTERVAL);
#ifndef CONFIG_LINUX
if (r->cache_clean_interval != 0) {
error_setg(errp, QCOW2_OPT_CACHE_CLEAN_INTERVAL
diff --git a/block/qcow2.h b/block/qcow2.h
index e699a55d02..5e94f7ffc4 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -78,6 +78,7 @@
#define DEFAULT_CLUSTER_SIZE (64 * KiB)
+#define DEFAULT_CACHE_CLEAN_INTERVAL 600 /* seconds */
#define QCOW2_OPT_LAZY_REFCOUNTS "lazy-refcounts"
#define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request"
diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt
index 6ad1081d1a..684147ad45 100644
--- a/docs/qcow2-cache.txt
+++ b/docs/qcow2-cache.txt
@@ -204,8 +204,8 @@ This example removes all unused cache entries every 15
minutes:
-drive file=hd.qcow2,cache-clean-interval=900
-If unset, the default value for this parameter is 0 and it disables
-this feature.
+If unset, the default value for this parameter is 600. Setting it to 0
+disables this feature.
Note that this functionality currently relies on the MADV_DONTNEED
argument for madvise() to actually free the memory. This is a
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5b9084a394..9a6a708a37 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2830,7 +2830,8 @@
#
# @cache-clean-interval: clean unused entries in the L2 and refcount
# caches. The interval is in seconds. The default value
-# is 0 and it disables this feature (since 2.5)
+# is 600, and 0 disables this feature. (since 2.5)
+#
# @encrypt: Image decryption options. Mandatory for
# encrypted images, except when doing a metadata-only
# probe of the image. (since 2.10)
diff --git a/qemu-options.hx b/qemu-options.hx
index 4c44cdbc23..6abf3631ec 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -767,7 +767,7 @@ it which is not used for the L2 cache)
@item cache-clean-interval
Clean unused entries in the L2 and refcount caches. The interval is in seconds.
-The default value is 0 and it disables this feature.
+The default value is 600. Setting it to 0 disables this feature.
@item pass-discard-request
Whether discard requests to the qcow2 device should be forwarded to the data
--
2.17.1
[Qemu-devel] [PATCH v7 7/9] qcow2: Resize the cache upon image resizing
The caches are now recalculated upon image resizing. This is done
because the new default behavior of assigning L2 cache relatively to
the image size, implies that the cache will be adapted accordingly
after an image resize.
Signed-off-by: Leonid Bloch
Reviewed-by: Alberto Garcia
---
block/qcow2.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index 434fb89076..ba4dfae735 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3418,6 +3418,7 @@ static int coroutine_fn
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
uint64_t old_length;
int64_t new_l1_size;
int ret;
+QDict *options;
if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA &&
prealloc != PREALLOC_MODE_FALLOC && prealloc != PREALLOC_MODE_FULL)
@@ -3642,6 +3643,8 @@ static int coroutine_fn
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
}
}
+bs->total_sectors = offset / BDRV_SECTOR_SIZE;
+
/* write updated header.size */
offset = cpu_to_be64(offset);
ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, size),
@@ -3652,6 +3655,13 @@ static int coroutine_fn
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
}
s->l1_vm_state_index = new_l1_size;
+
+/* Update cache sizes */
+options = qdict_clone_shallow(bs->options);
+ret = qcow2_update_options(bs, options, s->flags, errp);
+if (ret < 0) {
+goto fail;
+}
ret = 0;
fail:
qemu_co_mutex_unlock(&s->lock);
--
2.17.1
[Qemu-devel] [PATCH v7 3/9] qcow2: Make sizes more humanly readable
Signed-off-by: Leonid Bloch
---
block/qcow2.c | 2 +-
block/qcow2.h | 9 +
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 3f4abc394e..7a2d7a1d48 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -831,7 +831,7 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts
*opts,
}
} else {
if (!l2_cache_size_set) {
-*l2_cache_size = MAX(DEFAULT_L2_CACHE_BYTE_SIZE,
+*l2_cache_size = MAX(DEFAULT_L2_CACHE_SIZE,
(uint64_t)DEFAULT_L2_CACHE_CLUSTERS
* s->cluster_size);
}
diff --git a/block/qcow2.h b/block/qcow2.h
index 81b844e936..39e1b279f8 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -27,6 +27,7 @@
#include "crypto/block.h"
#include "qemu/coroutine.h"
+#include "qemu/units.h"
//#define DEBUG_ALLOC
//#define DEBUG_ALLOC2
@@ -43,11 +44,11 @@
/* 8 MB refcount table is enough for 2 PB images at 64k cluster size
* (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
-#define QCOW_MAX_REFTABLE_SIZE 0x80
+#define QCOW_MAX_REFTABLE_SIZE (8 * MiB)
/* 32 MB L1 table is enough for 2 PB images at 64k cluster size
* (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
-#define QCOW_MAX_L1_SIZE 0x200
+#define QCOW_MAX_L1_SIZE (32 * MiB)
/* Allow for an average of 1k per snapshot table entry, should be plenty of
* space for snapshot names and IDs */
@@ -75,9 +76,9 @@
/* Whichever is more */
#define DEFAULT_L2_CACHE_CLUSTERS 8 /* clusters */
-#define DEFAULT_L2_CACHE_BYTE_SIZE 1048576 /* bytes */
+#define DEFAULT_L2_CACHE_SIZE (1 * MiB)
-#define DEFAULT_CLUSTER_SIZE 65536
+#define DEFAULT_CLUSTER_SIZE (64 * KiB)
#define QCOW2_OPT_LAZY_REFCOUNTS "lazy-refcounts"
--
2.17.1
[Qemu-devel] [PATCH v7 2/9] qcow2: Cosmetic changes
Some refactoring for better readability is done here.
Signed-off-by: Leonid Bloch
---
block/qcow2.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index ec9e6238a0..3f4abc394e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -790,8 +790,9 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts
*opts,
*refcount_cache_size = qemu_opt_get_size(opts,
QCOW2_OPT_REFCOUNT_CACHE_SIZE, 0);
-*l2_cache_entry_size = qemu_opt_get_size(
-opts, QCOW2_OPT_L2_CACHE_ENTRY_SIZE, s->cluster_size);
+*l2_cache_entry_size = qemu_opt_get_size(opts,
+ QCOW2_OPT_L2_CACHE_ENTRY_SIZE,
+ s->cluster_size);
if (combined_cache_size_set) {
if (l2_cache_size_set && refcount_cache_size_set) {
@@ -823,8 +824,8 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts
*opts,
*l2_cache_size = max_l2_cache;
*refcount_cache_size = combined_cache_size - *l2_cache_size;
} else {
-*refcount_cache_size =
-MIN(combined_cache_size, min_refcount_cache);
+*refcount_cache_size = MIN(combined_cache_size,
+ min_refcount_cache);
*l2_cache_size = combined_cache_size - *refcount_cache_size;
}
}
--
2.17.1
[Qemu-devel] [PATCH v7 0/9] Take the image size into account when allocating the L2 cache
This series makes the qcow2 L2 cache assignment aware of the image size, with the intention for it to cover the entire image. The importance of this change is in noticeable performance improvement, especially with heavy random I/O. The memory overhead is not big in most cases, as only 1 MB of cache for every 8 GB of image size is used. For cases with very large images and/or small cluster sizes, or systems with limited RAM resources, there is an upper limit on the default L2 cache: 32 MB. To modify this limit one can use the already existing 'l2-cache-size' and 'cache-size' options. Moreover, this fixes the behavior of 'l2-cache-size', as it was documented as the *maximum* L2 cache size, but in practice behaved as the absolute size. To compensate the memory overhead which may be increased following this behavior, the default cache-clean-interval is set to 10 minutes by default (was disabled by default before). The L2 cache is also resized accordingly, by default, if the image is resized. Additionally, few minor changes are made (refactoring and documentation fixes). Differences from v1: * .gitignore modification patch removed (unneeded). * The grammar fix in conflicting cache sizing patch removed (merged). * The update total_sectors when resizing patch squashed with the resizing patch. * L2 cache is now capped at 32 MB. * The default cache-clean-interval is set to 30 seconds. Differences from v2: * Made it clear in the documentation that setting cache-clean-interval to 0 disables this feature. Differences from v3: * The default cache-clean-interval is set to 10 minutes instead of 30 seconds before. * Commit message changes to better explain the patches. * Some refactoring. Differences from v4: * Refactoring. * Documentation and commit message fixes. Differences from v5: * A patch with cosmetic changes is split from the main patch * A patch for avoiding duplication in refcount cache size assignment is split from the main patch. * In the main patch the cap on the L2 cache size is set to only 1 MB (in order to be close to the previous behavior) and a separate patch sets it to 32 MB. * Changes in the documentation fixes patch [1/8]. Differences from v6: * A patch is added to make the defined sizes more humanly readable Leonid Bloch (9): qcow2: Options' documentation fixes qcow2: Cosmetic changes qcow2: Make sizes more humanly readable qcow2: Avoid duplication in setting the refcount cache size qcow2: Assign the L2 cache relatively to the image size qcow2: Increase the default upper limit on the L2 cache size qcow2: Resize the cache upon image resizing qcow2: Set the default cache-clean-interval to 10 minutes qcow2: Explicit number replaced by a constant block/qcow2.c | 54 +- block/qcow2.h | 12 - docs/qcow2-cache.txt | 33 ++- qapi/block-core.json | 3 ++- qemu-options.hx| 11 +--- tests/qemu-iotests/137 | 1 - tests/qemu-iotests/137.out | 1 - 7 files changed, 66 insertions(+), 49 deletions(-) -- 2.17.1
[Qemu-devel] [PATCH v7 1/9] qcow2: Options' documentation fixes
Signed-off-by: Leonid Bloch --- docs/qcow2-cache.txt | 16 +++- qemu-options.hx | 9 ++--- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt index 8a09a5cc5f..0f157d859a 100644 --- a/docs/qcow2-cache.txt +++ b/docs/qcow2-cache.txt @@ -97,12 +97,15 @@ need: l2_cache_size = disk_size_GB * 131072 refcount_cache_size = disk_size_GB * 32768 -QEMU has a default L2 cache of 1MB (1048576 bytes) and a refcount -cache of 256KB (262144 bytes), so using the formulas we've just seen -we have +With the default cluster size, to cover each 8 GB of the virtual image +size, 1MB of L2 cache is needed: - 1048576 / 131072 = 8 GB of virtual disk covered by that cache -262144 / 32768 = 8 GB + 65536 / 8 = 8192 = 8 GB / 1 MB + +A default refcount cache is 4 times the cluster size, which defaults to +256 KB (262144 bytes). This is sufficient for 8 GB of image size: + + 262144 / 32768 = 8 GB How to configure the cache sizes @@ -130,6 +133,9 @@ There are a few things that need to be taken into account: memory as possible to the L2 cache before increasing the refcount cache size. + - At most two of "l2-cache-size", "refcount-cache-size", and "cache-size" + can be set simultaneously. + Unlike L2 tables, refcount blocks are not used during normal I/O but only during allocations and internal snapshots. In most cases they are accessed sequentially (even during random guest I/O) so increasing the diff --git a/qemu-options.hx b/qemu-options.hx index b1bf0f485f..f6804758d3 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -752,15 +752,18 @@ image file) @item cache-size The maximum total size of the L2 table and refcount block caches in bytes -(default: 1048576 bytes or 8 clusters, whichever is larger) +(default: the sum of l2-cache-size and refcount-cache-size) @item l2-cache-size The maximum size of the L2 table cache in bytes -(default: 4/5 of the total cache size) +(default: if cache-size is not defined - 1048576 bytes or 8 clusters, whichever +is larger; otherwise, as large as possible or needed within the cache-size, +while permitting the requested or the minimal refcount cache size) @item refcount-cache-size The maximum size of the refcount block cache in bytes -(default: 1/5 of the total cache size) +(default: 4 times the cluster size; or if cache-size is specified, the part of +it which is not used for the L2 cache) @item cache-clean-interval Clean unused entries in the L2 and refcount caches. The interval is in seconds. -- 2.17.1
Re: [Qemu-devel] [PATCH 1/4] tests: fix crumple/recursive leak
Marc-André Lureau writes: > Spotted by ASAN: > > = > ==27907==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 4120 byte(s) in 1 object(s) allocated from: > #0 0x7f913458ce50 in calloc (/lib64/libasan.so.5+0xeee50) > #1 0x7f9133fd641d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d) > #2 0x5561c6643c95 in qdict_crumple_test_recursive > /home/elmarco/src/qq/tests/check-block-qdict.c:438 > #3 0x7f9133ff7c49 (/lib64/libglib-2.0.so.0+0x73c49) > > Signed-off-by: Marc-André Lureau > --- > tests/check-block-qdict.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/check-block-qdict.c b/tests/check-block-qdict.c > index 478807f839..73d3e9f574 100644 > --- a/tests/check-block-qdict.c > +++ b/tests/check-block-qdict.c > @@ -491,6 +491,7 @@ static void qdict_crumple_test_recursive(void) > empty_list_0 = qobject_to(QDict, qlist_pop(empty_list)); > g_assert(empty_list_0); > g_assert_cmpint(qdict_size(empty_list_0), ==, 0); > +qobject_unref(empty_list_0); > > qobject_unref(src); > qobject_unref(dst); Screwed up in commit 2860b2b2cb8. I can add that to the commit message when I apply. Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Enforce multiline comment syntax
Peter Maydell writes:
> We now require Linux-kernel-style multiline comments:
> /*
> * line one
> * line two
> */
>
> Enforce this in checkpatch.pl, by backporting the relevant
> parts of the Linux kernel's checkpatch.pl. (The only changes
> needed are that Linux's checkpatch.pl WARN() function takes
> an extra argument that ours does not.)
Really? [*]
>
> The kernel's checkpatch does not enforce "leading /* on
> a line of its own, so that part is unique to QEMU's checkpatch.
>
> Sample warning output:
>
> WARNING: Block comments use a leading /* on a separate line
> #34: FILE: hw/intc/arm_gicv3_common.c:39:
> +/* Older versions of QEMU had a bug in the handling of state save/restore
>
> Signed-off-by: Peter Maydell
> ---
> I'm still not used to the leeading-/*-on-it's-own style,
> so having checkpatch catch my lapses is handy...
Yes, please!
> I used WARN level severity mostly because Linux does.
> ---
> scripts/checkpatch.pl | 48 +++
> 1 file changed, 48 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 42e1c50dd80..18bc3c59c85 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1566,6 +1566,54 @@ sub process {
> # check we are in a valid C source file if not then ignore this hunk
> next if ($realfile !~ /\.(h|c|cpp)$/);
>
> +# Block comment styles
> +
> + # Block comments use /* on a line of its own
> + if ($rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ && #inline /*...*/
> + $rawline =~ m@^\+.*/\*\*?[ \t]*.+[ \t]*$@) { # /* or /**
> non-blank
> + WARN("Block comments use a leading /* on a separate
> line\n" . $herecurr);
> + }
> +
[*] The kernel's has
# Networking with an initial /*
if ($realfile =~ m@^(drivers/net/|net/)@ &&
$prevrawline =~ /^\+[ \t]*\/\*[ \t]*$/ &&
$rawline =~ /^\+[ \t]*\*/ &&
$realline > 2) {
WARN("NETWORKING_BLOCK_COMMENT_STYLE",
"networking block comments don't use an empty
/* line, use /* Comment...\n" . $hereprev);
}
which makes no sense for us. Your code looks good to me, but your
commit message claims it doesn't exist :)
The remainder matches the kernel's version.
> +# Block comments use * on subsequent lines
> + if ($prevline =~ /$;[ \t]*$/ && #ends in comment
> + $prevrawline =~ /^\+.*?\/\*/ && #starting /*
> + $prevrawline !~ /\*\/[ \t]*$/ &&#no trailing */
> + $rawline =~ /^\+/ &&#line is new
> + $rawline !~ /^\+[ \t]*\*/) {#no leading *
> + WARN("Block comments use * on subsequent lines\n" .
> $hereprev);
> + }
> +
> +# Block comments use */ on trailing lines
> + if ($rawline !~ m@^\+[ \t]*\*/[ \t]*$@ && #trailing */
> + $rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ && #inline /*...*/
> + $rawline !~ m@^\+.*\*{2,}/[ \t]*$@ && #trailing **/
> + $rawline =~ m@^\+[ \t]*.+\*\/[ \t]*$@) {#non blank */
> + WARN("Block comments use a trailing */ on a separate
> line\n" . $herecurr);
> + }
> +
> +# Block comment * alignment
> + if ($prevline =~ /$;[ \t]*$/ && #ends in comment
> + $line =~ /^\+[ \t]*$;/ && #leading comment
> + $rawline =~ /^\+[ \t]*\*/ &&#leading *
> + (($prevrawline =~ /^\+.*?\/\*/ && #leading /*
> + $prevrawline !~ /\*\/[ \t]*$/) || #no trailing */
> + $prevrawline =~ /^\+[ \t]*\*/)) { #leading *
> + my $oldindent;
> + $prevrawline =~ m@^\+([ \t]*/?)\*@;
> + if (defined($1)) {
> + $oldindent = expand_tabs($1);
> + } else {
> + $prevrawline =~ m@^\+(.*/?)\*@;
> + $oldindent = expand_tabs($1);
> + }
> + $rawline =~ m@^\+([ \t]*)\*@;
> + my $newindent = $1;
> + $newindent = expand_tabs($newindent);
> + if (length($oldindent) ne length($newindent)) {
> + WARN("Block comments should align the * on each
> line\n" . $hereprev);
> + }
> + }
> +
> # Check for potential 'bare' types
> my ($stat, $cond, $line_nr_next, $remain_next, $off_next,
> $realline_next);
With a corrected commit message:
Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH v2 4/4] tests/microbit-test: Check nRF51 UART functionality
On Wed, Aug 8, 2018 at 10:07 PM, Julia Suvorova wrote: > ret = g_test_run(); > > qtest_quit(global_qtest); > + > +close(sock_fd); > +rmdir(serialtmpdir); This temporary directory is leaked if the test fails. Please remove it immediately after connect().
Re: [Qemu-devel] [PATCH v3 0/5] qmp: add pmemload command
Simon Ruderich writes: > On Mon, May 21, 2018 at 05:36:39PM +0200, Simon Ruderich wrote: >> Hello, >> >> This is third version of this patch set, rebased on current >> master. >> >> As I've received no answers to [1] (and I'd prefer to keep the >> patch as is for now if possible) this doesn't include any changes >> to address the comments to [2]. > > Hello again, > > I haven't received any answers in quite some time. Is there > anything I can do to get these patches merged? Are there any > issues left to resolve? You neglected to cc: maintainers. I spotted this cry for help only by chance. To find maintainers to cc:, you can use get_maintainer.pl like this: $ scripts/get_maintainer.pl *patch scripts/get_maintainer.pl: file '-cover-letter.patch' doesn't appear to be a patch. Add -f to options? Paolo Bonzini (maintainer:Overall) Peter Crosthwaite (maintainer:Overall) Richard Henderson (maintainer:Overall) "Dr. David Alan Gilbert" (maintainer:Human Monitor (HMP)) Eric Blake (supporter:QAPI Schema) Markus Armbruster (supporter:QAPI Schema) qemu-devel@nongnu.org (open list:Overall) It's output is advice, not gospel. Use your judgement. I'm copying maintainers for you now. This should get you some review.
Re: [Qemu-devel] [PATCH v2 2/4] hw/arm/nrf51_soc: Connect UART to nRF51 SoC
On Wed, Aug 8, 2018 at 10:07 PM, Julia Suvorova wrote:
> @@ -70,6 +73,19 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error
> **errp)
> }
> memory_region_add_subregion(&s->container, SRAM_BASE, &s->sram);
>
> +/* UART */
> +qdev_prop_set_chr(DEVICE(&s->uart), "chardev", serial_hd(0));
> +object_property_set_bool(OBJECT(&s->uart), true, "realized", &err);
> +if (err) {
> +error_propagate(errp, err);
> +return;
Is there cleanup missing (e.g. unrealizing sub-devices or freeing resources)?
Re: [Qemu-devel] [PATCH v2 1/4] hw/char: Implement nRF51 SoC UART
On Wed, Aug 8, 2018 at 10:07 PM, Julia Suvorova wrote:
> +static uint64_t uart_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +NRF51UARTState *s = NRF51_UART(opaque);
> +uint64_t r;
> +
> +if (!s->enabled) {
> +return 0;
> +}
> +
> +switch (addr) {
> +case A_UART_RXD:
> +r = s->rx_fifo[s->rx_fifo_pos];
> +if (s->rx_started && s->rx_fifo_len) {
> +qemu_chr_fe_accept_input(&s->chr);
Should this be called after popping a byte from the rx fifo? That way
.can_receive() will return true again.
> +static void nrf51_uart_realize(DeviceState *dev, Error **errp)
> +{
> +NRF51UARTState *s = NRF51_UART(dev);
> +
> +qemu_chr_fe_set_handlers(&s->chr, uart_can_receive, uart_receive,
> + uart_event, NULL, s, NULL, true);
> +}
unrealize() should set the handlers to NULL. That way the device can
be removed without leaving callbacks registered.
Re: [Qemu-devel] [PATCH v7 00/80] Add nanoMIPS support to QEMU
Hi Aleksandar, On 08/06/2018 01:59 PM, Aleksandar Markovic wrote: > From: Aleksandar Markovic [snip] > This series of patches implements recently announced nanoMIPS on QEMU. > nanoMIPS is a variable length ISA containing 16, 32 and 48-bit wide > instructions. It is designed to be portable at assembly level with > other MIPS and microMIPS code, but contains a number of changes that > enhance code density and efficiency. The largest portion of patches > is nanoMIPS decoding engine. [snip] I think you can stage 1-12 for the next MIPS pull request on 3.1 cycle opens. Then respins of this series will be a bit shorter ;) Regards, Phil.
Re: [Qemu-devel] [PATCH v7 11/80] elf: Add ELF flags for MIPS machine variants
On 08/06/2018 01:59 PM, Aleksandar Markovic wrote: > From: Aleksandar Markovic > > Add MIPS machine variants ELF flags so that the emulation behavior > can be adjusted if needed. > > Reviewed-by: Laurent Vivier > Reviewed-by: Richard Henderson > Signed-off-by: Aleksandar Markovic > Signed-off-by: Stefan Markovic > --- > include/elf.h | 23 +++ > 1 file changed, 23 insertions(+) > > diff --git a/include/elf.h b/include/elf.h > index 60f6cbe..002a002 100644 > --- a/include/elf.h > +++ b/include/elf.h > @@ -62,6 +62,29 @@ typedef int64_t Elf64_Sxword; > #define EF_MIPS_FP64 0x0200 > #define EF_MIPS_NAN2008 0x0400 > > +/* MIPS machine variant */ > +#define EF_MIPS_MACH_NONE 0x /* A standard MIPS implementation > */ > +#define EF_MIPS_MACH_3900 0x0081 /* Toshiba R3900 > */ Yeah \o/ > +#define EF_MIPS_MACH_4010 0x0082 /* LSI R4010 > */ > +#define EF_MIPS_MACH_4100 0x0083 /* NEC VR4100 > */ > +#define EF_MIPS_MACH_4650 0x0085 /* MIPS R4650 > */ > +#define EF_MIPS_MACH_4120 0x0087 /* NEC VR4120 > */ > +#define EF_MIPS_MACH_4111 0x0088 /* NEC VR4111/VR4181 > */ > +#define EF_MIPS_MACH_SB1 0x008a /* Broadcom SB-1 > */ > +#define EF_MIPS_MACH_OCTEON 0x008b /* Cavium Networks Octeon > */ > +#define EF_MIPS_MACH_XLR 0x008c /* RMI Xlr > */ > +#define EF_MIPS_MACH_OCTEON2 0x008d /* Cavium Networks Octeon2 > */ > +#define EF_MIPS_MACH_OCTEON3 0x008e /* Cavium Networks Octeon3 > */ > +#define EF_MIPS_MACH_5400 0x0091 /* NEC VR5400 > */ > +#define EF_MIPS_MACH_5900 0x0092 /* MIPS R5900 > */ > +#define EF_MIPS_MACH_5500 0x0098 /* NEC VR5500 > */ > +#define EF_MIPS_MACH_9000 0x0099 /* Unknown > */ This is the PMC-Sierra's RM9000. Reviewed-by: Philippe Mathieu-Daudé > +#define EF_MIPS_MACH_LS2E 0x00a0 /* ST Microelectronics Loongson 2E > */ > +#define EF_MIPS_MACH_LS2F 0x00a1 /* ST Microelectronics Loongson 2F > */ > +#define EF_MIPS_MACH_LS3A 0x00a2 /* Loongson 3A > */ > +#define EF_MIPS_MACH 0x00ff /* EF_MIPS_MACH_xxx selection mask > */ > + > + > /* These constants define the different elf file types */ > #define ET_NONE 0 > #define ET_REL1 >
[Qemu-devel] [PATCH] hw/ppc: on 40p machine, change default firmware to OpenBIOS
OpenBIOS gained 40p support in 5b20e4cacecb62fb2bdc6867c11d44cddd77c4ff
Use it, instead of relying on an unmaintained and very limited firmware.
Signed-off-by: Hervé Poussineau
---
hw/ppc/prep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 3401570d98..1558855247 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -736,7 +736,7 @@ static void ibm_40p_init(MachineState *machine)
/* PCI host */
dev = qdev_create(NULL, "raven-pcihost");
if (!bios_name) {
-bios_name = BIOS_FILENAME;
+bios_name = "openbios-ppc";
}
qdev_prop_set_string(dev, "bios-name", bios_name);
qdev_prop_set_uint32(dev, "elf-machine", PPC_ELF_MACHINE);
--
2.11.0
Re: [Qemu-devel] [PATCH 1/2] hw/ppc: on 40p machine, change default firmware to OpenBIOS
Le 10/08/2018 à 03:04, David Gibson a écrit :
On Thu, Aug 09, 2018 at 10:40:09PM +0200, Hervé Poussineau wrote:
OpenBIOS gained 40p support in 5b20e4cacecb62fb2bdc6867c11d44cddd77c4ff
Use it, instead of relying on the presence of a proprietary
firmware.
Uh.. openhackware might be, well, a hack, but I don't think it's
propietary. Replacing it with openbios might well be a good idea, but
we want an accurrate reason for doing so.
Oops, you're right. That's what happens when you try to change/expand
your commit messages just before sending the patch. I will resend.
Signed-off-by: Hervé Poussineau
---
hw/ppc/prep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 3401570d98..1558855247 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -736,7 +736,7 @@ static void ibm_40p_init(MachineState *machine)
/* PCI host */
dev = qdev_create(NULL, "raven-pcihost");
if (!bios_name) {
-bios_name = BIOS_FILENAME;
+bios_name = "openbios-ppc";
}
qdev_prop_set_string(dev, "bios-name", bios_name);
qdev_prop_set_uint32(dev, "elf-machine", PPC_ELF_MACHINE);
Re: [Qemu-devel] [PATCH v6 5/8] qcow2: Increase the default upper limit on the L2 cache size
--- a/block/qcow2.h +++ b/block/qcow2.h @@ -73,7 +73,7 @@ /* Must be at least 4 to cover all cases of refcount table growth */ #define MIN_REFCOUNT_CACHE_SIZE 4 /* clusters */ -#define DEFAULT_L2_CACHE_MAX_SIZE 1048576 /* bytes */ +#define DEFAULT_L2_CACHE_MAX_SIZE 0x200U /* bytes */ I'd spell this as '(64 * MiB)' with the help of "qemu/units.h". You mean '(32 * MiB)', right? Good idea! Done. Leonid.
Re: [Qemu-devel] [PATCH v7 09/80] target/mips: Check ELPA flag only in some cases of MFHC0 and MTHC0
On 08/06/2018 01:59 PM, Aleksandar Markovic wrote:
> From: Yongbok Kim
>
> MFHC0 and MTHC0 used to handle EntryLo0 and EntryLo1 registers only,
> and placing ELPA flag checks before switch statement were technically
> correct. However, after adding handling more registers, these checks
> should be moved to act only in cases of handling EntryLo0 and
> EntryLo1.
>
> Reviewed-by: Aleksandar Markovic
> Reviewed-by: Richard Henderson
> Signed-off-by: Yongbok Kim
> Signed-off-by: Aleksandar Markovic
Reviewed-by: Philippe Mathieu-Daudé
> ---
> target/mips/translate.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 841c0c8..bc1f21f 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -4896,12 +4896,11 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg,
> int reg, int sel)
> {
> const char *rn = "invalid";
>
> -CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
> -
> switch (reg) {
> case 2:
> switch (sel) {
> case 0:
> +CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
> gen_mfhc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo0));
> rn = "EntryLo0";
> break;
> @@ -4912,6 +4911,7 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int
> reg, int sel)
> case 3:
> switch (sel) {
> case 0:
> +CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
> gen_mfhc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo1));
> rn = "EntryLo1";
> break;
> @@ -4964,12 +4964,11 @@ static void gen_mthc0(DisasContext *ctx, TCGv arg,
> int reg, int sel)
> const char *rn = "invalid";
> uint64_t mask = ctx->PAMask >> 36;
>
> -CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
> -
> switch (reg) {
> case 2:
> switch (sel) {
> case 0:
> +CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
> tcg_gen_andi_tl(arg, arg, mask);
> gen_mthc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo0));
> rn = "EntryLo0";
> @@ -4981,6 +4980,7 @@ static void gen_mthc0(DisasContext *ctx, TCGv arg, int
> reg, int sel)
> case 3:
> switch (sel) {
> case 0:
> +CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
> tcg_gen_andi_tl(arg, arg, mask);
> gen_mthc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo1));
> rn = "EntryLo1";
>
Re: [Qemu-devel] [PATCH v7 04/80] target/mips: Fix two instances of shadow variables
On 08/06/2018 01:59 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic
>
> Fix two instances of shadow variables. This cleans up entire file
> translate.c from shadow variables.
>
> Reviewed-by: Richard Henderson
> Signed-off-by: Aleksandar Markovic
> Signed-off-by: Stefan Markovic
Reviewed-by: Philippe Mathieu-Daudé
> ---
> target/mips/translate.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index e32fd5f..d6eccc9 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -13247,7 +13247,7 @@ static void gen_pool16c_r6_insn(DisasContext *ctx)
> gen_arith_imm(ctx, OPC_ADDIU, 29, 29, imm << 2);
> } else {
> /* JRC16 */
> -int rs = extract32(ctx->opcode, 5, 5);
> +rs = extract32(ctx->opcode, 5, 5);
> gen_compute_branch(ctx, OPC_JR, 2, rs, 0, 0, 0);
> }
> break;
> @@ -15249,7 +15249,7 @@ static void decode_micromips32_opc(CPUMIPSState *env,
> DisasContext *ctx)
> } else {
> /* ADDIUPC */
> int reg = mmreg(ZIMM(ctx->opcode, 23, 3));
> -int offset = SIMM(ctx->opcode, 0, 23) << 2;
> +offset = SIMM(ctx->opcode, 0, 23) << 2;
>
> gen_addiupc(ctx, reg, offset, 0, 0);
> }
>
Re: [Qemu-devel] [PATCH v7 03/80] target/mips: Mark switch fallthroughs with interpretable comments
On 08/06/2018 01:59 PM, Aleksandar Markovic wrote: > From: Aleksandar Markovic > > Mark switch fallthroughs with comments, in cases fallthroughs > are intentional. > > The comments "/* fall through */" are interpreted by compilers and > other tools, and they will not issue warnings in such cases. For gcc, > the warning is turnend on by -Wimplicit-fallthrough. With this patch, > there will be no such warnings in target/mips directory. If such > warning appears in future, it should be checked if it is intentional, > and, if yes, marked with a comment similar to those from this patch. > > The comment must be just before next "case", otherwise gcc won't > understand it. > > Reviewed-by: Richard Henderson > Signed-off-by: Aleksandar Markovic > Signed-off-by: Stefan Markovic Reviewed-by: Philippe Mathieu-Daudé > --- > target/mips/translate.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/target/mips/translate.c b/target/mips/translate.c > index 051dda5..e32fd5f 100644 > --- a/target/mips/translate.c > +++ b/target/mips/translate.c > @@ -14255,8 +14255,8 @@ static void decode_micromips32_opc(CPUMIPSState *env, > DisasContext *ctx) > case SDP: > check_insn(ctx, ISA_MIPS3); > check_mips_64(ctx); > -/* Fallthrough */ > #endif > +/* fall through */ > case LWP: > case SWP: > gen_ldst_pair(ctx, minor, rt, rs, SIMM(ctx->opcode, 0, 12)); > @@ -14266,8 +14266,8 @@ static void decode_micromips32_opc(CPUMIPSState *env, > DisasContext *ctx) > case SDM: > check_insn(ctx, ISA_MIPS3); > check_mips_64(ctx); > -/* Fallthrough */ > #endif > +/* fall through */ > case LWM32: > case SWM32: > gen_ldst_multiple(ctx, minor, rt, rs, SIMM(ctx->opcode, 0, 12)); > @@ -20023,6 +20023,7 @@ static void decode_opc(CPUMIPSState *env, > DisasContext *ctx) > case OPC_MTHC1: > check_cp1_enabled(ctx); > check_insn(ctx, ISA_MIPS32R2); > +/* fall through */ > case OPC_MFC1: > case OPC_CFC1: > case OPC_MTC1: >
Re: [Qemu-devel] [Qemu-arm] [PATCH 10/16] hw/dma/pl080: Allow use as embedded-struct device
On 08/10/2018 02:18 AM, Philippe Mathieu-Daudé wrote:
> On 08/09/2018 10:01 AM, Peter Maydell wrote:
>> Create a new include file for the pl081's device struct,
>> type macros, etc, so that it can be instantiated using
>> the "embedded struct" coding style.
[...]
>> +#ifndef HW_DMA_PL080_H
>> +#define HW_DMA_PL080_H
>> +
>> +#include "hw/sysbus.h"
>> +
>> +#define PL080_MAX_CHANNELS 8
>> +
>> +typedef struct {
>> +uint32_t src;
>> +uint32_t dest;
>> +uint32_t lli;
>> +uint32_t ctrl;
>> +uint32_t conf;
>> +} pl080_channel;
>> +
>> +#define TYPE_PL080 "pl080"
>> +#define TYPE_PL081 "pl081"
>> +#define PL080(obj) OBJECT_CHECK(PL080State, (obj), TYPE_PL080)
>
> The PL080() macro can stay in the source.
Oh this respect the "coding style" indeed.
Can you add the PL081() companion? Thanks.
>
> Regardless:
> Reviewed-by: Philippe Mathieu-Daudé
>
>> +
>> +typedef struct PL080State {
>> +SysBusDevice parent_obj;
>> +
>> +MemoryRegion iomem;
>> +uint8_t tc_int;
>> +uint8_t tc_mask;
>> +uint8_t err_int;
>> +uint8_t err_mask;
>> +uint32_t conf;
>> +uint32_t sync;
>> +uint32_t req_single;
>> +uint32_t req_burst;
>> +pl080_channel chan[PL080_MAX_CHANNELS];
>> +int nchannels;
>> +/* Flag to avoid recursive DMA invocations. */
>> +int running;
>> +qemu_irq irq;
>> +} PL080State;
>> +
>> +#endif
>> diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c
>> index 7724c93b8f2..0f79c2d8a6c 100644
>> --- a/hw/dma/pl080.c
>> +++ b/hw/dma/pl080.c
>> @@ -11,8 +11,8 @@
>> #include "hw/sysbus.h"
>> #include "exec/address-spaces.h"
>> #include "qemu/log.h"
>> +#include "hw/dma/pl080.h"
>>
>> -#define PL080_MAX_CHANNELS 8
>> #define PL080_CONF_E0x1
>> #define PL080_CONF_M1 0x2
>> #define PL080_CONF_M2 0x4
>> @@ -30,36 +30,6 @@
>> #define PL080_CCTRL_D 0x0200
>> #define PL080_CCTRL_S 0x0100
>>
>> -typedef struct {
>> -uint32_t src;
>> -uint32_t dest;
>> -uint32_t lli;
>> -uint32_t ctrl;
>> -uint32_t conf;
>> -} pl080_channel;
>> -
>> -#define TYPE_PL080 "pl080"
>> -#define PL080(obj) OBJECT_CHECK(PL080State, (obj), TYPE_PL080)
>> -
>> -typedef struct PL080State {
>> -SysBusDevice parent_obj;
>> -
>> -MemoryRegion iomem;
>> -uint8_t tc_int;
>> -uint8_t tc_mask;
>> -uint8_t err_int;
>> -uint8_t err_mask;
>> -uint32_t conf;
>> -uint32_t sync;
>> -uint32_t req_single;
>> -uint32_t req_burst;
>> -pl080_channel chan[PL080_MAX_CHANNELS];
>> -int nchannels;
>> -/* Flag to avoid recursive DMA invocations. */
>> -int running;
>> -qemu_irq irq;
>> -} PL080State;
>> -
>> static const VMStateDescription vmstate_pl080_channel = {
>> .name = "pl080_channel",
>> .version_id = 1,
>> @@ -408,7 +378,7 @@ static const TypeInfo pl080_info = {
>> };
>>
>> static const TypeInfo pl081_info = {
>> -.name = "pl081",
>> +.name = TYPE_PL081,
>> .parent= TYPE_PL080,
>> .instance_init = pl081_init,
>> };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 5d1a3645dd4..92ccca716c6 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -444,6 +444,7 @@ F: hw/char/pl011.c
>> F: include/hw/char/pl011.h
>> F: hw/display/pl110*
>> F: hw/dma/pl080.c
>> +F: include/hw/dma/pl080.h
>> F: hw/dma/pl330.c
>> F: hw/gpio/pl061.c
>> F: hw/input/pl050.c
>>
Re: [Qemu-devel] [Qemu-arm] [PATCH 10/16] hw/dma/pl080: Allow use as embedded-struct device
On 08/09/2018 10:01 AM, Peter Maydell wrote:
> Create a new include file for the pl081's device struct,
> type macros, etc, so that it can be instantiated using
> the "embedded struct" coding style.
>
> Signed-off-by: Peter Maydell
> ---
> include/hw/dma/pl080.h | 62 ++
> hw/dma/pl080.c | 34 ++-
> MAINTAINERS| 1 +
> 3 files changed, 65 insertions(+), 32 deletions(-)
> create mode 100644 include/hw/dma/pl080.h
>
> diff --git a/include/hw/dma/pl080.h b/include/hw/dma/pl080.h
> new file mode 100644
> index 000..7deb46c8578
> --- /dev/null
> +++ b/include/hw/dma/pl080.h
> @@ -0,0 +1,62 @@
> +/*
> + * ARM PrimeCell PL080/PL081 DMA controller
> + *
> + * Copyright (c) 2006 CodeSourcery.
> + * Copyright (c) 2018 Linaro Limited
> + * Written by Paul Brook, Peter Maydell
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 or
> + * (at your option) any later version.
> + */
> +
> +/* This is a model of the Arm PrimeCell PL080/PL081 DMA controller:
> + * The PL080 TRM is:
> + * http://infocenter.arm.com/help/topic/com.arm.doc.ddi0196g/DDI0196.pdf
> + * and the PL081 TRM is:
> + * http://infocenter.arm.com/help/topic/com.arm.doc.ddi0218e/DDI0218.pdf
> + *
> + * QEMU interface:
> + * + sysbus IRQ: DMACINTR combined interrupt line
> + * + sysbus MMIO region 0: MemoryRegion for the device's registers
> + */
> +
> +#ifndef HW_DMA_PL080_H
> +#define HW_DMA_PL080_H
> +
> +#include "hw/sysbus.h"
> +
> +#define PL080_MAX_CHANNELS 8
> +
> +typedef struct {
> +uint32_t src;
> +uint32_t dest;
> +uint32_t lli;
> +uint32_t ctrl;
> +uint32_t conf;
> +} pl080_channel;
> +
> +#define TYPE_PL080 "pl080"
> +#define TYPE_PL081 "pl081"
> +#define PL080(obj) OBJECT_CHECK(PL080State, (obj), TYPE_PL080)
The PL080() macro can stay in the source.
Regardless:
Reviewed-by: Philippe Mathieu-Daudé
> +
> +typedef struct PL080State {
> +SysBusDevice parent_obj;
> +
> +MemoryRegion iomem;
> +uint8_t tc_int;
> +uint8_t tc_mask;
> +uint8_t err_int;
> +uint8_t err_mask;
> +uint32_t conf;
> +uint32_t sync;
> +uint32_t req_single;
> +uint32_t req_burst;
> +pl080_channel chan[PL080_MAX_CHANNELS];
> +int nchannels;
> +/* Flag to avoid recursive DMA invocations. */
> +int running;
> +qemu_irq irq;
> +} PL080State;
> +
> +#endif
> diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c
> index 7724c93b8f2..0f79c2d8a6c 100644
> --- a/hw/dma/pl080.c
> +++ b/hw/dma/pl080.c
> @@ -11,8 +11,8 @@
> #include "hw/sysbus.h"
> #include "exec/address-spaces.h"
> #include "qemu/log.h"
> +#include "hw/dma/pl080.h"
>
> -#define PL080_MAX_CHANNELS 8
> #define PL080_CONF_E0x1
> #define PL080_CONF_M1 0x2
> #define PL080_CONF_M2 0x4
> @@ -30,36 +30,6 @@
> #define PL080_CCTRL_D 0x0200
> #define PL080_CCTRL_S 0x0100
>
> -typedef struct {
> -uint32_t src;
> -uint32_t dest;
> -uint32_t lli;
> -uint32_t ctrl;
> -uint32_t conf;
> -} pl080_channel;
> -
> -#define TYPE_PL080 "pl080"
> -#define PL080(obj) OBJECT_CHECK(PL080State, (obj), TYPE_PL080)
> -
> -typedef struct PL080State {
> -SysBusDevice parent_obj;
> -
> -MemoryRegion iomem;
> -uint8_t tc_int;
> -uint8_t tc_mask;
> -uint8_t err_int;
> -uint8_t err_mask;
> -uint32_t conf;
> -uint32_t sync;
> -uint32_t req_single;
> -uint32_t req_burst;
> -pl080_channel chan[PL080_MAX_CHANNELS];
> -int nchannels;
> -/* Flag to avoid recursive DMA invocations. */
> -int running;
> -qemu_irq irq;
> -} PL080State;
> -
> static const VMStateDescription vmstate_pl080_channel = {
> .name = "pl080_channel",
> .version_id = 1,
> @@ -408,7 +378,7 @@ static const TypeInfo pl080_info = {
> };
>
> static const TypeInfo pl081_info = {
> -.name = "pl081",
> +.name = TYPE_PL081,
> .parent= TYPE_PL080,
> .instance_init = pl081_init,
> };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5d1a3645dd4..92ccca716c6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -444,6 +444,7 @@ F: hw/char/pl011.c
> F: include/hw/char/pl011.h
> F: hw/display/pl110*
> F: hw/dma/pl080.c
> +F: include/hw/dma/pl080.h
> F: hw/dma/pl330.c
> F: hw/gpio/pl061.c
> F: hw/input/pl050.c
>
Re: [Qemu-devel] [Qemu-arm] [PATCH 15/16] hw/dma/pl080: Remove hw_error() if DMA is enabled
On 08/09/2018 10:01 AM, Peter Maydell wrote:
> The PL08x model currently will unconditionally call hw_error()
> if the DMA engine is enabled by the guest. This has been
> present since the PL080 model was edded in 2006, and is
> presumably either unintentional debug code left enabled,
> or a guard against untested DMA engine code being used.
>
> Remove the hw_error(), since we now have a guest which
> will actually try to use the DMA engine (the self-test
> binary for the AN505 MPS2 FPGA image).
>
> Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> hw/dma/pl080.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c
> index 8f92550392b..ef15d3e628d 100644
> --- a/hw/dma/pl080.c
> +++ b/hw/dma/pl080.c
> @@ -110,7 +110,6 @@ static void pl080_run(PL080State *s)
> if ((s->conf & PL080_CONF_E) == 0)
> return;
>
> -hw_error("DMA active\n");
> /* If we are already in the middle of a DMA operation then indicate that
> there may be new DMA requests and return immediately. */
> if (s->running) {
>
Re: [Qemu-devel] [Qemu-arm] [PATCH 13/16] hw/dma/pl080: Provide device reset function
On 08/09/2018 10:01 AM, Peter Maydell wrote:
> The PL080/PL081 model is missing a reset function; implement it.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> hw/dma/pl080.c | 25 +
> 1 file changed, 25 insertions(+)
>
> diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c
> index 8f9f3e08d9a..a7aacad74f0 100644
> --- a/hw/dma/pl080.c
> +++ b/hw/dma/pl080.c
> @@ -348,6 +348,30 @@ static const MemoryRegionOps pl080_ops = {
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> +static void pl080_reset(DeviceState *dev)
> +{
> +PL080State *s = PL080(dev);
> +int i;
> +
> +s->tc_int = 0;
> +s->tc_mask = 0;
> +s->err_int = 0;
> +s->err_mask = 0;
> +s->conf = 0;
> +s->sync = 0;
> +s->req_single = 0;
> +s->req_burst = 0;
> +s->running = 0;
> +
> +for (i = 0; i < s->nchannels; i++) {
> +s->chan[i].src = 0;
> +s->chan[i].dest = 0;
> +s->chan[i].lli = 0;
> +s->chan[i].ctrl = 0;
> +s->chan[i].conf = 0;
> +}
> +}
> +
> static void pl080_init(Object *obj)
> {
> SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> @@ -393,6 +417,7 @@ static void pl080_class_init(ObjectClass *oc, void *data)
> dc->vmsd = &vmstate_pl080;
> dc->realize = pl080_realize;
> dc->props = pl080_properties;
> +dc->reset = pl080_reset;
> }
>
> static const TypeInfo pl080_info = {
>
Re: [Qemu-devel] [Qemu-arm] [PATCH 12/16] hw/dma/pl080: Don't use CPU address space for DMA accesses
On 08/09/2018 10:01 AM, Peter Maydell wrote:
> Currently our PL080/PL081 model uses a combination of the CPU's
> address space (via cpu_physical_memory_{read,write}()) and the
> system address space for performing DMA accesses.
>
> For the PL081s in the MPS FPGA images, their DMA accesses
> must go via Master Security Controllers. Switch the
> PL080/PL081 model to take a MemoryRegion property which
> defines its downstream for making DMA accesses.
>
> Since the PL08x are only used in two board models, we
> make provision of the 'downstream' link mandatory and convert
> both users at once, rather than having it be optional with
> a default to the system address space.
>
> Signed-off-by: Peter Maydell
> ---
> include/hw/dma/pl080.h | 5 +
> hw/arm/realview.c | 8 +++-
> hw/arm/versatilepb.c | 9 -
> hw/dma/pl080.c | 35 +--
> 4 files changed, 49 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/dma/pl080.h b/include/hw/dma/pl080.h
> index 7c6a4184833..9d4b3df143f 100644
> --- a/include/hw/dma/pl080.h
> +++ b/include/hw/dma/pl080.h
> @@ -21,6 +21,8 @@
> * + sysbus IRQ 1: DMACINTERR error interrupt request
> * + sysbus IRQ 2: DMACINTTC count interrupt request
> * + sysbus MMIO region 0: MemoryRegion for the device's registers
> + * + QOM property "downstream": MemoryRegion defining where DMA
> + * bus master transactions are made
> */
>
> #ifndef HW_DMA_PL080_H
> @@ -61,6 +63,9 @@ typedef struct PL080State {
> qemu_irq irq;
> qemu_irq interr;
> qemu_irq inttc;
> +
> +MemoryRegion *downstream;
> +AddressSpace downstream_as;
> } PL080State;
>
> #endif
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index cd585d94694..ab8c14fde38 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -201,7 +201,13 @@ static void realview_init(MachineState *machine,
> pl011_create(0x1000c000, pic[15], serial_hd(3));
>
> /* DMA controller is optional, apparently. */
> -sysbus_create_simple("pl081", 0x1003, pic[24]);
> +dev = qdev_create(NULL, "pl081");
> +object_property_set_link(OBJECT(dev), OBJECT(sysmem), "downstream",
> + &error_fatal);
Nice!
Reviewed-by: Philippe Mathieu-Daudé
> +qdev_init_nofail(dev);
> +busdev = SYS_BUS_DEVICE(dev);
> +sysbus_mmio_map(busdev, 0, 0x1003);
> +sysbus_connect_irq(busdev, 0, pic[24]);
>
> sysbus_create_simple("sp804", 0x10011000, pic[4]);
> sysbus_create_simple("sp804", 0x10012000, pic[5]);
> diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
> index a5a06b6d408..8b748570596 100644
> --- a/hw/arm/versatilepb.c
> +++ b/hw/arm/versatilepb.c
> @@ -287,7 +287,14 @@ static void versatile_init(MachineState *machine, int
> board_id)
> pl011_create(0x101f3000, pic[14], serial_hd(2));
> pl011_create(0x10009000, sic[6], serial_hd(3));
>
> -sysbus_create_simple("pl080", 0x1013, pic[17]);
> +dev = qdev_create(NULL, "pl080");
> +object_property_set_link(OBJECT(dev), OBJECT(sysmem), "downstream",
> + &error_fatal);
> +qdev_init_nofail(dev);
> +busdev = SYS_BUS_DEVICE(dev);
> +sysbus_mmio_map(busdev, 0, 0x1013);
> +sysbus_connect_irq(busdev, 0, pic[17]);
> +
> sysbus_create_simple("sp804", 0x101e2000, pic[4]);
> sysbus_create_simple("sp804", 0x101e3000, pic[5]);
>
> diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c
> index 301030dd118..8f9f3e08d9a 100644
> --- a/hw/dma/pl080.c
> +++ b/hw/dma/pl080.c
> @@ -12,6 +12,7 @@
> #include "exec/address-spaces.h"
> #include "qemu/log.h"
> #include "hw/dma/pl080.h"
> +#include "qapi/error.h"
>
> #define PL080_CONF_E0x1
> #define PL080_CONF_M1 0x2
> @@ -161,14 +162,16 @@ again:
> swidth = 1 << ((ch->ctrl >> 18) & 7);
> dwidth = 1 << ((ch->ctrl >> 21) & 7);
> for (n = 0; n < dwidth; n+= swidth) {
> -cpu_physical_memory_read(ch->src, buff + n, swidth);
> +address_space_read(&s->downstream_as, ch->src,
> + MEMTXATTRS_UNSPECIFIED, buff + n, swidth);
> if (ch->ctrl & PL080_CCTRL_SI)
> ch->src += swidth;
> }
> xsize = (dwidth < swidth) ? swidth : dwidth;
> /* ??? This may pad the value incorrectly for dwidth < 32. */
> for (n = 0; n < xsize; n += dwidth) {
> -cpu_physical_memory_write(ch->dest + n, buff + n, dwidth);
> +address_space_write(&s->downstream_as, ch->dest + n,
> +MEMTXATTRS_UNSPECIFIED, buff + n,
> dwidth);
> if (ch->ctrl & PL080_CCTRL_DI)
> ch->dest += swidth;
> }
> @@ -178,19 +181,19 @@ again:
> if (size == 0) {
> /* Transfer complete. */
> if (ch->lli) {
> -ch-
Re: [Qemu-devel] [Qemu-arm] [PATCH 02/16] nvic: Expose NMI line
On 08/09/2018 10:01 AM, Peter Maydell wrote:
> On real v7M hardware, the NMI line is an externally visible signal
> that an SoC or board can toggle to assert an NMI. Expose it in
> our QEMU NVIC and armv7m container objects so that a board model
> can wire it up if it needs to.
>
> In particular, the MPS2 watchdog is wired to NMI.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> hw/arm/armv7m.c | 1 +
> hw/intc/armv7m_nvic.c | 19 +++
> hw/intc/trace-events | 1 +
> 3 files changed, 21 insertions(+)
>
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 6b076660574..66217a6053a 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -202,6 +202,7 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
> */
> qdev_pass_gpios(DEVICE(&s->nvic), dev, NULL);
> qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
> +qdev_pass_gpios(DEVICE(&s->nvic), dev, "NMI");
>
> /* Wire the NVIC up to the CPU */
> sbd = SYS_BUS_DEVICE(&s->nvic);
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index cd1e7f17299..be7771e9d1f 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -772,6 +772,24 @@ static void set_irq_level(void *opaque, int n, int level)
> }
> }
>
> +/* callback when external NMI line is changed */
> +static void nvic_nmi_trigger(void *opaque, int n, int level)
> +{
> +NVICState *s = opaque;
> +
> +trace_nvic_set_nmi_level(level);
> +
> +/*
> + * The architecture doesn't specify whether NMI should share
> + * the normal-interrupt behaviour of being resampled on
> + * exception handler return. We choose not to, so just
> + * set NMI pending here and don't track the current level.
> + */
> +if (level) {
> +armv7m_nvic_set_pending(s, ARMV7M_EXCP_NMI, false);
> +}
> +}
> +
> static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs)
> {
> ARMCPU *cpu = s->cpu;
> @@ -2310,6 +2328,7 @@ static void armv7m_nvic_instance_init(Object *obj)
> qdev_init_gpio_out_named(dev, &nvic->sysresetreq, "SYSRESETREQ", 1);
> qdev_init_gpio_in_named(dev, nvic_systick_trigger, "systick-trigger",
> M_REG_NUM_BANKS);
> +qdev_init_gpio_in_named(dev, nvic_nmi_trigger, "NMI", 1);
> }
>
> static void armv7m_nvic_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/intc/trace-events b/hw/intc/trace-events
> index 5fb18e65c97..33e932fb918 100644
> --- a/hw/intc/trace-events
> +++ b/hw/intc/trace-events
> @@ -184,6 +184,7 @@ nvic_acknowledge_irq(int irq, int prio) "NVIC acknowledge
> IRQ: %d now active (pr
> nvic_get_pending_irq_info(int irq, bool secure) "NVIC next IRQ %d:
> targets_secure: %d"
> nvic_complete_irq(int irq, bool secure) "NVIC complete IRQ %d (secure %d)"
> nvic_set_irq_level(int irq, int level) "NVIC external irq %d level set to %d"
> +nvic_set_nmi_level(int level) "NVIC external NMI level set to %d"
> nvic_sysreg_read(uint64_t addr, uint32_t value, unsigned size) "NVIC sysreg
> read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
> nvic_sysreg_write(uint64_t addr, uint32_t value, unsigned size) "NVIC sysreg
> write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
>
>
Re: [Qemu-devel] [Qemu-arm] [PATCH v4 5/6] loader: Implement .hex file loader
Hi Su,
On 08/03/2018 11:47 AM, Stefan Hajnoczi wrote:
> From: Su Hang
>
> This patch adds Intel Hexadecimal Object File format support to the
> generic loader device. The file format specification is available here:
> http://www.piclist.com/techref/fileext/hex/intel.htm
>
> This file format is often used with microcontrollers such as the
> micro:bit, Arduino, STM32, etc. Users expect to be able to run .hex
> files directly with without first converting them to ELF. Most
> micro:bit code is developed in web-based IDEs without direct user access
> to binutils so it is important for QEMU to handle this file format
> natively.
>
> Signed-off-by: Su Hang
> Signed-off-by: Stefan Hajnoczi
> ---
> include/hw/loader.h | 12 ++
> hw/core/generic-loader.c | 4 +
> hw/core/loader.c | 243 +++
> 3 files changed, 259 insertions(+)
>
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 5235f119a3..3c112975f4 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -28,6 +28,18 @@ ssize_t load_image_size(const char *filename, void *addr,
> size_t size);
> int load_image_targphys_as(const char *filename,
> hwaddr addr, uint64_t max_sz, AddressSpace *as);
>
> +/**load_targphys_hex_as:
> + * @filename: Path to the .hex file
> + * @entry: Store the entry point given by the .hex file
> + * @as: The AddressSpace to load the .hex file to. The value of
> + * address_space_memory is used if nothing is supplied here.
> + *
> + * Load a fixed .hex file into memory.
> + *
> + * Returns the size of the loaded .hex file on success, -1 otherwise.
> + */
> +int load_targphys_hex_as(const char *filename, hwaddr *entry, AddressSpace
> *as);
> +
> /** load_image_targphys:
> * Same as load_image_targphys_as(), but doesn't allow the caller to specify
> * an AddressSpace.
> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
> index cb0e68486d..fde32cbda1 100644
> --- a/hw/core/generic-loader.c
> +++ b/hw/core/generic-loader.c
> @@ -147,6 +147,10 @@ static void generic_loader_realize(DeviceState *dev,
> Error **errp)
> size = load_uimage_as(s->file, &entry, NULL, NULL, NULL,
> NULL,
>as);
> }
> +
> +if (size < 0) {
> +size = load_targphys_hex_as(s->file, &entry, as);
> +}
> }
>
> if (size < 0 || s->force_raw) {
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 612420b870..072bf8b434 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -1321,3 +1321,246 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict)
> }
> }
> }
> +
> +typedef enum HexRecord HexRecord;
> +enum HexRecord {
> +DATA_RECORD = 0,
> +EOF_RECORD,
> +EXT_SEG_ADDR_RECORD,
> +START_SEG_ADDR_RECORD,
> +EXT_LINEAR_ADDR_RECORD,
> +START_LINEAR_ADDR_RECORD,
> +};
> +
> +#define DATA_FIELD_MAX_LEN 0xff
> +#define LEN_EXCEPT_DATA 0x5
> +/* 0x5 = sizeof(byte_count) + sizeof(address) + sizeof(record_type) +
> + * sizeof(checksum) */
> +typedef struct {
> +uint8_t byte_count;
> +uint16_t address;
> +uint8_t record_type;
> +uint8_t data[DATA_FIELD_MAX_LEN];
> +uint8_t checksum;
> +} HexLine;
> +
> +/* return 0 or -1 if error */
> +static bool parse_record(HexLine *line, uint8_t *our_checksum, const uint8_t
> c,
> + uint32_t *index, const bool in_process)
> +{
> +/* +---+---+---+-++
> + * | byte | |record | ||
> + * | count |address| type |data |checksum|
> + * +---+---+---+-++
> + * ^ ^ ^ ^ ^^
> + * |1 byte |2 bytes|1 byte | 0-255 bytes | 1 byte |
> + */
> +uint8_t value = 0;
> +uint32_t idx = *index;
> +/* ignore space */
> +if (g_ascii_isspace(c)) {
> +return true;
> +}
> +if (!g_ascii_isxdigit(c) || !in_process) {
> +return false;
> +}
> +value = g_ascii_xdigit_value(c);
> +value = idx & 0x1 ? value & 0xf : value << 4;
This construction is slightly easier to read as:
value = (idx & 0x1) ? (value & 0xf) : (value << 4);
> +if (idx < 2) {
> +line->byte_count |= value;
> +} else if (2 <= idx && idx < 6) {
> +line->address <<= 4;
> +line->address += g_ascii_xdigit_value(c);
> +} else if (6 <= idx && idx < 8) {
> +line->record_type |= value;
> +} else if (8 <= idx && idx < 8 + 2 * line->byte_count) {
> +line->data[(idx - 8) >> 1] |= value;
> +} else if (8 + 2 * line->byte_count <= idx &&
> + idx < 10 + 2 * line->byte_count) {
> +line->checksum |= value;
> +} else {
> +return false;
> +}
> +*our_chec
Re: [Qemu-devel] [Qemu-arm] [PATCH v4 6/6] Add QTest testcase for the Intel Hexadecimal
Hi Su,
On 08/03/2018 11:47 AM, Stefan Hajnoczi wrote:
> From: Su Hang
>
> 'test.hex' file is a bare metal ARM software stored in Hexadecimal
> Object Format. When it's loaded by QEMU, it will print "Hello world!\n"
> on console.
... on the console of the Emcraft SmartFusion2 machine.
>
> `pre_store` array in 'hexloader-test.c' file, stores the binary format
> of 'test.hex' file, which is used to verify correctness.
>
> Reviewed-by: Stefan Hajnoczi
> Suggested-by: Steffen Gortz
> Suggested-by: Stefan Hajnoczi
> Signed-off-by: Su Hang
> Signed-off-by: Stefan Hajnoczi
> ---
> MAINTAINERS | 6 +++
> configure| 4 ++
> tests/Makefile.include | 2 +
> tests/hexloader-test.c | 60
> tests/hex-loader-check-data/test.hex | 12 ++
> 5 files changed, 84 insertions(+)
> create mode 100644 tests/hexloader-test.c
> create mode 100644 tests/hex-loader-check-data/test.hex
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 666e936812..c48d9271cf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1323,6 +1323,12 @@ F: hw/core/generic-loader.c
> F: include/hw/core/generic-loader.h
> F: docs/generic-loader.txt
>
> +Intel Hexadecimal Object File Loader
> +M: Su Hang
> +S: Maintained
> +F: tests/hexloader-test.c
> +F: tests/hex-loader-check-data/test.hex
> +
> CHRP NVRAM
> M: Thomas Huth
> S: Maintained
> diff --git a/configure b/configure
> index 2a7796ea80..db97930314 100755
> --- a/configure
> +++ b/configure
> @@ -7382,6 +7382,10 @@ for test_file in $(find
> $source_path/tests/acpi-test-data -type f)
> do
> FILES="$FILES tests/acpi-test-data$(echo $test_file | sed -e
> 's/.*acpi-test-data//')"
> done
> +for test_file in $(find $source_path/tests/hex-loader-check-data -type f)
> +do
> +FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed -e
> 's/.*hex-loader-check-data//')"
> +done
> mkdir -p $DIRS
> for f in $FILES ; do
> if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; then
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index a49282704e..760a0f18b6 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -386,6 +386,7 @@ check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
> gcov-files-arm-y += hw/timer/arm_mptimer.c
> check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
> check-qtest-arm-y += tests/sdhci-test$(EXESUF)
> +check-qtest-arm-y += tests/hexloader-test$(EXESUF)
>
> check-qtest-aarch64-y = tests/numa-test$(EXESUF)
> check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
> @@ -773,6 +774,7 @@ tests/qmp-test$(EXESUF): tests/qmp-test.o
> tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
> tests/rtc-test$(EXESUF): tests/rtc-test.o
> tests/m48t59-test$(EXESUF): tests/m48t59-test.o
> +tests/hexloader-test$(EXESUF): tests/hexloader-test.o
> tests/endianness-test$(EXESUF): tests/endianness-test.o
> tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o $(libqos-obj-y)
> tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
> diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
> new file mode 100644
> index 00..8818bd45f8
> --- /dev/null
> +++ b/tests/hexloader-test.c
> @@ -0,0 +1,60 @@
> +/*
> + * QTest testcase for the Intel Hexadecimal Object File Loader
> + *
> + * Authors:
> + * Su Hang 2018
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +
> +#define BIN_SIZE 146
This size is not needed, just use sizeof().
> +
> +static unsigned char pre_store[BIN_SIZE] = {
I'd call this blob 'emcraft_sf2_uart_hello'.
It can also be const:
static const unsigned char emcraft_sf2_uart_hello[] = {
...
Can you add a comment providing the .S asm source and how to
build/generate this blob? (I think this is required to respect the GPLv2
license in your header).
> + 0x04, 0xd0, 0x9f, 0xe5, 0x16, 0x00, 0x00, 0xeb, 0xfe, 0xff, 0xff, 0xea,
> + 0x98, 0x10, 0x01, 0x00, 0x04, 0xb0, 0x2d, 0xe5, 0x00, 0xb0, 0x8d, 0xe2,
> + 0x0c, 0xd0, 0x4d, 0xe2, 0x08, 0x00, 0x0b, 0xe5, 0x06, 0x00, 0x00, 0xea,
> + 0x08, 0x30, 0x1b, 0xe5, 0x00, 0x20, 0xd3, 0xe5, 0x2c, 0x30, 0x9f, 0xe5,
> + 0x00, 0x20, 0x83, 0xe5, 0x08, 0x30, 0x1b, 0xe5, 0x01, 0x30, 0x83, 0xe2,
> + 0x08, 0x30, 0x0b, 0xe5, 0x08, 0x30, 0x1b, 0xe5, 0x00, 0x30, 0xd3, 0xe5,
> + 0x00, 0x00, 0x53, 0xe3, 0xf4, 0xff, 0xff, 0x1a, 0x00, 0x00, 0xa0, 0xe1,
> + 0x00, 0xd0, 0x8b, 0xe2, 0x04, 0xb0, 0x9d, 0xe4, 0x1e, 0xff, 0x2f, 0xe1,
> + 0x00, 0x10, 0x1f, 0x10, 0x00, 0x48, 0x2d, 0xe9, 0x04, 0xb0, 0x8d, 0xe2,
> + 0x08, 0x00, 0x9f, 0xe5, 0xe6, 0xff, 0xff, 0xeb, 0x00, 0x00, 0xa0, 0xe1,
> + 0x00, 0x88, 0xbd, 0xe8, 0x84, 0x00, 0x01, 0x00, 0x00, 0x10, 0x1f, 0x10,
> + 'H', 'e', 'l', 'l','o', ' ', 'w', '
[Qemu-devel] [PATCH 3/4] dmg: including dmg-lzfse module inside dmg block driver.
This commit includes the support to new module dmg-lzfse into dmg block
driver. It includes the support for block type ULFO (0x8007).
Signed-off-by: Julio Faracco
---
block/dmg.c | 28
block/dmg.h | 3 +++
2 files changed, 31 insertions(+)
diff --git a/block/dmg.c b/block/dmg.c
index c9b3c519c4..390ab67e53 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -33,6 +33,9 @@
int (*dmg_uncompress_bz2)(char *next_in, unsigned int avail_in,
char *next_out, unsigned int avail_out);
+int (*dmg_uncompress_lzfse)(char *next_in, unsigned int avail_in,
+char *next_out, unsigned int avail_out);
+
enum {
/* Limit chunk sizes to prevent unreasonable amounts of memory being used
* or truncating when converting to 32-bit types
@@ -107,6 +110,7 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t
chunk,
switch (s->types[chunk]) {
case 0x8005: /* zlib compressed */
case 0x8006: /* bzip2 compressed */
+case 0x8007: /* lzfse compressed */
compressed_size = s->lengths[chunk];
uncompressed_sectors = s->sectorcounts[chunk];
break;
@@ -188,6 +192,8 @@ static bool dmg_is_known_block_type(uint32_t entry_type)
return true;
case 0x8006:/* bzip2 */
return !!dmg_uncompress_bz2;
+case 0x8007:/* lzfse */
+ return !!dmg_uncompress_lzfse;
default:
return false;
}
@@ -431,6 +437,7 @@ static int dmg_open(BlockDriverState *bs, QDict *options,
int flags,
}
block_module_load_one("dmg-bz2");
+block_module_load_one("dmg-lzfse");
s->n_chunks = 0;
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
@@ -629,6 +636,27 @@ static inline int dmg_read_chunk(BlockDriverState *bs,
uint64_t sector_num)
return ret;
}
break;
+ case 0x8007:
+ if (!dmg_uncompress_lzfse) {
+break;
+}
+/* we need to buffer, because only the chunk as whole can be
+ * inflated. */
+ret = bdrv_pread(bs->file, s->offsets[chunk],
+ s->compressed_chunk, s->lengths[chunk]);
+if (ret != s->lengths[chunk]) {
+return -1;
+}
+
+ret = dmg_uncompress_lzfse((char *)s->compressed_chunk,
+ (unsigned int) s->lengths[chunk],
+ (char *)s->uncompressed_chunk,
+ (unsigned int)
+ (512 * s->sectorcounts[chunk]));
+if (ret < 0) {
+return ret;
+}
+break;
case 1: /* copy */
ret = bdrv_pread(bs->file, s->offsets[chunk],
s->uncompressed_chunk, s->lengths[chunk]);
diff --git a/block/dmg.h b/block/dmg.h
index 2ecf239ba5..f28929998f 100644
--- a/block/dmg.h
+++ b/block/dmg.h
@@ -55,4 +55,7 @@ typedef struct BDRVDMGState {
extern int (*dmg_uncompress_bz2)(char *next_in, unsigned int avail_in,
char *next_out, unsigned int avail_out);
+extern int (*dmg_uncompress_lzfse)(char *next_in, unsigned int avail_in,
+ char *next_out, unsigned int avail_out);
+
#endif
--
2.17.1
[Qemu-devel] [PATCH 2/4] configure: adding support to lzfse library.
This commit includes the support to lzfse opensource library. With this
library dmg block driver can decompress images with this type of
compression inside.
Signed-off-by: Julio Faracco
---
block/Makefile.objs | 2 ++
configure | 32
2 files changed, 34 insertions(+)
diff --git a/block/Makefile.objs b/block/Makefile.objs
index c8337bf186..f4ddbb9c7b 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -47,6 +47,8 @@ ssh.o-cflags := $(LIBSSH2_CFLAGS)
ssh.o-libs := $(LIBSSH2_LIBS)
block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o
dmg-bz2.o-libs := $(BZIP2_LIBS)
+block-obj-$(if $(CONFIG_LZFSE),m,n) += dmg-lzfse.o
+dmg-lzfse.o-libs := $(LZFSE_LIBS)
qcow.o-libs:= -lz
linux-aio.o-libs := -laio
parallels.o-cflags := $(LIBXML2_CFLAGS)
diff --git a/configure b/configure
index 2a7796ea80..b12a16f2bf 100755
--- a/configure
+++ b/configure
@@ -432,6 +432,7 @@ capstone=""
lzo=""
snappy=""
bzip2=""
+lzfse=""
guest_agent=""
guest_agent_with_vss="no"
guest_agent_ntddscsi="no"
@@ -1300,6 +1301,10 @@ for opt do
;;
--enable-bzip2) bzip2="yes"
;;
+ --enable-lzfse) lzfse="yes"
+ ;;
+ --disable-lzfse) lzfse="no"
+ ;;
--enable-guest-agent) guest_agent="yes"
;;
--disable-guest-agent) guest_agent="no"
@@ -1689,6 +1694,8 @@ disabled with --disable-FEATURE, default is enabled if
available:
snappy support of snappy compression library
bzip2 support of bzip2 compression library
(for reading bzip2-compressed dmg images)
+ lzfse support of lzfse compression library
+ (for reading lzfse-compressed dmg images)
seccomp seccomp support
coroutine-pool coroutine freelist (better performance)
glusterfs GlusterFS backend
@@ -2213,6 +2220,25 @@ EOF
fi
fi
+##
+# lzfse check
+
+if test "$lzfse" != "no" ; then
+cat > $TMPC << EOF
+#include
+int main(void) { lzfse_decode_scratch_size(); return 0; }
+EOF
+if compile_prog "" "-llzfse" ; then
+libs_softmmu="$libs_softmmu -llzfse"
+lzfse="yes"
+else
+if test "$lzfse" = "yes"; then
+feature_not_found "lzfse" "Install lzfse devel"
+fi
+lzfse="no"
+fi
+fi
+
##
# libseccomp check
@@ -6001,6 +6027,7 @@ echo "Live block migration $live_block_migration"
echo "lzo support $lzo"
echo "snappy support$snappy"
echo "bzip2 support $bzip2"
+echo "lzfse support $lzfse"
echo "NUMA host support $numa"
echo "libxml2 $libxml2"
echo "tcmalloc support $tcmalloc"
@@ -6525,6 +6552,11 @@ if test "$bzip2" = "yes" ; then
echo "BZIP2_LIBS=-lbz2" >> $config_host_mak
fi
+if test "$lzfse" = "yes" ; then
+ echo "CONFIG_LZFSE=y" >> $config_host_mak
+ echo "LZFSE_LIBS=-llzfse" >> $config_host_mak
+fi
+
if test "$libiscsi" = "yes" ; then
echo "CONFIG_LIBISCSI=m" >> $config_host_mak
echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak
--
2.17.1
[Qemu-devel] [PATCH 4/4] dmg: exchanging hardcoded dmg UDIF block types to enum.
This change is better to understand what kind of block type is being
handled by the code. Using a syntax similar to the DMG documentation is
easier than tracking all hex values assigned to a block type.
Signed-off-by: Julio Faracco
---
block/dmg.c | 43 ---
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 390ab67e53..ae379adb20 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -44,6 +44,19 @@ enum {
DMG_SECTORCOUNTS_MAX = DMG_LENGTHS_MAX / 512,
};
+enum {
+/* DMG Block Type */
+UDZE=0, /* Zeroes */
+UDRW, /* RAW type */
+UDIG, /* Ignore */
+UDCO=0x8004,
+UDZO,
+UDBZ,
+ULFO,
+UDCM=0x7ffe, /* Comments */
+UDLE /* Last Entry */
+};
+
static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
{
int len;
@@ -108,16 +121,16 @@ static void update_max_chunk_size(BDRVDMGState *s,
uint32_t chunk,
uint32_t uncompressed_sectors = 0;
switch (s->types[chunk]) {
-case 0x8005: /* zlib compressed */
-case 0x8006: /* bzip2 compressed */
-case 0x8007: /* lzfse compressed */
+case UDCO: /* zlib compressed */
+case UDBZ: /* bzip2 compressed */
+case ULFO: /* lzfse compressed */
compressed_size = s->lengths[chunk];
uncompressed_sectors = s->sectorcounts[chunk];
break;
-case 1: /* copy */
+case UDRW: /* copy */
uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);
break;
-case 2: /* zero */
+case UDIG: /* zero */
/* as the all-zeroes block may be large, it is treated specially: the
* sector is not copied from a large buffer, a simple memset is used
* instead. Therefore uncompressed_sectors does not need to be set. */
@@ -186,13 +199,13 @@ typedef struct DmgHeaderState {
static bool dmg_is_known_block_type(uint32_t entry_type)
{
switch (entry_type) {
-case 0x0001:/* uncompressed */
-case 0x0002:/* zeroes */
-case 0x8005:/* zlib */
+case UDRW:/* uncompressed */
+case UDIG:/* zeroes */
+case UDCO:/* zlib */
return true;
-case 0x8006:/* bzip2 */
+case UDBZ:/* bzip2 */
return !!dmg_uncompress_bz2;
-case 0x8007:/* lzfse */
+case ULFO:/* lzfse */
return !!dmg_uncompress_lzfse;
default:
return false;
@@ -592,7 +605,7 @@ static inline int dmg_read_chunk(BlockDriverState *bs,
uint64_t sector_num)
s->current_chunk = s->n_chunks;
switch (s->types[chunk]) { /* block entry type */
-case 0x8005: { /* zlib compressed */
+case UDZO: { /* zlib compressed */
/* we need to buffer, because only the chunk as whole can be
* inflated. */
ret = bdrv_pread(bs->file, s->offsets[chunk],
@@ -615,7 +628,7 @@ static inline int dmg_read_chunk(BlockDriverState *bs,
uint64_t sector_num)
return -1;
}
break; }
-case 0x8006: /* bzip2 compressed */
+case UDBZ: /* bzip2 compressed */
if (!dmg_uncompress_bz2) {
break;
}
@@ -636,7 +649,7 @@ static inline int dmg_read_chunk(BlockDriverState *bs,
uint64_t sector_num)
return ret;
}
break;
- case 0x8007:
+ case ULFO:
if (!dmg_uncompress_lzfse) {
break;
}
@@ -657,14 +670,14 @@ static inline int dmg_read_chunk(BlockDriverState *bs,
uint64_t sector_num)
return ret;
}
break;
-case 1: /* copy */
+case UDRW: /* copy */
ret = bdrv_pread(bs->file, s->offsets[chunk],
s->uncompressed_chunk, s->lengths[chunk]);
if (ret != s->lengths[chunk]) {
return -1;
}
break;
-case 2: /* zero */
+case UDIG: /* zero */
/* see dmg_read, it is treated specially. No buffer needs to be
* pre-filled, the zeroes can be set directly. */
break;
--
2.17.1
[Qemu-devel] [PATCH 0/4] Adding LZFSE compression support for DMG block driver.
Since Mac OS X El Capitain (v10.11), Apple uses LZFSE compression to generate compressed DMGs as an alternative to BZIP2. Possible, Apple want to keep this algorithm as default in long term. Some years ago, Apple opened the LZFSE algorithm to opensource and the main source (or the most active repo), can be found at: https://github.com/lzfse/lzfse Julio Faracco (4): block: adding lzfse decompressing support as a module. configure: adding support to lzfse library. dmg: including dmg-lzfse module inside dmg block driver. dmg: exchanging hardcoded dmg UDIF block types to enum. block/Makefile.objs | 2 ++ block/dmg-lzfse.c | 54 + block/dmg.c | 65 - block/dmg.h | 3 +++ configure | 32 ++ 5 files changed, 144 insertions(+), 12 deletions(-) create mode 100644 block/dmg-lzfse.c -- 2.17.1
[Qemu-devel] [PATCH 1/4] block: adding lzfse decompressing support as a module.
QEMU dmg support includes zlib and bzip2, but it does not contains lzfse
support. This commit adds the source file to extend compression support
for new DMGs.
Signed-off-by: Julio Faracco
---
block/dmg-lzfse.c | 54 +++
1 file changed, 54 insertions(+)
create mode 100644 block/dmg-lzfse.c
diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c
new file mode 100644
index 00..d09b544a0a
--- /dev/null
+++ b/block/dmg-lzfse.c
@@ -0,0 +1,54 @@
+/*
+ * DMG lzfse uncompression
+ *
+ * Copyright (c) 2018 Julio Cesar Faracco
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "dmg.h"
+#include
+
+static int dmg_uncompress_lzfse_do(char *next_in, unsigned int avail_in,
+ char *next_out, unsigned int avail_out)
+{
+void *aux;
+size_t aux_allocated;
+size_t out_size;
+
+aux_allocated = lzfse_decode_scratch_size();
+aux = g_malloc(aux_allocated);
+
+if (aux_allocated != 0 && aux == 0) {
+return -1;
+}
+
+out_size = lzfse_decode_buffer((uint8_t *) next_out, avail_out,
+ (uint8_t *) next_in, avail_in, aux);
+
+return out_size;
+}
+
+__attribute__((constructor))
+static void dmg_lzfse_init(void)
+{
+assert(!dmg_uncompress_lzfse);
+dmg_uncompress_lzfse = dmg_uncompress_lzfse_do;
+}
--
2.17.1
Re: [Qemu-devel] [Qemu-arm] [PATCH v4 3/6] loader: extract rom_free() function
On 08/03/2018 11:47 AM, Stefan Hajnoczi wrote:
> The next patch will need to free a rom. There is already code to do
> this in rom_add_file().
>
> Note that rom_add_file() uses:
>
> rom = g_malloc0(sizeof(*rom));
> ...
> if (rom->fw_dir) {
> g_free(rom->fw_dir);
> g_free(rom->fw_file);
> }
>
> The conditional is unnecessary since g_free(NULL) is a no-op.
>
> Signed-off-by: Stefan Hajnoczi
Reviewed-by: Philippe Mathieu-Daudé
> ---
> hw/core/loader.c | 21 -
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index bbb6e65bb5..0c72e7c05a 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -847,6 +847,17 @@ struct Rom {
> static FWCfgState *fw_cfg;
> static QTAILQ_HEAD(, Rom) roms = QTAILQ_HEAD_INITIALIZER(roms);
>
> +/* rom->data must be heap-allocated (do not use with rom_add_elf_program())
> */
> +static void rom_free(Rom *rom)
> +{
> +g_free(rom->data);
> +g_free(rom->path);
> +g_free(rom->name);
> +g_free(rom->fw_dir);
> +g_free(rom->fw_file);
> +g_free(rom);
> +}
> +
> static inline bool rom_order_compare(Rom *rom, Rom *item)
> {
> return ((uintptr_t)(void *)rom->as > (uintptr_t)(void *)item->as) ||
> @@ -995,15 +1006,7 @@ err:
> if (fd != -1)
> close(fd);
>
> -g_free(rom->data);
> -g_free(rom->path);
> -g_free(rom->name);
> -if (fw_dir) {
> -g_free(rom->fw_dir);
> -g_free(rom->fw_file);
> -}
> -g_free(rom);
> -
> +rom_free(rom);
> return -1;
> }
>
>
Re: [Qemu-devel] [Qemu-arm] [PATCH v4 2/6] target/arm: add "cortex-m0" CPU model
On 08/03/2018 11:47 AM, Stefan Hajnoczi wrote:
> Define a "cortex-m0" ARMv6-M CPU model.
>
> Most of the register reset values set by other CPU models are not
> relevant for the cut-down ARMv6-M architecture.
>
> Signed-off-by: Stefan Hajnoczi
> Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> target/arm/cpu.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 3848ef46aa..7e477c0d23 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1255,6 +1255,15 @@ static void arm11mpcore_initfn(Object *obj)
> cpu->reset_auxcr = 1;
> }
>
> +static void cortex_m0_initfn(Object *obj)
> +{
> +ARMCPU *cpu = ARM_CPU(obj);
> +set_feature(&cpu->env, ARM_FEATURE_V6);
> +set_feature(&cpu->env, ARM_FEATURE_M);
> +
> +cpu->midr = 0x410cc200;
> +}
> +
> static void cortex_m3_initfn(Object *obj)
> {
> ARMCPU *cpu = ARM_CPU(obj);
> @@ -1845,6 +1854,8 @@ static const ARMCPUInfo arm_cpus[] = {
> { .name = "arm1136", .initfn = arm1136_initfn },
> { .name = "arm1176", .initfn = arm1176_initfn },
> { .name = "arm11mpcore", .initfn = arm11mpcore_initfn },
> +{ .name = "cortex-m0", .initfn = cortex_m0_initfn,
> + .class_init = arm_v7m_class_init },
> { .name = "cortex-m3", .initfn = cortex_m3_initfn,
> .class_init = arm_v7m_class_init },
> { .name = "cortex-m4", .initfn = cortex_m4_initfn,
>
Re: [Qemu-devel] [Qemu-arm] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model
On 07/30/2018 02:51 PM, Peter Maydell wrote:
> On 27 July 2018 at 06:26, Philippe Mathieu-Daudé wrote:
>> Hi Stefan,
>>
>> On 07/25/2018 05:59 AM, Stefan Hajnoczi wrote:
>>> Define a "cortex-m0" ARMv6-M CPU model.
>>>
>>> Most of the register reset values set by other CPU models are not
>>> relevant for the cut-down ARMv6-M architecture.
>>>
>>> Signed-off-by: Stefan Hajnoczi
>>> ---
>>> target/arm/cpu.c | 11 +++
>>> 1 file changed, 11 insertions(+)
>>>
>>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>>> index 3848ef46aa..7e477c0d23 100644
>>> --- a/target/arm/cpu.c
>>> +++ b/target/arm/cpu.c
>>> @@ -1255,6 +1255,15 @@ static void arm11mpcore_initfn(Object *obj)
>>> cpu->reset_auxcr = 1;
>>> }
>>>
>>> +static void cortex_m0_initfn(Object *obj)
>>> +{
>>> +ARMCPU *cpu = ARM_CPU(obj);
>>> +set_feature(&cpu->env, ARM_FEATURE_V6);
>>> +set_feature(&cpu->env, ARM_FEATURE_M);
>>
>> What about ARM_FEATURE_THUMB2 (T32)?
>
> No, the M0 doesn't have Thumb2. It has Thumb1 plus a tiny set
> of 32-bit instructions (which we deal with specially in translate.c).
Oh I thought it as Thumb2, ok.
>
> (As it happens we generally can't get to the checks on the THUMB2
> feature for a v6M core, so I think but have not checked thoroughly
> that it would make no difference to QEMU's behaviour whether the
> feature bit was set or not.)
>
>> Peter: Since the M0 (optionally?) supports 32x32 multiply, should this
>> cpu use the ARM_FEATURE_THUMB_DSP feature? Else this might trigger an
>> 'Undefined Instruction' in disas_thumb2_insn().
>
> ARM_FEATURE_THUMB_DSP only enables checks in disas_thumb2_insn()
> (and 32x32->32 multiply is not one of the insns it gates).
> The only insns in that function that a v6M core can execute are
> msr, mrs, dsb, dmb, isb and bl, none of which are affected by that
> feature switch.
Yes.
>
>> And what about optional ARM_FEATURE_PMSA?
>> Oh this would be cortex_m0plus_initfn() for "cortex-m0-plus", ok.
>
> Yep, plain M0 has no MPU (and so we're postponing the work
> of implementing the v6M PMSA, which IIRC isn't quite the
> same as the v7M one).
Thanks for your detailed explanations :)
Regards,
Phil.
Re: [Qemu-devel] [RFC PATCH 1/4] scripts/decodetree.py: add a disassembly generator (HACK!)
On Wed, Aug 08, 2018 at 01:39:31PM +0100, Alex Bennée wrote: > Given our issues with failing disassembly we could try and re-use the > decode tree data to output what instruction is being decoded. This > will be used if registered as a fall-back for when the "proper" > disassembler fails to decode an instruction. > > Signed-off-by: Alex Bennée I don't have an opinion on the approach you are taking, but the Python code you are adding is consistent with the existing style of the script. That said, I find the existing code full of output() calls very hard to read. If anybody wants to volunteer to improve the readability of the output generation, it would be welcome. Acked-by: Eduardo Habkost -- Eduardo
Re: [Qemu-devel] [Qemu-arm] [PATCH v4 1/6] hw/arm: make bitbanded IO optional on ARMv7-M
On 08/03/2018 11:47 AM, Stefan Hajnoczi wrote:
> Some ARM CPUs have bitbanded IO, a memory region that allows convenient
> bit access via 32-bit memory loads/stores. This eliminates the need for
> read-modify-update instruction sequences.
>
> This patch makes this optional feature an ARMv7MState qdev property,
> allowing boards to choose whether they want bitbanding or not.
>
> Status of boards:
> * iotkit (Cortex M33), no bitband
> * mps2 (Cortex M3), bitband
> * msf2 (Cortex M3), bitband
> * stellaris (Cortex M3), bitband
> * stm32f205 (Cortex M3), bitband
>
> As a side-effect of this patch, Peter Maydell noted that the Ethernet
> controller on mps2 board is now accessible. Previously they were hidden
> by the bitband region (which does not exist on the real board).
>
> Signed-off-by: Stefan Hajnoczi
Reviewed-by: Philippe Mathieu-Daudé
> ---
> include/hw/arm/armv7m.h | 2 ++
> hw/arm/armv7m.c | 37 -
> hw/arm/mps2.c | 1 +
> hw/arm/msf2-soc.c | 1 +
> hw/arm/stellaris.c | 1 +
> hw/arm/stm32f205_soc.c | 1 +
> 6 files changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
> index 78308d1484..2ba24953b6 100644
> --- a/include/hw/arm/armv7m.h
> +++ b/include/hw/arm/armv7m.h
> @@ -43,6 +43,7 @@ typedef struct {
> * devices will be automatically layered on top of this view.)
> * + Property "idau": IDAU interface (forwarded to CPU object)
> * + Property "init-svtor": secure VTOR reset value (forwarded to CPU object)
> + * + Property "enable-bitband": expose bitbanded IO
> */
> typedef struct ARMv7MState {
> /*< private >*/
> @@ -63,6 +64,7 @@ typedef struct ARMv7MState {
> MemoryRegion *board_memory;
> Object *idau;
> uint32_t init_svtor;
> +bool enable_bitband;
> } ARMv7MState;
>
> #endif
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 6b07666057..878613994d 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -211,25 +211,27 @@ static void armv7m_realize(DeviceState *dev, Error
> **errp)
> memory_region_add_subregion(&s->container, 0xe000e000,
> sysbus_mmio_get_region(sbd, 0));
>
> -for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> -Object *obj = OBJECT(&s->bitband[i]);
> -SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
> +if (s->enable_bitband) {
> +for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> +Object *obj = OBJECT(&s->bitband[i]);
> +SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
>
> -object_property_set_int(obj, bitband_input_addr[i], "base", &err);
> -if (err != NULL) {
> -error_propagate(errp, err);
> -return;
> -}
> -object_property_set_link(obj, OBJECT(s->board_memory),
> - "source-memory", &error_abort);
> -object_property_set_bool(obj, true, "realized", &err);
> -if (err != NULL) {
> -error_propagate(errp, err);
> -return;
> -}
> +object_property_set_int(obj, bitband_input_addr[i], "base",
> &err);
> +if (err != NULL) {
> +error_propagate(errp, err);
> +return;
> +}
> +object_property_set_link(obj, OBJECT(s->board_memory),
> + "source-memory", &error_abort);
> +object_property_set_bool(obj, true, "realized", &err);
> +if (err != NULL) {
> +error_propagate(errp, err);
> +return;
> +}
>
> -memory_region_add_subregion(&s->container, bitband_output_addr[i],
> -sysbus_mmio_get_region(sbd, 0));
> +memory_region_add_subregion(&s->container,
> bitband_output_addr[i],
> +sysbus_mmio_get_region(sbd, 0));
> +}
> }
> }
>
> @@ -239,6 +241,7 @@ static Property armv7m_properties[] = {
> MemoryRegion *),
> DEFINE_PROP_LINK("idau", ARMv7MState, idau, TYPE_IDAU_INTERFACE, Object
> *),
> DEFINE_PROP_UINT32("init-svtor", ARMv7MState, init_svtor, 0),
> +DEFINE_PROP_BOOL("enable-bitband", ARMv7MState, enable_bitband, false),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
> index c3946da317..0a0ae867d9 100644
> --- a/hw/arm/mps2.c
> +++ b/hw/arm/mps2.c
> @@ -186,6 +186,7 @@ static void mps2_common_init(MachineState *machine)
> g_assert_not_reached();
> }
> qdev_prop_set_string(armv7m, "cpu-type", machine->cpu_type);
> +qdev_prop_set_bit(armv7m, "enable-bitband", true);
> object_property_set_link(OBJECT(&mms->armv7m), OBJECT(system_memory),
> "memory", &error_abort);
> object_property_set_bool(OBJECT(&mms->armv7m), true, "realized",
> diff --git a/hw/a
[Qemu-devel] [PATCH 7/7] target/s390x: implement CVB, CVBY and CVBG
Convert to Binary - counterparts of the already implemented Convert
to Decimal (CVD*) instructions.
Example from the Principles of Operation: 25594C becomes 63FA.
Signed-off-by: Pavel Zbitskiy
---
target/s390x/helper.h | 1 +
target/s390x/insn-data.def | 4 +++
target/s390x/int_helper.c | 50 +
target/s390x/translate.c| 10 +++
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/cvb.c | 18
6 files changed, 84 insertions(+)
create mode 100644 tests/tcg/s390x/cvb.c
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 97c60ca7bc..46baaee0ab 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -88,6 +88,7 @@ DEF_HELPER_FLAGS_4(tcxb, TCG_CALL_NO_RWG_SE, i32, env, i64,
i64, i64)
DEF_HELPER_FLAGS_2(sqeb, TCG_CALL_NO_WG, i64, env, i64)
DEF_HELPER_FLAGS_2(sqdb, TCG_CALL_NO_WG, i64, env, i64)
DEF_HELPER_FLAGS_3(sqxb, TCG_CALL_NO_WG, i64, env, i64, i64)
+DEF_HELPER_FLAGS_3(cvb, TCG_CALL_NO_WG, i64, env, i64, i32)
DEF_HELPER_FLAGS_1(cvd, TCG_CALL_NO_RWG_SE, i64, s32)
DEF_HELPER_FLAGS_4(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 9c7b434fca..1b29a5e044 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -284,6 +284,10 @@
D(0xec73, CLFIT, RIE_a, GIE, r1_32u, i2_32u, 0, 0, ct, 0, 1)
D(0xec71, CLGIT, RIE_a, GIE, r1_o, i2_32u, 0, 0, ct, 0, 1)
+/* CONVERT TO BINARY */
+C(0x4f00, CVB, RX_a, Z, 0, a2, new, r1_32, cvb, 0)
+C(0xe306, CVBY,RXY_a, LD, 0, a2, new, r1_32, cvb, 0)
+C(0xe30e, CVBG,RXY_a, Z, 0, a2, r1, 0, cvb, 0)
/* CONVERT TO DECIMAL */
C(0x4e00, CVD, RX_a, Z, r1_o, a2, 0, 0, cvd, 0)
C(0xe326, CVDY,RXY_a, LD, r1_o, a2, 0, 0, cvd, 0)
diff --git a/target/s390x/int_helper.c b/target/s390x/int_helper.c
index abf77a94e6..66c6dc8b3a 100644
--- a/target/s390x/int_helper.c
+++ b/target/s390x/int_helper.c
@@ -24,6 +24,7 @@
#include "exec/exec-all.h"
#include "qemu/host-utils.h"
#include "exec/helper-proto.h"
+#include "exec/cpu_ldst.h"
/* #define DEBUG_HELPER */
#ifdef DEBUG_HELPER
@@ -118,6 +119,55 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah,
uint64_t al,
return ret;
}
+static void general_operand_exception(CPUS390XState *env, uintptr_t ra)
+{
+#ifndef CONFIG_USER_ONLY
+LowCore *lowcore;
+
+lowcore = cpu_map_lowcore(env);
+lowcore->data_exc_code = 0;
+cpu_unmap_lowcore(lowcore);
+#endif
+s390_program_interrupt(env, PGM_DATA, ILEN_AUTO, ra);
+}
+
+uint64_t HELPER(cvb)(CPUS390XState *env, uint64_t src, uint32_t n)
+{
+int i, j;
+uintptr_t ra = GETPC();
+int64_t dec, sign = 0, digit, val = 0, pow10 = 0;
+
+for (i = 0; i < n; i++) {
+dec = cpu_ldq_data_ra(env, src + (n - i - 1) * 8, ra);
+for (j = 0; j < 16; j++, dec >>= 4) {
+if (i == 0 && j == 0) {
+sign = dec & 0xf;
+if (sign < 0xa) {
+general_operand_exception(env, ra);
+}
+continue;
+}
+digit = dec & 0xf;
+if (digit > 0x9) {
+general_operand_exception(env, ra);
+}
+if (i == 0 && j == 1) {
+if (sign == 0xb || sign == 0xd) {
+val = -digit;
+pow10 = -10;
+} else {
+val = digit;
+pow10 = 10;
+}
+} else {
+val += digit * pow10;
+pow10 *= 10;
+}
+}
+}
+return val;
+}
+
uint64_t HELPER(cvd)(int32_t reg)
{
/* positive 0 */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 6f8fbda222..48a97a37ff 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2107,6 +2107,16 @@ static DisasJumpType op_csp(DisasContext *s, DisasOps *o)
}
#endif
+static DisasJumpType op_cvb(DisasContext *s, DisasOps *o)
+{
+uint64_t n = ((s->fields->op == 0xE3) && (s->fields->op2 == 0x0E)) ?
+/* CVBG */ 2 :
+/* CVB, CVBY */ 1;
+
+gen_helper_cvb(o->out, cpu_env, o->in2, tcg_const_i32(n));
+return DISAS_NEXT;
+}
+
static DisasJumpType op_cvd(DisasContext *s, DisasOps *o)
{
TCGv_i64 t1 = tcg_temp_new_i64();
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 151dc075aa..990dfb26ff 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -6,3 +6,4 @@ TESTS+=ipm
TESTS+=exrl-trt
TESTS+=exrl-trtr
TESTS+=pack
+TESTS+=cvb
diff --git a/tests/tcg/s390x/cvb.c b/tests/tcg/s390x/cvb.c
new file mode 100644
index 00..3a72e132aa
--- /dev/null
+++ b/tests/tcg/s390x/cvb.c
@@ -0,0 +1,18 @@
+#include
+#include
+
+int main(void)
+{
+uint64_t data = 0x000
[Qemu-devel] [PATCH 5/7] target/s390x: add EX support for TRT and TRTR
Improves "b213c9f5: target/s390x: Implement TRTR" by introducing the
intermediate functions, which are compatible with dx_helper type.
Signed-off-by: Pavel Zbitskiy
---
target/s390x/mem_helper.c | 16 +++
tests/tcg/s390x/Makefile.target | 2 ++
tests/tcg/s390x/exrl-trt.c | 48 +
tests/tcg/s390x/exrl-trtr.c | 48 +
4 files changed, 114 insertions(+)
create mode 100644 tests/tcg/s390x/exrl-trt.c
create mode 100644 tests/tcg/s390x/exrl-trtr.c
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index c94dbf3fcb..704d0193b5 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1299,12 +1299,26 @@ static inline uint32_t do_helper_trt(CPUS390XState
*env, int len,
return 0;
}
+static uint32_t do_helper_trt_fwd(CPUS390XState *env, uint32_t len,
+ uint64_t array, uint64_t trans,
+ uintptr_t ra)
+{
+return do_helper_trt(env, len, array, trans, 1, ra);
+}
+
uint32_t HELPER(trt)(CPUS390XState *env, uint32_t len, uint64_t array,
uint64_t trans)
{
return do_helper_trt(env, len, array, trans, 1, GETPC());
}
+static uint32_t do_helper_trt_bkwd(CPUS390XState *env, uint32_t len,
+ uint64_t array, uint64_t trans,
+ uintptr_t ra)
+{
+return do_helper_trt(env, len, array, trans, -1, ra);
+}
+
uint32_t HELPER(trtr)(CPUS390XState *env, uint32_t len, uint64_t array,
uint64_t trans)
{
@@ -2193,12 +2207,14 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen,
uint64_t r1, uint64_t addr)
typedef uint32_t (*dx_helper)(CPUS390XState *, uint32_t, uint64_t,
uint64_t, uintptr_t);
static const dx_helper dx[16] = {
+[0x0] = do_helper_trt_bkwd,
[0x2] = do_helper_mvc,
[0x4] = do_helper_nc,
[0x5] = do_helper_clc,
[0x6] = do_helper_oc,
[0x7] = do_helper_xc,
[0xc] = do_helper_tr,
+[0xd] = do_helper_trt_fwd,
};
dx_helper helper = dx[opc & 0xf];
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index c800a582e5..7de4376f52 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -3,3 +3,5 @@ CFLAGS+=-march=zEC12 -m64
TESTS+=hello-s390x
TESTS+=csst
TESTS+=ipm
+TESTS+=exrl-trt
+TESTS+=exrl-trtr
diff --git a/tests/tcg/s390x/exrl-trt.c b/tests/tcg/s390x/exrl-trt.c
new file mode 100644
index 00..3c5323aecb
--- /dev/null
+++ b/tests/tcg/s390x/exrl-trt.c
@@ -0,0 +1,48 @@
+#include
+#include
+
+int main(void)
+{
+char op1[] = "hello";
+char op2[256];
+uint64_t r1 = 0xull;
+uint64_t r2 = 0xull;
+uint64_t cc;
+int i;
+
+for (i = 0; i < 256; i++) {
+if (i == 0) {
+op2[i] = 0xaa;
+} else {
+op2[i] = 0;
+}
+}
+asm volatile(
+"j 2f\n"
+"1: trt 0(1,%[op1]),0(%[op2])\n"
+"2: exrl %[op1_len],1b\n"
+"lgr %[r1],%%r1\n"
+"lgr %[r2],%%r2\n"
+"ipm %[cc]\n"
+: [r1] "+r" (r1),
+ [r2] "+r" (r2),
+ [cc] "=r" (cc)
+: [op1] "r" (&op1),
+ [op1_len] "r" (5),
+ [op2] "r" (&op2)
+: "r1", "r2", "cc");
+cc = (cc >> 28) & 3;
+if (cc != 2) {
+write(1, "bad cc\n", 7);
+return 1;
+}
+if ((char *)r1 != &op1[5]) {
+write(1, "bad r1\n", 7);
+return 1;
+}
+if (r2 != 0xffaaull) {
+write(1, "bad r2\n", 7);
+return 1;
+}
+return 0;
+}
diff --git a/tests/tcg/s390x/exrl-trtr.c b/tests/tcg/s390x/exrl-trtr.c
new file mode 100644
index 00..c33153ad7e
--- /dev/null
+++ b/tests/tcg/s390x/exrl-trtr.c
@@ -0,0 +1,48 @@
+#include
+#include
+
+int main(void)
+{
+char op1[] = {0, 1, 2, 3};
+char op2[256];
+uint64_t r1 = 0xull;
+uint64_t r2 = 0xull;
+uint64_t cc;
+int i;
+
+for (i = 0; i < 256; i++) {
+if (i == 1) {
+op2[i] = 0xbb;
+} else {
+op2[i] = 0;
+}
+}
+asm volatile(
+"j 2f\n"
+"1: trtr 3(1,%[op1]),0(%[op2])\n"
+"2: exrl %[op1_len],1b\n"
+"lgr %[r1],%%r1\n"
+"lgr %[r2],%%r2\n"
+"ipm %[cc]\n"
+: [r1] "+r" (r1),
+ [r2] "+r" (r2),
+ [cc] "=r" (cc)
+: [op1] "r" (&op1),
+ [op1_len] "r" (3),
+ [op2] "r" (&op2)
+: "r1", "r2", "cc");
+cc = (cc >> 28) & 3;
+if (cc != 1) {
+write(1, "bad cc\n", 7);
+return 1;
+}
+if ((char *)r1 != &op1[1]) {
+write(1, "bad r1\n", 7);
+return 1;
+}
+if (r2 != 0xff
[Qemu-devel] [PATCH 3/7] target/s390x: fix CSST decoding and runtime alignment check
CSST is defined as:
C(0xc802, CSST,SSF, CASS, la1, a2, 0, 0, csst, 0)
It means that the first parameter is handled by in1_la1().
in1_la1() fills addr1 field, and not in1.
Furthermore, when extract32() is used for the alignment check, the
third parameter should specify the number of trailing bits that must
be 0. For FC these numbers are:
FC=0 (word, 4 bytes):2
FC=1 (double word, 8 bytes): 3
FC=2 (quad word, 16 bytes): 4
For SC these numbers correspond to the size:
SC=0: 0
SC=1: 1
SC=2: 2
SC=3: 3
SC=4: 4
Signed-off-by: Pavel Zbitskiy
---
target/s390x/mem_helper.c | 2 +-
target/s390x/translate.c| 4 +--
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/csst.c | 43 +
4 files changed, 47 insertions(+), 3 deletions(-)
create mode 100644 tests/tcg/s390x/csst.c
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index e21a47fb4d..c94dbf3fcb 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1442,7 +1442,7 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t r3,
uint64_t a1,
}
/* Sanity check the alignments. */
-if (extract32(a1, 0, 4 << fc) || extract32(a2, 0, 1 << sc)) {
+if (extract32(a1, 0, fc + 2) || extract32(a2, 0, sc)) {
goto spec_exception;
}
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 316ff79250..d0d2c3412f 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2051,9 +2051,9 @@ static DisasJumpType op_csst(DisasContext *s, DisasOps *o)
TCGv_i32 t_r3 = tcg_const_i32(r3);
if (tb_cflags(s->base.tb) & CF_PARALLEL) {
-gen_helper_csst_parallel(cc_op, cpu_env, t_r3, o->in1, o->in2);
+gen_helper_csst_parallel(cc_op, cpu_env, t_r3, o->addr1, o->in2);
} else {
-gen_helper_csst(cc_op, cpu_env, t_r3, o->in1, o->in2);
+gen_helper_csst(cc_op, cpu_env, t_r3, o->addr1, o->in2);
}
tcg_temp_free_i32(t_r3);
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 9f4076901f..f62f950d8e 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -1,3 +1,4 @@
VPATH+=$(SRC_PATH)/tests/tcg/s390x
CFLAGS+=-march=zEC12 -m64
TESTS+=hello-s390x
+TESTS+=csst
diff --git a/tests/tcg/s390x/csst.c b/tests/tcg/s390x/csst.c
new file mode 100644
index 00..1dae9071fb
--- /dev/null
+++ b/tests/tcg/s390x/csst.c
@@ -0,0 +1,43 @@
+#include
+#include
+
+int main(void)
+{
+uint64_t parmlist[] = {
+0xfedcba9876543210ull,
+0,
+0xull,
+0,
+};
+uint64_t op1 = 0x0123456789abcdefull;
+uint64_t op2 = 0;
+uint64_t op3 = op1;
+uint64_t cc;
+
+asm volatile(
+"lghi %%r0,%[flags]\n"
+"la %%r1,%[parmlist]\n"
+"csst %[op1],%[op2],%[op3]\n"
+"ipm %[cc]\n"
+: [op1] "+m" (op1),
+ [op2] "+m" (op2),
+ [op3] "+r" (op3),
+ [cc] "=r" (cc)
+: [flags] "K" (0x0301),
+ [parmlist] "m" (parmlist)
+: "r0", "r1", "cc", "memory");
+cc = (cc >> 28) & 3;
+if (cc) {
+write(1, "bad cc\n", 7);
+return 1;
+}
+if (op1 != parmlist[0]) {
+write(1, "bad op1\n", 8);
+return 1;
+}
+if (op2 != parmlist[2]) {
+write(1, "bad op2\n", 8);
+return 1;
+}
+return 0;
+}
--
2.18.0
[Qemu-devel] [PATCH 4/7] target/s390x: fix IPM polluting irrelevant bits
Suppose psw.mask=0x8000, cc=2, r1=0 and we do "ipm 1".
This command must touch only bits 32-39, so the expected output
is r1=0x2000. However, currently qemu yields r1=0x20008000,
because irrelevant parts of PSW leak into r1 during program mask
transfer.
Signed-off-by: Pavel Zbitskiy
---
target/s390x/translate.c| 17 +++--
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/ipm.c | 22 ++
3 files changed, 30 insertions(+), 10 deletions(-)
create mode 100644 tests/tcg/s390x/ipm.c
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index d0d2c3412f..6f8fbda222 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2437,20 +2437,17 @@ static DisasJumpType op_insi(DisasContext *s, DisasOps
*o)
static DisasJumpType op_ipm(DisasContext *s, DisasOps *o)
{
-TCGv_i64 t1;
+TCGv_i64 t1, t2;
gen_op_calc_cc(s);
-tcg_gen_andi_i64(o->out, o->out, ~0xff00ull);
-
t1 = tcg_temp_new_i64();
-tcg_gen_shli_i64(t1, psw_mask, 20);
-tcg_gen_shri_i64(t1, t1, 36);
-tcg_gen_or_i64(o->out, o->out, t1);
-
-tcg_gen_extu_i32_i64(t1, cc_op);
-tcg_gen_shli_i64(t1, t1, 28);
-tcg_gen_or_i64(o->out, o->out, t1);
+tcg_gen_extract_i64(t1, psw_mask, 40, 4);
+t2 = tcg_temp_new_i64();
+tcg_gen_extu_i32_i64(t2, cc_op);
+tcg_gen_deposit_i64(t1, t1, t2, 4, 60);
+tcg_gen_deposit_i64(o->out, o->out, t1, 24, 8);
tcg_temp_free_i64(t1);
+tcg_temp_free_i64(t2);
return DISAS_NEXT;
}
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index f62f950d8e..c800a582e5 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -2,3 +2,4 @@ VPATH+=$(SRC_PATH)/tests/tcg/s390x
CFLAGS+=-march=zEC12 -m64
TESTS+=hello-s390x
TESTS+=csst
+TESTS+=ipm
diff --git a/tests/tcg/s390x/ipm.c b/tests/tcg/s390x/ipm.c
new file mode 100644
index 00..742f3a18c5
--- /dev/null
+++ b/tests/tcg/s390x/ipm.c
@@ -0,0 +1,22 @@
+#include
+#include
+
+int main(void)
+{
+uint32_t op1 = 0x;
+uint32_t op2 = 0x;
+uint64_t cc = 0xull;
+
+asm volatile(
+"clc 0(4,%[op1]),0(%[op2])\n"
+"ipm %[cc]\n"
+: [cc] "+r" (cc)
+: [op1] "r" (&op1),
+ [op2] "r" (&op2)
+: "cc");
+if (cc != 0x20ffull) {
+write(1, "bad cc\n", 7);
+return 1;
+}
+return 0;
+}
--
2.18.0
[Qemu-devel] [PATCH 6/7] target/s390x: fix PACK reading 1 byte less and writing 1 byte more
PACK fails on the test from the Principles of Operation: F1F2F3F4
becomes 234C instead of 0001234C due to an off-by-one error.
Furthermore, it overwrites one extra byte to the left of F1.
If len_dest is 0, then we only want to flip the 1st byte and never loop
over the rest. Therefore, the loop condition should be > and not >=.
If len_src is 1, then we should flip the 1st byte and pack the 2nd.
Since len_src is already decremented before the loop, the first
condition should be >=, and not >.
Likewise for len_src == 2 and the second condition.
Signed-off-by: Pavel Zbitskiy
---
target/s390x/mem_helper.c | 6 +++---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/pack.c | 21 +
3 files changed, 25 insertions(+), 3 deletions(-)
create mode 100644 tests/tcg/s390x/pack.c
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 704d0193b5..bacae4f503 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1019,15 +1019,15 @@ void HELPER(pack)(CPUS390XState *env, uint32_t len,
uint64_t dest, uint64_t src)
len_src--;
/* now pack every value */
-while (len_dest >= 0) {
+while (len_dest > 0) {
b = 0;
-if (len_src > 0) {
+if (len_src >= 0) {
b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
src--;
len_src--;
}
-if (len_src > 0) {
+if (len_src >= 0) {
b |= cpu_ldub_data_ra(env, src, ra) << 4;
src--;
len_src--;
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 7de4376f52..151dc075aa 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -5,3 +5,4 @@ TESTS+=csst
TESTS+=ipm
TESTS+=exrl-trt
TESTS+=exrl-trtr
+TESTS+=pack
diff --git a/tests/tcg/s390x/pack.c b/tests/tcg/s390x/pack.c
new file mode 100644
index 00..4be36f29a7
--- /dev/null
+++ b/tests/tcg/s390x/pack.c
@@ -0,0 +1,21 @@
+#include
+
+int main(void)
+{
+char data[] = {0xaa, 0xaa, 0xf1, 0xf2, 0xf3, 0xc4, 0xaa, 0xaa};
+char exp[] = {0xaa, 0xaa, 0x00, 0x01, 0x23, 0x4c, 0xaa, 0xaa};
+int i;
+
+asm volatile(
+"pack 2(4,%[data]),2(4,%[data])\n"
+:
+: [data] "r" (&data[0])
+: "memory");
+for (i = 0; i < 8; i++) {
+if (data[i] != exp[i]) {
+write(1, "bad data\n", 9);
+return 1;
+}
+}
+return 0;
+}
--
2.18.0
[Qemu-devel] [PATCH 2/7] target/s390x: add BAL and BALR instructions
These instructions are provided for compatibility purposes and are
used only by old software, in the new code BAS and BASR are preferred.
The difference between the old and new instruction exists only in the
24-bit mode.
Signed-off-by: Pavel Zbitskiy
---
target/s390x/insn-data.def | 3 +++
target/s390x/translate.c | 33 +
2 files changed, 36 insertions(+)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 5c6f33ed9c..9c7b434fca 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -102,6 +102,9 @@
D(0x9400, NI, SI,Z, la1, i2_8u, new, 0, ni, nz64, MO_UB)
D(0xeb54, NIY, SIY, LD, la1, i2_8u, new, 0, ni, nz64, MO_UB)
+/* BRANCH AND LINK */
+C(0x0500, BALR,RR_a, Z, 0, r2_nz, r1, 0, bal, 0)
+C(0x4500, BAL, RX_a, Z, 0, a2, r1, 0, bal, 0)
/* BRANCH AND SAVE */
C(0x0d00, BASR,RR_a, Z, 0, r2_nz, r1, 0, bas, 0)
C(0x4d00, BAS, RX_a, Z, 0, a2, r1, 0, bas, 0)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 57c03cbf58..316ff79250 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1463,6 +1463,39 @@ static DisasJumpType op_bas(DisasContext *s, DisasOps *o)
}
}
+static void save_link_info(DisasContext *s, DisasOps *o)
+{
+TCGv_i64 t;
+
+if (s->base.tb->flags & (FLAG_MASK_32 | FLAG_MASK_64)) {
+tcg_gen_movi_i64(o->out, pc_to_link_info(s, s->pc_tmp));
+return;
+}
+gen_op_calc_cc(s);
+tcg_gen_andi_i64(o->out, o->out, 0xull);
+tcg_gen_ori_i64(o->out, o->out, ((s->ilen / 2) << 30) | s->pc_tmp);
+t = tcg_temp_new_i64();
+tcg_gen_shri_i64(t, psw_mask, 16);
+tcg_gen_andi_i64(t, t, 0x0f00);
+tcg_gen_or_i64(o->out, o->out, t);
+tcg_gen_extu_i32_i64(t, cc_op);
+tcg_gen_shli_i64(t, t, 28);
+tcg_gen_or_i64(o->out, o->out, t);
+tcg_temp_free_i64(t);
+}
+
+static DisasJumpType op_bal(DisasContext *s, DisasOps *o)
+{
+save_link_info(s, o);
+if (o->in2) {
+tcg_gen_mov_i64(psw_addr, o->in2);
+per_branch(s, false);
+return DISAS_PC_UPDATED;
+} else {
+return DISAS_NEXT;
+}
+}
+
static DisasJumpType op_basi(DisasContext *s, DisasOps *o)
{
tcg_gen_movi_i64(o->out, pc_to_link_info(s, s->pc_tmp));
--
2.18.0
[Qemu-devel] [PATCH 1/7] tests/tcg: add a simple s390x test
Copied from alpha.
Signed-off-by: Pavel Zbitskiy
---
tests/tcg/s390x/Makefile.target | 3 +++
tests/tcg/s390x/hello-s390x.c | 7 +++
2 files changed, 10 insertions(+)
create mode 100644 tests/tcg/s390x/Makefile.target
create mode 100644 tests/tcg/s390x/hello-s390x.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
new file mode 100644
index 00..9f4076901f
--- /dev/null
+++ b/tests/tcg/s390x/Makefile.target
@@ -0,0 +1,3 @@
+VPATH+=$(SRC_PATH)/tests/tcg/s390x
+CFLAGS+=-march=zEC12 -m64
+TESTS+=hello-s390x
diff --git a/tests/tcg/s390x/hello-s390x.c b/tests/tcg/s390x/hello-s390x.c
new file mode 100644
index 00..3dc0a05f2b
--- /dev/null
+++ b/tests/tcg/s390x/hello-s390x.c
@@ -0,0 +1,7 @@
+#include
+
+int main(void)
+{
+write(1, "hello\n", 6);
+return 0;
+}
--
2.18.0
[Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support
Found while attempting to run an old tool in qemu. * BAL and BALR:Added. * CSST:Qemu crashed after an accidental jump to garbage. * IPM: A tool produced an incorrect output. * EX TRT/TRTR: A tool ran quite slow. * PACK:A tool produced an incorrect output. * CVB, CVBY, CVBG: Added. Changes since v1: * Tests. * Call pc_to_link_info() instead of op_bas(). * Clarified CSST commit message. * Rewrote IPM using extract/deposit. * Clarified PACK commit message. * Do not use LowCore for CONFIG_USER_ONLY. * Reduce duplication in CVB code. Pavel Zbitskiy (7): tests/tcg: add a simple s390x test target/s390x: add BAL and BALR instructions target/s390x: fix CSST decoding and runtime alignment check target/s390x: fix IPM polluting irrelevant bits target/s390x: add EX support for TRT and TRTR target/s390x: fix PACK reading 1 byte less and writing 1 byte more target/s390x: implement CVB, CVBY and CVBG target/s390x/helper.h | 1 + target/s390x/insn-data.def | 7 target/s390x/int_helper.c | 50 ++ target/s390x/mem_helper.c | 24 ++--- target/s390x/translate.c| 64 ++--- tests/tcg/s390x/Makefile.target | 9 + tests/tcg/s390x/csst.c | 43 ++ tests/tcg/s390x/cvb.c | 18 ++ tests/tcg/s390x/exrl-trt.c | 48 + tests/tcg/s390x/exrl-trtr.c | 48 + tests/tcg/s390x/hello-s390x.c | 7 tests/tcg/s390x/ipm.c | 22 tests/tcg/s390x/pack.c | 21 +++ 13 files changed, 346 insertions(+), 16 deletions(-) create mode 100644 tests/tcg/s390x/Makefile.target create mode 100644 tests/tcg/s390x/csst.c create mode 100644 tests/tcg/s390x/cvb.c create mode 100644 tests/tcg/s390x/exrl-trt.c create mode 100644 tests/tcg/s390x/exrl-trtr.c create mode 100644 tests/tcg/s390x/hello-s390x.c create mode 100644 tests/tcg/s390x/ipm.c create mode 100644 tests/tcg/s390x/pack.c -- 2.18.0
[Qemu-devel] [resend][PATCH] qga-win: add support for qmp_guest_fsfreeze_freeze_list
From: Chen Hanxiao
This patch add support for freeze specified fs.
The valid mountpoints list member are [1]:
The path of a mounted folder, for example, Y:\MountX\
A drive letter, for example, D:\
A volume GUID path of the form \\?\Volume{GUID}\,
where GUID identifies the volume
A UNC path that specifies a remote file share,
for example, \\Clusterx\Share1\
[1]
https://docs.microsoft.com/en-us/windows/desktop/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset
Cc: Michael Roth
Signed-off-by: Chen Hanxiao
---
qga/commands-win32.c| 21 ++
qga/main.c | 2 +-
qga/vss-win32.c | 5 ++--
qga/vss-win32.h | 3 +-
qga/vss-win32/requester.cpp | 56 +++--
qga/vss-win32/requester.h | 13 +++--
6 files changed, 72 insertions(+), 28 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 98d9735389..1d627f73c1 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -776,6 +776,13 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
* The frozen state is limited for up to 10 seconds by VSS.
*/
int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+{
+return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
+}
+
+int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
+ strList *mountpoints,
+ Error **errp)
{
int i;
Error *local_err = NULL;
@@ -790,7 +797,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
/* cannot risk guest agent blocking itself on a write in this state */
ga_set_frozen(ga_state);
-qga_vss_fsfreeze(&i, true, &local_err);
+qga_vss_fsfreeze(&i, true, mountpoints, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto error;
@@ -808,15 +815,6 @@ error:
return 0;
}
-int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
- strList *mountpoints,
- Error **errp)
-{
-error_setg(errp, QERR_UNSUPPORTED);
-
-return 0;
-}
-
/*
* Thaw local file systems using Volume Shadow-copy Service.
*/
@@ -829,7 +827,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
return 0;
}
-qga_vss_fsfreeze(&i, false, errp);
+qga_vss_fsfreeze(&i, false, NULL, errp);
ga_unset_frozen(ga_state);
return i;
@@ -1646,7 +1644,6 @@ GList *ga_command_blacklist_init(GList *blacklist)
"guest-set-vcpus",
"guest-get-memory-blocks", "guest-set-memory-blocks",
"guest-get-memory-block-size",
-"guest-fsfreeze-freeze-list",
NULL};
char **p = (char **)list_unsupported;
diff --git a/qga/main.c b/qga/main.c
index 87372d40ef..098c783f1f 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -152,7 +152,7 @@ static void quit_handler(int sig)
WaitForSingleObject(hEventTimeout, 0);
CloseHandle(hEventTimeout);
}
-qga_vss_fsfreeze(&i, false, &err);
+qga_vss_fsfreeze(&i, false, NULL, &err);
if (err) {
g_debug("Error unfreezing filesystems prior to exiting: %s",
error_get_pretty(err));
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index a541f3ae01..f444a25a70 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -147,7 +147,8 @@ void ga_uninstall_vss_provider(void)
}
/* Call VSS requester and freeze/thaw filesystems and applications */
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp)
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+ strList *mountpoints, Error **errp)
{
const char *func_name = freeze ? "requester_freeze" : "requester_thaw";
QGAVSSRequesterFunc func;
@@ -164,5 +165,5 @@ void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error
**errp)
return;
}
-func(nr_volume, &errset);
+func(nr_volume, mountpoints, &errset);
}
diff --git a/qga/vss-win32.h b/qga/vss-win32.h
index 4f8e39aa5c..ce2abe5a72 100644
--- a/qga/vss-win32.h
+++ b/qga/vss-win32.h
@@ -22,6 +22,7 @@ bool vss_initialized(void);
int ga_install_vss_provider(void);
void ga_uninstall_vss_provider(void);
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp);
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+ strList *mountpints, Error **errp);
#endif
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 3d9c9716c0..0bd170eddc 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -234,7 +234,7 @@ out:
}
}
-void requester_freeze(int *num_vols, ErrorSet *errset)
+void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset)
{
COMPointer pAsync;
HANDLE volume;
@@ -245,7 +245,9 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
SECURITY_ATTRIBUTES sa;
WCHAR short_volume_name[64], *display_name = sho
Re: [Qemu-devel] [PATCH v6 3/4] spapr: introduce a IRQ controller backend to the machine
On Mon, Jul 30, 2018 at 04:11:33PM +0200, Cédric Le Goater wrote:
> This proposal moves all the related IRQ routines of the sPAPR machine
> behind a sPAPR IRQ backend interface 'spapr_irq' to prepare for future
> changes. First of which will be to increase the size of the IRQ number
> space, then, will follow a new backend for the POWER9 XIVE IRQ controller.
>
> Signed-off-by: Cédric Le Goater
Applied to ppc-for-3.1.
> ---
> include/hw/ppc/spapr.h | 11 +-
> include/hw/ppc/spapr_irq.h | 22
> hw/ppc/spapr.c | 180 +
> hw/ppc/spapr_cpu_core.c| 1 +
> hw/ppc/spapr_irq.c | 230 +
> 5 files changed, 259 insertions(+), 185 deletions(-)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 73067f5ee8aa..ad4d7cfd97b0 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -4,7 +4,6 @@
> #include "qemu/units.h"
> #include "sysemu/dma.h"
> #include "hw/boards.h"
> -#include "hw/ppc/xics.h"
> #include "hw/ppc/spapr_drc.h"
> #include "hw/mem/pc-dimm.h"
> #include "hw/ppc/spapr_ovec.h"
> @@ -16,6 +15,7 @@ struct sPAPRNVRAM;
> typedef struct sPAPREventLogEntry sPAPREventLogEntry;
> typedef struct sPAPREventSource sPAPREventSource;
> typedef struct sPAPRPendingHPT sPAPRPendingHPT;
> +typedef struct ICSState ICSState;
>
> #define HPTE64_V_HPTE_DIRTY 0x0040ULL
> #define SPAPR_ENTRY_POINT 0x100
> @@ -110,6 +110,7 @@ struct sPAPRMachineClass {
>unsigned n_dma, uint32_t *liobns, Error **errp);
> sPAPRResizeHPT resize_hpt_default;
> sPAPRCapabilities default_caps;
> +sPAPRIrq *irq;
> };
>
> /**
> @@ -780,14 +781,6 @@ int spapr_get_vcpu_id(PowerPCCPU *cpu);
> void spapr_set_vcpu_id(PowerPCCPU *cpu, int cpu_index, Error **errp);
> PowerPCCPU *spapr_find_cpu(int vcpu_id);
>
> -int spapr_irq_find(sPAPRMachineState *spapr, int num, bool align,
> - Error **errp);
> -#define spapr_irq_findone(spapr, errp) spapr_irq_find(spapr, 1, false, errp)
> -int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error
> **errp);
> -void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
> -qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
> -
> -
> int spapr_caps_pre_load(void *opaque);
> int spapr_caps_pre_save(void *opaque);
>
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 6f7f50548809..0e98c4474bb2 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -29,4 +29,26 @@ int spapr_irq_msi_alloc(sPAPRMachineState *spapr, uint32_t
> num, bool align,
> void spapr_irq_msi_free(sPAPRMachineState *spapr, int irq, uint32_t num);
> void spapr_irq_msi_reset(sPAPRMachineState *spapr);
>
> +typedef struct sPAPRIrq {
> +uint32_tnr_irqs;
> +
> +void (*init)(sPAPRMachineState *spapr, Error **errp);
> +int (*claim)(sPAPRMachineState *spapr, int irq, bool lsi, Error **errp);
> +void (*free)(sPAPRMachineState *spapr, int irq, int num);
> +qemu_irq (*qirq)(sPAPRMachineState *spapr, int irq);
> +void (*print_info)(sPAPRMachineState *spapr, Monitor *mon);
> +} sPAPRIrq;
> +
> +extern sPAPRIrq spapr_irq_xics;
> +
> +int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error
> **errp);
> +void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
> +qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
> +
> +/*
> + * XICS legacy routines
> + */
> +int spapr_irq_find(sPAPRMachineState *spapr, int num, bool align, Error
> **errp);
> +#define spapr_irq_findone(spapr, errp) spapr_irq_find(spapr, 1, false, errp)
> +
> #endif
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 792e24453d8b..d9f8cca49208 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -54,7 +54,6 @@
> #include "hw/ppc/spapr.h"
> #include "hw/ppc/spapr_vio.h"
> #include "hw/pci-host/spapr.h"
> -#include "hw/ppc/xics.h"
> #include "hw/pci/msi.h"
>
> #include "hw/pci/pci.h"
> @@ -117,33 +116,6 @@ static bool spapr_is_thread0_in_vcore(sPAPRMachineState
> *spapr,
> return spapr_get_vcpu_id(cpu) % spapr->vsmt == 0;
> }
>
> -static ICSState *spapr_ics_create(sPAPRMachineState *spapr,
> - const char *type_ics,
> - int nr_irqs, Error **errp)
> -{
> -Error *local_err = NULL;
> -Object *obj;
> -
> -obj = object_new(type_ics);
> -object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort);
> -object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr),
> - &error_abort);
> -object_property_set_int(obj, nr_irqs, "nr-irqs", &local_err);
> -if (local_err) {
> -goto error;
> -}
> -object_property_set_bool(obj, true, "realized", &local_err);
> -if (local_err) {
> -goto error;
> -}
> -
> -return ICS_BASE(obj);
> -
> -error:
> -error_p
Re: [Qemu-devel] [PATCH 2/2] hw/ppc: deprecate the machine type 'prep', replaced by '40p'
On Thu, Aug 09, 2018 at 10:40:10PM +0200, Hervé Poussineau wrote:
> - prep machine is a fictional machine, so has no specifications. Which
> devices can be changed/added/removed without impact? Are interrupts
> correctly mapped?
> - prep firmware (OHW) has support only for IDE drives (no SCSI).
> Booting from IDE has been broken approximatively 3 years ago, and nobody
> complained.
> - OHW is limited on IDE boot to a specific set of OS loaders.
> These operating systems are of the 2004 time frame.
> - OHW can use -kernel. Linux kernel freezes a long time after PS/2 mouse
> detection, and then screen becomes garbage. This was already broken in
> QEMU v2.7, 2 years ago, and nobody complained.
>
> On the other side:
> - 40p is a real machine, so emulation can be checked against
> hardware specifications
> - OpenBIOS has support for SCSI block devices, including 40p LSI adapter
> - OpenBIOS can start mostly all Linux kernels (including recent ones)
> and recent operating system (like NetBSD 7.1.2)
>
> Signed-off-by: Hervé Poussineau
Applied to ppc-for-3.1.
> ---
> hw/ppc/prep.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 1558855247..fbcaebabc0 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -676,6 +676,7 @@ static void ppc_prep_init(MachineState *machine)
>
> static void prep_machine_init(MachineClass *mc)
> {
> +mc->deprecation_reason = "use 40p machine type instead";
> mc->desc = "PowerPC PREP platform";
> mc->init = ppc_prep_init;
> mc->block_default_type = IF_IDE;
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH 1/2] hw/ppc: on 40p machine, change default firmware to OpenBIOS
On Thu, Aug 09, 2018 at 10:40:09PM +0200, Hervé Poussineau wrote:
> OpenBIOS gained 40p support in 5b20e4cacecb62fb2bdc6867c11d44cddd77c4ff
> Use it, instead of relying on the presence of a proprietary
> firmware.
Uh.. openhackware might be, well, a hack, but I don't think it's
propietary. Replacing it with openbios might well be a good idea, but
we want an accurrate reason for doing so.
> Signed-off-by: Hervé Poussineau
> ---
> hw/ppc/prep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 3401570d98..1558855247 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -736,7 +736,7 @@ static void ibm_40p_init(MachineState *machine)
> /* PCI host */
> dev = qdev_create(NULL, "raven-pcihost");
> if (!bios_name) {
> -bios_name = BIOS_FILENAME;
> +bios_name = "openbios-ppc";
> }
> qdev_prop_set_string(dev, "bios-name", bios_name);
> qdev_prop_set_uint32(dev, "elf-machine", PPC_ELF_MACHINE);
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH 3/3] vhost-scsi: expose 't10_pi' property for VIRTIO_SCSI_F_T10_PI
> On 8 Aug 2018, at 12:52, Greg Edwards wrote: > > Allow toggling on/off the VIRTIO_SCSI_F_T10_PI feature bit for both > vhost-scsi and vhost-user-scsi devices. > > Signed-off-by: Greg Edwards > --- > hw/scsi/vhost-scsi.c | 3 +++ > hw/scsi/vhost-user-scsi.c | 3 +++ > 2 files changed, 6 insertions(+) Reviewed-by: Felipe Franciosi
Re: [Qemu-devel] [PATCH 2/3] vhost-scsi: unify vhost-scsi get_features implementations
> On 8 Aug 2018, at 12:52, Greg Edwards wrote: > > Move the enablement of preset host features into the common > vhost_scsi_common_get_features() function. This is in preparation for > having vhost-scsi also make use of host_features. > > Signed-off-by: Greg Edwards > --- > hw/scsi/vhost-scsi-common.c | 3 +++ > hw/scsi/vhost-user-scsi.c | 14 +- > 2 files changed, 4 insertions(+), 13 deletions(-) Reviewed-by: Felipe Franciosi
Re: [Qemu-devel] [PATCH 1/3] vhost-user-scsi: move host_features into VHostSCSICommon
> On 8 Aug 2018, at 12:52, Greg Edwards wrote: > > In preparation for having vhost-scsi also make use of host_features, > move it from struct VHostUserSCSI into struct VHostSCSICommon. > > Signed-off-by: Greg Edwards > --- > hw/scsi/vhost-user-scsi.c | 15 --- > include/hw/virtio/vhost-scsi-common.h | 1 + > include/hw/virtio/vhost-user-scsi.h | 1 - > 3 files changed, 9 insertions(+), 8 deletions(-) > Reviewed-by: Felipe Franciosi
Re: [Qemu-devel] [RFC PATCH 0/4] "pc: acpi: _CST support"
On Thu, Aug 09, 2018 at 12:13:24PM +0200, Igor Mammedov wrote: > On Wed, 8 Aug 2018 23:20:25 +0300 > "Michael S. Tsirkin" wrote: > > > On Wed, Aug 08, 2018 at 05:15:45PM +0200, Igor Mammedov wrote: > > > It's an alternative approach to > > > 1) [PATCH hack dontapply v2 0/7] Dynamic _CST generation > > > which instead of dynamic AML loading uses static AML with > > > dynamic values. It allows us to keep firmware blob static and > > > to avoid split firmware issue (1) in case of cross version migration. > > > > I think there's a misunderstanding. That patch only declares a couple of > > states but that is just for debugging/demonstration purposes. A typical > > real CPU has more states (e.g. some intel CPUs have ~10 levels). > yep, baremetal has more CStates, but that depends on what we are trying to > achieve here. > If goal is to use mwait/monitor instead o halt when idle > to reduce wakeup latencies then 1 entry in _CST is sufficient. 1 entry is not sufficient in my testing. My testing shows we want to stay in guest for states with latency up to ~200 usec to get measureable latency gains. > If target is reduce power consumption than we would need more entries. > In the later case _CST is probably not sufficient as it provides only > power/latency pair while intel_idle also provides target residence. > So if power mgmt target is considered, we need to use new _LPI > which means guest will need support for it as well, not sure if x86 > kernel supports it nor aware of such support in Windows (hopefully it > would just ignore unknown method) I think we want to address both goals. Linux on x86 does support _LPI and I agree it's a good idea to be able to pass that in. Will take a look. But intel_idle seems to just set target = latency * 2 so I am not sure it's all that important. > > > > > ABI in this case is confined to cpu hotplug IO registers > > > (i.e. do it old school way, like we used to do so far). > > > This way we don't have to add yet another ABI to keep dynamic > > > AML code under control (1). > > > > > > Tested with: XPsp3 - ws2106 guests. > > > > > > CC: "Michael S. Tsirkin" > > > > > > > > > Igor Mammedov (3): > > > acpi: add aml_create_byte_field() > > > pc: acpi: add _CST support > > > acpi: add support for CST update notification > > > > > > Michael S. Tsirkin (1): > > > acpi: aml: add aml_register() > > > > > > include/hw/acpi/aml-build.h | 6 ++ > > > include/hw/acpi/cpu.h | 10 +++ > > > docs/specs/acpi_cpu_hotplug.txt | 21 +- > > > hw/acpi/aml-build.c | 28 +++ > > > hw/acpi/cpu.c | 158 > > > +++- > > > hw/acpi/piix4.c | 2 + > > > hw/i386/acpi-build.c| 5 +- > > > tests/bios-tables-test.c| 1 + > > > 8 files changed, 225 insertions(+), 6 deletions(-) > > > > > > -- > > > 2.7.4
Re: [Qemu-devel] [RFC PATCH 3/4] pc: acpi: add _CST support
On Thu, Aug 09, 2018 at 12:00:11PM +0200, Igor Mammedov wrote: > On Wed, 8 Aug 2018 23:17:48 +0300 > "Michael S. Tsirkin" wrote: > > > On Wed, Aug 08, 2018 at 05:15:48PM +0200, Igor Mammedov wrote: > > > Reuse CPU hotplug IO registers for passing a CST entry > > > containing package for shalowest C1 using mwait and > > > read it out in guest with new CCST AML method. > > > > I don't see how 1 entry is enough. We need to describe full _CST package so > > that guest can do reasonable power management on the CPU. > I was assuming that CST are to be used for mwait/monitor idle combo > to help reduce latencies/vmexits on cpus wakeup from idle state, hence 1 > entry. > It's trivial to add as much as needed entries though. > If you are fine with approach I can respin patch with arbitrary > number of CStates. You will find that the code then becomes very messy, and dynamic size of the package is one of the things that makes it messy. > > > The CState support is optional and could be turned on > > > with '-global PIIX4_PM.cstate=on' CLI option. > > > > > > Signed-off-by: Igor Mammedov > > > --- > [...]
Re: [Qemu-devel] [PATCH v2 09/22] virtio: Remove unneeded includes
On Thu, Aug 09, 2018 at 08:03:03PM +0100, Dr. David Alan Gilbert wrote: > * Juan Quintela (quint...@redhat.com) wrote: > > They are all already included in virtio_pci.h. All I see in virtio_pci.h is: #include "standard-headers/linux/types.h" Weird. BTW what's the point of this patch? Generally it's best not to depend on headers including each other, it makes refactoring harder. > > > > Signed-off-by: Juan Quintela > > > cc'ing in mst for virtio goodness. > > > > --- > > hw/virtio/virtio-pci.c | 15 +-- > > 1 file changed, 1 insertion(+), 14 deletions(-) > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > index 3a01fe90f0..cb2ddd3f41 100644 > > --- a/hw/virtio/virtio-pci.c > > +++ b/hw/virtio/virtio-pci.c > > @@ -18,24 +18,11 @@ > > #include "qemu/osdep.h" > > > > #include "standard-headers/linux/virtio_pci.h" > > -#include "hw/virtio/virtio.h" > > -#include "hw/virtio/virtio-blk.h" > > -#include "hw/virtio/virtio-net.h" > > -#include "hw/virtio/virtio-serial.h" > > -#include "hw/virtio/virtio-scsi.h" > > -#include "hw/virtio/virtio-balloon.h" > > -#include "hw/virtio/virtio-input.h" > > -#include "hw/pci/pci.h" > > +#include "virtio-pci.h" > > #include "qapi/error.h" > > -#include "qemu/error-report.h" > > -#include "hw/pci/msi.h" > > #include "hw/pci/msix.h" > > -#include "hw/loader.h" > > #include "sysemu/kvm.h" > > -#include "virtio-pci.h" > > #include "qemu/range.h" > > -#include "hw/virtio/virtio-bus.h" > > -#include "qapi/visitor.h" > > > > #define VIRTIO_PCI_REGION_SIZE(dev) > > VIRTIO_PCI_CONFIG_OFF(msix_present(dev)) > > > > -- > > 2.17.1 > > > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] virtio: add support for in-order feature
On Thu, Aug 09, 2018 at 07:54:37PM +0300, Ilya Maximets wrote:
> New feature bit for in-order feature of the upcoming
> virtio 1.1. It's already supported by DPDK vhost-user
> and virtio implementations. These changes required to
> allow feature negotiation.
>
> Signed-off-by: Ilya Maximets
> ---
>
> I just wanted to test this new feature in DPDK but failed
> to found required patch for QEMU side. So, I implemented it.
> At least it will be helpful for someone like me, who wants
> to evaluate VIRTIO_F_IN_ORDER with DPDK.
>
> hw/net/vhost_net.c | 1 +
> include/hw/virtio/virtio.h | 12 +++-
> include/standard-headers/linux/virtio_config.h | 7 +++
> 3 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index e037db6..86879c5 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -78,6 +78,7 @@ static const int user_feature_bits[] = {
> VIRTIO_NET_F_MRG_RXBUF,
> VIRTIO_NET_F_MTU,
> VIRTIO_F_IOMMU_PLATFORM,
> +VIRTIO_F_IN_ORDER,
>
> /* This bit implies RARP isn't sent by QEMU out of band */
> VIRTIO_NET_F_GUEST_ANNOUNCE,
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 9c1fa07..a422025 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -254,16 +254,18 @@ typedef struct virtio_input_conf virtio_input_conf;
> typedef struct VirtIOSCSIConf VirtIOSCSIConf;
> typedef struct VirtIORNGConf VirtIORNGConf;
>
> -#define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \
> +#define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \
> DEFINE_PROP_BIT64("indirect_desc", _state, _field,\
>VIRTIO_RING_F_INDIRECT_DESC, true), \
> DEFINE_PROP_BIT64("event_idx", _state, _field,\
>VIRTIO_RING_F_EVENT_IDX, true), \
> DEFINE_PROP_BIT64("notify_on_empty", _state, _field, \
> - VIRTIO_F_NOTIFY_ON_EMPTY, true), \
> -DEFINE_PROP_BIT64("any_layout", _state, _field, \
> - VIRTIO_F_ANY_LAYOUT, true), \
> -DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
> + VIRTIO_F_NOTIFY_ON_EMPTY, true),\
> +DEFINE_PROP_BIT64("any_layout", _state, _field, \
> + VIRTIO_F_ANY_LAYOUT, true), \
> +DEFINE_PROP_BIT64("in_order", _state, _field, \
> + VIRTIO_F_IN_ORDER, true), \
> +DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
>VIRTIO_F_IOMMU_PLATFORM, false)
Is in_order really right for all virtio devices?
> hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> diff --git a/include/standard-headers/linux/virtio_config.h
> b/include/standard-headers/linux/virtio_config.h
> index b777069..d20398c 100644
> --- a/include/standard-headers/linux/virtio_config.h
> +++ b/include/standard-headers/linux/virtio_config.h
> @@ -71,4 +71,11 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Inorder feature indicates that all buffers are used by the device
> + * in the same order in which they have been made available.
> + */
> +#define VIRTIO_F_IN_ORDER 35
> +
> #endif /* _LINUX_VIRTIO_CONFIG_H */
> --
> 2.7.4
Re: [Qemu-devel] [PATCH v2 00/11] block: Deal with filters
On 2018-08-10 00:31, Max Reitz wrote: [...] > v2: > - Patch 4: We must clear BDS.exact_filename for filter nodes, or we > basically end up with a random filename for them. This is achieved by > pulling the !drv->is_filter check into an inner condition. > (Fixes iotests 40 and 184) > > - Patch 7: iotest 228 tried to use QAPI's backing_file information > (which was very inconsistent, so with 228 doing some funky backing > stuff, it really showed) to get some idea of what qemu thinks about > the image header. Let's make it use full-backing-filename (and that > now really reflects the image header). *cough* *cough* *cough* git-backport-diff against v1: Key: [] : patches are identical [] : number of functional differences between upstream/downstream patch [down] : patch is downstream-only The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively 001/11:[] [--] 'block: Mark commit and mirror as filter drivers' 002/11:[] [--] 'blockdev: Check @replaces in blockdev_mirror_common' 003/11:[] [--] 'block: Filtered children access functions' 004/11:[0005] [FC] 'block: Storage child access function' 005/11:[] [--] 'block: Fix check_to_replace_node()' 006/11:[] [--] 'iotests: Add tests for mirror @replaces loops' 007/11:[0012] [FC] 'block: Leave BDS.backing_file constant' 008/11:[] [--] 'iotests: Add filter commit test cases' 009/11:[] [--] 'iotests: Add filter mirror test cases' 010/11:[] [--] 'iotests: Add test for commit in sub directory' 011/11:[] [--] 'iotests: Test committing to overridden backing' > Max Reitz (11): > block: Mark commit and mirror as filter drivers > blockdev: Check @replaces in blockdev_mirror_common > block: Filtered children access functions > block: Storage child access function > block: Fix check_to_replace_node() > iotests: Add tests for mirror @replaces loops > block: Leave BDS.backing_file constant > iotests: Add filter commit test cases > iotests: Add filter mirror test cases > iotests: Add test for commit in sub directory > iotests: Test committing to overridden backing > > qapi/block-core.json | 4 + > include/block/block.h | 2 + > include/block/block_int.h | 53 - > block.c| 341 - > block/backup.c | 8 +- > block/block-backend.c | 16 +- > block/commit.c | 38 ++-- > block/io.c | 47 +++-- > block/mirror.c | 39 ++-- > block/qapi.c | 38 ++-- > block/snapshot.c | 40 ++-- > block/stream.c | 15 +- > blockdev.c | 167 > migration/block-dirty-bitmap.c | 4 +- > nbd/server.c | 8 +- > qemu-img.c | 24 ++- > tests/qemu-iotests/020 | 36 > tests/qemu-iotests/020.out | 10 + > tests/qemu-iotests/040 | 191 ++ > tests/qemu-iotests/040.out | 4 +- > tests/qemu-iotests/041 | 270 +- > tests/qemu-iotests/041.out | 4 +- > tests/qemu-iotests/191.out | 1 - > tests/qemu-iotests/228 | 6 +- > tests/qemu-iotests/228.out | 6 +- > 25 files changed, 1141 insertions(+), 231 deletions(-) > signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v2 10/11] iotests: Add test for commit in sub directory
Add a test for committing an overlay in a sub directory to one of the
images in its backing chain, using both relative and absolute filenames.
Signed-off-by: Max Reitz
---
tests/qemu-iotests/020 | 36
tests/qemu-iotests/020.out | 10 ++
2 files changed, 46 insertions(+)
diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020
index eac5080f83..d0ac33922d 100755
--- a/tests/qemu-iotests/020
+++ b/tests/qemu-iotests/020
@@ -32,6 +32,11 @@ _cleanup()
_cleanup_test_img
rm -f "$TEST_IMG.base"
rm -f "$TEST_IMG.orig"
+
+rm -f "$TEST_DIR/subdir/t.$IMGFMT.base"
+rm -f "$TEST_DIR/subdir/t.$IMGFMT.mid"
+rm -f "$TEST_DIR/subdir/t.$IMGFMT"
+rmdir "$TEST_DIR/subdir" &> /dev/null
}
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -135,6 +140,37 @@ $QEMU_IO -c 'writev 0 64k' "$TEST_IMG" | _filter_qemu_io
$QEMU_IMG commit "$TEST_IMG"
_cleanup
+
+echo
+echo 'Testing commit in sub-directory with relative filenames'
+echo
+
+pushd "$TEST_DIR" > /dev/null
+
+mkdir subdir
+
+TEST_IMG="subdir/t.$IMGFMT.base" _make_test_img 1M
+TEST_IMG="subdir/t.$IMGFMT.mid" _make_test_img -b "t.$IMGFMT.base"
+TEST_IMG="subdir/t.$IMGFMT" _make_test_img -b "t.$IMGFMT.mid"
+
+# Should work
+$QEMU_IMG commit -b "t.$IMGFMT.mid" "subdir/t.$IMGFMT"
+
+# Might theoretically work, but does not in practice (we have to
+# decide between this and the above; and since we always represent
+# backing file names as relative to the overlay, we go for the above)
+$QEMU_IMG commit -b "subdir/t.$IMGFMT.mid" "subdir/t.$IMGFMT" 2>&1 | \
+_filter_imgfmt
+
+# This should work as well
+$QEMU_IMG commit -b "$TEST_DIR/subdir/t.$IMGFMT.mid" "subdir/t.$IMGFMT"
+
+popd > /dev/null
+
+# Now let's try with just absolute filenames
+$QEMU_IMG commit -b "$TEST_DIR/subdir/t.$IMGFMT.mid" \
+"$TEST_DIR/subdir/t.$IMGFMT"
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/020.out b/tests/qemu-iotests/020.out
index 4b722b2dd0..228c37dded 100644
--- a/tests/qemu-iotests/020.out
+++ b/tests/qemu-iotests/020.out
@@ -1094,4 +1094,14 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
backing_file=json:{'driv
wrote 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qemu-img: Block job failed: No space left on device
+
+Testing commit in sub-directory with relative filenames
+
+Formatting 'subdir/t.IMGFMT.base', fmt=IMGFMT size=1048576
+Formatting 'subdir/t.IMGFMT.mid', fmt=IMGFMT size=1048576
backing_file=t.IMGFMT.base
+Formatting 'subdir/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=t.IMGFMT.mid
+Image committed.
+qemu-img: Did not find 'subdir/t.IMGFMT.mid' in the backing chain of
'subdir/t.IMGFMT'
+Image committed.
+Image committed.
*** done
--
2.17.1
Re: [Qemu-devel] [PATCH v2 06/11] iotests: Add tests for mirror @replaces loops
On 2018-08-10 00:31, Max Reitz wrote: > This adds two tests for cases where our old check_to_replace_node() > function failed to detect that executing this job with these parameters > would result in a cyclic graph. > > Signed-off-by: Max Reitz > --- > tests/qemu-iotests/041 | 124 + > tests/qemu-iotests/041.out | 4 +- > 2 files changed, 126 insertions(+), 2 deletions(-) Sometimes, I hate myself a little bit. I totally forgot to run the test with QED. So: > diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 > index c20ac7da87..186bd0f031 100755 > --- a/tests/qemu-iotests/041 > +++ b/tests/qemu-iotests/041 > @@ -1061,5 +1061,129 @@ class TestOrphanedSource(iotests.QMPTestCase): [...] > +'driver': 'qcow2', [...] > +'driver': 'qcow2', [...] > +'driver': 'qcow2', Imagine these to be "'driver': iotests.imgfmt". With that, it passes. (I won't send a v3 specifically for this, but it will be included in v3. Eventually.) Max signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v2 09/11] iotests: Add filter mirror test cases
This patch adds some test cases how mirroring relates to filters. One
of them tests what happens when you mirror off a filtered COW node, two
others use the mirror filter node as basically our only example of an
implicitly created filter node so far (besides the commit filter).
Signed-off-by: Max Reitz
---
tests/qemu-iotests/041 | 146 -
tests/qemu-iotests/041.out | 4 +-
2 files changed, 147 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 186bd0f031..c21c8b36d5 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -20,8 +20,9 @@
import time
import os
+import json
import iotests
-from iotests import qemu_img, qemu_io
+from iotests import qemu_img, qemu_img_pipe, qemu_io
backing_img = os.path.join(iotests.test_dir, 'backing.img')
target_backing_img = os.path.join(iotests.test_dir, 'target-backing.img')
@@ -1185,5 +1186,148 @@ class TestReplaces(iotests.QMPTestCase):
os.remove(test_img)
os.remove(target_img)
+# Tests for mirror with filters (and how the mirror filter behaves, as
+# an example for an implicit filter)
+class TestFilters(iotests.QMPTestCase):
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, backing_img, '1M')
+qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, test_img)
+qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, target_img)
+
+qemu_io('-c', 'write -P 1 0 512k', backing_img)
+qemu_io('-c', 'write -P 2 512k 512k', test_img)
+
+self.vm = iotests.VM()
+self.vm.launch()
+
+result = self.vm.qmp('blockdev-add', **{
+'node-name': 'target',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': target_img
+},
+'backing': None
+})
+self.assert_qmp(result, 'return', {})
+
+self.filterless_chain = {
+'node-name': 'source',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': test_img
+},
+'backing': {
+'node-name': 'backing',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': backing_img
+}
+}
+}
+
+def tearDown(self):
+self.vm.shutdown()
+
+os.remove(test_img)
+os.remove(target_img)
+os.remove(backing_img)
+
+def test_cor(self):
+result = self.vm.qmp('blockdev-add', **{
+'node-name': 'filter',
+'driver': 'copy-on-read',
+'file': self.filterless_chain
+})
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('blockdev-mirror',
+ job_id='mirror',
+ device='filter',
+ target='target',
+ sync='top')
+self.assert_qmp(result, 'return', {})
+
+self.complete_and_wait('mirror')
+
+self.vm.qmp('blockdev-del', node_name='target')
+
+target_map = qemu_img_pipe('map', '--output=json', target_img)
+target_map = json.loads(target_map)
+
+assert target_map[0]['start'] == 0
+assert target_map[0]['length'] == 512 * 1024
+assert target_map[0]['depth'] == 1
+
+assert target_map[1]['start'] == 512 * 1024
+assert target_map[1]['length'] == 512 * 1024
+assert target_map[1]['depth'] == 0
+
+def test_implicit_mirror_filter(self):
+result = self.vm.qmp('blockdev-add', **self.filterless_chain)
+self.assert_qmp(result, 'return', {})
+
+# We need this so we can query from above the mirror node
+result = self.vm.qmp('device_add',
+ driver='virtio-blk',
+ id='virtio',
+ bus='pci.0',
+ drive='source')
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('blockdev-mirror',
+ job_id='mirror',
+ device='source',
+ target='target',
+ sync='top')
+self.assert_qmp(result, 'return', {})
+
+# The mirror filter is now an implicit node, so it should be
+# invisible when querying the backing chain
+device_info = self.vm.qmp('query-block')['return'][0]
+assert device_info['qdev'] ==
'/machine/peripheral/vir
[Qemu-devel] [PATCH v2 07/11] block: Leave BDS.backing_file constant
Parts of the block layer treat BDS.backing_file as if it were whatever
the image header says (i.e., if it is a relative path, it is relative to
the overlay), other parts treat it like a cache for
bs->backing->bs->filename (relative paths are relative to the CWD).
Considering bs->backing->bs->filename exists, let us make it mean the
former.
Among other things, this now allows the user to specify a base when
using qemu-img to commit an image file in a directory that is not the
CWD (assuming, everything uses relative filenames).
Before this patch:
$ ./qemu-img create -f qcow2 foo/bot.qcow2 1M
$ ./qemu-img create -f qcow2 -b bot.qcow2 foo/mid.qcow2
$ ./qemu-img create -f qcow2 -b mid.qcow2 foo/top.qcow2
$ ./qemu-img commit -b mid.qcow2 foo/top.qcow2
qemu-img: Did not find 'mid.qcow2' in the backing chain of 'foo/top.qcow2'
$ ./qemu-img commit -b foo/mid.qcow2 foo/top.qcow2
qemu-img: Did not find 'foo/mid.qcow2' in the backing chain of 'foo/top.qcow2'
$ ./qemu-img commit -b $PWD/foo/mid.qcow2 foo/top.qcow2
qemu-img: Did not find '[...]/foo/mid.qcow2' in the backing chain of
'foo/top.qcow2'
After this patch:
$ ./qemu-img commit -b mid.qcow2 foo/top.qcow2
Image committed.
$ ./qemu-img commit -b foo/mid.qcow2 foo/top.qcow2
qemu-img: Did not find 'foo/mid.qcow2' in the backing chain of 'foo/top.qcow2'
$ ./qemu-img commit -b $PWD/foo/mid.qcow2 foo/top.qcow2
Image committed.
With this change, bdrv_find_backing_image() must look at whether the
user has overridden a BDS's backing file. If so, it can no longer use
bs->backing_file, but must instead compare the given filename against
the backing node's filename directly.
Note that this changes the QAPI output for a node's backing_file. We
had very inconsistent output there (sometimes what the image header
said, sometimes the actual filename of the backing image). This
inconsistent output was effectively useless, so we have to decide one
way or the other. Considering that bs->backing_file usually at runtime
contained the path to the image relative to qemu's CWD (or absolute),
this patch changes QAPI's backing_file to always report the
bs->backing->bs->filename from now on. If you want to receive the image
header information, you have to refer to full-backing-filename.
This necessitates a change to iotest 228. The interesting information
it really wanted is the image header, and it can get that now, but it
has to use full-backing-filename instead of backing_file. Because of
this patch's changes to bs->backing_file's behavior, we also need some
reference output changes.
Along with the changes to bs->backing_file, stop updating
BDS.backing_format in bdrv_backing_attach() as well. This necessitates
a change to the reference output of iotest 191.
Signed-off-by: Max Reitz
---
include/block/block_int.h | 14 +-
block.c| 29 ++---
block/qapi.c | 7 ---
qemu-img.c | 12 ++--
tests/qemu-iotests/191.out | 1 -
tests/qemu-iotests/228 | 6 +++---
tests/qemu-iotests/228.out | 6 +++---
7 files changed, 51 insertions(+), 24 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index d3d8b22155..8f2c515ec1 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -737,11 +737,15 @@ struct BlockDriverState {
bool walking_aio_notifiers; /* to make removal during iteration safe */
char filename[PATH_MAX];
-char backing_file[PATH_MAX]; /* if non zero, the image is a diff of
-this file image */
-/* The backing filename indicated by the image header; if we ever
- * open this file, then this is replaced by the resulting BDS's
- * filename (i.e. after a bdrv_refresh_filename() run). */
+/* If non-zero, the image is a diff of this image file. Note that
+ * this the name given in the image header and may therefore not
+ * be equal to .backing->bs->filename, and relative paths are
+ * resolved relatively to their overlay. */
+char backing_file[PATH_MAX];
+/* The backing filename indicated by the image header. Contrary
+ * to backing_file, if we ever open this file, auto_backing_file
+ * is replaced by the resulting BDS's filename (i.e. after a
+ * bdrv_refresh_filename() run). */
char auto_backing_file[PATH_MAX];
char backing_format[16]; /* if non-zero and backing_file exists */
diff --git a/block.c b/block.c
index 69b56771f7..434074b7dd 100644
--- a/block.c
+++ b/block.c
@@ -78,6 +78,8 @@ static BlockDriverState *bdrv_open_inherit(const char
*filename,
const BdrvChildRole *child_role,
Error **errp);
+static bool bdrv_backing_overridden(BlockDriverState *bs);
+
/* If non-zero, use only whitelisted block drivers */
static int use_bdrv_whitelist;
@@ -995,10 +997,6 @@ static void bdrv_backing_attach(BdrvChild *c)
bdrv_refre
[Qemu-devel] [PATCH v2 06/11] iotests: Add tests for mirror @replaces loops
This adds two tests for cases where our old check_to_replace_node()
function failed to detect that executing this job with these parameters
would result in a cyclic graph.
Signed-off-by: Max Reitz
---
tests/qemu-iotests/041 | 124 +
tests/qemu-iotests/041.out | 4 +-
2 files changed, 126 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index c20ac7da87..186bd0f031 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1061,5 +1061,129 @@ class TestOrphanedSource(iotests.QMPTestCase):
target='dest-ro')
self.assert_qmp(result, 'error/class', 'GenericError')
+# Various tests for the @replaces option (independent of quorum)
+class TestReplaces(iotests.QMPTestCase):
+def setUp(self):
+self.vm = iotests.VM()
+self.vm.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+
+def test_drive_mirror_loop(self):
+qemu_img('create', '-f', iotests.imgfmt, test_img, '1M')
+
+result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg')
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('blockdev-add', **{
+'node-name': 'source',
+'driver': 'throttle',
+'throttle-group': 'tg',
+'file': {
+'node-name': 'filtered',
+'driver': 'qcow2',
+'file': {
+'driver': 'file',
+'filename': test_img
+}
+}
+})
+self.assert_qmp(result, 'return', {})
+
+# Mirror from @source to @target in sync=none, so that @source
+# will be @target's backing file; but replace @filtered.
+# Then, @target's backing file will be @source, whose backing
+# file is now @target instead of @filtered. That is a loop.
+# (But apart from the loop, replacing @filtered instead of
+# @source is fine, because both are just filtered versions of
+# each other.)
+result = self.vm.qmp('drive-mirror',
+ job_id='mirror',
+ device='source',
+ target=target_img,
+ format=iotests.imgfmt,
+ node_name='target',
+ sync='none',
+ replaces='filtered')
+if 'error' in result:
+# This is the correct result
+self.assert_qmp(result, 'error/class', 'GenericError')
+else:
+# This is wrong, but let's run it to the bitter conclusion
+self.complete_and_wait(drive='mirror')
+# Fail for good measure, although qemu should have crashed
+# anyway
+self.fail('Loop creation was successful')
+
+os.remove(test_img)
+try:
+os.remove(target_img)
+except OSError:
+pass
+
+def test_blockdev_mirror_loop(self):
+qemu_img('create', '-f', iotests.imgfmt, test_img, '1M')
+qemu_img('create', '-f', iotests.imgfmt, target_img, '1M')
+
+result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg')
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('blockdev-add', **{
+'node-name': 'source',
+'driver': 'throttle',
+'throttle-group': 'tg',
+'file': {
+'node-name': 'middle',
+'driver': 'throttle',
+'throttle-group': 'tg',
+'file': {
+'node-name': 'bottom',
+'driver': 'qcow2',
+'file': {
+'driver': 'file',
+'filename': test_img
+}
+}
+}
+})
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('blockdev-add', **{
+'node-name': 'target',
+'driver': 'qcow2',
+'file': {
+'driver': 'file',
+'filename': target_img
+},
+'backing': 'middle'
+})
+
+# Mirror from @source to @target. With blockdev-mirror, the
+# current (old) backing file is retained (which is @middle).
+# By replacing @bottom, @middle's file will be @target, whose
+# backing file is @middle again. That is a loop.
+# (But apart from the loop, replacing @bottom instead of
+# @source is fine, because both are just filtered versions of
+# each other.)
+
[Qemu-devel] [PATCH v2 00/11] block: Deal with filters
Note 1: This series depends on v10 of my “block: Fix some filename generation issues” series. Based-on: <20180809213528.14738-1-mre...@redhat.com> Note 2: This is technically the first part of my active mirror followup. But just very technically. I noticed that that followup started to consist of two parts, namely (A) fix filtery things in the block layer, and (B) fix active mirror. So I decided to split it. This is part A. Part B comes later. When we introduced filters, we did it a bit casually. Sure, we talked a lot about them before, but that was mostly discussion about where implicit filters should be added to the graph (note that we currently only have two implicit filters, those being mirror and commit). But in the end, we really just designated some drivers filters (Quorum, blkdebug, etc.) and added some specifically (throttle, COR), without really looking through the block layer to see where issues might occur. It turns out vast areas of the block layer just don't know about filters and cannot really handle them. Many cases will work in practice, in others, well, too bad, you cannot use some feature because some part deep inside the block layer looks at your filters and thinks they are format nodes. This series sets out to correct a bit of that. I lost my head many times and I'm sure this series is incomplete in many ways, but it really doesn't do any good if it sits on my disk any longer, it needs to go out now. The most important patches of this series are patches 3 and 4. These introduce functions to encapsulate bs->backing and bs->file accesses. Because sometimes, bs->backing means COW, sometimes it means filtered node. And sometimes, bs->file means metadata storage, and sometimes it means filtered node. With this functions, it's always clear what the caller wants, and it will always get what it wants. Besides that, patch 3 introduces functions to skip filters which may be used by parts of the block layer that just don't care about them. Secondly, the restraints put on mirror's @replaces parameter are revisited and fixed. Thirdly, BDS.backing_file is changed to be constant. I don't quite know why we modify it whenever we change a BDS's backing file, but that's definitely not quite right. This fixes things like being able to perform a commit on a file (using relative filenames) in a directory that's not qemu's CWD. Finally, a number of tests are added. There are probably many things that are worthy of discussion, of which only some come to my head, e.g.: - In which cases do we want to skip filters, in which cases do we want to skip implicit filters? My approach was to basically never skip explicitly added filters, except when it's about finding a file in some tree (e.g. in a backing chain). Maybe there are cases where you think we should skip even explicitly added filters. - I made interesting decisions like “When you mirror from a node, we should indeed mirror from that node, but when replacing it, we should skip leave all implicit filters on top intact.” You may disagree with that. (My reasoning here is that users aren't supposed to know about implicit filters, and therefore, they should not intend to remove them. Also, mirror accepts only root nodes as the source, so you cannot really specify the node below the implicit filters. But you can use @replaces to drop the implicit filters, if you know they are there.) v2: - Patch 4: We must clear BDS.exact_filename for filter nodes, or we basically end up with a random filename for them. This is achieved by pulling the !drv->is_filter check into an inner condition. (Fixes iotests 40 and 184) - Patch 7: iotest 228 tried to use QAPI's backing_file information (which was very inconsistent, so with 228 doing some funky backing stuff, it really showed) to get some idea of what qemu thinks about the image header. Let's make it use full-backing-filename (and that now really reflects the image header). Max Reitz (11): block: Mark commit and mirror as filter drivers blockdev: Check @replaces in blockdev_mirror_common block: Filtered children access functions block: Storage child access function block: Fix check_to_replace_node() iotests: Add tests for mirror @replaces loops block: Leave BDS.backing_file constant iotests: Add filter commit test cases iotests: Add filter mirror test cases iotests: Add test for commit in sub directory iotests: Test committing to overridden backing qapi/block-core.json | 4 + include/block/block.h | 2 + include/block/block_int.h | 53 - block.c| 341 - block/backup.c | 8 +- block/block-backend.c | 16 +- block/commit.c | 38 ++-- block/io.c | 47 +++-- block/mirror.c | 39 ++-- block/qapi.c | 38 ++-- block/snapshot.c |
[Qemu-devel] [PATCH v2 08/11] iotests: Add filter commit test cases
This patch adds some tests on how commit copes with filter nodes.
Signed-off-by: Max Reitz
---
tests/qemu-iotests/040 | 130 +
tests/qemu-iotests/040.out | 4 +-
2 files changed, 132 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 1beb5e6dab..f0544d6107 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -346,5 +346,135 @@ class TestReopenOverlay(ImageCommitTestCase):
def test_reopen_overlay(self):
self.run_commit_test(self.img1, self.img0)
+class TestCommitWithFilters(iotests.QMPTestCase):
+img0 = os.path.join(iotests.test_dir, '0.img')
+img1 = os.path.join(iotests.test_dir, '1.img')
+img2 = os.path.join(iotests.test_dir, '2.img')
+img3 = os.path.join(iotests.test_dir, '3.img')
+
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, self.img0, '1M')
+qemu_img('create', '-f', iotests.imgfmt, self.img1, '1M')
+qemu_img('create', '-f', iotests.imgfmt, self.img2, '1M')
+qemu_img('create', '-f', iotests.imgfmt, self.img3, '1M')
+
+self.vm = iotests.VM()
+self.vm.launch()
+result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg')
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('blockdev-add', **{
+'node-name': 'top-filter',
+'driver': 'throttle',
+'throttle-group': 'tg',
+'file': {
+'node-name': 'cow-3',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img3
+},
+'backing': {
+'node-name': 'cow-2',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img2
+},
+'backing': {
+'node-name': 'cow-1',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img1
+},
+'backing': {
+'node-name': 'bottom-filter',
+'driver': 'throttle',
+'throttle-group': 'tg',
+'file': {
+'node-name': 'cow-0',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img0
+}
+}
+}
+}
+}
+}
+})
+self.assert_qmp(result, 'return', {})
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(self.img3)
+os.remove(self.img2)
+os.remove(self.img1)
+os.remove(self.img0)
+
+# Filters make for funny filenames, so we cannot just use
+# self.imgX for the block-commit parameters
+def get_filename(self, node):
+return self.vm.node_info(node)['image']['filename']
+
+def test_filterless_commit(self):
+self.assert_no_active_block_jobs()
+result = self.vm.qmp('block-commit',
+ job_id='commit',
+ device='top-filter',
+ top=self.get_filename('cow-2'),
+ base=self.get_filename('cow-1'))
+self.assert_qmp(result, 'return', {})
+self.wait_until_completed(drive='commit')
+
+def test_commit_through_filter(self):
+self.assert_no_active_block_jobs()
+result = self.vm.qmp('block-commit',
+ job_id='commit',
+ device='top-filter',
+ top=self.get_filename('cow-1'),
+ base=self.get_filename('cow-0'))
+# Cannot commit through explicitly added filters (yet,
+# although in the future we probably want to make users use
+# blockdev-copy for this)
+self.assert_qmp(result, 'error/class', 'GenericError')
+self.assert_qmp(result, 'error/desc', 'Cannot commit through explicit
filter nodes')
+
+def test_filtered_active_commit_with_filter(self):
+self.assert_no_active_block_jobs()
+result = self.vm.qmp('block-commit',
+ job_id='commit',
+ device='top-filter',
+ base=self.get_filename('cow-2'))
+# Not spe
[Qemu-devel] [PATCH] i386: Disable TOPOEXT by default on "-cpu host"
Enabling TOPOEXT is always allowed, but it can't be enabled
blindly by "-cpu host" because it may make guests crash if the
rest of the cache topology information isn't provided or isn't
consistent.
This addresses the bug reported at:
https://bugzilla.redhat.com/show_bug.cgi?id=1613277
Signed-off-by: Eduardo Habkost
---
target/i386/cpu.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 723e02221e..3ac627978f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -849,6 +849,12 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
.cpuid_eax = 0x8001, .cpuid_reg = R_ECX,
.tcg_features = TCG_EXT3_FEATURES,
+/*
+ * TOPOEXT is always allowed but can't be enabled blindly by
+ * "-cpu host", as it requires consistent cache topology info
+ * to be provided so it doesn't confuse guests.
+ */
+.no_autoenable_flags = CPUID_EXT3_TOPOEXT,
},
[FEAT_C000_0001_EDX] = {
.feat_names = {
--
2.18.0.rc1.1.g3f1ff2140
[Qemu-devel] [PATCH v2 03/11] block: Filtered children access functions
What bs->file and bs->backing mean depends on the node. For filter
nodes, both signify a node that will eventually receive all R/W
accesses. For format nodes, bs->file contains metadata and data, and
bs->backing will not receive writes -- instead, writes are COWed to
bs->file. Usually.
In any case, it is not trivial to guess what a child means exactly with
our currently limited form of expression. It is better to introduce
some functions that actually guarantee a meaning:
- bdrv_filtered_cow_child() will return the child that receives requests
filtered through COW. That is, reads may or may not be forwarded
(depending on the overlay's allocation status), but writes never go to
this child.
- bdrv_filtered_rw_child() will return the child that receives requests
filtered through some very plain process. Reads and writes issued to
the parent will go to the child as well (although timing, etc. may be
modified).
- All drivers but quorum (but quorum is pretty opaque to the general
block layer anyway) always only have one of these children: All read
requests must be served from the filtered_rw_child (if it exists), so
if there was a filtered_cow_child in addition, it would not receive
any requests at all.
(The closest here is mirror, where all requests are passed on to the
source, but with write-blocking, write requests are "COWed" to the
target. But that just means that the target is a special child that
cannot be introspected by the generic block layer functions, and that
source is a filtered_rw_child.)
Therefore, we can also add bdrv_filtered_child() which returns that
one child (or NULL, if there is no filtered child).
Also, many places in the current block layer should be skipping filters
(all filters or just the ones added implicitly, it depends) when going
through a block node chain. They do not do that currently, but this
patch makes them.
Signed-off-by: Max Reitz
---
qapi/block-core.json | 4 +
include/block/block.h | 1 +
include/block/block_int.h | 33 +-
block.c| 184 -
block/backup.c | 8 +-
block/block-backend.c | 16 ++-
block/commit.c | 36 ---
block/io.c | 27 ++---
block/mirror.c | 37 ---
block/qapi.c | 26 ++---
block/stream.c | 15 ++-
blockdev.c | 84 ---
migration/block-dirty-bitmap.c | 4 +-
nbd/server.c | 8 +-
qemu-img.c | 12 ++-
15 files changed, 363 insertions(+), 132 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f20efc97f7..a71df88eb2 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2248,6 +2248,10 @@
# On successful completion the image file is updated to drop the backing file
# and the BLOCK_JOB_COMPLETED event is emitted.
#
+# In case @device is a filter node, block-stream modifies the first non-filter
+# overlay node below it to point to base's backing node (or NULL if @base was
+# not specified) instead of modifying @device itself.
+#
# @job-id: identifier for the newly-created block job. If
# omitted, the device name will be used. (Since 2.7)
#
diff --git a/include/block/block.h b/include/block/block.h
index 7ef118a704..a01986495d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -452,6 +452,7 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
const char *node_name,
Error **errp);
bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base);
+bool bdrv_legacy_chain_contains(BlockDriverState *top, BlockDriverState *base);
BlockDriverState *bdrv_next_node(BlockDriverState *bs);
BlockDriverState *bdrv_next_all_states(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 90217512b5..fa9154899d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -91,6 +91,7 @@ struct BlockDriver {
* certain callbacks that refer to data (see block.c) to their bs->file if
* the driver doesn't implement them. Drivers that do not wish to forward
* must implement them and return -ENOTSUP.
+ * Note that filters are not allowed to modify data.
*/
bool is_filter;
/* for snapshots block filter like Quorum can implement the
@@ -887,11 +888,6 @@ typedef enum BlockMirrorBackingMode {
MIRROR_LEAVE_BACKING_CHAIN,
} BlockMirrorBackingMode;
-static inline BlockDriverState *backing_bs(BlockDriverState *bs)
-{
-return bs->backing ? bs->backing->bs : NULL;
-}
-
/* Essential block drivers which must always be statically linked into qemu,
and
* which therefore can be accessed without using bdrv_find_format() */
@@ -1215,4 +1211,31 @@ int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src,
uint64_t src_of
[Qemu-devel] [PATCH v2 02/11] blockdev: Check @replaces in blockdev_mirror_common
There is no reason why the constraints we put on @replaces should be
limited to drive-mirror. Therefore, move the sanity checks from
qmp_drive_mirror() to blockdev_mirror_common() so they apply to
blockdev-mirror as well.
Signed-off-by: Max Reitz
---
blockdev.c | 55 --
1 file changed, 33 insertions(+), 22 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 9b143d9e72..2d61588a9a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3644,6 +3644,39 @@ static void blockdev_mirror_common(const char *job_id,
BlockDriverState *bs,
sync = MIRROR_SYNC_MODE_FULL;
}
+if (has_replaces) {
+BlockDriverState *to_replace_bs;
+AioContext *replace_aio_context;
+int64_t bs_size, replace_size;
+
+bs_size = bdrv_getlength(bs);
+if (bs_size < 0) {
+error_setg_errno(errp, -bs_size, "Failed to query device's size");
+return;
+}
+
+to_replace_bs = check_to_replace_node(bs, replaces, errp);
+if (!to_replace_bs) {
+return;
+}
+
+replace_aio_context = bdrv_get_aio_context(to_replace_bs);
+aio_context_acquire(replace_aio_context);
+replace_size = bdrv_getlength(to_replace_bs);
+aio_context_release(replace_aio_context);
+
+if (replace_size < 0) {
+error_setg_errno(errp, -replace_size,
+ "Failed to query the replacement node's size");
+return;
+}
+if (bs_size != replace_size) {
+error_setg(errp, "cannot replace image with a mirror image of "
+ "different size");
+return;
+}
+}
+
/* pass the node name to replace to mirror start since it's loose coupling
* and will allow to check whether the node still exist at mirror
completion
*/
@@ -3704,33 +3737,11 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
}
if (arg->has_replaces) {
-BlockDriverState *to_replace_bs;
-AioContext *replace_aio_context;
-int64_t replace_size;
-
if (!arg->has_node_name) {
error_setg(errp, "a node-name must be provided when replacing a"
" named node of the graph");
goto out;
}
-
-to_replace_bs = check_to_replace_node(bs, arg->replaces, &local_err);
-
-if (!to_replace_bs) {
-error_propagate(errp, local_err);
-goto out;
-}
-
-replace_aio_context = bdrv_get_aio_context(to_replace_bs);
-aio_context_acquire(replace_aio_context);
-replace_size = bdrv_getlength(to_replace_bs);
-aio_context_release(replace_aio_context);
-
-if (size != replace_size) {
-error_setg(errp, "cannot replace image with a mirror image of "
- "different size");
-goto out;
-}
}
if (arg->mode == NEW_IMAGE_MODE_ABSOLUTE_PATHS) {
--
2.17.1
[Qemu-devel] [PATCH v2 11/11] iotests: Test committing to overridden backing
Signed-off-by: Max Reitz
---
tests/qemu-iotests/040 | 61 ++
tests/qemu-iotests/040.out | 4 +--
2 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index f0544d6107..90c03e745b 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -476,5 +476,66 @@ class TestCommitWithFilters(iotests.QMPTestCase):
self.assert_qmp(result, 'error/desc',
'Top image file %s not found' % cow3_name)
+class TestCommitWithOverriddenBacking(iotests.QMPTestCase):
+img_base_a = os.path.join(iotests.test_dir, 'base_a.img')
+img_base_b = os.path.join(iotests.test_dir, 'base_b.img')
+img_top = os.path.join(iotests.test_dir, 'top.img')
+
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, self.img_base_a, '1M')
+qemu_img('create', '-f', iotests.imgfmt, self.img_base_b, '1M')
+qemu_img('create', '-f', iotests.imgfmt, '-b', self.img_base_a, \
+ self.img_top)
+
+self.vm = iotests.VM()
+self.vm.launch()
+
+# Use base_b instead of base_a as the backing of top
+result = self.vm.qmp('blockdev-add', **{
+'node-name': 'top',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img_top
+},
+'backing': {
+'node-name': 'base',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img_base_b
+}
+}
+})
+self.assert_qmp(result, 'return', {})
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(self.img_top)
+os.remove(self.img_base_a)
+os.remove(self.img_base_b)
+
+def test_commit_to_a(self):
+# Try committing to base_a (which should fail, as top's
+# backing image is base_b instead)
+result = self.vm.qmp('block-commit',
+ job_id='commit',
+ device='top',
+ base=self.img_base_a)
+self.assert_qmp(result, 'error/class', 'GenericError')
+
+def test_commit_to_b(self):
+# Try committing to base_b (which should work, since that is
+# actually top's backing image)
+result = self.vm.qmp('block-commit',
+ job_id='commit',
+ device='top',
+ base=self.img_base_b)
+self.assert_qmp(result, 'return', {})
+
+self.vm.event_wait('BLOCK_JOB_READY')
+self.vm.qmp('block-job-complete', device='commit')
+self.vm.event_wait('BLOCK_JOB_COMPLETED')
+
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out
index 49f84261d0..cfa5c0d0e6 100644
--- a/tests/qemu-iotests/040.out
+++ b/tests/qemu-iotests/040.out
@@ -1,5 +1,5 @@
-.
+...
--
-Ran 33 tests
+Ran 35 tests
OK
--
2.17.1
Re: [Qemu-devel] [PATCH v6 5/8] qcow2: Increase the default upper limit on the L2 cache size
On 08/09/2018 04:53 PM, Leonid Bloch wrote: The upper limit on the L2 cache size is increased from 1 MB to 32 MB. This is done in order to allow default full coverage of an image with the L2 cache for images of up to 256 GB in size (was 8 GB). Note, that only the needed amount to cover the full image is allocated. The value which is changed here is just the upper limit on the L2 cache size, beyond which it will not grow, even if the size of the image requires it to. Signed-off-by: Leonid Bloch --- block/qcow2.h| 2 +- docs/qcow2-cache.txt | 6 +++--- qemu-options.hx | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index 0bab3d8b94..d77a31d932 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -73,7 +73,7 @@ /* Must be at least 4 to cover all cases of refcount table growth */ #define MIN_REFCOUNT_CACHE_SIZE 4 /* clusters */ -#define DEFAULT_L2_CACHE_MAX_SIZE 1048576 /* bytes */ +#define DEFAULT_L2_CACHE_MAX_SIZE 0x200U /* bytes */ I'd spell this as '(64 * MiB)' with the help of "qemu/units.h". -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Qemu-devel] [PATCH v2 05/11] block: Fix check_to_replace_node()
Currently, check_to_replace_node() only allows mirror to replace a node
in the chain of the source node, and only if it is the first non-filter
node below the source. Well, technically, the idea is that you can
exactly replace a quorum child by mirroring from quorum.
This has (probably) two reasons:
(1) We do not want to create loops.
(2) @replaces and @device should have exactly the same content so
replacing them does not cause visible data to change.
This has two issues:
(1) It is overly restrictive. It is completely fine for @replaces to be
a filter.
(2) It is not restrictive enough. You can create loops with this as
follows:
$ qemu-img create -f qcow2 /tmp/source.qcow2 64M
$ qemu-system-x86_64 -qmp stdio
{"execute": "qmp_capabilities"}
{"execute": "object-add",
"arguments": {"qom-type": "throttle-group", "id": "tg0"}}
{"execute": "blockdev-add",
"arguments": {
"node-name": "source",
"driver": "throttle",
"throttle-group": "tg0",
"file": {
"node-name": "filtered",
"driver": "qcow2",
"file": {
"driver": "file",
"filename": "/tmp/source.qcow2"
} } } }
{"execute": "drive-mirror",
"arguments": {
"job-id": "mirror",
"device": "source",
"target": "/tmp/target.qcow2",
"format": "qcow2",
"node-name": "target",
"sync" :"none",
"replaces": "filtered"
} }
{"execute": "block-job-complete", "arguments": {"device": "mirror"}}
And qemu crashes because of a stack overflow due to the loop being
created (target's backing file is source, so when it replaces filtered,
it points to itself through source).
(blockdev-mirror can be broken similarly.)
So let us make the checks for the two conditions above explicit, which
makes the whole function exactly as restrictive as it needs to be.
Signed-off-by: Max Reitz
---
include/block/block.h | 1 +
block.c | 77 +++
blockdev.c| 32 --
3 files changed, 101 insertions(+), 9 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index a01986495d..a738fef601 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -385,6 +385,7 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate);
/* check if a named node can be replaced when doing drive-mirror */
BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs,
+BlockDriverState *backing_bs,
const char *node_name, Error **errp);
/* async block I/O */
diff --git a/block.c b/block.c
index a3a121551a..69b56771f7 100644
--- a/block.c
+++ b/block.c
@@ -5161,7 +5161,55 @@ bool bdrv_is_first_non_filter(BlockDriverState
*candidate)
return false;
}
+static bool is_child_of(BlockDriverState *child, BlockDriverState *parent)
+{
+BdrvChild *c;
+
+if (!parent) {
+return false;
+}
+
+QLIST_FOREACH(c, &parent->children, next) {
+if (c->bs == child || is_child_of(child, c->bs)) {
+return true;
+}
+}
+
+return false;
+}
+
+/* Return true if there are only filters in [@top, @base). Note that
+ * this may include quorum (which bdrv_chain_contains() cannot
+ * handle). */
+static bool is_filtered_child(BlockDriverState *top, BlockDriverState *base)
+{
+BdrvChild *c;
+
+if (!top) {
+return false;
+}
+
+if (top == base) {
+return true;
+}
+
+if (!top->drv->is_filter) {
+return false;
+}
+
+QLIST_FOREACH(c, &top->children, next) {
+if (is_filtered_child(c->bs, base)) {
+return true;
+}
+}
+
+return false;
+}
+
+/* @parent_bs is mirror's source BDS, @backing_bs is the BDS which
+ * will be attached to the target when mirror completes */
BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs,
+BlockDriverState *backing_bs,
const char *node_name, Error **errp)
{
BlockDriverState *to_replace_bs = bdrv_find_node(node_name);
@@ -5180,13 +5228,28 @@ BlockDriverState
*check_to_replace_node(BlockDriverState *parent_bs,
goto out;
}
-/* We don't want arbitrary node of the BDS chain to be replaced only the
top
- * most non filter in order to prevent data corruption.
- * Another benefit is that this tests exclude backing files which are
- * blocked by the backing blockers.
- */
-if (!bdrv_recurse_is_first_non_filter(parent_bs, to_replace_bs)) {
-error_setg(errp, "Only top most non filter can be replaced");
+/* If to_replace_bs is (recursively) a child of backing_bs,
+ * replacing it may create a loop. We cannot allow that. */
+if (to_replace_bs == backing_bs || is_child_of(to_replace_bs, backing_bs))
{
+error_setg(errp, "Replacing this node would result in a loop");
+to_replace_bs = NUL
Re: [Qemu-devel] [PATCH 21/56] json: Reject invalid UTF-8 sequences
On 08/08/2018 07:02 AM, Markus Armbruster wrote:
We reject bytes that can't occur in valid UTF-8 (\xC0..\xC1,
\xF5..\xFF in the lexer. That's insufficient; there's plenty of
invalid UTF-8 not containing these bytes, as demonstrated by
check-qjson:
* Malformed sequences
- Unexpected continuation bytes
- Missing continuation bytes after start bytes other than
\xC0..\xC1, \xF5..\xFD.
* Overlong sequences with start bytes other than \xC0..\xC1,
\xF5..\xFD.
* Invalid code points
Fixing this in the lexer would be bothersome. Fixing it in the parser
is straightforward, so do that.
Signed-off-by: Markus Armbruster
---
@@ -193,12 +198,15 @@ static QString
*qstring_from_escaped_str(JSONParserContext *ctxt,
goto out;
}
} else {
-char dummy[2];
-
-dummy[0] = *ptr;
-dummy[1] = 0;
-
-qstring_append(str, dummy);
+cp = mod_utf8_codepoint(ptr, 6, &end);
Why are you hard-coding 6 here, rather than computing min(6,
strchr(ptr,0)-ptr)? If the user passes an invalid sequence at the end
of the string, can we end up making mod_utf8_codepoint() read beyond the
end of our string? Would it be better to just always pass the remaining
string length (mod_utf8_codepoint() only cares about stopping short of 6
bytes, but never reads beyond there even if you pass a larger number)?
+if (cp <= 0) {
+parse_error(ctxt, token, "invalid UTF-8 sequence in string");
+goto out;
+}
+ptr = end - 1;
+len = mod_utf8_encode(utf8_buf, sizeof(utf8_buf), cp);
+assert(len >= 0);
+qstring_append(str, utf8_buf);
}
}
+++ b/util/unicode.c
@@ -13,6 +13,21 @@
#include "qemu/osdep.h"
#include "qemu/unicode.h"
+ssize_t mod_utf8_encode(char buf[], size_t bufsz, int codepoint)
+{
+assert(bufsz >= 5);
+
+if (!is_valid_codepoint(codepoint)) {
+return -1;
+}
+
+if (codepoint > 0 && codepoint <= 0x7F) {
+buf[0] = codepoint & 0x7F;
Dead use of binary &. But acceptable for symmetry with the other code
branches.
+buf[1] = 0;
+return 1;
+}
+if (codepoint <= 0x7FF) {
+buf[0] = 0xC0 | ((codepoint >> 6) & 0x1F);
+buf[1] = 0x80 | (codepoint & 0x3F);
+buf[2] = 0;
+return 2;
+}
+if (codepoint <= 0x) {
+buf[0] = 0xE0 | ((codepoint >> 12) & 0x0F);
+buf[1] = 0x80 | ((codepoint >> 6) & 0x3F);
+buf[2] = 0x80 | (codepoint & 0x3F);
+buf[3] = 0;
+return 3;
+}
+buf[0] = 0xF0 | ((codepoint >> 18) & 0x07);
+buf[1] = 0x80 | ((codepoint >> 12) & 0x3F);
+buf[2] = 0x80 | ((codepoint >> 6) & 0x3F);
+buf[3] = 0x80 | (codepoint & 0x3F);
+buf[4] = 0;
+return 4;
+}
Overall, looks nice.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[Qemu-devel] [PATCH v2 04/11] block: Storage child access function
For completeness' sake, add a function for accessing a node's storage
child, too. For filters, this is there filtered child; for non-filters,
this is bs->file.
Some places are deliberately left unconverted:
- BDS opening/closing functions where bs->file is handled specially
(which is basically wrong, but at least simplifies probing)
- bdrv_co_block_status_from_file(), because its name implies that it
points to ->file
- bdrv_snapshot_goto() in one places unrefs bs->file. Such a
modification is not covered by this patch and is therefore just
safeguarded by an additional assert(), but otherwise kept as-is.
Signed-off-by: Max Reitz
---
include/block/block_int.h | 6 +
block.c | 51 ---
block/io.c| 20 +--
block/qapi.c | 7 +++---
block/snapshot.c | 40 +-
5 files changed, 82 insertions(+), 42 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index fa9154899d..d3d8b22155 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1214,6 +1214,7 @@ int refresh_total_sectors(BlockDriverState *bs, int64_t
hint);
BdrvChild *bdrv_filtered_cow_child(BlockDriverState *bs);
BdrvChild *bdrv_filtered_rw_child(BlockDriverState *bs);
BdrvChild *bdrv_filtered_child(BlockDriverState *bs);
+BdrvChild *bdrv_storage_child(BlockDriverState *bs);
BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs);
BlockDriverState *bdrv_skip_rw_filters(BlockDriverState *bs);
BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs);
@@ -1238,4 +1239,9 @@ static inline BlockDriverState
*bdrv_filtered_bs(BlockDriverState *bs)
return child_bs(bdrv_filtered_child(bs));
}
+static inline BlockDriverState *bdrv_storage_bs(BlockDriverState *bs)
+{
+return child_bs(bdrv_storage_child(bs));
+}
+
#endif /* BLOCK_INT_H */
diff --git a/block.c b/block.c
index 61a2fe14eb..a3a121551a 100644
--- a/block.c
+++ b/block.c
@@ -3835,15 +3835,21 @@ exit:
int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
{
BlockDriver *drv = bs->drv;
+BlockDriverState *storage_bs;
+
if (!drv) {
return -ENOMEDIUM;
}
+
if (drv->bdrv_get_allocated_file_size) {
return drv->bdrv_get_allocated_file_size(bs);
}
-if (bs->file) {
-return bdrv_get_allocated_file_size(bs->file->bs);
+
+storage_bs = bdrv_storage_bs(bs);
+if (storage_bs) {
+return bdrv_get_allocated_file_size(storage_bs);
}
+
return -ENOTSUP;
}
@@ -4252,7 +4258,7 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const
char *event,
const char *tag)
{
while (bs && bs->drv && !bs->drv->bdrv_debug_breakpoint) {
-bs = bs->file ? bs->file->bs : NULL;
+bs = bdrv_storage_bs(bs);
}
if (bs && bs->drv && bs->drv->bdrv_debug_breakpoint) {
@@ -4265,7 +4271,7 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const
char *event,
int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag)
{
while (bs && bs->drv && !bs->drv->bdrv_debug_remove_breakpoint) {
-bs = bs->file ? bs->file->bs : NULL;
+bs = bdrv_storage_bs(bs);
}
if (bs && bs->drv && bs->drv->bdrv_debug_remove_breakpoint) {
@@ -4278,7 +4284,7 @@ int bdrv_debug_remove_breakpoint(BlockDriverState *bs,
const char *tag)
int bdrv_debug_resume(BlockDriverState *bs, const char *tag)
{
while (bs && (!bs->drv || !bs->drv->bdrv_debug_resume)) {
-bs = bs->file ? bs->file->bs : NULL;
+bs = bdrv_storage_bs(bs);
}
if (bs && bs->drv && bs->drv->bdrv_debug_resume) {
@@ -4291,7 +4297,7 @@ int bdrv_debug_resume(BlockDriverState *bs, const char
*tag)
bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag)
{
while (bs && bs->drv && !bs->drv->bdrv_debug_is_suspended) {
-bs = bs->file ? bs->file->bs : NULL;
+bs = bdrv_storage_bs(bs);
}
if (bs && bs->drv && bs->drv->bdrv_debug_is_suspended) {
@@ -5388,14 +5394,21 @@ void bdrv_refresh_filename(BlockDriverState *bs)
bs->exact_filename[0] = '\0';
drv->bdrv_refresh_filename(bs);
-} else if (bs->file) {
-/* Try to reconstruct valid information from the underlying file */
+} else if (bdrv_storage_child(bs)) {
+/* Try to reconstruct valid information from the underlying
+ * file -- this only works for format nodes (filter nodes
+ * cannot be probed and as such must be selected by the user
+ * either through an options dict, or through a special
+ * filename which the filter driver must construct in its
+ * .bdrv_refresh_filename() implementation) */
+BlockDriverState *storage_bs = bdrv_storage_bs(bs);
bs->exact_filename[0] = '\0';
/*
* We can use the underlying file's filename if:
* - it has a filename,
+
[Qemu-devel] [PATCH v2 01/11] block: Mark commit and mirror as filter drivers
The commit and mirror block nodes are filters, so they should be marked
as such.
Signed-off-by: Max Reitz
---
block/commit.c | 2 ++
block/mirror.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/block/commit.c b/block/commit.c
index 14788b0708..a95b87bb3a 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -261,6 +261,8 @@ static BlockDriver bdrv_commit_top = {
.bdrv_refresh_filename = bdrv_commit_top_refresh_filename,
.bdrv_close = bdrv_commit_top_close,
.bdrv_child_perm= bdrv_commit_top_child_perm,
+
+.is_filter = true,
};
void commit_start(const char *job_id, BlockDriverState *bs,
diff --git a/block/mirror.c b/block/mirror.c
index b4287a1e2b..5c561c6241 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1457,6 +1457,8 @@ static BlockDriver bdrv_mirror_top = {
.bdrv_refresh_filename = bdrv_mirror_top_refresh_filename,
.bdrv_close = bdrv_mirror_top_close,
.bdrv_child_perm= bdrv_mirror_top_child_perm,
+
+.is_filter = true,
};
static void mirror_start_job(const char *job_id, BlockDriverState *bs,
--
2.17.1
[Qemu-devel] [PATCH v6 7/8] qcow2: Set the default cache-clean-interval to 10 minutes
The default cache-clean-interval is set to 10 minutes, in order to lower
the overhead of the qcow2 caches (before the default was 0, i.e.
disabled).
Signed-off-by: Leonid Bloch
Reviewed-by: Alberto Garcia
---
block/qcow2.c| 2 +-
block/qcow2.h| 1 +
docs/qcow2-cache.txt | 4 ++--
qapi/block-core.json | 3 ++-
qemu-options.hx | 2 +-
5 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index ba4dfae735..b4f291765b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -944,7 +944,7 @@ static int qcow2_update_options_prepare(BlockDriverState
*bs,
/* New interval for cache cleanup timer */
r->cache_clean_interval =
qemu_opt_get_number(opts, QCOW2_OPT_CACHE_CLEAN_INTERVAL,
-s->cache_clean_interval);
+DEFAULT_CACHE_CLEAN_INTERVAL);
#ifndef CONFIG_LINUX
if (r->cache_clean_interval != 0) {
error_setg(errp, QCOW2_OPT_CACHE_CLEAN_INTERVAL
diff --git a/block/qcow2.h b/block/qcow2.h
index d77a31d932..587b053453 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -77,6 +77,7 @@
#define DEFAULT_CLUSTER_SIZE 65536
+#define DEFAULT_CACHE_CLEAN_INTERVAL 600 /* seconds */
#define QCOW2_OPT_LAZY_REFCOUNTS "lazy-refcounts"
#define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request"
diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt
index 6ad1081d1a..684147ad45 100644
--- a/docs/qcow2-cache.txt
+++ b/docs/qcow2-cache.txt
@@ -204,8 +204,8 @@ This example removes all unused cache entries every 15
minutes:
-drive file=hd.qcow2,cache-clean-interval=900
-If unset, the default value for this parameter is 0 and it disables
-this feature.
+If unset, the default value for this parameter is 600. Setting it to 0
+disables this feature.
Note that this functionality currently relies on the MADV_DONTNEED
argument for madvise() to actually free the memory. This is a
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5b9084a394..9a6a708a37 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2830,7 +2830,8 @@
#
# @cache-clean-interval: clean unused entries in the L2 and refcount
# caches. The interval is in seconds. The default value
-# is 0 and it disables this feature (since 2.5)
+# is 600, and 0 disables this feature. (since 2.5)
+#
# @encrypt: Image decryption options. Mandatory for
# encrypted images, except when doing a metadata-only
# probe of the image. (since 2.10)
diff --git a/qemu-options.hx b/qemu-options.hx
index 4c44cdbc23..6abf3631ec 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -767,7 +767,7 @@ it which is not used for the L2 cache)
@item cache-clean-interval
Clean unused entries in the L2 and refcount caches. The interval is in seconds.
-The default value is 0 and it disables this feature.
+The default value is 600. Setting it to 0 disables this feature.
@item pass-discard-request
Whether discard requests to the qcow2 device should be forwarded to the data
--
2.17.1
Re: [Qemu-devel] [PATCH 00/11] block: Deal with filters
Let's pretend you didn't see this, as it breaks some iotests... *cough* *cough* (This is what I get for last-minute changes and rebases without proper test running. Yes, I'm ashamed.) Max On 2018-08-09 23:37, Max Reitz wrote: > Note 1: This series depends on v10 of my “block: Fix some filename > generation issues” series. > > Based-on: <20180809213528.14738-1-mre...@redhat.com> > > Note 2: This is technically the first part of my active mirror followup. > But just very technically. I noticed that that followup started to > consist of two parts, namely (A) fix filtery things in the block layer, > and (B) fix active mirror. So I decided to split it. This is part A. > Part B comes later. > > > When we introduced filters, we did it a bit casually. Sure, we talked a > lot about them before, but that was mostly discussion about where > implicit filters should be added to the graph (note that we currently > only have two implicit filters, those being mirror and commit). But in > the end, we really just designated some drivers filters (Quorum, > blkdebug, etc.) and added some specifically (throttle, COR), without > really looking through the block layer to see where issues might occur. > > It turns out vast areas of the block layer just don't know about filters > and cannot really handle them. Many cases will work in practice, in > others, well, too bad, you cannot use some feature because some part > deep inside the block layer looks at your filters and thinks they are > format nodes. > > This series sets out to correct a bit of that. I lost my head many > times and I'm sure this series is incomplete in many ways, but it really > doesn't do any good if it sits on my disk any longer, it needs to go out > now. > > The most important patches of this series are patches 3 and 4. These > introduce functions to encapsulate bs->backing and bs->file accesses. > Because sometimes, bs->backing means COW, sometimes it means filtered > node. And sometimes, bs->file means metadata storage, and sometimes it > means filtered node. With this functions, it's always clear what the > caller wants, and it will always get what it wants. > > Besides that, patch 3 introduces functions to skip filters which may be > used by parts of the block layer that just don't care about them. > > > Secondly, the restraints put on mirror's @replaces parameter are > revisited and fixed. > > > Thirdly, BDS.backing_file is changed to be constant. I don't quite know > why we modify it whenever we change a BDS's backing file, but that's > definitely not quite right. This fixes things like being able to > perform a commit on a file (using relative filenames) in a directory > that's not qemu's CWD. > > > Finally, a number of tests are added. > > > There are probably many things that are worthy of discussion, of which > only some come to my head, e.g.: > > - In which cases do we want to skip filters, in which cases do we want > to skip implicit filters? > My approach was to basically never skip explicitly added filters, > except when it's about finding a file in some tree (e.g. in a backing > chain). Maybe there are cases where you think we should skip even > explicitly added filters. > > - I made interesting decisions like “When you mirror from a node, we > should indeed mirror from that node, but when replacing it, we should > skip leave all implicit filters on top intact.” You may disagree with > that. > (My reasoning here is that users aren't supposed to know about > implicit filters, and therefore, they should not intend to remove > them. Also, mirror accepts only root nodes as the source, so you > cannot really specify the node below the implicit filters. But you > can use @replaces to drop the implicit filters, if you know they are > there.) > > > Max Reitz (11): > block: Mark commit and mirror as filter drivers > blockdev: Check @replaces in blockdev_mirror_common > block: Filtered children access functions > block: Storage child access function > block: Fix check_to_replace_node() > iotests: Add tests for mirror @replaces loops > block: Leave BDS.backing_file constant > iotests: Add filter commit test cases > iotests: Add filter mirror test cases > iotests: Add test for commit in sub directory > iotests: Test committing to overridden backing > > qapi/block-core.json | 4 + > include/block/block.h | 2 + > include/block/block_int.h | 53 +- > block.c| 338 - > block/backup.c | 8 +- > block/block-backend.c | 16 +- > block/commit.c | 38 ++-- > block/io.c | 47 +++-- > block/mirror.c | 39 ++-- > block/qapi.c | 38 ++-- > block/snapshot.c | 40 ++-- > block/stream.c | 15 +- > blockdev.c | 167 > migrati
[Qemu-devel] [PATCH v6 8/8] qcow2: Explicit number replaced by a constant
Signed-off-by: Leonid Bloch
Reviewed-by: Alberto Garcia
---
block/qcow2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index b4f291765b..b0e20aeffc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1324,7 +1324,7 @@ static int coroutine_fn qcow2_do_open(BlockDriverState
*bs, QDict *options,
/* 2^(s->refcount_order - 3) is the refcount width in bytes */
s->refcount_block_bits = s->cluster_bits - (s->refcount_order - 3);
s->refcount_block_size = 1 << s->refcount_block_bits;
-bs->total_sectors = header.size / 512;
+bs->total_sectors = header.size / BDRV_SECTOR_SIZE;
s->csize_shift = (62 - (s->cluster_bits - 8));
s->csize_mask = (1 << (s->cluster_bits - 8)) - 1;
s->cluster_offset_mask = (1LL << s->csize_shift) - 1;
@@ -3450,7 +3450,7 @@ static int coroutine_fn
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
goto fail;
}
-old_length = bs->total_sectors * 512;
+old_length = bs->total_sectors * BDRV_SECTOR_SIZE;
new_l1_size = size_to_l1(s, offset);
if (offset < old_length) {
--
2.17.1
[Qemu-devel] [PATCH v6 6/8] qcow2: Resize the cache upon image resizing
The caches are now recalculated upon image resizing. This is done
because the new default behavior of assigning L2 cache relatively to
the image size, implies that the cache will be adapted accordingly
after an image resize.
Signed-off-by: Leonid Bloch
Reviewed-by: Alberto Garcia
---
block/qcow2.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index 434fb89076..ba4dfae735 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3418,6 +3418,7 @@ static int coroutine_fn
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
uint64_t old_length;
int64_t new_l1_size;
int ret;
+QDict *options;
if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA &&
prealloc != PREALLOC_MODE_FALLOC && prealloc != PREALLOC_MODE_FULL)
@@ -3642,6 +3643,8 @@ static int coroutine_fn
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
}
}
+bs->total_sectors = offset / BDRV_SECTOR_SIZE;
+
/* write updated header.size */
offset = cpu_to_be64(offset);
ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, size),
@@ -3652,6 +3655,13 @@ static int coroutine_fn
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
}
s->l1_vm_state_index = new_l1_size;
+
+/* Update cache sizes */
+options = qdict_clone_shallow(bs->options);
+ret = qcow2_update_options(bs, options, s->flags, errp);
+if (ret < 0) {
+goto fail;
+}
ret = 0;
fail:
qemu_co_mutex_unlock(&s->lock);
--
2.17.1
[Qemu-devel] [PATCH v6 3/8] qcow2: Avoid duplication in setting the refcount cache size
The refcount cache size does not need to be set to its minimum value in
read_cache_sizes(), as it is set to at least its minimum value in
qcow2_update_options_prepare().
Signed-off-by: Leonid Bloch
---
block/qcow2.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 3f4abc394e..a8b65189fc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -829,16 +829,13 @@ static void read_cache_sizes(BlockDriverState *bs,
QemuOpts *opts,
*l2_cache_size = combined_cache_size - *refcount_cache_size;
}
}
-} else {
-if (!l2_cache_size_set) {
-*l2_cache_size = MAX(DEFAULT_L2_CACHE_BYTE_SIZE,
- (uint64_t)DEFAULT_L2_CACHE_CLUSTERS
- * s->cluster_size);
-}
-if (!refcount_cache_size_set) {
-*refcount_cache_size = min_refcount_cache;
-}
+} else if (!l2_cache_size_set) {
+*l2_cache_size = MAX(DEFAULT_L2_CACHE_BYTE_SIZE,
+ (uint64_t)DEFAULT_L2_CACHE_CLUSTERS
+ * s->cluster_size);
}
+/* If refcount-cache-size is not specified, it will be set to minimum
+ * in qcow2_update_options_prepare(). No need to set it here. */
if (*l2_cache_entry_size < (1 << MIN_CLUSTER_BITS) ||
*l2_cache_entry_size > s->cluster_size ||
--
2.17.1
[Qemu-devel] [PATCH v6 0/8] qcow2: Take the image size into account when allocating the L2 cache
This series makes the qcow2 L2 cache assignment aware of the image size, with the intention for it to cover the entire image. The importance of this change is in noticeable performance improvement, especially with heavy random I/O. The memory overhead is not big in most cases, as only 1 MB of cache for every 8 GB of image size is used. For cases with very large images and/or small cluster sizes, or systems with limited RAM resources, there is an upper limit on the default L2 cache: 32 MB. To modify this limit one can use the already existing 'l2-cache-size' and 'cache-size' options. Moreover, this fixes the behavior of 'l2-cache-size', as it was documented as the *maximum* L2 cache size, but in practice behaved as the absolute size. To compensate the memory overhead which may be increased following this behavior, the default cache-clean-interval is set to 10 minutes by default (was disabled by default before). The L2 cache is also resized accordingly, by default, if the image is resized. Additionally, few minor changes are made (refactoring and documentation fixes). Differences from v1: * .gitignore modification patch removed (unneeded). * The grammar fix in conflicting cache sizing patch removed (merged). * The update total_sectors when resizing patch squashed with the resizing patch. * L2 cache is now capped at 32 MB. * The default cache-clean-interval is set to 30 seconds. Differences from v2: * Made it clear in the documentation that setting cache-clean-interval to 0 disables this feature. Differences from v3: * The default cache-clean-interval is set to 10 minutes instead of 30 seconds before. * Commit message changes to better explain the patches. * Some refactoring. Differences from v4: * Refactoring. * Documentation and commit message fixes. Differences from v5: * A patch with cosmetic changes is split from the main patch * A patch for avoiding duplication in refcount cache size assignment is split from the main patch. * In the main patch the cap on the L2 cache size is set to only 1 MB (in order to be close to the previous behavior) and a separate patch sets it to 32 MB. * Changes in the documentation fixes patch [1/8]. Leonid Bloch (8): qcow2: Options' documentation fixes qcow2: Cosmetic changes qcow2: Avoid duplication in setting the refcount cache size qcow2: Assign the L2 cache relatively to the image size qcow2: Increase the default upper limit on the L2 cache size qcow2: Resize the cache upon image resizing qcow2: Set the default cache-clean-interval to 10 minutes qcow2: Explicit number replaced by a constant block/qcow2.c | 54 +- block/qcow2.h | 5 ++-- docs/qcow2-cache.txt | 33 ++- qapi/block-core.json | 3 ++- qemu-options.hx| 11 +--- tests/qemu-iotests/137 | 1 - tests/qemu-iotests/137.out | 1 - 7 files changed, 62 insertions(+), 46 deletions(-) -- 2.17.1
Re: [Qemu-devel] [PATCH for-3.0] slirp: Correct size check in m_inc()
Dr. David Alan Gilbert, le jeu. 09 août 2018 12:32:05 +0100, a ecrit:
> >|--datasize>|---m_len--->
> >|--m_size-->
> >|M_ROOM>
> > |-M_FREEROOM-->
> >
> >^ ^ ^
> >m_dat m_data end of buffer
> >
> > ("datasize" is a bit misnamed, as it's "size of the leading
> > gap between the start of the buffer and the data"; "gapsize"
> > would be more helpful.)
> >
> > Anyway, we allocate size + datasize, and
> > m_size == datasize + M_ROOM. We know that size >= M_ROOM,
> > so the allocated buffer must be at least m_size big.
>
> Ah OK, thanks.
> (That ascii art could do with being in a comment somewhere!)
Indeed. Peter, maybe your Signed-off-by on this? :)
Samuel
commit 4be85a1eeb6b19e91491e689d4d0d054030cbb49
Author: Peter Maydell
Date: Thu Aug 9 23:52:59 2018 +0200
slirp: document mbuf pointers and sizes
Signed-off-by: Samuel Thibault
diff --git a/slirp/mbuf.h b/slirp/mbuf.h
index 33b84485d6..a5bb3f9e66 100644
--- a/slirp/mbuf.h
+++ b/slirp/mbuf.h
@@ -47,6 +47,16 @@
* free the m_ext. This is inefficient memory-wise, but who cares.
*/
+/*
+ * |--gapsize->|---m_len--->
+ * |--m_size-->
+ * |M_ROOM>
+ * |-M_FREEROOM-->
+ *
+ * ^ ^ ^
+ * m_dat/m_ext m_data end of buffer
+ */
+
/*
* How much room is in the mbuf, from m_data to the end of the mbuf
*/
[Qemu-devel] [PATCH 3/3] target/riscv: call tcg_lookup_and_goto_ptr on DISAS_TOO_MANY
Performance impact of this and the previous commit, measured with
the very-easy-to-cross-compile rv8-bench:
https://github.com/rv8-io/rv8-bench
Host: Intel(R) Core(TM) i7-4790K CPU @ 4.00GHz
- Key:
before: master
after1,2,3: the 3 commits in this series (i.e. 3 is this commit)
- User-mode:
bench before after1 after2 after3 final speedup
-
aes1.12s 1.12s 1.10s 1.00s 1.12
bigint 0.78s 0.78s 0.78s 0.78s 1
dhrystone 0.96s 0.97s 0.49s 0.49s 1.9591837
miniz 1.94s 1.94s 1.88s 1.86s 1.0430108
norx 0.51s 0.51s 0.49s 0.48s 1.0625
primes 0.85s 0.85s 0.84s 0.84s 1.0119048
qsort 4.87s 4.88s 1.86s 1.86s 2.6182796
sha512 0.76s 0.77s 0.64s 0.64s 1.1875
(after1 only applies to softmmu, so no surprises here)
- Full-system (fedora):
bench before after1 after2 after3 final speedup
-
aes2.68s 2.54s 2.60s 2.34s 1.1452991
bigint 1.61s 1.56s 1.55s 1.64s 0.98170732
dhrystone 1.78s 1.67s 1.25s 1.24s 1.4354839
miniz 3.53s 3.35s 3.28s 3.35s 1.0537313
norx 1.13s 1.09s 1.07s 1.06s 1.0660377
primes 15.37s 15.41s 15.20s 15.37s 1
qsort 7.20s 6.71s 3.85s 3.96s 1.8181818
sha512 1.07s 1.04s 0.90s 0.90s 1.189
SoftMMU slows things down, so the numbers are less sensitive.
Cross-page jumps improve things a little bit, though.
Note that I'm not showing here averages, just results from a
single run, so with primes there isn't much to worry about.
Signed-off-by: Emilio G. Cota
---
target/riscv/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 66a80ca772..98e0311606 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1872,7 +1872,7 @@ static void riscv_tr_tb_stop(DisasContextBase *dcbase,
CPUState *cpu)
if (ctx->base.singlestep_enabled) {
gen_exception_debug();
} else {
-tcg_gen_exit_tb(NULL, 0);
+tcg_gen_lookup_and_goto_ptr();
}
break;
case DISAS_NORETURN:
--
2.17.1
Re: [Qemu-devel] [PATCH v4 2/5] qcow2: Assign the L2 cache relatively to image size
On 8/9/18 8:37 PM, Eric Blake wrote: On 08/09/2018 11:46 AM, Leonid Bloch wrote: There are no functional changes, why do you need to change the indentation here? It's in the "immediate area (few lines) of the lines [I'm] changing". But there's no need to change those lines unless there's an obvious mistake. In this case it's just a matter of style so they just add noise to the patch. Again, it just looks nicer, more readable, compliant to the generally accepted style, and right next to the functional changes. It's a style improvement which is in the immediate vicinity of the functional improvements. I made another one, you must have seen it already, in v5. Look, it just looks better. It's possible to make another patch for these cosmetic changes, but is it worth it when they are right next to the functional changes? It's a bit of noise in the patch, versus noise in the Git history. Patch splitting is an art form. But it IS easier to review two patches (one that fixes style but has no semantic change, and one that does semantic change in as few lines as possible) than to review one (that mixes both steps at once). The more things you do in a single patch, the more likely you were to be better off by having split it into independent patches. A longer git history is not a problem. Our bottleneck is reviewer time, and everything you can do to make life easier for reviewers is a net win in overall time spent on the project. And splitting two distinct changes IS worthwhile, especially when a reviewer has requested that split. Along those lines, I'll also comment that I've seen Berto request that you consider splitting even the functional part of this patch into two pieces - one raising the default value, and the other fixing things to use only what is needed rather than the full specified length when the specified/default length is larger than necessary. It's not a hard split, and while you've continued to argue against the split, I tend to agree that doing the two parts separately makes the series a bit easier to backport to other stable branches (for example, if a distro wants to change to yet a different default value, but still use your patch that changes to not overallocate when the specified/default is larger than needed). Hi Eric, I agree with your arguments here, splitting the cosmetic fixes being the reviewer's request, and splitting the size setting for the reason that you have mentioned above. Thanks! Sending v6. Leonid.
[Qemu-devel] [PATCH v6 5/8] qcow2: Increase the default upper limit on the L2 cache size
The upper limit on the L2 cache size is increased from 1 MB to 32 MB. This is done in order to allow default full coverage of an image with the L2 cache for images of up to 256 GB in size (was 8 GB). Note, that only the needed amount to cover the full image is allocated. The value which is changed here is just the upper limit on the L2 cache size, beyond which it will not grow, even if the size of the image requires it to. Signed-off-by: Leonid Bloch --- block/qcow2.h| 2 +- docs/qcow2-cache.txt | 6 +++--- qemu-options.hx | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index 0bab3d8b94..d77a31d932 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -73,7 +73,7 @@ /* Must be at least 4 to cover all cases of refcount table growth */ #define MIN_REFCOUNT_CACHE_SIZE 4 /* clusters */ -#define DEFAULT_L2_CACHE_MAX_SIZE 1048576 /* bytes */ +#define DEFAULT_L2_CACHE_MAX_SIZE 0x200U /* bytes */ #define DEFAULT_CLUSTER_SIZE 65536 diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt index 69af306267..6ad1081d1a 100644 --- a/docs/qcow2-cache.txt +++ b/docs/qcow2-cache.txt @@ -125,8 +125,8 @@ There are a few things that need to be taken into account: (or the cache entry size: see "Using smaller cache sizes" below). - The default L2 cache size will cover the entire virtual size of an - image, up to a certain maximum. This maximum is 1 MB by default - (enough for image sizes of up to 8 GB with the default cluster size) + image, up to a certain maximum. This maximum is 32 MB by default + (enough for image sizes of up to 256 GB with the default cluster size) and it can be reduced or enlarged using the "l2-cache-size" option. The minimum is 2 clusters (or 2 cache entries, see below). @@ -186,7 +186,7 @@ Some things to take into account: always uses the cluster size as the entry size. - If the L2 cache is big enough to hold all of the image's L2 tables - (the default behavior for images of up to 8 GB in size) then none + (the default behavior for images of up to 256 GB in size) then none of this is necessary and you can omit the "l2-cache-entry-size" parameter altogether. diff --git a/qemu-options.hx b/qemu-options.hx index 22e8e2d113..4c44cdbc23 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -756,7 +756,7 @@ The maximum total size of the L2 table and refcount block caches in bytes @item l2-cache-size The maximum size of the L2 table cache in bytes -(default: if cache-size is not specified - 1M; otherwise, as large as possible +(default: if cache-size is not specified - 32M; otherwise, as large as possible within the cache-size, while permitting the requested or the minimal refcount cache size) -- 2.17.1
[Qemu-devel] [PATCH 10/11] iotests: Add test for commit in sub directory
Add a test for committing an overlay in a sub directory to one of the
images in its backing chain, using both relative and absolute filenames.
Signed-off-by: Max Reitz
---
tests/qemu-iotests/020 | 36
tests/qemu-iotests/020.out | 10 ++
2 files changed, 46 insertions(+)
diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020
index eac5080f83..d0ac33922d 100755
--- a/tests/qemu-iotests/020
+++ b/tests/qemu-iotests/020
@@ -32,6 +32,11 @@ _cleanup()
_cleanup_test_img
rm -f "$TEST_IMG.base"
rm -f "$TEST_IMG.orig"
+
+rm -f "$TEST_DIR/subdir/t.$IMGFMT.base"
+rm -f "$TEST_DIR/subdir/t.$IMGFMT.mid"
+rm -f "$TEST_DIR/subdir/t.$IMGFMT"
+rmdir "$TEST_DIR/subdir" &> /dev/null
}
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -135,6 +140,37 @@ $QEMU_IO -c 'writev 0 64k' "$TEST_IMG" | _filter_qemu_io
$QEMU_IMG commit "$TEST_IMG"
_cleanup
+
+echo
+echo 'Testing commit in sub-directory with relative filenames'
+echo
+
+pushd "$TEST_DIR" > /dev/null
+
+mkdir subdir
+
+TEST_IMG="subdir/t.$IMGFMT.base" _make_test_img 1M
+TEST_IMG="subdir/t.$IMGFMT.mid" _make_test_img -b "t.$IMGFMT.base"
+TEST_IMG="subdir/t.$IMGFMT" _make_test_img -b "t.$IMGFMT.mid"
+
+# Should work
+$QEMU_IMG commit -b "t.$IMGFMT.mid" "subdir/t.$IMGFMT"
+
+# Might theoretically work, but does not in practice (we have to
+# decide between this and the above; and since we always represent
+# backing file names as relative to the overlay, we go for the above)
+$QEMU_IMG commit -b "subdir/t.$IMGFMT.mid" "subdir/t.$IMGFMT" 2>&1 | \
+_filter_imgfmt
+
+# This should work as well
+$QEMU_IMG commit -b "$TEST_DIR/subdir/t.$IMGFMT.mid" "subdir/t.$IMGFMT"
+
+popd > /dev/null
+
+# Now let's try with just absolute filenames
+$QEMU_IMG commit -b "$TEST_DIR/subdir/t.$IMGFMT.mid" \
+"$TEST_DIR/subdir/t.$IMGFMT"
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/020.out b/tests/qemu-iotests/020.out
index 4b722b2dd0..228c37dded 100644
--- a/tests/qemu-iotests/020.out
+++ b/tests/qemu-iotests/020.out
@@ -1094,4 +1094,14 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
backing_file=json:{'driv
wrote 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qemu-img: Block job failed: No space left on device
+
+Testing commit in sub-directory with relative filenames
+
+Formatting 'subdir/t.IMGFMT.base', fmt=IMGFMT size=1048576
+Formatting 'subdir/t.IMGFMT.mid', fmt=IMGFMT size=1048576
backing_file=t.IMGFMT.base
+Formatting 'subdir/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=t.IMGFMT.mid
+Image committed.
+qemu-img: Did not find 'subdir/t.IMGFMT.mid' in the backing chain of
'subdir/t.IMGFMT'
+Image committed.
+Image committed.
*** done
--
2.17.1
[Qemu-devel] [PATCH v6 4/8] qcow2: Assign the L2 cache relatively to the image size
Sufficient L2 cache can noticeably improve the performance when using
large images with frequent I/O.
Previously, the L2 cache was allocated without considering the image
size, and an option existed to manually determine its size. Thus to
achieve a full coverage of an image by the L2 cache (i.e. use more than
the default value of MAX(1 MB, 8 clusters)), a user needed to calculate
the required size manually, or with a script, and pass this value to the
'l2-cache-size' option.
Now, the L2 cache is assigned taking the actual image size into account,
and will cover the entire image, unless the size needed for that is
larger than a certain maximum. This maximum is set to 1 MB by default
(enough to cover an 8 GB image with the default cluster size) but can
be increased or decreased using the 'l2-cache-size' option. This option
was previously documented as the *maximum* L2 cache size, and this patch
makes it behave as such, instead of as a constant size. Also, the
existing option 'cache-size' can limit the sum of both L2 and refcount
caches, as previously.
Signed-off-by: Leonid Bloch
---
block/qcow2.c | 22 ++
block/qcow2.h | 4 +---
docs/qcow2-cache.txt | 13 -
qemu-options.hx| 6 +++---
tests/qemu-iotests/137 | 1 -
tests/qemu-iotests/137.out | 1 -
6 files changed, 22 insertions(+), 25 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index a8b65189fc..434fb89076 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -777,16 +777,19 @@ static void read_cache_sizes(BlockDriverState *bs,
QemuOpts *opts,
uint64_t *refcount_cache_size, Error **errp)
{
BDRVQcow2State *s = bs->opaque;
-uint64_t combined_cache_size;
+uint64_t combined_cache_size, l2_cache_max_setting;
bool l2_cache_size_set, refcount_cache_size_set, combined_cache_size_set;
int min_refcount_cache = MIN_REFCOUNT_CACHE_SIZE * s->cluster_size;
+uint64_t virtual_disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
+uint64_t max_l2_cache = virtual_disk_size / (s->cluster_size / 8);
combined_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_CACHE_SIZE);
l2_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_L2_CACHE_SIZE);
refcount_cache_size_set = qemu_opt_get(opts,
QCOW2_OPT_REFCOUNT_CACHE_SIZE);
combined_cache_size = qemu_opt_get_size(opts, QCOW2_OPT_CACHE_SIZE, 0);
-*l2_cache_size = qemu_opt_get_size(opts, QCOW2_OPT_L2_CACHE_SIZE, 0);
+l2_cache_max_setting = qemu_opt_get_size(opts, QCOW2_OPT_L2_CACHE_SIZE,
+ DEFAULT_L2_CACHE_MAX_SIZE);
*refcount_cache_size = qemu_opt_get_size(opts,
QCOW2_OPT_REFCOUNT_CACHE_SIZE, 0);
@@ -794,13 +797,16 @@ static void read_cache_sizes(BlockDriverState *bs,
QemuOpts *opts,
QCOW2_OPT_L2_CACHE_ENTRY_SIZE,
s->cluster_size);
+*l2_cache_size = MIN(max_l2_cache, l2_cache_max_setting);
+
if (combined_cache_size_set) {
if (l2_cache_size_set && refcount_cache_size_set) {
error_setg(errp, QCOW2_OPT_CACHE_SIZE ", " QCOW2_OPT_L2_CACHE_SIZE
" and " QCOW2_OPT_REFCOUNT_CACHE_SIZE " may not be set "
"at the same time");
return;
-} else if (*l2_cache_size > combined_cache_size) {
+} else if (l2_cache_size_set &&
+ (l2_cache_max_setting > combined_cache_size)) {
error_setg(errp, QCOW2_OPT_L2_CACHE_SIZE " may not exceed "
QCOW2_OPT_CACHE_SIZE);
return;
@@ -815,13 +821,9 @@ static void read_cache_sizes(BlockDriverState *bs,
QemuOpts *opts,
} else if (refcount_cache_size_set) {
*l2_cache_size = combined_cache_size - *refcount_cache_size;
} else {
-uint64_t virtual_disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
-uint64_t max_l2_cache = virtual_disk_size / (s->cluster_size / 8);
-
/* Assign as much memory as possible to the L2 cache, and
* use the remainder for the refcount cache */
-if (combined_cache_size >= max_l2_cache + min_refcount_cache) {
-*l2_cache_size = max_l2_cache;
+if (combined_cache_size >= *l2_cache_size + min_refcount_cache) {
*refcount_cache_size = combined_cache_size - *l2_cache_size;
} else {
*refcount_cache_size = MIN(combined_cache_size,
@@ -829,10 +831,6 @@ static void read_cache_sizes(BlockDriverState *bs,
QemuOpts *opts,
*l2_cache_size = combined_cache_size - *refcount_cache_size;
}
}
-} else if (!l2_cache_size_set) {
-*l2_cache_size = MAX(DEFAULT_L2_CACHE_BYTE_SIZE,
- (uint64_t)DEFAULT_L2_CACHE_CLUSTERS
- * s->c
[Qemu-devel] [PATCH 08/11] iotests: Add filter commit test cases
This patch adds some tests on how commit copes with filter nodes.
Signed-off-by: Max Reitz
---
tests/qemu-iotests/040 | 130 +
tests/qemu-iotests/040.out | 4 +-
2 files changed, 132 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 1beb5e6dab..f0544d6107 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -346,5 +346,135 @@ class TestReopenOverlay(ImageCommitTestCase):
def test_reopen_overlay(self):
self.run_commit_test(self.img1, self.img0)
+class TestCommitWithFilters(iotests.QMPTestCase):
+img0 = os.path.join(iotests.test_dir, '0.img')
+img1 = os.path.join(iotests.test_dir, '1.img')
+img2 = os.path.join(iotests.test_dir, '2.img')
+img3 = os.path.join(iotests.test_dir, '3.img')
+
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, self.img0, '1M')
+qemu_img('create', '-f', iotests.imgfmt, self.img1, '1M')
+qemu_img('create', '-f', iotests.imgfmt, self.img2, '1M')
+qemu_img('create', '-f', iotests.imgfmt, self.img3, '1M')
+
+self.vm = iotests.VM()
+self.vm.launch()
+result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg')
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('blockdev-add', **{
+'node-name': 'top-filter',
+'driver': 'throttle',
+'throttle-group': 'tg',
+'file': {
+'node-name': 'cow-3',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img3
+},
+'backing': {
+'node-name': 'cow-2',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img2
+},
+'backing': {
+'node-name': 'cow-1',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img1
+},
+'backing': {
+'node-name': 'bottom-filter',
+'driver': 'throttle',
+'throttle-group': 'tg',
+'file': {
+'node-name': 'cow-0',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img0
+}
+}
+}
+}
+}
+}
+})
+self.assert_qmp(result, 'return', {})
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(self.img3)
+os.remove(self.img2)
+os.remove(self.img1)
+os.remove(self.img0)
+
+# Filters make for funny filenames, so we cannot just use
+# self.imgX for the block-commit parameters
+def get_filename(self, node):
+return self.vm.node_info(node)['image']['filename']
+
+def test_filterless_commit(self):
+self.assert_no_active_block_jobs()
+result = self.vm.qmp('block-commit',
+ job_id='commit',
+ device='top-filter',
+ top=self.get_filename('cow-2'),
+ base=self.get_filename('cow-1'))
+self.assert_qmp(result, 'return', {})
+self.wait_until_completed(drive='commit')
+
+def test_commit_through_filter(self):
+self.assert_no_active_block_jobs()
+result = self.vm.qmp('block-commit',
+ job_id='commit',
+ device='top-filter',
+ top=self.get_filename('cow-1'),
+ base=self.get_filename('cow-0'))
+# Cannot commit through explicitly added filters (yet,
+# although in the future we probably want to make users use
+# blockdev-copy for this)
+self.assert_qmp(result, 'error/class', 'GenericError')
+self.assert_qmp(result, 'error/desc', 'Cannot commit through explicit
filter nodes')
+
+def test_filtered_active_commit_with_filter(self):
+self.assert_no_active_block_jobs()
+result = self.vm.qmp('block-commit',
+ job_id='commit',
+ device='top-filter',
+ base=self.get_filename('cow-2'))
+# Not spe
[Qemu-devel] [PATCH v6 2/8] qcow2: Cosmetic changes
Some refactoring for better readability is done here.
Signed-off-by: Leonid Bloch
---
block/qcow2.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index ec9e6238a0..3f4abc394e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -790,8 +790,9 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts
*opts,
*refcount_cache_size = qemu_opt_get_size(opts,
QCOW2_OPT_REFCOUNT_CACHE_SIZE, 0);
-*l2_cache_entry_size = qemu_opt_get_size(
-opts, QCOW2_OPT_L2_CACHE_ENTRY_SIZE, s->cluster_size);
+*l2_cache_entry_size = qemu_opt_get_size(opts,
+ QCOW2_OPT_L2_CACHE_ENTRY_SIZE,
+ s->cluster_size);
if (combined_cache_size_set) {
if (l2_cache_size_set && refcount_cache_size_set) {
@@ -823,8 +824,8 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts
*opts,
*l2_cache_size = max_l2_cache;
*refcount_cache_size = combined_cache_size - *l2_cache_size;
} else {
-*refcount_cache_size =
-MIN(combined_cache_size, min_refcount_cache);
+*refcount_cache_size = MIN(combined_cache_size,
+ min_refcount_cache);
*l2_cache_size = combined_cache_size - *refcount_cache_size;
}
}
--
2.17.1
[Qemu-devel] [PATCH 1/3] target/riscv: optimize cross-page direct jumps in softmmu
Signed-off-by: Emilio G. Cota
---
target/riscv/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0b6be74f2d..ec2988b4f6 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -135,7 +135,7 @@ static void gen_goto_tb(DisasContext *ctx, int n,
target_ulong dest)
if (ctx->base.singlestep_enabled) {
gen_exception_debug();
} else {
-tcg_gen_exit_tb(NULL, 0);
+tcg_gen_lookup_and_goto_ptr();
}
}
}
--
2.17.1
[Qemu-devel] [PATCH 07/11] block: Leave BDS.backing_file constant
Parts of the block layer treat BDS.backing_file as if it were whatever
the image header says (i.e., if it is a relative path, it is relative to
the overlay), other parts treat it like a cache for
bs->backing->bs->filename (relative paths are relative to the CWD).
Considering bs->backing->bs->filename exists, let us make it mean the
former.
Among other things, this now allows the user to specify a base when
using qemu-img to commit an image file in a directory that is not the
CWD (assuming, everything uses relative filenames).
Before this patch:
$ ./qemu-img create -f qcow2 foo/bot.qcow2 1M
$ ./qemu-img create -f qcow2 -b bot.qcow2 foo/mid.qcow2
$ ./qemu-img create -f qcow2 -b mid.qcow2 foo/top.qcow2
$ ./qemu-img commit -b mid.qcow2 foo/top.qcow2
qemu-img: Did not find 'mid.qcow2' in the backing chain of 'foo/top.qcow2'
$ ./qemu-img commit -b foo/mid.qcow2 foo/top.qcow2
qemu-img: Did not find 'foo/mid.qcow2' in the backing chain of 'foo/top.qcow2'
$ ./qemu-img commit -b $PWD/foo/mid.qcow2 foo/top.qcow2
qemu-img: Did not find '[...]/foo/mid.qcow2' in the backing chain of
'foo/top.qcow2'
After this patch:
$ ./qemu-img commit -b mid.qcow2 foo/top.qcow2
Image committed.
$ ./qemu-img commit -b foo/mid.qcow2 foo/top.qcow2
qemu-img: Did not find 'foo/mid.qcow2' in the backing chain of 'foo/top.qcow2'
$ ./qemu-img commit -b $PWD/foo/mid.qcow2 foo/top.qcow2
Image committed.
With this change, bdrv_find_backing_image() must look at whether the
user has overridden a BDS's backing file. If so, it can no longer use
bs->backing_file, but must instead compare the given filename against
the backing node's filename directly.
Along with this, stop updating BDS.backing_format in
bdrv_backing_attach() as well. This necessitates a change to the
reference output of iotest 191.
Signed-off-by: Max Reitz
---
include/block/block_int.h | 14 +-
block.c| 29 ++---
block/qapi.c | 7 ---
qemu-img.c | 12 ++--
tests/qemu-iotests/191.out | 1 -
5 files changed, 45 insertions(+), 18 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index d3d8b22155..8f2c515ec1 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -737,11 +737,15 @@ struct BlockDriverState {
bool walking_aio_notifiers; /* to make removal during iteration safe */
char filename[PATH_MAX];
-char backing_file[PATH_MAX]; /* if non zero, the image is a diff of
-this file image */
-/* The backing filename indicated by the image header; if we ever
- * open this file, then this is replaced by the resulting BDS's
- * filename (i.e. after a bdrv_refresh_filename() run). */
+/* If non-zero, the image is a diff of this image file. Note that
+ * this the name given in the image header and may therefore not
+ * be equal to .backing->bs->filename, and relative paths are
+ * resolved relatively to their overlay. */
+char backing_file[PATH_MAX];
+/* The backing filename indicated by the image header. Contrary
+ * to backing_file, if we ever open this file, auto_backing_file
+ * is replaced by the resulting BDS's filename (i.e. after a
+ * bdrv_refresh_filename() run). */
char auto_backing_file[PATH_MAX];
char backing_format[16]; /* if non-zero and backing_file exists */
diff --git a/block.c b/block.c
index 9784ccb385..7fc7dbf364 100644
--- a/block.c
+++ b/block.c
@@ -78,6 +78,8 @@ static BlockDriverState *bdrv_open_inherit(const char
*filename,
const BdrvChildRole *child_role,
Error **errp);
+static bool bdrv_backing_overridden(BlockDriverState *bs);
+
/* If non-zero, use only whitelisted block drivers */
static int use_bdrv_whitelist;
@@ -995,10 +997,6 @@ static void bdrv_backing_attach(BdrvChild *c)
bdrv_refresh_filename(backing_hd);
parent->open_flags &= ~BDRV_O_NO_BACKING;
-pstrcpy(parent->backing_file, sizeof(parent->backing_file),
-backing_hd->filename);
-pstrcpy(parent->backing_format, sizeof(parent->backing_format),
-backing_hd->drv ? backing_hd->drv->format_name : "");
bdrv_op_block_all(backing_hd, parent->backing_blocker);
/* Otherwise we won't be able to commit or stream */
@@ -4318,6 +4316,7 @@ BlockDriverState
*bdrv_find_backing_image(BlockDriverState *bs,
char *backing_file_full = NULL;
char *filename_tmp = NULL;
int is_protocol = 0;
+bool filenames_refreshed = false;
BlockDriverState *curr_bs = NULL;
BlockDriverState *retval = NULL;
@@ -4340,9 +4339,25 @@ BlockDriverState
*bdrv_find_backing_image(BlockDriverState *bs,
{
BlockDriverState *bs_below = bdrv_backing_chain_next(curr_bs);
-/* If either of the filename paths is actually a protocol, then
- * compare unmodified paths; otherwise make pa
[Qemu-devel] [PATCH 11/11] iotests: Test committing to overridden backing
Signed-off-by: Max Reitz
---
tests/qemu-iotests/040 | 61 ++
tests/qemu-iotests/040.out | 4 +--
2 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index f0544d6107..90c03e745b 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -476,5 +476,66 @@ class TestCommitWithFilters(iotests.QMPTestCase):
self.assert_qmp(result, 'error/desc',
'Top image file %s not found' % cow3_name)
+class TestCommitWithOverriddenBacking(iotests.QMPTestCase):
+img_base_a = os.path.join(iotests.test_dir, 'base_a.img')
+img_base_b = os.path.join(iotests.test_dir, 'base_b.img')
+img_top = os.path.join(iotests.test_dir, 'top.img')
+
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, self.img_base_a, '1M')
+qemu_img('create', '-f', iotests.imgfmt, self.img_base_b, '1M')
+qemu_img('create', '-f', iotests.imgfmt, '-b', self.img_base_a, \
+ self.img_top)
+
+self.vm = iotests.VM()
+self.vm.launch()
+
+# Use base_b instead of base_a as the backing of top
+result = self.vm.qmp('blockdev-add', **{
+'node-name': 'top',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img_top
+},
+'backing': {
+'node-name': 'base',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img_base_b
+}
+}
+})
+self.assert_qmp(result, 'return', {})
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(self.img_top)
+os.remove(self.img_base_a)
+os.remove(self.img_base_b)
+
+def test_commit_to_a(self):
+# Try committing to base_a (which should fail, as top's
+# backing image is base_b instead)
+result = self.vm.qmp('block-commit',
+ job_id='commit',
+ device='top',
+ base=self.img_base_a)
+self.assert_qmp(result, 'error/class', 'GenericError')
+
+def test_commit_to_b(self):
+# Try committing to base_b (which should work, since that is
+# actually top's backing image)
+result = self.vm.qmp('block-commit',
+ job_id='commit',
+ device='top',
+ base=self.img_base_b)
+self.assert_qmp(result, 'return', {})
+
+self.vm.event_wait('BLOCK_JOB_READY')
+self.vm.qmp('block-job-complete', device='commit')
+self.vm.event_wait('BLOCK_JOB_COMPLETED')
+
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out
index 49f84261d0..cfa5c0d0e6 100644
--- a/tests/qemu-iotests/040.out
+++ b/tests/qemu-iotests/040.out
@@ -1,5 +1,5 @@
-.
+...
--
-Ran 33 tests
+Ran 35 tests
OK
--
2.17.1
[Qemu-devel] [PATCH 0/3] target/riscv: use tcg_lookup_and_goto_ptr
There are a few more places where the lookup could be inserted, but I think these are the ones that will matter performance-wise. Perf results in patch 3's log. Regarding the benchmarks: I'd have used SPEC06 but I don't have much time to get it to compile. Is there a guide on how to do so? Ideally I'd like to use the fedora image, which works very well. Thanks, Emilio
[Qemu-devel] [PATCH 04/11] block: Storage child access function
For completeness' sake, add a function for accessing a node's storage
child, too. For filters, this is there filtered child; for non-filters,
this is bs->file.
Some places are deliberately left unconverted:
- BDS opening/closing functions where bs->file is handled specially
(which is basically wrong, but at least simplifies probing)
- bdrv_co_block_status_from_file(), because its name implies that it
points to ->file
- bdrv_snapshot_goto() in one places unrefs bs->file. Such a
modification is not covered by this patch and is therefore just
safeguarded by an additional assert(), but otherwise kept as-is.
Signed-off-by: Max Reitz
---
include/block/block_int.h | 6 +
block.c | 48 ---
block/io.c| 20 +---
block/qapi.c | 7 +++---
block/snapshot.c | 40 ++--
5 files changed, 80 insertions(+), 41 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index fa9154899d..d3d8b22155 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1214,6 +1214,7 @@ int refresh_total_sectors(BlockDriverState *bs, int64_t
hint);
BdrvChild *bdrv_filtered_cow_child(BlockDriverState *bs);
BdrvChild *bdrv_filtered_rw_child(BlockDriverState *bs);
BdrvChild *bdrv_filtered_child(BlockDriverState *bs);
+BdrvChild *bdrv_storage_child(BlockDriverState *bs);
BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs);
BlockDriverState *bdrv_skip_rw_filters(BlockDriverState *bs);
BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs);
@@ -1238,4 +1239,9 @@ static inline BlockDriverState
*bdrv_filtered_bs(BlockDriverState *bs)
return child_bs(bdrv_filtered_child(bs));
}
+static inline BlockDriverState *bdrv_storage_bs(BlockDriverState *bs)
+{
+return child_bs(bdrv_storage_child(bs));
+}
+
#endif /* BLOCK_INT_H */
diff --git a/block.c b/block.c
index 61a2fe14eb..307c583dfd 100644
--- a/block.c
+++ b/block.c
@@ -3835,15 +3835,21 @@ exit:
int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
{
BlockDriver *drv = bs->drv;
+BlockDriverState *storage_bs;
+
if (!drv) {
return -ENOMEDIUM;
}
+
if (drv->bdrv_get_allocated_file_size) {
return drv->bdrv_get_allocated_file_size(bs);
}
-if (bs->file) {
-return bdrv_get_allocated_file_size(bs->file->bs);
+
+storage_bs = bdrv_storage_bs(bs);
+if (storage_bs) {
+return bdrv_get_allocated_file_size(storage_bs);
}
+
return -ENOTSUP;
}
@@ -4252,7 +4258,7 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const
char *event,
const char *tag)
{
while (bs && bs->drv && !bs->drv->bdrv_debug_breakpoint) {
-bs = bs->file ? bs->file->bs : NULL;
+bs = bdrv_storage_bs(bs);
}
if (bs && bs->drv && bs->drv->bdrv_debug_breakpoint) {
@@ -4265,7 +4271,7 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const
char *event,
int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag)
{
while (bs && bs->drv && !bs->drv->bdrv_debug_remove_breakpoint) {
-bs = bs->file ? bs->file->bs : NULL;
+bs = bdrv_storage_bs(bs);
}
if (bs && bs->drv && bs->drv->bdrv_debug_remove_breakpoint) {
@@ -4278,7 +4284,7 @@ int bdrv_debug_remove_breakpoint(BlockDriverState *bs,
const char *tag)
int bdrv_debug_resume(BlockDriverState *bs, const char *tag)
{
while (bs && (!bs->drv || !bs->drv->bdrv_debug_resume)) {
-bs = bs->file ? bs->file->bs : NULL;
+bs = bdrv_storage_bs(bs);
}
if (bs && bs->drv && bs->drv->bdrv_debug_resume) {
@@ -4291,7 +4297,7 @@ int bdrv_debug_resume(BlockDriverState *bs, const char
*tag)
bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag)
{
while (bs && bs->drv && !bs->drv->bdrv_debug_is_suspended) {
-bs = bs->file ? bs->file->bs : NULL;
+bs = bdrv_storage_bs(bs);
}
if (bs && bs->drv && bs->drv->bdrv_debug_is_suspended) {
@@ -5388,8 +5394,14 @@ void bdrv_refresh_filename(BlockDriverState *bs)
bs->exact_filename[0] = '\0';
drv->bdrv_refresh_filename(bs);
-} else if (bs->file) {
-/* Try to reconstruct valid information from the underlying file */
+} else if (bdrv_storage_child(bs) && !drv->is_filter) {
+/* Try to reconstruct valid information from the underlying
+ * file -- this only works for format nodes (filter nodes
+ * cannot be probed and as such must be selected by the user
+ * either through an options dict, or through a special
+ * filename which the filter driver must construct in its
+ * .bdrv_refresh_filename() implementation) */
+BlockDriverState *storage_bs = bdrv_storage_bs(bs);
bs->exact_filename[0] = '\0';
@@ -5404,11 +5416,10 @@ void bdrv_refresh_filename(BlockDriverState *bs)
* - no
