[Qemu-devel] [PATCH v7 1/2] Implement .hex file loader
This patch adds Intel Hexadecimal Object File format support to
the loader. The file format specification is available here:
http://www.piclist.com/techref/fileext/hex/intel.htm
The file format is mainly intended for embedded systems
and microcontrollers, such as Micro:bit Arduino, ARM, STM32, etc.
Suggested-by: Stefan Hajnoczi
Signed-off-by: Su Hang
---
hw/arm/boot.c | 10 ++-
hw/core/loader.c| 246
include/hw/loader.h | 15
3 files changed, 269 insertions(+), 2 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 9ae6ab2689e6..4acd5259d5fe 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -1066,12 +1066,18 @@ static void arm_load_kernel_notify(Notifier *notifier,
void *data)
kernel_size = load_uimage_as(info->kernel_filename, &entry, NULL,
&is_linux, NULL, NULL, as);
}
+if (kernel_size < 0) {
+entry = 0;
+/* 32-bit ARM Intel HEX file */
+kernel_size = load_targphys_hex_as(info->kernel_filename, &entry, as);
+}
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
kernel_size = load_aarch64_image(info->kernel_filename,
info->loader_start, &entry, as);
is_linux = 1;
-} else if (kernel_size < 0) {
-/* 32-bit ARM */
+}
+if (kernel_size < 0) {
+/* 32-bit ARM binary file */
entry = info->loader_start + KERNEL_LOAD_ADDR;
kernel_size = load_image_targphys_as(info->kernel_filename, entry,
info->ram_size - KERNEL_LOAD_ADDR,
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 06bdbca53709..c02a734743e2 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1286,3 +1286,249 @@ 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;
+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_checksum += value;
+++(*index);
+return true;
+}
+
+typedef struct {
+const char *filename;
+HexLine line;
+uint8_t *bin_buf;
+hwaddr *start_addr;
+int total_size;
+uint32_t next_address_to_write;
+uint32_t current_address;
+uint32_t current_rom_index;
+uint32_t rom_start_address;
+AddressSpace *as;
+} HexParser;
+
+/* return size or -1 if error */
+static int handle_record_type(HexParser *parser)
+{
+HexLine *line = &(parser->line);
+switch (line->record_type) {
+case DATA_RECORD:
+parser->current_address =
+(parser->next_address_to_write & 0x) | line->address;
+/* verify this is a contiguous block of memory */
+if (parser->current_address != parser->next_address_to_write) {
+if (parser->current_rom_index != 0) {
+rom_add_hex_blob_as(parser->filename, parser->bin_buf,
+parser->current_rom_index,
+parser->rom_start_address, parser->as);
+
[Qemu-devel] [PATCH v7 2/2] Add QTest testcase for the Intel Hexadecimal Object File Loader.
'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.
`pre_store` array in 'hexloader-test.c' file, stores the binary format
of 'test.hex' file, which is used to verify correctness.
Signed-off-by: Su Hang
---
MAINTAINERS | 6
tests/Makefile.include | 2 ++
tests/hex-loader-check-data/test.hex | 12
tests/hexloader-test.c | 56
4 files changed, 76 insertions(+)
create mode 100644 tests/hex-loader-check-data/test.hex
create mode 100644 tests/hexloader-test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 24b70169bc37..3d37d04c3345 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1291,6 +1291,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/tests/Makefile.include b/tests/Makefile.include
index 3b9a5e31a2c2..f4a3e71f34ee 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -380,6 +380,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)
@@ -755,6 +756,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/hex-loader-check-data/test.hex
b/tests/hex-loader-check-data/test.hex
new file mode 100644
index ..7e99b452f5cc
--- /dev/null
+++ b/tests/hex-loader-check-data/test.hex
@@ -0,0 +1,12 @@
+:02040001F9
+:14D09FE516EBFEEA9810010008
+:100014B02DE500B08DE20CD04DE208000BE5F8
+:100026EA08301BE50020D3E52C309FE5F0
+:1000302083E508301BE5013083E208300BE542
+:100048301BE50030D3E553E3F41A4E
+:10005000A0E100D08BE204B09DE41EFF2FE180
+:100060101F1000482DE904B08DE208009FE544
+:10007000E6EBA0E10088BDE8840001007E
+:100080101F1048656C6C6F20776F726C6421D4
+:02009A0064
+:0001FF
diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
new file mode 100644
index ..70f99ac03c6b
--- /dev/null
+++ b/tests/hexloader-test.c
@@ -0,0 +1,56 @@
+/*
+ * 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
+
+static unsigned char pre_store[BIN_SIZE] = {
+4, 208, 159, 229, 22, 0, 0, 235, 254, 255, 255, 234, 152, 16, 1,
+0, 4, 176, 45, 229, 0, 176, 141, 226, 12, 208, 77, 226, 8, 0,
+11, 229, 6, 0, 0, 234, 8, 48, 27, 229, 0, 32, 211, 229, 44,
+48, 159, 229, 0, 32, 131, 229, 8, 48, 27, 229, 1, 48, 131, 226,
+8, 48, 11, 229, 8, 48, 27, 229, 0, 48, 211, 229, 0, 0, 83,
+227, 244, 255, 255, 26, 0, 0, 160, 225, 0, 208, 139, 226, 4, 176,
+157, 228, 30, 255, 47, 225, 0, 16, 31, 16, 0, 72, 45, 233, 4,
+176, 141, 226, 8, 0, 159, 229, 230, 255, 255, 235, 0, 0, 160, 225,
+0, 136, 189, 232, 132, 0, 1, 0, 0, 16, 31, 16, 72, 101, 108,
+108, 111, 32, 119, 111, 114, 108, 100, 33, 10, 0};
+
+/* success if no crash or abort */
+static void hex_loader_test(void)
+{
+unsigned int i;
+unsigned char memory_content[BIN_SIZE];
+const unsigned int base_addr = 0x0001;
+
+QTestState *s = qtest_startf(
+"-M versatilepb -m 128M -nographic -kernel
../tests/hex-loader-check-data/test.hex");
+
+for (i = 0; i < BIN_SIZE; ++i) {
+memory_content[i] = qtest_readb(s, base_addr + i);
+g_assert_cmpuint(memory_content[i], ==, pre_store[i]);
+}
+qtest_quit(s);
+}
+
+int main(int argc, char **argv)
+{
+int ret;
+
+g_test_init(&argc, &argv, NULL);
+
+qtest_add_func("/tmp/hex_loader", hex_loader_test);
+ret = g_test_run();
+
+return ret;
+}
--
2.7.4
[Qemu-devel] [PATCH v7 0/2] Implement Hex file loader and add test case
These series of patchs implement Intel Hexadecimal File loader and
add QTest testcase to verify the correctness of Loader.
v1:
-- Basic version.
v2:
-- Replace `do{}while(cond);` block with `for(;;)` block.
v3:
-- Add two new files information in MAINTAINERS.
v4:
-- Correct the 'test.hex' path in hexloader-test.c.
v5:
-- Split huge parse_hex_blob() function into four small function;
-- Add check for memory bounds;
-- Check validation for Record type;
-- Replace function ctoh() with glib function g_ascii_xdigit_value();
-- Remove check for '.hex' suffix;
-- Remove unnecessary type cast;
-- Remove duplicate zero-initialization;
-- Correct typos;
v6:
-- Call Intel HEX loader after load_uimage_as();
-- Remove use of KERNEL_LOAD_ADDR;
-- Change (hwaddr) type argument to (hwaddr *) type;
-- Use new struct HexParser to contain all arguments needed by
handle_record_type();
-- Enable discontiguous data records (again);
-- Remove unnecessary memory clean: bin_buf and HexLine line;
-- Correct typo;
-- Remove unnecessary check for hex file size;
-- Add entry point handle for START_SEG_ADDR_RECORD and
START_LINEAR_ADDR_RECORD record type;
-- Use hwaddr * type to store entry point;
v7:
-- Remove unnecessary code style changes;
-- Replace `bool` with `size_t` for function `parse_record` return type;
-- Replace `int` with `size_t` for struct `HexParser` member field
`total_size` type;
-- Replace `int` with `size_t` for function `handle_record_type` return type;
-- Return an error when encountered unimplemented data type
`START_SEG_ADDR_RECORD`;
-- Add check for `START_LINEAR_ADDR_RECORD` data type:
byte_count == 4 and address == 0;
-- Rename struct `HexParser` member field `addr` to `start_addr`;
-- Replace `int` with `size_t` for function `parse_hex_blob` return type;
-- Stop incrementing `record_index` when encountered whitespace;
-- Replace
`if ((record_index >> 1) - LEN_EXCEPT_DATA != parser.line.byte_count)`
with
`if ((LEN_EXCEPT_DATA + parser.line.byte_count) * 2 != record_index)`
-- Remove unused field `max_sz`;
-- Replace `int` with `size_t` for local variable `total_size`
in function `parse_hex_blob`
-- Rename function `load_image_targphys_hex_as` argument `addr` to `entry`;
Su Hang (2):
Implement .hex file loader
Add QTest testcase for the Intel Hexadecimal Object File Loader.
MAINTAINERS | 6 +
hw/arm/boot.c| 10 +-
hw/core/loader.c | 246 +++
include/hw/loader.h | 15 +++
tests/Makefile.include | 2 +
tests/hex-loader-check-data/test.hex | 12 ++
tests/hexloader-test.c | 56
7 files changed, 345 insertions(+), 2 deletions(-)
create mode 100644 tests/hex-loader-check-data/test.hex
create mode 100644 tests/hexloader-test.c
--
2.7.4
Re: [Qemu-devel] [PATCH 04/13] qapi: Formalize qcow2 encryption probing
On Wed, May 09, 2018 at 06:55:21PM +0200, Max Reitz wrote:
> Currently, you can give no encryption format for a qcow2 file while
> still passing a key-secret. That does not conform to the schema, so
> this patch changes the schema to allow it.
>
> Signed-off-by: Max Reitz
> ---
> qapi/block-core.json | 44
> 1 file changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 71c9ab8538..092a1aba2d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -43,6 +43,19 @@
> { 'struct': 'ImageInfoSpecificQCow2EncryptionBase',
>'data': { 'format': 'BlockdevQcow2EncryptionFormat'}}
>
> +##
> +# @ImageInfoSpecificQCow2EncryptionNoInfo:
> +#
> +# Only used for the qcow2 encryption format "from-image" in which the
> +# actual encryption format is determined from the image header.
> +# Therefore, this encryption format will never be reported in
> +# ImageInfoSpecificQCow2Encryption.
> +#
> +# Since: 2.13
> +##
> +{ 'struct': 'ImageInfoSpecificQCow2EncryptionNoInfo',
> + 'data': { } }
> +
> ##
> # @ImageInfoSpecificQCow2Encryption:
> #
> @@ -52,7 +65,8 @@
>'base': 'ImageInfoSpecificQCow2EncryptionBase',
>'discriminator': 'format',
>'data': { 'aes': 'QCryptoBlockInfoQCow',
> -'luks': 'QCryptoBlockInfoLUKS' } }
> +'luks': 'QCryptoBlockInfoLUKS',
> +'from-image': 'ImageInfoSpecificQCow2EncryptionNoInfo' } }
>
> ##
> # @ImageInfoSpecificQCow2:
> @@ -2739,10 +2753,30 @@
> # @BlockdevQcow2EncryptionFormat:
> # @aes: AES-CBC with plain64 initialization venctors
> #
> +# @from-image: Determine the encryption format from the image
> +# header. This only allows the use of the
> +# key-secret option. (Since: 2.13)
> +#
> # Since: 2.10
> ##
> { 'enum': 'BlockdevQcow2EncryptionFormat',
> - 'data': [ 'aes', 'luks' ] }
> + 'data': [ 'aes', 'luks', 'from-image' ] }
> +
> +##
> +# @BlockdevQcow2EncryptionSecret:
> +#
> +# Allows specifying a key-secret without specifying the exact
> +# encryption format, which is determined automatically from the image
> +# header.
> +#
> +# @key-secret: The ID of a QCryptoSecret object providing the
> +# decryption key. Mandatory except when probing
> +# image for metadata only.
> +#
> +# Since: 2.13
> +##
> +{ 'struct': 'BlockdevQcow2EncryptionSecret',
> + 'data': { '*key-secret': 'str' } }
>
> ##
> # @BlockdevQcow2Encryption:
> @@ -2750,10 +2784,12 @@
> # Since: 2.10
> ##
> { 'union': 'BlockdevQcow2Encryption',
> - 'base': { 'format': 'BlockdevQcow2EncryptionFormat' },
> + 'base': { '*format': 'BlockdevQcow2EncryptionFormat' },
>'discriminator': 'format',
> + 'default-variant': 'from-image',
>'data': { 'aes': 'QCryptoBlockOptionsQCow',
> -'luks': 'QCryptoBlockOptionsLUKS'} }
> +'luks': 'QCryptoBlockOptionsLUKS',
> +'from-image': 'BlockdevQcow2EncryptionSecret' } }
Bike-shedding on name, how about "auto" or "probe" ?
IIUC, this schema addition means the QAPI parser now allows
encrypt.format=from-image,encrypt.key-secret=sec0,...other opts...
but the code will not accept "from-image" as a valid string.
eg qcow2_update_options_prepare() will do
case QCOW_CRYPT_AES:
if (encryptfmt && !g_str_equal(encryptfmt, "aes")) {
error_setg(errp,
"Header reported 'aes' encryption format but "
"options specify '%s'", encryptfmt);
ret = -EINVAL;
goto fail;
}
...snip
case QCOW_CRYPT_LUKS:
if (encryptfmt && !g_str_equal(encryptfmt, "luks")) {
error_setg(errp,
"Header reported 'luks' encryption format but "
"options specify '%s'", encryptfmt);
ret = -EINVAL;
goto fail;
}
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [virtio-dev] [PATCH V2 1/1] virtio-balloon: fix internal stat name array to match Linux kernel
On Wed, May 09, 2018 at 03:53:05PM -0400, Thomas Tai wrote:
> The Linux kernel commit b4325044 ("virtio_balloon: add array
> of stat names") defines an array of stat name strings for consumers
> of the virtio interface to use via the virtio_balloon.h header
> file, rather than requiring each consumer to define its own. But at
> present, the stat names defined in this array by the Linux kernel
> do not match up with those defined internally by QEMU.
>
> This patch fixes this inconsistency by importing the header file
> from linux and use the new macro to construct the balloon_stat_names
> array.
>
> Signed-off-by: Jonathan Helman
> Signed-off-by: Thomas Tai
> Cc: Rob Gardner
> ---
> docs/virtio-balloon-stats.txt | 2 ++
> hw/virtio/virtio-balloon.c | 13 ++---
> include/standard-headers/linux/virtio_balloon.h | 19 ++-
> 3 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/docs/virtio-balloon-stats.txt b/docs/virtio-balloon-stats.txt
> index 7a66d25..7c69fdb 100644
> --- a/docs/virtio-balloon-stats.txt
> +++ b/docs/virtio-balloon-stats.txt
> @@ -34,6 +34,8 @@ which will return a dictionary containing:
>- stat-total-memory
>- stat-available-memory
>- stat-disk-caches
> + - stat-hugetlb-allocations
> + - stat-hugetlb-failures
>
>o A key named last-update, which contains the last stats update
> timestamp in seconds. Since this timestamp is generated by the host,
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index f456cea..646f2e0 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -43,17 +43,8 @@ static void balloon_page(void *addr, int deflate)
> }
> }
>
> -static const char *balloon_stat_names[] = {
> - [VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in",
> - [VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out",
> - [VIRTIO_BALLOON_S_MAJFLT] = "stat-major-faults",
> - [VIRTIO_BALLOON_S_MINFLT] = "stat-minor-faults",
> - [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
> - [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
> - [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
> - [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
> - [VIRTIO_BALLOON_S_NR] = NULL
> -};
> +static const char *balloon_stat_names[VIRTIO_BALLOON_S_NR] = \
> + VIRTIO_BALLOON_S_NAMES_WITH_PREFIX("stat-");
>
> /*
> * reset_stats - Mark all items in the stats array as unset
> diff --git a/include/standard-headers/linux/virtio_balloon.h
> b/include/standard-headers/linux/virtio_balloon.h
> index 7b0a41b..4dbb7dc 100644
> --- a/include/standard-headers/linux/virtio_balloon.h
> +++ b/include/standard-headers/linux/virtio_balloon.h
> @@ -53,7 +53,24 @@ struct virtio_balloon_config {
> #define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */
> #define VIRTIO_BALLOON_S_AVAIL6 /* Available memory as in /proc */
> #define VIRTIO_BALLOON_S_CACHES 7 /* Disk caches */
> -#define VIRTIO_BALLOON_S_NR 8
> +#define VIRTIO_BALLOON_S_HTLB_PGALLOC 8 /* Hugetlb page allocations */
> +#define VIRTIO_BALLOON_S_HTLB_PGFAIL 9 /* Hugetlb page allocation
> failures */
> +#define VIRTIO_BALLOON_S_NR 10
> +
> +#define VIRTIO_BALLOON_S_NAMES_WITH_PREFIX(VIRTIO_BALLOON_S_NAMES_prefix) { \
> + VIRTIO_BALLOON_S_NAMES_prefix "swap-in", \
> + VIRTIO_BALLOON_S_NAMES_prefix "swap-out", \
> + VIRTIO_BALLOON_S_NAMES_prefix "major-faults", \
> + VIRTIO_BALLOON_S_NAMES_prefix "minor-faults", \
> + VIRTIO_BALLOON_S_NAMES_prefix "free-memory", \
> + VIRTIO_BALLOON_S_NAMES_prefix "total-memory", \
> + VIRTIO_BALLOON_S_NAMES_prefix "available-memory", \
> + VIRTIO_BALLOON_S_NAMES_prefix "disk-caches", \
> + VIRTIO_BALLOON_S_NAMES_prefix "hugetlb-allocations", \
> + VIRTIO_BALLOON_S_NAMES_prefix "hugetlb-failures" \
> +}
> +
> +#define VIRTIO_BALLOON_S_NAMES VIRTIO_BALLOON_S_NAMES_WITH_PREFIX("")
>
> /*
> * Memory statistics structure.
> --
> 1.8.3.1
>
>
> -
> To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
>
signature.asc
Description: PGP signature
Re: [Qemu-devel] [virtio-dev] [PATCH V2 1/1] virtio-balloon: fix internal stat name array to match Linux kernel
On Wed, May 09, 2018 at 03:53:05PM -0400, Thomas Tai wrote:
> The Linux kernel commit b4325044 ("virtio_balloon: add array
> of stat names") defines an array of stat name strings for consumers
> of the virtio interface to use via the virtio_balloon.h header
> file, rather than requiring each consumer to define its own. But at
> present, the stat names defined in this array by the Linux kernel
> do not match up with those defined internally by QEMU.
>
> This patch fixes this inconsistency by importing the header file
> from linux and use the new macro to construct the balloon_stat_names
> array.
>
> Signed-off-by: Jonathan Helman
> Signed-off-by: Thomas Tai
> Cc: Rob Gardner
> ---
> docs/virtio-balloon-stats.txt | 2 ++
> hw/virtio/virtio-balloon.c | 13 ++---
> include/standard-headers/linux/virtio_balloon.h | 19 ++-
> 3 files changed, 22 insertions(+), 12 deletions(-)
Reviewed-by: Stefan Hajnoczi
signature.asc
Description: PGP signature
Re: [Qemu-devel] [Qemu-block] Some question about savem/qcow2 incremental snapshot
On Wed, May 09, 2018 at 07:54:31PM +0200, Max Reitz wrote: > On 2018-05-09 12:16, Stefan Hajnoczi wrote: > > On Tue, May 08, 2018 at 05:03:09PM +0200, Kevin Wolf wrote: > >> Am 08.05.2018 um 16:41 hat Eric Blake geschrieben: > >>> On 12/25/2017 01:33 AM, He Junyan wrote: > >> 2. Make the nvdimm device use the QEMU block layer so that it is backed > >>by a non-raw disk image (such as a qcow2 file representing the > >>content of the nvdimm) that supports snapshots. > >> > >>This part is hard because it requires some completely new > >>infrastructure such as mapping clusters of the image file to guest > >>pages, and doing cluster allocation (including the copy on write > >>logic) by handling guest page faults. > >> > >> I think it makes sense to invest some effort into such interfaces, but > >> be prepared for a long journey. > > > > I like the suggestion but it needs to be followed up with a concrete > > design that is feasible and fair for Junyan and others to implement. > > Otherwise the "long journey" is really just a way of rejecting this > > feature. > > > > Let's discuss the details of using the block layer for NVDIMM and try to > > come up with a plan. > > > > The biggest issue with using the block layer is that persistent memory > > applications use load/store instructions to directly access data. This > > is fundamentally different from the block layer, which transfers blocks > > of data to and from the device. > > > > Because of block DMA, QEMU is able to perform processing at each block > > driver graph node. This doesn't exist for persistent memory because > > software does not trap I/O. Therefore the concept of filter nodes > > doesn't make sense for persistent memory - we certainly do not want to > > trap every I/O because performance would be terrible. > > > > Another difference is that persistent memory I/O is synchronous. > > Load/store instructions execute quickly. Perhaps we could use KVM async > > page faults in cases where QEMU needs to perform processing, but again > > the performance would be bad. > > Let me first say that I have no idea how the interface to NVDIMM looks. > I just assume it works pretty much like normal RAM (so the interface is > just that it’s a part of the physical address space). > > Also, it sounds a bit like you are already discarding my idea, but here > goes anyway. > > Would it be possible to introduce a buffering block driver that presents > the guest an area of RAM/NVDIMM through an NVDIMM interface (so I > suppose as part of the guest address space)? For writing, we’d keep a > dirty bitmap on it, and then we’d asynchronously move the dirty areas > through the block layer, so basically like mirror. On flushing, we’d > block until everything is clean. > > For reading, we’d follow a COR/stream model, basically, where everything > is unpopulated in the beginning and everything is loaded through the > block layer both asynchronously all the time and on-demand whenever the > guest needs something that has not been loaded yet. > > Now I notice that that looks pretty much like a backing file model where > we constantly run both a stream and a commit job at the same time. > > The user could decide how much memory to use for the buffer, so it could > either hold everything or be partially unallocated. > > You’d probably want to back the buffer by NVDIMM normally, so that > nothing is lost on crashes (though this would imply that for partial > allocation the buffering block driver would need to know the mapping > between the area in real NVDIMM and its virtual representation of it). > > Just my two cents while scanning through qemu-block to find emails that > don’t actually concern me... The guest kernel already implements this - it's the page cache and the block layer! Doing it in QEMU with dirty memory logging enabled is less efficient than doing it in the guest. That's why I said it's better to just use block devices than to implement buffering. I'm saying that persistent memory emulation on top of the iscsi:// block driver (for example) does not make sense. It could be implemented but the performance wouldn't be better than block I/O and the complexity/code size in QEMU isn't justified IMO. Stefan > > Most protocol drivers do not support direct memory access. iscsi, curl, > > etc just don't fit the model. One might be tempted to implement > > buffering but at that point it's better to just use block devices. > > > > I have CCed Pankaj, who is working on the virtio-pmem device. I need to > > be clear that emulated NVDIMM cannot be supported with the block layer > > since it lacks a guest flush mechanism. There is no way for > > applications to let the hypervisor know the file needs to be fsynced. > > That's what virtio-pmem addresses. > > > > Summary: > > A subset of the block layer could be used to back virtio-pmem. This > > requires a new block driver API and the KVM async page fault mechanism > > for trappin
Re: [Qemu-devel] [PATCH 0/2] xen-hvm: use new resource mapping API
> -Original Message- > From: Eric Blake [mailto:ebl...@redhat.com] > Sent: 09 May 2018 18:26 > To: Paul Durrant ; qemu-devel@nongnu.org > Cc: xen-de...@lists.xenproject.org; f...@redhat.com > Subject: Re: [Qemu-devel] [PATCH 0/2] xen-hvm: use new resource mapping > API > > On 05/09/2018 11:05 AM, Paul Durrant wrote: > > >> xenforeignmemory_map_resource() to map ioreq pages... > >> ERROR: spaces required around that '*' (ctx:WxV) > >> #164: FILE: include/hw/xen/xen_common.h:128: > >> +xenforeignmemory_handle *fmem, domid_t domid, unsigned int > type, > >> ^ > >> > >> total: 1 errors, 0 warnings, 138 lines checked > >> > >> Your patch has style problems, please review. If any of these errors > >> are false positives report them to the maintainer, see > >> CHECKPATCH in MAINTAINERS. > > > > This style warning appears to be spurious. > > Yep, and it's because xenforeignmemory_handle doesn't follow our usual > conventions for a type name. See commit 5ac067a if you want to add it > to the list of whitelisted exception type names, to silence messages > like this. Thanks Eric. I'll send a v2 series with a suitable additional patch. Cheers, Paul > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v3 1/9] block: Introduce API for copy offloading
On Wed, May 09, 2018 at 10:58:07PM +0800, Fam Zheng wrote:
Please include a commit description:
Introduce the bdrv_co_copy_range() API for copy offloading. Block
drivers implementing this API support efficient copy operations that
avoid reading each block from the source device and writing it to the
destination devices. Examples of copy offload primitives are SCSI
EXTENDED COPY and Linux copy_file_range(2).
This explains the purpose of the new API.
> +/* Copy range from @bs to @dst. */
There is no @bs argument, I guess it should be @src.
What about the return value?
What about the flags?
You don't need to duplicate this information, just reference
BlockDriver.bdrv_co_copy_range_from() if you want.
> +int bdrv_co_copy_range_from(BdrvChild *src, uint64_t src_offset,
> +BdrvChild *dst, uint64_t dst_offset,
> +uint64_t bytes, BdrvRequestFlags flags)
> +{
> +return bdrv_co_copy_range_internal(src, src_offset, dst, dst_offset,
> + bytes, flags, true);
> +}
> +
> +/* Copy range from @src to @bs. Should only be called by block drivers when
> @bs
> + * is the leaf. */
Same here.
> diff --git a/include/block/block.h b/include/block/block.h
> index cdec3639a3..72ac011b2b 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -604,4 +604,8 @@ bool bdrv_can_store_new_dirty_bitmap(BlockDriverState
> *bs, const char *name,
> */
> void bdrv_register_buf(BlockDriverState *bs, void *host, size_t size);
> void bdrv_unregister_buf(BlockDriverState *bs, void *host);
> +
> +int bdrv_co_copy_range(BdrvChild *bs, uint64_t offset,
> + BdrvChild *src, uint64_t src_offset,
> + uint64_t bytes, BdrvRequestFlags flags);
This is the public API. It's missing a doc comment.
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v3 4/9] file-posix: Implement bdrv_co_copy_range
On Wed, May 09, 2018 at 10:58:10PM +0800, Fam Zheng wrote:
> +static off_t copy_file_range(int in_fd, off_t *in_off, int out_fd,
> + off_t *out_off, size_t len, unsigned int flags)
> +{
> +#ifdef __NR_copy_file_range
> +return syscall(__NR_copy_file_range, in_fd, in_off, out_fd,
> + out_off, len, flags);
> +#else
> +errno = ENOSYS;
> +return -1;
> +#endif
> +}
> +
> +static ssize_t handle_aiocb_copy_range(RawPosixAIOData *aiocb)
> +{
> +uint64_t bytes = aiocb->aio_nbytes;
> +off_t in_off = aiocb->aio_offset;
> +off_t out_off = aiocb->aio_offset2;
> +
> +while (bytes) {
> +ssize_t ret = copy_file_range(aiocb->aio_fildes, &in_off,
> + aiocb->aio_fd2, &out_off,
> + bytes, 0);
> +if (ret == -EINTR) {
> +continue;
> +}
> +if (ret < 0) {
> +return -errno;
Convert ENOSYS to ENOTSUP?
Reviewed-by: Stefan Hajnoczi
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v3 5/9] iscsi: Query and save device designator when opening
On Wed, May 09, 2018 at 10:58:11PM +0800, Fam Zheng wrote: > The device designator data returned in INQUIRY command will be useful to > fill in source/target fields during copy offloading. Do this when > connecting to the target and save the data for later use. > > Signed-off-by: Fam Zheng > --- > block/iscsi.c| 41 + > include/scsi/constants.h | 2 ++ > 2 files changed, 43 insertions(+) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v3 6/9] iscsi: Create and use iscsi_co_wait_for_task
On Wed, May 09, 2018 at 10:58:12PM +0800, Fam Zheng wrote: > This loop is repeated a growing number times. Make a helper. > > Signed-off-by: Fam Zheng > --- > block/iscsi.c | 54 +- > 1 file changed, 17 insertions(+), 37 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v3 7/9] iscsi: Implement copy offloading
On Wed, May 09, 2018 at 10:58:13PM +0800, Fam Zheng wrote:
> +static int iscsi_populate_target_desc(unsigned char *desc, IscsiLun *lun)
The return values of these iscsi_populate_*() functions are unused and
don't make sense since the caller must already know the size ahead of
time. I suggest removing them to simplify the code.
> +{
> +struct scsi_inquiry_device_designator *dd = lun->dd;
> +
> +memset(desc, 0, 32);
> +desc[0] = IDENT_DESCR_TGT_DESCR;
> +desc[4] = dd->code_set;
> +desc[5] = (dd->designator_type & 0xF)
> +| ((dd->association & 3) << 4);
> +desc[7] = dd->designator_length;
> +memcpy(desc + 8, dd->designator, dd->designator_length);
> +
> +desc[28] = 0;
> +desc[29] = (lun->block_size >> 16) & 0xFF;
> +desc[30] = (lun->block_size >> 8) & 0xFF;
> +desc[31] = lun->block_size & 0xFF;
> +
> +return 32;
> +}
> +
> +static int iscsi_xcopy_desc_hdr(uint8_t *hdr, int dc, int cat, int src_index,
> +int dst_index)
> +{
> +int desc_len = 28;
s/desc_len/XCOPY_BLK2BLK_SEG_DESC_SIZE/ ?
Then the 28 constant doesn't need to be duplicated.
Reviewed-by: Stefan Hajnoczi
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v3 8/9] block-backend: Add blk_co_copy_range
On Wed, May 09, 2018 at 10:58:14PM +0800, Fam Zheng wrote: > It's a BlockBackend wrapper of the BDS interface. > > Signed-off-by: Fam Zheng > --- > block/block-backend.c | 18 ++ > include/sysemu/block-backend.h | 4 > 2 files changed, 22 insertions(+) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v3 9/9] qemu-img: Convert with copy offloading
On Wed, May 09, 2018 at 10:58:15PM +0800, Fam Zheng wrote:
> The new blk_co_copy_range interface offers a more efficient way in the
> case of network based storage. Make use of it to allow faster convert
> operation.
>
> Since copy offloading cannot do zero detection ('-S') and compression
> (-c), only try it when these options are not used.
>
> Update 178.out.qcow2 as we are now a little more efficient on the actual
> file size.
>
> Signed-off-by: Fam Zheng
> ---
> qemu-img.c | 50
> ++--
> tests/qemu-iotests/178.out.qcow2 | 8 +++
> 2 files changed, 52 insertions(+), 6 deletions(-)
Reviewed-by: Stefan Hajnoczi
signature.asc
Description: PGP signature
Re: [Qemu-devel] [RFC 13/24] avocado_qemu: Functional test for RHBZ#1431939
On Mon, May 07, 2018 at 11:03:29AM -0300, Eduardo Habkost wrote:
> On Mon, Apr 30, 2018 at 02:02:46PM +0100, Stefan Hajnoczi wrote:
> > On Fri, Apr 20, 2018 at 03:19:40PM -0300, Eduardo Habkost wrote:
> > > +def test_hotplug_memory_default_policy(self):
> > > +"""
> > > +According to the RHBZ1431939, the issue is 'host nodes'
> > > +returning '128'. It should return empty value when memory
> > > +hotplug default policy is used.
> > > +
> > > +Fixed in commit d81d857f4421d205395d55200425daa6591c28a5.
> > > +:avocado: tags=RHBZ1431939
> > > +"""
> > > +
> > > +cmd = 'object_add memory-backend-ram,id=mem1,size=1G'
> > > +res = self.vm.qmp('human-monitor-command', command_line=cmd)
> > > +self.assertEqual('', res['return'])
> >
> > General question about QMP test coding style:
> >
> > What happens if res['return'] does not exist because the QMP command
> > failed?
> >
> > I tend to use dict.get() to prevent KeyError. That way the
> > assertEqual() will fail instead of an unhandled KeyError in the test
> > code.
>
> It looks like vm.command() would be appropriate on most cases, as
> it will check for errors and return res['result'] automatically.
>
> vm.qmp() seems to be useful only if you really don't want an
> exception to be raised in case of QMP errors.
>
> Maybe we should rename .qmp() to .raw_qmp() to discourage people
> from using it.
Sounds good.
Stefan
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH v2 1/3] xen-hvm: create separate function for ioreq server initialization
The code is sufficiently substantial that it improves code readability
to put it in a new function called by xen_hvm_init() rather than having
it inline.
Signed-off-by: Paul Durrant
---
Cc: Stefano Stabellini
Cc: Anthony Perard
---
hw/i386/xen/xen-hvm.c | 76 +++
1 file changed, 46 insertions(+), 30 deletions(-)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index caa563be3d..6ffa3c22cc 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -95,7 +95,8 @@ typedef struct XenIOState {
CPUState **cpu_by_vcpu_id;
/* the evtchn port for polling the notification, */
evtchn_port_t *ioreq_local_port;
-/* evtchn local port for buffered io */
+/* evtchn remote and local ports for buffered io */
+evtchn_port_t bufioreq_remote_port;
evtchn_port_t bufioreq_local_port;
/* the evtchn fd for polling */
xenevtchn_handle *xce_handle;
@@ -1236,12 +1237,52 @@ static void xen_wakeup_notifier(Notifier *notifier,
void *data)
xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
}
-void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
+static int xen_map_ioreq_server(XenIOState *state)
{
-int i, rc;
xen_pfn_t ioreq_pfn;
xen_pfn_t bufioreq_pfn;
evtchn_port_t bufioreq_evtchn;
+int rc;
+
+rc = xen_get_ioreq_server_info(xen_domid, state->ioservid,
+ &ioreq_pfn, &bufioreq_pfn,
+ &bufioreq_evtchn);
+if (rc < 0) {
+error_report("failed to get ioreq server info: error %d handle=%p",
+ errno, xen_xc);
+return rc;
+}
+
+DPRINTF("shared page at pfn %lx\n", ioreq_pfn);
+DPRINTF("buffered io page at pfn %lx\n", bufioreq_pfn);
+DPRINTF("buffered io evtchn is %x\n", bufioreq_evtchn);
+
+state->shared_page = xenforeignmemory_map(xen_fmem, xen_domid,
+ PROT_READ | PROT_WRITE,
+ 1, &ioreq_pfn, NULL);
+if (state->shared_page == NULL) {
+error_report("map shared IO page returned error %d handle=%p",
+ errno, xen_xc);
+return -1;
+}
+
+state->buffered_io_page = xenforeignmemory_map(xen_fmem, xen_domid,
+ PROT_READ | PROT_WRITE,
+ 1, &bufioreq_pfn, NULL);
+if (state->buffered_io_page == NULL) {
+error_report("map buffered IO page returned error %d", errno);
+return -1;
+}
+
+state->bufioreq_remote_port = bufioreq_evtchn;
+
+return 0;
+}
+
+void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
+{
+int i, rc;
+xen_pfn_t ioreq_pfn;
XenIOState *state;
state = g_malloc0(sizeof (XenIOState));
@@ -1269,25 +1310,8 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion
**ram_memory)
state->wakeup.notify = xen_wakeup_notifier;
qemu_register_wakeup_notifier(&state->wakeup);
-rc = xen_get_ioreq_server_info(xen_domid, state->ioservid,
- &ioreq_pfn, &bufioreq_pfn,
- &bufioreq_evtchn);
+rc = xen_map_ioreq_server(state);
if (rc < 0) {
-error_report("failed to get ioreq server info: error %d handle=%p",
- errno, xen_xc);
-goto err;
-}
-
-DPRINTF("shared page at pfn %lx\n", ioreq_pfn);
-DPRINTF("buffered io page at pfn %lx\n", bufioreq_pfn);
-DPRINTF("buffered io evtchn is %x\n", bufioreq_evtchn);
-
-state->shared_page = xenforeignmemory_map(xen_fmem, xen_domid,
- PROT_READ|PROT_WRITE,
- 1, &ioreq_pfn, NULL);
-if (state->shared_page == NULL) {
-error_report("map shared IO page returned error %d handle=%p",
- errno, xen_xc);
goto err;
}
@@ -1308,14 +1332,6 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion
**ram_memory)
goto err;
}
-state->buffered_io_page = xenforeignmemory_map(xen_fmem, xen_domid,
- PROT_READ|PROT_WRITE,
- 1, &bufioreq_pfn, NULL);
-if (state->buffered_io_page == NULL) {
-error_report("map buffered IO page returned error %d", errno);
-goto err;
-}
-
/* Note: cpus is empty at this point in init */
state->cpu_by_vcpu_id = g_malloc0(max_cpus * sizeof(CPUState *));
@@ -1340,7 +1356,7 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion
**ram_memory)
}
rc = xenevtchn_bind_interdomain(state->xce_handle, xen_domid,
-bufioreq_evtchn);
+state->bufioreq_remote_port);
if (rc == -1) {
error_report("buffered evtchn bind erro
[Qemu-devel] [PATCH v2 2/3] checkpatch: generalize xen handle matching in the list of types
All the xen stable APIs define handle types of the form:
_handle
and some define additional handle types of the form:
__handle
Examples of these are xenforeignmemory_handle and
xenforeignmemory_resource_handle.
Both of these types will be misparsed by checkpatch if they appear as the
first token in a line since, as types defined by an external library, they
do not conform to the QEMU CODING_STYLE, which suggests CamelCase.
A previous patch (5ac067a24a8) added xendevicemodel_handle to the list
of types. This patch changes that to xen\w+_handle such that it will
match all Xen stable API handles of the forms detailed above.
Signed-off-by: Paul Durrant
---
Cc: Eric Blake
Cc: Paolo Bonzini
Cc: Daniel P. Berrange
v2:
- New in this version
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5b8735defb..98ed799f29 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -266,7 +266,7 @@ our @typeList = (
qr{target_(?:u)?long},
qr{hwaddr},
qr{xml${Ident}},
- qr{xendevicemodel_handle},
+ qr{xen\w+_handle},
);
# This can be modified by sub possible. Since it can be empty, be careful
--
2.11.0
[Qemu-devel] [PATCH v2 0/2] xen-hvm: use new resource mapping API
This series modifies QEMU to use the new guest resource mapping API (available in Xen 4.11+) to map ioreq pages. v2: - Add a patch to checkpatch to avoid misparsing of Xen stable API handles Paul Durrant (3): xen-hvm: create separate function for ioreq server initialization checkpatch: generalize xen handle matching in the list of types xen-hvm: try to use xenforeignmemory_map_resource() to map ioreq pages configure | 5 ++ hw/i386/xen/trace-events| 1 + hw/i386/xen/xen-hvm.c | 114 include/hw/xen/xen_common.h | 14 ++ scripts/checkpatch.pl | 2 +- 5 files changed, 105 insertions(+), 31 deletions(-) --- Cc: Anthony Perard Cc: Daniel P. Berrange Cc: Eric Blake Cc: Paolo Bonzini Cc: Stefano Stabellini -- 2.11.0
[Qemu-devel] [PATCH v2 3/3] xen-hvm: try to use xenforeignmemory_map_resource() to map ioreq pages
Xen 4.11 has a new API to directly map guest resources. Among the resources
that can be mapped using this API are ioreq pages.
This patch modifies QEMU to attempt to use the new API should it exist,
falling back to the previous mechanism if it is unavailable.
Signed-off-by: Paul Durrant
---
Cc: Stefano Stabellini
Cc: Anthony Perard
---
configure | 5
hw/i386/xen/trace-events| 1 +
hw/i386/xen/xen-hvm.c | 68 +++--
include/hw/xen/xen_common.h | 14 ++
4 files changed, 73 insertions(+), 15 deletions(-)
diff --git a/configure b/configure
index 1443422e83..0f9c2f000e 100755
--- a/configure
+++ b/configure
@@ -2229,12 +2229,17 @@ EOF
#undef XC_WANT_COMPAT_DEVICEMODEL_API
#define __XEN_TOOLS__
#include
+#include
int main(void) {
xendevicemodel_handle *xd;
+ xenforeignmemory_handle *xfmem;
xd = xendevicemodel_open(0, 0);
xendevicemodel_pin_memory_cacheattr(xd, 0, 0, 0, 0);
+ xfmem = xenforeignmemory_open(0, 0);
+ xenforeignmemory_map_resource(xfmem, 0, 0, 0, 0, 0, NULL, 0, 0);
+
return 0;
}
EOF
diff --git a/hw/i386/xen/trace-events b/hw/i386/xen/trace-events
index 8dab7bcfe0..38616b698f 100644
--- a/hw/i386/xen/trace-events
+++ b/hw/i386/xen/trace-events
@@ -15,6 +15,7 @@ cpu_ioreq_pio(void *req, uint32_t dir, uint32_t df, uint32_t
data_is_ptr, uint64
cpu_ioreq_pio_read_reg(void *req, uint64_t data, uint64_t addr, uint32_t size)
"I/O=%p pio read reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d"
cpu_ioreq_pio_write_reg(void *req, uint64_t data, uint64_t addr, uint32_t
size) "I/O=%p pio write reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d"
cpu_ioreq_move(void *req, uint32_t dir, uint32_t df, uint32_t data_is_ptr,
uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p copy
dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d"
+xen_map_resource_ioreq(uint32_t id, void *addr) "id: %u addr: %p"
# xen-mapcache.c
xen_map_cache(uint64_t phys_addr) "want 0x%"PRIx64
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 6ffa3c22cc..664cc52532 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1239,13 +1239,41 @@ static void xen_wakeup_notifier(Notifier *notifier,
void *data)
static int xen_map_ioreq_server(XenIOState *state)
{
+void *addr = NULL;
+xenforeignmemory_resource_handle *fres;
xen_pfn_t ioreq_pfn;
xen_pfn_t bufioreq_pfn;
evtchn_port_t bufioreq_evtchn;
int rc;
+/*
+ * Attempt to map using the resource API and fall back to normal
+ * foreign mapping if this is not supported.
+ */
+QEMU_BUILD_BUG_ON(XENMEM_resource_ioreq_server_frame_bufioreq != 0);
+QEMU_BUILD_BUG_ON(XENMEM_resource_ioreq_server_frame_ioreq(0) != 1);
+fres = xenforeignmemory_map_resource(xen_fmem, xen_domid,
+ XENMEM_resource_ioreq_server,
+ state->ioservid, 0, 2,
+ &addr,
+ PROT_READ | PROT_WRITE, 0);
+if (fres != NULL) {
+trace_xen_map_resource_ioreq(state->ioservid, addr);
+state->buffered_io_page = addr;
+state->shared_page = addr + TARGET_PAGE_SIZE;
+} else {
+error_report("failed to map ioreq server resources: error %d
handle=%p",
+ errno, xen_xc);
+if (errno != EOPNOTSUPP) {
+return -1;
+}
+}
+
rc = xen_get_ioreq_server_info(xen_domid, state->ioservid,
- &ioreq_pfn, &bufioreq_pfn,
+ (state->shared_page == NULL) ?
+ &ioreq_pfn : NULL,
+ (state->buffered_io_page == NULL) ?
+ &bufioreq_pfn : NULL,
&bufioreq_evtchn);
if (rc < 0) {
error_report("failed to get ioreq server info: error %d handle=%p",
@@ -1253,27 +1281,37 @@ static int xen_map_ioreq_server(XenIOState *state)
return rc;
}
-DPRINTF("shared page at pfn %lx\n", ioreq_pfn);
-DPRINTF("buffered io page at pfn %lx\n", bufioreq_pfn);
-DPRINTF("buffered io evtchn is %x\n", bufioreq_evtchn);
-
-state->shared_page = xenforeignmemory_map(xen_fmem, xen_domid,
- PROT_READ | PROT_WRITE,
- 1, &ioreq_pfn, NULL);
if (state->shared_page == NULL) {
-error_report("map shared IO page returned error %d handle=%p",
- errno, xen_xc);
-return -1;
+DPRINTF("shared page at pfn %lx\n", ioreq_pfn);
+
+state->shared_page = xenforeignmemory_map(xen_fmem, xen_domid,
+ PROT_READ | PROT_WRITE,
+ 1, &ioreq_pfn, NULL);
+if (state->
Re: [Qemu-devel] [PATCH v3 3/9] qcow2: Implement copy offloading
On Wed, May 09, 2018 at 10:58:09PM +0800, Fam Zheng wrote:
> +static int qcow2_co_copy_range_from(BlockDriverState *bs,
> +BdrvChild *src, uint64_t src_offset,
> +BdrvChild *dst, uint64_t dst_offset,
> +uint64_t bytes, BdrvRequestFlags flags)
> +{
> +BDRVQcow2State *s = bs->opaque;
> +int offset_in_cluster;
> +int ret;
> +unsigned int cur_bytes; /* number of bytes in current iteration */
> +uint64_t cluster_offset = 0;
> +BdrvChild *child = NULL;
> +
> +assert(!bs->encrypted);
> +qemu_co_mutex_lock(&s->lock);
> +
> +while (bytes != 0) {
> +
> +/* prepare next request */
> +cur_bytes = MIN(bytes, INT_MAX);
> +
> +ret = qcow2_get_cluster_offset(bs, src_offset, &cur_bytes,
> &cluster_offset);
> +if (ret < 0) {
> +goto out;
> +}
> +
> +offset_in_cluster = offset_into_cluster(s, src_offset);
> +
> +switch (ret) {
> +case QCOW2_CLUSTER_UNALLOCATED:
> +if (bs->backing) {
> +child = bs->backing;
> +} else {
> +flags |= BDRV_REQ_ZERO_WRITE;
> +}
> +break;
Do we need a special case if the backing file is shorter than this
image?
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH] linux-user: move thunk.c to linux-user/
thunk.c is part of linux-user and only used by it, so move it to the linux-user directory. Signed-off-by: Laurent Vivier --- Makefile.target | 2 +- linux-user/Makefile.objs | 2 +- thunk.c => linux-user/thunk.c | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename thunk.c => linux-user/thunk.c (100%) diff --git a/Makefile.target b/Makefile.target index d0ec77a307..98f25c91c7 100644 --- a/Makefile.target +++ b/Makefile.target @@ -112,7 +112,7 @@ QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR) \ -I$(SRC_PATH)/linux-user obj-y += linux-user/ -obj-y += gdbstub.o thunk.o +obj-y += gdbstub.o endif #CONFIG_LINUX_USER diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs index 59a5c17354..1fc1dd2762 100644 --- a/linux-user/Makefile.objs +++ b/linux-user/Makefile.objs @@ -1,7 +1,7 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \ elfload.o linuxload.o uaccess.o uname.o \ safe-syscall.o $(TARGET_ABI_DIR)/signal.o \ -$(TARGET_ABI_DIR)/cpu_loop.o +$(TARGET_ABI_DIR)/cpu_loop.o thunk.o obj-$(TARGET_HAS_BFLT) += flatload.o obj-$(TARGET_I386) += vm86.o diff --git a/thunk.c b/linux-user/thunk.c similarity index 100% rename from thunk.c rename to linux-user/thunk.c -- 2.14.3
Re: [Qemu-devel] [PATCH] linux-user: move thunk.c to linux-user/
On 10 May 2018 at 10:29, Laurent Vivier wrote: > thunk.c is part of linux-user and only used by it, > so move it to the linux-user directory. > > Signed-off-by: Laurent Vivier > --- > Makefile.target | 2 +- > linux-user/Makefile.objs | 2 +- > thunk.c => linux-user/thunk.c | 0 > 3 files changed, 2 insertions(+), 2 deletions(-) > rename thunk.c => linux-user/thunk.c (100%) bsd-user doesn't use it now, but I have a feeling maybe the currently-out-of-tree improvements do? thanks -- PMM
[Qemu-devel] [PATCH v3 1/5] fpu/softfloat: int_to_float ensure r fully initialised
Reported by Coverity (CID1390635). We ensure this for uint_to_float
later on so we might as well mirror that.
Signed-off-by: Alex Bennée
---
fpu/softfloat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 70e0c40a1c..3adf6a06e4 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1517,7 +1517,7 @@ FLOAT_TO_UINT(64, 64)
static FloatParts int_to_float(int64_t a, float_status *status)
{
-FloatParts r;
+FloatParts r = {};
if (a == 0) {
r.cls = float_class_zero;
r.sign = false;
--
2.17.0
[Qemu-devel] [PATCH v3 0/5] refactor float-to-float and fix AHP
Hi, Hi, I've not included the test case in the series but you can find it in my TCG fixup branch: https://github.com/stsquad/qemu/blob/testing/tcg-tests-revival-v4/tests/tcg/arm/fcvt.c Some of the ARMv7 versions are commented out as they where not supported until later revs. I do have a build that includes that but unfortunately the Debian compiler it too old to build it. : patch 0001/fpu softfloat int_to_float ensure r fully initial.patch needs review : patch 0004/target arm convert conversion helpers to fpst ahp.patch needs review : patch 0005/target arm squash FZ16 behaviour for conversions.patch needs review Alex Bennée (5): fpu/softfloat: int_to_float ensure r fully initialised fpu/softfloat: re-factor float to float conversions fpu/softfloat: support ARM Alternative half-precision target/arm: convert conversion helpers to fpst/ahp_flag target/arm: squash FZ16 behaviour for conversions fpu/softfloat-specialize.h | 40 --- fpu/softfloat.c| 546 +++-- include/fpu/softfloat.h| 8 +- target/arm/helper.c| 66 ++--- target/arm/helper.h| 12 +- target/arm/translate-a64.c | 38 ++- target/arm/translate.c | 70 +++-- target/arm/translate.h | 15 + 8 files changed, 306 insertions(+), 489 deletions(-) -- 2.17.0
[Qemu-devel] [PATCH v3 2/5] fpu/softfloat: re-factor float to float conversions
This allows us to delete a lot of additional boilerplate code which is
no longer needed. Currently the ieee flag is ignored (everything is
assumed to be ieee). Handling for ARM AHP will be in the next patch.
Signed-off-by: Alex Bennée
Reviewed-by: Richard Henderson
---
v2
- pass FloatFmt to float_to_float instead of sizes
- split AHP handling to another patch
- use rth's suggested re-packing (+ setting .exp)
v3
- also rm extractFloat16Sign
---
fpu/softfloat-specialize.h | 40
fpu/softfloat.c| 452 +++--
include/fpu/softfloat.h| 8 +-
3 files changed, 88 insertions(+), 412 deletions(-)
diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 27834af0de..a20b440159 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -293,46 +293,6 @@ float16 float16_maybe_silence_nan(float16 a_, float_status
*status)
return a_;
}
-/*
-| Returns the result of converting the half-precision floating-point NaN
-| `a' to the canonical NaN format. If `a' is a signaling NaN, the invalid
-| exception is raised.
-**/
-
-static commonNaNT float16ToCommonNaN(float16 a, float_status *status)
-{
-commonNaNT z;
-
-if (float16_is_signaling_nan(a, status)) {
-float_raise(float_flag_invalid, status);
-}
-z.sign = float16_val(a) >> 15;
-z.low = 0;
-z.high = ((uint64_t) float16_val(a)) << 54;
-return z;
-}
-
-/*
-| Returns the result of converting the canonical NaN `a' to the half-
-| precision floating-point format.
-**/
-
-static float16 commonNaNToFloat16(commonNaNT a, float_status *status)
-{
-uint16_t mantissa = a.high >> 54;
-
-if (status->default_nan_mode) {
-return float16_default_nan(status);
-}
-
-if (mantissa) {
-return make_float16(uint16_t) a.sign) << 15)
- | (0x1F << 10) | mantissa));
-} else {
-return float16_default_nan(status);
-}
-}
-
#ifdef NO_SIGNALING_NANS
int float32_is_quiet_nan(float32 a_, float_status *status)
{
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 3adf6a06e4..042e5c901d 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -123,15 +123,6 @@ static inline int extractFloat16Exp(float16 a)
return (float16_val(a) >> 10) & 0x1f;
}
-/*
-| Returns the sign bit of the single-precision floating-point value `a'.
-**/
-
-static inline flag extractFloat16Sign(float16 a)
-{
-return float16_val(a)>>15;
-}
-
/*
| Returns the fraction bits of the single-precision floating-point value `a'.
**/
@@ -1194,6 +1185,90 @@ float64 float64_div(float64 a, float64 b, float_status
*status)
return float64_round_pack_canonical(pr, status);
}
+/*
+ * Float to Float conversions
+ *
+ * Returns the result of converting one float format to another. The
+ * conversion is performed according to the IEC/IEEE Standard for
+ * Binary Floating-Point Arithmetic.
+ *
+ * The float_to_float helper only needs to take care of raising
+ * invalid exceptions and handling the conversion on NaNs.
+ */
+
+static FloatParts float_to_float(FloatParts a,
+ const FloatFmt *srcf, const FloatFmt *dstf,
+ float_status *s)
+{
+if (is_nan(a.cls)) {
+
+if (is_snan(a.cls)) {
+s->float_exception_flags |= float_flag_invalid;
+}
+
+if (s->default_nan_mode) {
+a.cls = float_class_dnan;
+return a;
+}
+
+/*
+ * Our only option now is to "re-pack" the NaN. As the
+ * canonilization process doesn't mess with fraction bits for
+ * NaNs we do it all here. We also reset a.exp to the
+ * destination format exp_max as the maybe_silence_nan code
+ * assumes it is correct (which is would be for non-conversions).
+ */
+a.frac = a.frac << (64 - srcf->frac_size) >> (64 - dstf->frac_size);
+a.exp = dstf->exp_max;
+a.cls = float_class_msnan;
+}
+
+return a;
+}
+
+float32 float16_to_float32(float16 a, bool ieee, float_status *s)
+{
+FloatParts p = float16_unpack_canonical(a, s);
+FloatParts pr = float_to_float(p, &float16_params, &float32_params, s);
+return float32_round_pack_canonical(pr, s);
+}
+
+float64 float16_to_float64(float16 a, bool ieee, float_status *s)
+{
+FloatParts p = float16
[Qemu-devel] [PATCH v3 3/5] fpu/softfloat: support ARM Alternative half-precision
For float16 ARM supports an alternative half-precision format which
sacrifices the ability to represent NaN/Inf in return for a higher
dynamic range. To support this I've added an additional
FloatFmt (float16_params_ahp).
The new FloatFmt flag (arm_althp) is then used to modify the behaviour
of canonicalize and round_canonical with respect to representation and
exception raising.
Finally the float16_to_floatN and floatN_to_float16 conversion
routines select the new alternative FloatFmt when !ieee.
Signed-off-by: Alex Bennée
Reviewed-by: Richard Henderson
---
v3
- squash NaN to 0 if destination is AHP F16
---
fpu/softfloat.c | 108 +---
1 file changed, 85 insertions(+), 23 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 042e5c901d..79ebc998d3 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -225,6 +225,8 @@ typedef struct {
* frac_lsb: least significant bit of fraction
* fram_lsbm1: the bit bellow the least significant bit (for rounding)
* round_mask/roundeven_mask: masks used for rounding
+ * The following optional modifiers are available:
+ * arm_althp: handle ARM Alternative Half Precision
*/
typedef struct {
int exp_size;
@@ -236,6 +238,7 @@ typedef struct {
uint64_t frac_lsbm1;
uint64_t round_mask;
uint64_t roundeven_mask;
+bool arm_althp;
} FloatFmt;
/* Expand fields based on the size of exponent and fraction */
@@ -248,12 +251,17 @@ typedef struct {
.frac_lsb = 1ull << (DECOMPOSED_BINARY_POINT - F), \
.frac_lsbm1 = 1ull << ((DECOMPOSED_BINARY_POINT - F) - 1), \
.round_mask = (1ull << (DECOMPOSED_BINARY_POINT - F)) - 1, \
-.roundeven_mask = (2ull << (DECOMPOSED_BINARY_POINT - F)) - 1
+.roundeven_mask = (2ull << (DECOMPOSED_BINARY_POINT - F)) - 1,
static const FloatFmt float16_params = {
FLOAT_PARAMS(5, 10)
};
+static const FloatFmt float16_params_ahp = {
+FLOAT_PARAMS(5, 10)
+.arm_althp = true
+};
+
static const FloatFmt float32_params = {
FLOAT_PARAMS(8, 23)
};
@@ -317,7 +325,7 @@ static inline float64 float64_pack_raw(FloatParts p)
static FloatParts canonicalize(FloatParts part, const FloatFmt *parm,
float_status *status)
{
-if (part.exp == parm->exp_max) {
+if (part.exp == parm->exp_max && !parm->arm_althp) {
if (part.frac == 0) {
part.cls = float_class_inf;
} else {
@@ -403,8 +411,9 @@ static FloatParts round_canonical(FloatParts p,
float_status *s,
exp += parm->exp_bias;
if (likely(exp > 0)) {
+bool maybe_inexact = false;
if (frac & round_mask) {
-flags |= float_flag_inexact;
+maybe_inexact = true;
frac += inc;
if (frac & DECOMPOSED_OVERFLOW_BIT) {
frac >>= 1;
@@ -413,14 +422,26 @@ static FloatParts round_canonical(FloatParts p,
float_status *s,
}
frac >>= frac_shift;
-if (unlikely(exp >= exp_max)) {
-flags |= float_flag_overflow | float_flag_inexact;
-if (overflow_norm) {
-exp = exp_max - 1;
-frac = -1;
-} else {
-p.cls = float_class_inf;
-goto do_inf;
+if (parm->arm_althp) {
+if (unlikely(exp >= exp_max + 1)) {
+flags |= float_flag_invalid;
+frac = -1;
+exp = exp_max;
+} else if (maybe_inexact) {
+flags |= float_flag_inexact;
+}
+} else {
+if (unlikely(exp >= exp_max)) {
+flags |= float_flag_overflow | float_flag_inexact;
+if (overflow_norm) {
+exp = exp_max - 1;
+frac = -1;
+} else {
+p.cls = float_class_inf;
+goto do_inf;
+}
+} else if (maybe_inexact) {
+flags |= float_flag_inexact;
}
}
} else if (s->flush_to_zero) {
@@ -465,7 +486,13 @@ static FloatParts round_canonical(FloatParts p,
float_status *s,
case float_class_inf:
do_inf:
exp = exp_max;
-frac = 0;
+if (parm->arm_althp) {
+flags |= float_flag_invalid;
+/* Alt HP returns result = sign:Ones(M-1) */
+frac = -1;
+} else {
+frac = 0;
+}
break;
case float_class_qnan:
@@ -483,12 +510,21 @@ static FloatParts round_canonical(FloatParts p,
float_status *s,
return p;
}
+/* Explicit FloatFmt version */
+static FloatParts float16a_unpack_canonical(const FloatFmt *params,
+float16 f, float_status *s
[Qemu-devel] [PATCH v3 5/5] target/arm: squash FZ16 behaviour for conversions
The ARM ARM specifies FZ16 is suppressed for conversions. Rather than
pushing this logic into the softfloat code we can simply save the FZ
state and temporarily disable it for the softfloat call.
Signed-off-by: Alex Bennée
---
target/arm/helper.c | 24
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 4dd28bb70c..17147be58b 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11459,12 +11459,20 @@ uint32_t HELPER(set_neon_rmode)(uint32_t rmode,
CPUARMState *env)
/* Half precision conversions. */
static float32 do_fcvt_f16_to_f32(float16 a, float_status *s, bool ahp)
{
-return float16_to_float32(a, !ahp, s);
+flag save_flush_to_zero = s->flush_to_zero;
+set_flush_to_zero(false, s);
+float32 r = float16_to_float32(a, !ahp, s);
+set_flush_to_zero(save_flush_to_zero, s);
+return r;
}
static float16 do_fcvt_f32_to_f16(float32 a, float_status *s, bool ahp)
{
-return float32_to_float16(a, !ahp, s);
+flag save_flush_to_zero = s->flush_to_zero;
+set_flush_to_zero(false, s);
+float16 r = float32_to_float16(a, !ahp, s);
+set_flush_to_zero(save_flush_to_zero, s);
+return float16_val(r);
}
float32 HELPER(neon_fcvt_f16_to_f32)(float16 a, void *fpstp, uint32_t ahp_mode)
@@ -11494,13 +11502,21 @@ float16 HELPER(vfp_fcvt_f32_to_f16)(float32 a, void
*fpstp, uint32_t ahp_mode)
float64 HELPER(vfp_fcvt_f16_to_f64)(float16 a, void *fpstp, uint32_t ahp_mode)
{
float_status *fpst = fpstp;
-return float16_to_float64(a, !ahp_mode, fpst);
+flag save_flush_to_zero = fpst->flush_to_zero;
+set_flush_to_zero(false, fpst);
+float64 r = float16_to_float64(a, !ahp_mode, fpst);
+set_flush_to_zero(save_flush_to_zero, fpst);
+return r;
}
float16 HELPER(vfp_fcvt_f64_to_f16)(float64 a, void *fpstp, uint32_t ahp_mode)
{
float_status *fpst = fpstp;
-return float64_to_float16(a, !ahp_mode, fpst);
+flag save_flush_to_zero = fpst->flush_to_zero;
+set_flush_to_zero(false, fpst);
+float16 r = float64_to_float16(a, !ahp_mode, fpst);
+set_flush_to_zero(save_flush_to_zero, fpst);
+return float16_val(r);
}
#define float32_two make_float32(0x4000)
--
2.17.0
[Qemu-devel] [PATCH v3 4/5] target/arm: convert conversion helpers to fpst/ahp_flag
Instead of passing env and leaving it up to the helper to get the
right fpstatus we pass it explicitly. There was already a get_fpstatus
helper for neon for the 32 bit code. We also add an get_ahp_flag() for
passing the state of the alternative FP16 format flag. This leaves
scope for later tracking the AHP state in translation flags.
Signed-off-by: Alex Bennée
---
target/arm/helper.c| 58 ---
target/arm/helper.h| 12 +++
target/arm/translate-a64.c | 38 +
target/arm/translate.c | 70 +-
target/arm/translate.h | 15
5 files changed, 128 insertions(+), 65 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0fef5d4d06..4dd28bb70c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11457,64 +11457,50 @@ uint32_t HELPER(set_neon_rmode)(uint32_t rmode,
CPUARMState *env)
}
/* Half precision conversions. */
-static float32 do_fcvt_f16_to_f32(uint32_t a, CPUARMState *env, float_status
*s)
+static float32 do_fcvt_f16_to_f32(float16 a, float_status *s, bool ahp)
{
-int ieee = (env->vfp.xregs[ARM_VFP_FPSCR] & (1 << 26)) == 0;
-float32 r = float16_to_float32(make_float16(a), ieee, s);
-if (ieee) {
-return float32_maybe_silence_nan(r, s);
-}
-return r;
+return float16_to_float32(a, !ahp, s);
}
-static uint32_t do_fcvt_f32_to_f16(float32 a, CPUARMState *env, float_status
*s)
+static float16 do_fcvt_f32_to_f16(float32 a, float_status *s, bool ahp)
{
-int ieee = (env->vfp.xregs[ARM_VFP_FPSCR] & (1 << 26)) == 0;
-float16 r = float32_to_float16(a, ieee, s);
-if (ieee) {
-r = float16_maybe_silence_nan(r, s);
-}
-return float16_val(r);
+return float32_to_float16(a, !ahp, s);
}
-float32 HELPER(neon_fcvt_f16_to_f32)(uint32_t a, CPUARMState *env)
+float32 HELPER(neon_fcvt_f16_to_f32)(float16 a, void *fpstp, uint32_t ahp_mode)
{
-return do_fcvt_f16_to_f32(a, env, &env->vfp.standard_fp_status);
+float_status *fpst = fpstp;
+return do_fcvt_f16_to_f32(a, fpst, ahp_mode);
}
-uint32_t HELPER(neon_fcvt_f32_to_f16)(float32 a, CPUARMState *env)
+float16 HELPER(neon_fcvt_f32_to_f16)(float32 a, void *fpstp, uint32_t ahp_mode)
{
-return do_fcvt_f32_to_f16(a, env, &env->vfp.standard_fp_status);
+float_status *fpst = fpstp;
+return do_fcvt_f32_to_f16(a, fpst, ahp_mode);
}
-float32 HELPER(vfp_fcvt_f16_to_f32)(uint32_t a, CPUARMState *env)
+float32 HELPER(vfp_fcvt_f16_to_f32)(float16 a, void *fpstp, uint32_t ahp_mode)
{
-return do_fcvt_f16_to_f32(a, env, &env->vfp.fp_status);
+float_status *fpst = fpstp;
+return do_fcvt_f16_to_f32(a, fpst, ahp_mode);
}
-uint32_t HELPER(vfp_fcvt_f32_to_f16)(float32 a, CPUARMState *env)
+float16 HELPER(vfp_fcvt_f32_to_f16)(float32 a, void *fpstp, uint32_t ahp_mode)
{
-return do_fcvt_f32_to_f16(a, env, &env->vfp.fp_status);
+float_status *fpst = fpstp;
+return do_fcvt_f32_to_f16(a, fpst, ahp_mode);
}
-float64 HELPER(vfp_fcvt_f16_to_f64)(uint32_t a, CPUARMState *env)
+float64 HELPER(vfp_fcvt_f16_to_f64)(float16 a, void *fpstp, uint32_t ahp_mode)
{
-int ieee = (env->vfp.xregs[ARM_VFP_FPSCR] & (1 << 26)) == 0;
-float64 r = float16_to_float64(make_float16(a), ieee, &env->vfp.fp_status);
-if (ieee) {
-return float64_maybe_silence_nan(r, &env->vfp.fp_status);
-}
-return r;
+float_status *fpst = fpstp;
+return float16_to_float64(a, !ahp_mode, fpst);
}
-uint32_t HELPER(vfp_fcvt_f64_to_f16)(float64 a, CPUARMState *env)
+float16 HELPER(vfp_fcvt_f64_to_f16)(float64 a, void *fpstp, uint32_t ahp_mode)
{
-int ieee = (env->vfp.xregs[ARM_VFP_FPSCR] & (1 << 26)) == 0;
-float16 r = float64_to_float16(a, ieee, &env->vfp.fp_status);
-if (ieee) {
-r = float16_maybe_silence_nan(r, &env->vfp.fp_status);
-}
-return float16_val(r);
+float_status *fpst = fpstp;
+return float64_to_float16(a, !ahp_mode, fpst);
}
#define float32_two make_float32(0x4000)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 34e8cc8904..288480a0e7 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -181,12 +181,12 @@ DEF_HELPER_3(vfp_ultoh, f16, i32, i32, ptr)
DEF_HELPER_FLAGS_2(set_rmode, TCG_CALL_NO_RWG, i32, i32, ptr)
DEF_HELPER_FLAGS_2(set_neon_rmode, TCG_CALL_NO_RWG, i32, i32, env)
-DEF_HELPER_2(vfp_fcvt_f16_to_f32, f32, i32, env)
-DEF_HELPER_2(vfp_fcvt_f32_to_f16, i32, f32, env)
-DEF_HELPER_2(neon_fcvt_f16_to_f32, f32, i32, env)
-DEF_HELPER_2(neon_fcvt_f32_to_f16, i32, f32, env)
-DEF_HELPER_FLAGS_2(vfp_fcvt_f16_to_f64, TCG_CALL_NO_RWG, f64, i32, env)
-DEF_HELPER_FLAGS_2(vfp_fcvt_f64_to_f16, TCG_CALL_NO_RWG, i32, f64, env)
+DEF_HELPER_3(vfp_fcvt_f16_to_f32, f32, f16, ptr, i32)
+DEF_HELPER_3(vfp_fcvt_f32_to_f16, f16, f32, ptr, i32)
+DEF_HELPER_3(neon_fcvt_f16_to_f32, f32, f16, ptr, i32)
+DEF_HELPER_3(neon_fcvt_f32_to_f16, f16, f32, ptr, i32)
+DEF_HELPER_FLAG
Re: [Qemu-devel] [PATCH] linux-user: move thunk.c to linux-user/
Le 10/05/2018 à 11:35, Peter Maydell a écrit : > On 10 May 2018 at 10:29, Laurent Vivier wrote: >> thunk.c is part of linux-user and only used by it, >> so move it to the linux-user directory. >> >> Signed-off-by: Laurent Vivier >> --- >> Makefile.target | 2 +- >> linux-user/Makefile.objs | 2 +- >> thunk.c => linux-user/thunk.c | 0 >> 3 files changed, 2 insertions(+), 2 deletions(-) >> rename thunk.c => linux-user/thunk.c (100%) > > bsd-user doesn't use it now, but I have a feeling maybe > the currently-out-of-tree improvements do? Yes, you're right, I've found it in https://github.com/seanbruno/qemu-bsd-user/blob/bsd-user/bsd-user/bsd-ioctl.c Thanks, Laurent
Re: [Qemu-devel] [PATCH v2 0/5] checkpatch: backport UTF-8 fixes and MAINTAINERS check
On Mon, Apr 30, 2018 at 01:46:46PM +0100, Stefan Hajnoczi wrote: > v2: > * Resolve missing CHK() [Thomas] > * Drop first argument to WARN() to match QEMU function arguments > > This series cherry picks checkpatch UTF-8 fixes and the MAINTAINERS file check > from Linux. Thomas Huth original did the backport in January 2017 but the > series was forgotten (<1485436265-12573-1-git-send-email-th...@redhat.com>). > I > did the cherry pick again from scratch. > > The MAINTAINERS file check prints a warning when new files are added without a > modification to ./MAINTAINERS. It is easy to forget to update ./MAINTAINERS > and this check should help us stay on top of new source files. > > Joe Perches (4): > checkpatch: add a --strict check for utf-8 in commit logs > checkpatch: ignore email headers better > checkpatch: emit a warning on file add/move/delete > checkpatch: reduce MAINTAINERS update message frequency > > Pasi Savanainen (1): > checkpatch: check utf-8 content from a commit log when it's missing > from charset > > scripts/checkpatch.pl | 56 > +++ > 1 file changed, 52 insertions(+), 4 deletions(-) > > -- > 2.14.3 > > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v2 1/2] qemu-iotests: reduce chance of races in 185
On Tue, May 08, 2018 at 09:26:03AM -0500, Eric Blake wrote:
> On 05/08/2018 08:54 AM, Stefan Hajnoczi wrote:
> > Commit 8565c3ab537e78f3e69977ec2c609dc9417a806e ("qemu-iotests: fix
> > 185") identified a race condition in a sub-test.
> >
> > Similar issues also affect the other sub-tests. If disk I/O completes
> > quickly, it races with the QMP 'quit' command. This causes spurious
> > test failures because QMP events are emitted in an unpredictable order.
> >
> > This test relies on QEMU internals and there is no QMP API for getting
> > deterministic behavior needed to make this test 100% reliable. At the
> > same time, the test is useful and it would be a shame to remove it.
> >
> > Add sleep 0.5 to reduce the chance of races. This is not a real fix but
> > appears to reduce spurious failures in practice.
> >
> > Cc: Vladimir Sementsov-Ogievskiy
> > Signed-off-by: Stefan Hajnoczi
> > ---
> > tests/qemu-iotests/185 | 12
> > 1 file changed, 12 insertions(+)
>
> I'm not opposed to this patch, but is there any way to write the test to
> take both events in either order, without logging the events as they arrive,
> but instead summarizing in a deterministic order which events were received
> after the fact? That way, no matter which way the race is won, we merely
> log that we got two expected events, and could avoid the extra sleep.
I don't think there is a practical way of doing that without big changes
to the test. It could be rewritten in Python to make filtering the QMP
events easier.
Hiding the race doesn't solve the deeper problem though: the test case
doesn't exercise the same code path each time. The test should really
cover all cancellation points in the block job lifecycle instead of just
one at random. If we solve this problem then we don't need to filter
the QMP event sequence.
Maybe it can be done with blkdebug. If not then maybe a blockjobdbg
interface is necessary to perform deterministic tests (eliminating the
need for the ratelimiting trick used by this test!).
Please share ideas, but I think this is a long-term item that shouldn't
block this series.
Stefan
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v5 2/4] monitor: protect mon->fds with mon_lock
On Wed, May 09, 2018 at 12:17:32PM +0800, Peter Xu wrote: > mon->fds were protected by BQL. Now protect it by mon_lock so that it > can even be used in monitor iothread. Only monitor_get_fd() can safely be called from the monitor iothread (oob). The other functions call close(2), which may block, and therefore aren't allowed in oob code. Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v5 3/4] monitor: more comments on lock-free fleids/funcs
On Wed, May 09, 2018 at 12:17:33PM +0800, Peter Xu wrote: > Add some explicit comment for both Readline and cpu_set/cpu_get helpers > that they do not need the mon_lock protection. > > Signed-off-by: Peter Xu > --- > monitor.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] MAINTAINERS: Add trace-events and qemu-option-trace.texi to tracing section
On Wed, May 09, 2018 at 06:38:20AM +0200, Thomas Huth wrote: > The "trace-events" and "qemu-option-trace.texi" files in the top directory > are currently "unmaintained" according to scripts/get_maintainer.pl. They > obviously belong to the Tracing section, so add an entry for them there. > > Signed-off-by: Thomas Huth > --- > MAINTAINERS | 2 ++ > 1 file changed, 2 insertions(+) Thanks for the discussion! I'm happy to serve as a catch-all for trace-events patches (especially trace events in unmaintained code). It is preferrable for trace-events changes to go via the subsystem, if there is a maintainer. Thanks, applied to my tracing tree: https://github.com/stefanha/qemu/commits/tracing Stefan signature.asc Description: PGP signature
[Qemu-devel] [Bug 1575607] Re: vm startup failed, qemu returned "kvm run failed Bad address"
have resolved it？ -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1575607 Title: vm startup failed, qemu returned "kvm run failed Bad address" Status in QEMU: New Bug description: create a virtual machine and start by libvirt. vm startup failed, qemu returned "kvm run failed Bad address" the error log is : error: kvm run failed Bad address EAX=7ffc EBX=7ffbffd0 ECX=fff0 EDX=002c ESI=6f5c EDI=7ffbffd0 EBP=7ffc ESP=6f34 EIP=000dec7b EFL=00010046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0010 00c09300 DPL=0 DS [-WA] CS =0008 00c09b00 DPL=0 CS32 [-RA] SS =0010 00c09300 DPL=0 DS [-WA] DS =0010 00c09300 DPL=0 DS [-WA] FS =0010 00c09300 DPL=0 DS [-WA] GS =0010 00c09300 DPL=0 DS [-WA] LDT= 8200 DPL=0 LDT TR = 8b00 DPL=0 TSS32-busy GDT= 000f6a80 0037 IDT= 000f6abe CR0=6011 CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6=0ff0 DR7=0400 EFER= Code=c3 29 d3 21 cb 39 c3 77 27 3b 5e 0c 72 22 85 ff 75 02 89 df <89> 5f 08 01 da 89 57 0c 89 47 10 89 5e 10 8b 56 04 89 f8 e8 8c fc ff ff 89 d8 eb 06 8b 36 we add numa in the vm, the numatune info is: the version of qemu is 2.5.0. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1575607/+subscriptions
Re: [Qemu-devel] [RFC PATCH V4 3/4] vfio: Add SaveVMHanlders for VFIO device to support live migration
Yulei Zhang wrote:
> Instead of using vm state description, add SaveVMHandlers for VFIO
> device to support live migration.
>
> Introduce new Ioctl VFIO_DEVICE_GET_DIRTY_BITMAP to fetch the memory
> bitmap that dirtied by vfio device during the iterative precopy stage
> to shorten the system downtime afterward.
>
> For vfio pci device status migrate, during the system downtime, it will
> save the following states
> 1. pci configuration space addr0~addr5
> 2. pci configuration space msi_addr msi_data
> 3. pci device status fetch from device driver
>
> And on the target side the vfio_load will restore the same states
> 1. re-setup the pci bar configuration
> 2. re-setup the pci device msi configuration
> 3. restore the pci device status
[...]
> +static uint64_t vfio_dirty_log_sync(VFIOPCIDevice *vdev)
> +{
> +RAMBlock *block;
> +struct vfio_device_get_dirty_bitmap *d;
> +uint64_t page = 0;
> +ram_addr_t size;
> +unsigned long nr, bitmap;
> +
> +RAMBLOCK_FOREACH(block) {
BTW, you are asking to sync all blocks of memory? Does vfio needs it?
Things like acpi tables, or similar weird blocks looks strange, no?
> +size = block->used_length;
> +nr = size >> TARGET_PAGE_BITS;
> +bitmap = (BITS_TO_LONGS(nr) + 1) * sizeof(unsigned long);
> +d = g_malloc0(sizeof(*d) + bitmap);
> +d->start_addr = block->offset;
> +d->page_nr = nr;
> +if (ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_DIRTY_BITMAP, d)) {
> +error_report("vfio: Failed to get device dirty bitmap");
> +g_free(d);
> +goto exit;
> +}
> +
> +if (d->page_nr) {
> +cpu_physical_memory_set_dirty_lebitmap(
> + (unsigned long *)&d->dirty_bitmap,
> + d->start_addr, d->page_nr);
> +page += d->page_nr;
Are you sure that this are the number of dirty pages? It looks to me to
the total number of pages in the region, no?
> +}
> +g_free(d);
> +}
> +
> +exit:
> +return page;
> +}
You walk the whole RAM on each bitmap. Could it be possible that driver
knows _what_ memory regions it can have dirty pages on?
> +static void vfio_save_live_pending(QEMUFile *f, void *opaque, uint64_t
> max_size,
> + uint64_t *non_postcopiable_pending,
> + uint64_t *postcopiable_pending)
> +{
> +VFIOPCIDevice *vdev = opaque;
> +uint64_t pending;
> +
> +qemu_mutex_lock_iothread();
> +rcu_read_lock();
> +pending = vfio_dirty_log_sync(vdev);
> +rcu_read_unlock();
> +qemu_mutex_unlock_iothread();
> +*non_postcopiable_pending += pending;
> +}
As said in other emails, this is not for iterative migration, see how we
do for ram (I have simplified it a bit):
static void ram_save_pending(QEMUFile *f, void *opaque, uint64_t max_size,
uint64_t *res_precopy_only,
uint64_t *res_compatible,
uint64_t *res_postcopy_only)
{
RAMState **temp = opaque;
RAMState *rs = *temp;
uint64_t remaining_size;
remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE;
if (remaining_size < max_size) {
qemu_mutex_lock_iothread();
rcu_read_lock();
migration_bitmap_sync(rs);
rcu_read_unlock();
qemu_mutex_unlock_iothread();
remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE;
}
*res_precopy_only += remaining_size;
}
I.e. we only do the sync stage if we don't have enough dirty pages on
the bitmap.
BTW, once that we are here, independently of this, perhaps we should
change the "sync" to take functions for each ramblock instead of for
the whole ram.
> +static int vfio_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +VFIOPCIDevice *vdev = opaque;
> +PCIDevice *pdev = &vdev->pdev;
> +int sz = vdev->device_state.size - VFIO_DEVICE_STATE_OFFSET;
> +uint8_t *buf = NULL;
> +uint32_t ctl, msi_lo, msi_hi, msi_data, bar_cfg, i;
> +bool msi_64bit;
> +
> +if (qemu_get_byte(f) == VFIO_SAVE_FLAG_SETUP) {
> +goto exit;
> +}
As said before, I would suggest that you add several fields:
- VERSION: So you can make incopatible changes
- FLAGS: compatible changes
- SIZE of the region? Rest of QEMU don't have it, I consider it an
error. Having it makes way easier to analyze the stream and seach for errors.
> +/* retore pci bar configuration */
> +ctl = pci_default_read_config(pdev, PCI_COMMAND, 2);
> +vfio_pci_write_config(pdev, PCI_COMMAND,
> + ctl & (!(PCI_COMMAND_IO | PCI_COMMAND_MEMORY)), 2);
Please forgive my pci ignorance, but are we really want to store the pci
configuration every time that we receive an iteration stage?
> +for (i = 0; i < PCI_ROM_SLOT; i++) {
Is this PCI_ROM_SLOT fixed by some spec or should we transfer it?
> +bar_cfg = qemu_get_be32(
Re: [Qemu-devel] [PATCH v3 1/5] fpu/softfloat: int_to_float ensure r fully initialised
On 10 May 2018 at 10:42, Alex Bennée wrote:
> Reported by Coverity (CID1390635). We ensure this for uint_to_float
> later on so we might as well mirror that.
>
> Signed-off-by: Alex Bennée
> ---
> fpu/softfloat.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 70e0c40a1c..3adf6a06e4 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -1517,7 +1517,7 @@ FLOAT_TO_UINT(64, 64)
>
> static FloatParts int_to_float(int64_t a, float_status *status)
> {
> -FloatParts r;
> +FloatParts r = {};
> if (a == 0) {
> r.cls = float_class_zero;
> r.sign = false;
> --
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [Qemu-devel] [PATCH v3 2/5] fpu/softfloat: re-factor float to float conversions
On 10 May 2018 at 10:42, Alex Bennée wrote:
> This allows us to delete a lot of additional boilerplate code which is
> no longer needed. Currently the ieee flag is ignored (everything is
> assumed to be ieee). Handling for ARM AHP will be in the next patch.
>
> Signed-off-by: Alex Bennée
> Reviewed-by: Richard Henderson
>
> ---
> v2
> - pass FloatFmt to float_to_float instead of sizes
> - split AHP handling to another patch
> - use rth's suggested re-packing (+ setting .exp)
> v3
> - also rm extractFloat16Sign
> ---
> fpu/softfloat-specialize.h | 40
> fpu/softfloat.c| 452 +++--
> include/fpu/softfloat.h| 8 +-
> 3 files changed, 88 insertions(+), 412 deletions(-)
This introduces a regression where we don't get tininess-before-rounding
for double/single to halfprec conversions. This is because we're
now using the fp_status_f16 status field, and it has not had
the detect_tininess setting initialized. This fixes it:
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index d175c5e94f..7939c6b8ae 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -324,6 +324,8 @@ static void arm_cpu_reset(CPUState *s)
&env->vfp.fp_status);
set_float_detect_tininess(float_tininess_before_rounding,
&env->vfp.standard_fp_status);
+set_float_detect_tininess(float_tininess_before_rounding,
+ &env->vfp.fp_status_f16);
#ifndef CONFIG_USER_ONLY
if (kvm_enabled()) {
kvm_arm_reset_vcpu(cpu);
(You can see this if you try something like fcvt h1, d0 where
d0 == 0x3f0f___ -- we get the right answer of 0x0400
but fail to set Underflow as well as Inexact.)
thanks
-- PMM
Re: [Qemu-devel] [PATCH v3 0/5] refactor float-to-float and fix AHP
On 10 May 2018 at 10:42, Alex Bennée wrote: > Hi, > > Hi, > > I've not included the test case in the series but you can find it in > my TCG fixup branch: > > > https://github.com/stsquad/qemu/blob/testing/tcg-tests-revival-v4/tests/tcg/arm/fcvt.c > > Some of the ARMv7 versions are commented out as they where not > supported until later revs. I do have a build that includes that but > unfortunately the Debian compiler it too old to build it. > > : patch 0001/fpu softfloat int_to_float ensure r fully initial.patch needs > review > : patch 0004/target arm convert conversion helpers to fpst ahp.patch needs > review > : patch 0005/target arm squash FZ16 behaviour for conversions.patch needs > review This still seems to regress the NaN conversion case I mentioned in review of the previous series: (3) Here's a NaN case we get wrong now: 64 to IEEE-16 conversion, input is 0x7ff1 (an SNaN), we produce 0x7c00 (infinity) but should produce 0x7e00 (a QNaN). thanks -- PMM
Re: [Qemu-devel] [virtio-dev] [PATCH V2 1/1] virtio-balloon: fix internal stat name array to match Linux kernel
On 2018-05-10 04:18 AM, Stefan Hajnoczi wrote:
On Wed, May 09, 2018 at 03:53:05PM -0400, Thomas Tai wrote:
The Linux kernel commit b4325044 ("virtio_balloon: add array
of stat names") defines an array of stat name strings for consumers
of the virtio interface to use via the virtio_balloon.h header
file, rather than requiring each consumer to define its own. But at
present, the stat names defined in this array by the Linux kernel
do not match up with those defined internally by QEMU.
This patch fixes this inconsistency by importing the header file
from linux and use the new macro to construct the balloon_stat_names
array.
Signed-off-by: Jonathan Helman
Signed-off-by: Thomas Tai
Cc: Rob Gardner
---
docs/virtio-balloon-stats.txt | 2 ++
hw/virtio/virtio-balloon.c | 13 ++---
include/standard-headers/linux/virtio_balloon.h | 19 ++-
3 files changed, 22 insertions(+), 12 deletions(-)
Reviewed-by: Stefan Hajnoczi
Thank you Stefan.
-Thomas
Re: [Qemu-devel] [PATCH] lm32: take BQL before writing IP/IM register
Michael Walle writes:
> Writing to these registers may raise an interrupt request. Actually,
> this prevents the milkymist board from starting.
>
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Michael Walle
Reviewed-by: Alex Bennée
> ---
> target/lm32/op_helper.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/lm32/op_helper.c b/target/lm32/op_helper.c
> index 30f670eee8..a766a1ece4 100644
> --- a/target/lm32/op_helper.c
> +++ b/target/lm32/op_helper.c
> @@ -102,12 +102,16 @@ void HELPER(wcsr_dc)(CPULM32State *env, uint32_t dc)
>
> void HELPER(wcsr_im)(CPULM32State *env, uint32_t im)
> {
> +qemu_mutex_lock_iothread();
> lm32_pic_set_im(env->pic_state, im);
> +qemu_mutex_unlock_iothread();
> }
>
> void HELPER(wcsr_ip)(CPULM32State *env, uint32_t im)
> {
> +qemu_mutex_lock_iothread();
> lm32_pic_set_ip(env->pic_state, im);
> +qemu_mutex_unlock_iothread();
> }
>
> void HELPER(wcsr_jtx)(CPULM32State *env, uint32_t jtx)
--
Alex Bennée
[Qemu-devel] [PATCH] tcg: Optionally log FPU state in TCG -d cpu logging
Usually the logging of the CPU state produced by -d cpu is sufficient
to diagnose problems, but sometimes you want to see the state of
the floating point registers as well. We don't want to enable that
by default as it adds a lot of extra data to the log; instead,
allow it to be optionally enabled via -d fpu.
Signed-off-by: Peter Maydell
---
I've found this helpful while tracking down fp-emulation related bugs.
include/qemu/log.h | 1 +
accel/tcg/cpu-exec.c | 9 ++---
util/log.c | 2 ++
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/qemu/log.h b/include/qemu/log.h
index ff92a8b86a..b097a6cae1 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -44,6 +44,7 @@ static inline bool qemu_log_separate(void)
#define CPU_LOG_PAGE (1 << 14)
/* LOG_TRACE (1 << 15) is defined in log-for-trace.h */
#define CPU_LOG_TB_OP_IND (1 << 16)
+#define CPU_LOG_TB_FPU (1 << 17)
/* Lock output for a series of related logs. Since this is not needed
* for a single qemu_log / qemu_log_mask / qemu_log_mask_and_addr, we
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 81153e7a13..0b154cc678 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -156,11 +156,14 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu,
TranslationBlock *itb)
if (qemu_loglevel_mask(CPU_LOG_TB_CPU)
&& qemu_log_in_addr_range(itb->pc)) {
qemu_log_lock();
+int flags = 0;
+if (qemu_loglevel_mask(CPU_LOG_TB_FPU)) {
+flags |= CPU_DUMP_FPU;
+}
#if defined(TARGET_I386)
-log_cpu_state(cpu, CPU_DUMP_CCOP);
-#else
-log_cpu_state(cpu, 0);
+flags |= CPU_DUMP_CCOP;
#endif
+log_cpu_state(cpu, flags);
qemu_log_unlock();
}
#endif /* DEBUG_DISAS */
diff --git a/util/log.c b/util/log.c
index 96f30dd21a..c0dbbd4700 100644
--- a/util/log.c
+++ b/util/log.c
@@ -256,6 +256,8 @@ const QEMULogItem qemu_log_items[] = {
"show trace before each executed TB (lots of logs)" },
{ CPU_LOG_TB_CPU, "cpu",
"show CPU registers before entering a TB (lots of logs)" },
+{ CPU_LOG_TB_FPU, "fpu",
+ "include FPU registers in the 'cpu' logging" },
{ CPU_LOG_MMU, "mmu",
"log MMU-related activities" },
{ CPU_LOG_PCALL, "pcall",
--
2.17.0
[Qemu-devel] Question about QEMU
Hi, This is Wonsang Ryu. I'm developer working in South Korea. Do you have forum or community for any developer? I want share my issues with other developers. I have some question about QEMU. I am porting an embedded OS for emulator to QEMU. My environment is Host OS : Ubuntu 14.04 64bit Guest OS : x86 Embedded Linux OS (like as open embedded) Guest UI Options : using SDL2, virglrenderer Basic running is ok. I have a problem about screen resolution. Recommended resolution of my guest OS is 1920x1080. If a QEMU window size is 1920x1080, it is clear. But, if I change smaller of a QEMU window size, it does not display clearly.(ex: 1280x720) In this case, QEMU windows size in host is 1280x720, and guest screen resolution is 1920x1080. For example, it is similar with same resolution support of different monitor size. I think, it need to enable the MultiSampling(or anti-aliasing) of OpenGL. Does not support MultiSampleing in QEMU yet? Or, is it related with virglrenderer? I found, QEMU does not use MULTISAMPLING option in "sdl2-gl.c". And, "nr_samples" is always zero in "virtio-gpu-3d.c". I think, it need to apply multisampling to texture received from virglrenderer. How can I enable a multisampling option in QEMU? I want to hold the resolution of the guest OS and scale the QEMU windows in the host OS. When scale QEMU windows, I want display is more clear even though small. Is it possible? Please help me. Thanks.
Re: [Qemu-devel] [PATCH v3 2/5] fpu/softfloat: re-factor float to float conversions
Peter Maydell writes:
> On 10 May 2018 at 10:42, Alex Bennée wrote:
>> This allows us to delete a lot of additional boilerplate code which is
>> no longer needed. Currently the ieee flag is ignored (everything is
>> assumed to be ieee). Handling for ARM AHP will be in the next patch.
>>
>> Signed-off-by: Alex Bennée
>> Reviewed-by: Richard Henderson
>>
>> ---
>> v2
>> - pass FloatFmt to float_to_float instead of sizes
>> - split AHP handling to another patch
>> - use rth's suggested re-packing (+ setting .exp)
>> v3
>> - also rm extractFloat16Sign
>> ---
>> fpu/softfloat-specialize.h | 40
>> fpu/softfloat.c| 452 +++--
>> include/fpu/softfloat.h| 8 +-
>> 3 files changed, 88 insertions(+), 412 deletions(-)
>
> This introduces a regression where we don't get tininess-before-rounding
> for double/single to halfprec conversions. This is because we're
> now using the fp_status_f16 status field, and it has not had
> the detect_tininess setting initialized. This fixes it:
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index d175c5e94f..7939c6b8ae 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -324,6 +324,8 @@ static void arm_cpu_reset(CPUState *s)
>&env->vfp.fp_status);
> set_float_detect_tininess(float_tininess_before_rounding,
>&env->vfp.standard_fp_status);
> +set_float_detect_tininess(float_tininess_before_rounding,
> + &env->vfp.fp_status_f16);
I'm now wondering if I should have tried harder to rationalise the
various float_status structures we've ended up with.
> #ifndef CONFIG_USER_ONLY
> if (kvm_enabled()) {
> kvm_arm_reset_vcpu(cpu);
>
> (You can see this if you try something like fcvt h1, d0 where
> d0 == 0x3f0f___ -- we get the right answer of 0x0400
> but fail to set Underflow as well as Inexact.)
>
> thanks
> -- PMM
--
Alex Bennée
Re: [Qemu-devel] [PATCH v1 0/8] MemoryDevice: introduce and use ResourceHandler
On Wed, 9 May 2018 16:13:14 +0200
David Hildenbrand wrote:
> On 03.05.2018 17:49, David Hildenbrand wrote:
> > Hotplug handlers usually have the following tasks:
> > 1. Allocate some resources for a new device
> > 2. Make the new device visible for the guest
> > 3. Notify the guest about the new device
> >
> > Hotplug handlers have right now one limitation: They handle their own
> > context and only care about resources they manage.
> >
> > We can have devices that need certain other resources that are e.g.
> > system resources managed by the machine. We need a clean way to assign
> > these resources (without violating layers as brought up by Igor).
> >
> > One example is virtio-mem/virtio-pmem. Both device types need to be
> > assigned some region in guest physical address space. This device memory
> > belongs to the machine and is managed by it. However, virito devices are
> > hotplugged using the hotplug handler their proxy device implements. So we
> > could trigger e.g. a PCI hotplug handler for virtio-pci or a CSS/CCW
> > hotplug handler for virtio-ccw. But definetly not the machine.
> >
> > So let's generalize the task of "assigning" resources and use it directly
> > for memory devices. We now have a clean way to support any kind of memory
> > device - independent of the underlying device type. Right now, only one
> > resource handler per device can be supported (in addition to the existing
> > hotplug handler).
> >
> > You can find more details in patch nr 2.
> >
> > This work is based on the already queued patch series
> > "[PATCH v4 00/11] pc-dimm: factor out MemoryDevice"
> >
> > David Hildenbrand (8):
> > memory-device: always compile support for memory devices for SOFTMMU
> > qdev: introduce ResourceHandler as a first-stage hotplug handler
> > machine: provide default resource handler
> > memory-device: new functions to handle resource assignment
> > pc-dimm: implement new memory device functions
> > machine: introduce enforce_memory_device_align() and add it for pc
> > memory-device: factor out pre-assign into default resource handler
> > memory-device: factor out (un)assign into default resource handler
> >
> > hw/Makefile.objs | 2 +-
> > hw/core/Makefile.objs | 1 +
> > hw/core/machine.c | 70 +++
> > hw/core/qdev.c | 41 +-
> > hw/core/resource-handler.c | 57 +++
> > hw/i386/pc.c | 31 ++-
> > hw/mem/Makefile.objs | 2 +-
> > hw/mem/memory-device.c | 122
> > +
> > hw/mem/pc-dimm.c | 53 --
> > hw/mem/trace-events| 4 +-
> > hw/ppc/spapr.c | 5 +-
> > include/hw/boards.h| 17 ++
> > include/hw/mem/memory-device.h | 17 --
> > include/hw/mem/pc-dimm.h | 3 +-
> > include/hw/resource-handler.h | 46
> > stubs/Makefile.objs| 1 -
> > stubs/qmp_memory_device.c | 13 -
> > 17 files changed, 364 insertions(+), 121 deletions(-)
> > create mode 100644 hw/core/resource-handler.c
> > create mode 100644 include/hw/resource-handler.h
> > delete mode 100644 stubs/qmp_memory_device.c
> >
>
> If there are no further comments, I'll send a v2 by the end of this
> week. Thanks!
I couldn't convince myself that ResourceHandler is really necessary.
My main gripe with it, is that it imposes specific ordering wrt hotplug
handler that resources will be touched. Other issue is that it looks
a bit over-engineered with a lot of code fragmentation. Hence,
I'd suggest use simple hotplug handler chaining instead,
which should take care of wiring up virtio-mem/virtio-pmem,
keeping code compact at the same time.
Could you try something along these lines (I'll post a patch to
override default hotplug handler as reply here for you to pick up,
that should make following work):
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 100dfdc..c400c0c 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -393,12 +393,30 @@ static void s390_machine_reset(void)
s390_ipl_prepare_cpu(ipl_cpu);
s390_cpu_set_state(S390_CPU_STATE_OPERATING, ipl_cpu);
}
+static void s390_machine_device_pre_plug(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_CCW)) {
+/* do checks && set default values if it weren't set by user */
+/* possibly pass to device's bus pre_plug handler if need */
+}
+}
static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
s390_cpu_plug(hotplug_dev, dev, errp);
+} if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_C
[Qemu-devel] [PATCH] qdev: let machine hotplug handler to override bus hotplug handler
it will allow to return another hotplug handler than the default
one for a specific bus based device type. Which is needed to handle
non trivial plug/unplug sequences that need the access to resources
configured outside of bus where device is attached.
That will allow for returned hotplug handler to orchestrate wiring
in arbitrary order, by chaining other hotplug handlers when
it's needed.
PS:
It could be used for hybrid virtio-mem and virtio-pmem devices
where it will return machine as hotplug handler which will do
necessary wiring at machine level and then pass control down
the chain to bus specific hotplug handler.
Example of top level hotplug handler override and custom plug sequence:
some_machine_get_hotplug_handler(machine){
if (object_dynamic_cast(OBJECT(dev), TYPE_SOME_BUS_DEVICE)) {
return HOTPLUG_HANDLER(machine);
}
return NULL;
}
some_machine_device_plug(hotplug_dev, dev) {
if (object_dynamic_cast(OBJECT(dev), TYPE_SOME_BUS_DEVICE)) {
/* do machine specific initialization */
some_machine_init_special_device(dev)
/* pass control to bus specific handler */
hotplug_handler_plug(dev->parent_bus->hotplug_handler, dev)
}
}
Signed-off-by: Igor Mammedov
---
include/hw/qdev-core.h | 11 +++
hw/core/qdev.c | 6 ++
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 9453588..e6a8eca 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -286,6 +286,17 @@ void qdev_init_nofail(DeviceState *dev);
void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
int required_for_version);
HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev);
+/**
+ * qdev_get_hotplug_handler: Get handler responsible for device wiring
+ *
+ * Find HOTPLUG_HANDLER for @dev that provides [pre|un]plug callbacks for it.
+ *
+ * Note: in case @dev has a parent bus, it will be returned as handler unless
+ * machine handler overrides it.
+ *
+ * Returns: pointer to object that implements TYPE_HOTPLUG_HANDLER interface
+ * or NULL if there aren't any.
+ */
HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
void qdev_unplug(DeviceState *dev, Error **errp);
void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f6f9247..885286f 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -261,12 +261,10 @@ HotplugHandler
*qdev_get_machine_hotplug_handler(DeviceState *dev)
HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
{
-HotplugHandler *hotplug_ctrl;
+HotplugHandler *hotplug_ctrl = qdev_get_machine_hotplug_handler(dev);
-if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
+if (hotplug_ctrl == NULL && dev->parent_bus) {
hotplug_ctrl = dev->parent_bus->hotplug_handler;
-} else {
-hotplug_ctrl = qdev_get_machine_hotplug_handler(dev);
}
return hotplug_ctrl;
}
--
2.7.4
Re: [Qemu-devel] [PATCH 1/3] nvdimm: fix typo in label-size definition
On Fri, 27 Apr 2018 15:53:12 -0600
Ross Zwisler wrote:
> Signed-off-by: Ross Zwisler
> Fixes: commit da6789c27c2e ("nvdimm: add a macro for property "label-size"")
> Cc: Haozhong Zhang
> Cc: Michael S. Tsirkin
> Cc: Stefan Hajnoczi
Reviewed-by: Igor Mammedov
> ---
> hw/mem/nvdimm.c | 2 +-
> include/hw/mem/nvdimm.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> index acb656b672..4087aca25e 100644
> --- a/hw/mem/nvdimm.c
> +++ b/hw/mem/nvdimm.c
> @@ -89,7 +89,7 @@ static void nvdimm_set_unarmed(Object *obj, bool value,
> Error **errp)
>
> static void nvdimm_init(Object *obj)
> {
> -object_property_add(obj, NVDIMM_LABLE_SIZE_PROP, "int",
> +object_property_add(obj, NVDIMM_LABEL_SIZE_PROP, "int",
> nvdimm_get_label_size, nvdimm_set_label_size, NULL,
> NULL, NULL);
> object_property_add_bool(obj, NVDIMM_UNARMED_PROP,
> diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
> index 7fd87c4e1c..74c60332e1 100644
> --- a/include/hw/mem/nvdimm.h
> +++ b/include/hw/mem/nvdimm.h
> @@ -48,7 +48,7 @@
> #define NVDIMM_GET_CLASS(obj) OBJECT_GET_CLASS(NVDIMMClass, (obj), \
> TYPE_NVDIMM)
>
> -#define NVDIMM_LABLE_SIZE_PROP "label-size"
> +#define NVDIMM_LABEL_SIZE_PROP "label-size"
> #define NVDIMM_UNARMED_PROP"unarmed"
>
> struct NVDIMMDevice {
Re: [Qemu-devel] [PATCH v5 4/6] s390x/vfio: ap: Introduce VFIO AP device
On 05/09/2018 10:28 AM, Halil Pasic wrote:
On 05/08/2018 02:25 PM, Tony Krowiak wrote:
Introduces a VFIO based AP device. The device is defined via
the QEMU command line by specifying:
-device vfio-ap,sysfsdev=
There may be only one vfio-ap device configured for a guest.
The mediated matrix device is created by the VFIO AP device
[..]
+ * directory.
+ */
+
+#include
+#include
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "hw/vfio/vfio.h"
+#include "hw/vfio/vfio-common.h"
+#include "hw/s390x/ap-device.h"
+#include "qemu/error-report.h"
+#include "qemu/queue.h"
+#include "qemu/option.h"
+#include "qemu/config-file.h"
+#include "cpu.h"
+#include "kvm_s390x.h"
+#include "sysemu/sysemu.h"
+
+#define VFIO_AP_DEVICE_TYPE "vfio-ap"
+
+typedef struct VFIOAPDevice {
+APDevice apdev;
+VFIODevice vdev;
+QTAILQ_ENTRY(VFIOAPDevice) sibling;
+} VFIOAPDevice;
+
+VFIOAPDevice *vfio_apdev;
+
+static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
+{
+vdev->needs_reset = false;
+}
+
+/*
+ * We don't need vfio_hot_reset_multi and vfio_eoi operations for
+ * vfio-ap-matrix device now.
+ */
+struct VFIODeviceOps vfio_ap_ops = {
+.vfio_compute_needs_reset = vfio_ap_compute_needs_reset,
+};
+
I'm not familiar with the vfio infrastructure, but AFAIR I
haven't seen any substantial reset handling (QEMU or kernel).
Did I miss something?
No, you didn't miss anything, there is no reset handling.
If I did not. I think this is a big problem. We need to at least
zeroize the queues (e.g. on system reset) to avoid leaking
sensitive information. Without this, there is no sane way to use
ap-passthrough. Or am I wrong?
I do not have a definitive answer, I will have to look into it.
I'm thinking that since we are using ap-passthrough, the AP bus
running on the guest would be responsible for handling reset possibly
by resetting or zeroizing its queues. I'll get back to you on this.
Regards,
Halil
Re: [Qemu-devel] [PATCH 01/13] qapi: Add default-variant for flat unions
On 05/09/2018 11:55 AM, Max Reitz wrote:
This patch allows specifying a discriminator that is an optional member
of the base struct. In such a case, a default value must be provided
that is used when no value is given.
Signed-off-by: Max Reitz
---
qapi/introspect.json | 8 ++
scripts/qapi/common.py | 57 ++
scripts/qapi/doc.py| 8 --
scripts/qapi/introspect.py | 10 +---
scripts/qapi/visit.py | 33 ++--
tests/qapi-schema/test-qapi.py | 2 ++
6 files changed, 101 insertions(+), 17 deletions(-)
I've been threatening that we might need this for some time, so I'm glad
to see it being implemented. We'll see if the tests in 2 and 3 cover
the code added here.
diff --git a/qapi/introspect.json b/qapi/introspect.json
index c7f67b7d78..2d7b1e3745 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -168,6 +168,13 @@
# @tag: the name of the member serving as type tag.
# An element of @members with this name must exist.
#
+# @default-variant: if the @tag element of @members is optional, this
+# is the default value for choosing a variant. Its
+# value must be a valid value for @tag.
Perhaps s/must/will/ as this struct is used for output (and therefore we
always meet the condition, rather than the user having to do something
correctly).
Nice that you remembered introspection.
+# Present exactly when @tag is present and the
+# associated element of @members is optional.
+# (Since: 2.13)
+#
# @variants: variant members, i.e. additional members that
#depend on the type tag's value. Present exactly when
#@tag is present. The variants are in no particular order,
@@ -181,6 +188,7 @@
{ 'struct': 'SchemaInfoObject',
'data': { 'members': [ 'SchemaInfoObjectMember' ],
'*tag': 'str',
+'*default-variant': 'str',
'*variants': [ 'SchemaInfoObjectVariant' ] } }
##
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index a032cec375..fbf0244f73 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -721,6 +721,7 @@ def check_union(expr, info):
name = expr['union']
base = expr.get('base')
discriminator = expr.get('discriminator')
+default_variant = expr.get('default-variant')
members = expr['data']
# Two types of unions, determined by discriminator.
@@ -745,16 +746,37 @@ def check_union(expr, info):
base_members = find_base_members(base)
assert base_members is not None
-# The value of member 'discriminator' must name a non-optional
-# member of the base struct.
+# The value of member 'discriminator' must name a member of
+# the base struct.
check_name(info, "Discriminator of flat union '%s'" % name,
discriminator)
-discriminator_type = base_members.get(discriminator)
-if not discriminator_type:
-raise QAPISemError(info,
- "Discriminator '%s' is not a member of base "
- "struct '%s'"
- % (discriminator, base))
+if default_variant is None:
+discriminator_type = base_members.get(discriminator)
+if not discriminator_type:
+if base_members.get('*' + discriminator) is None:
Unrelated to your patch, but this reminds me - we already had a question
on list about whether we should allow ANY member of the base struct,
rather than the members directly declared in the struct. (The use case
was that if you have a grandparent struct, then an intermediate base
struct, the code as written does not permit the discriminator to come
from the grandparent struct, which is awkward - but if I also recall
correctly, the question came up in relation to the query-cpus-fast
command, where we ended up not refactoring things after all due to
deprecation of query-cpus).
+raise QAPISemError(info,
+ "Discriminator '%s' is not a member of "
+ "base struct '%s'"
+ % (discriminator, base))
+else:
+raise QAPISemError(info,
+ "Default variant must be specified for "
+ "optional discriminator '%s'"
+ % discriminator)
+else:
+discriminator_type = base_members.get('*' + discriminator)
+if not discriminator_type:
+if base_members.get(discriminator) is None:
+raise QAPISemError(info,
+ "Discriminator '%s' is not a member of "
+
Re: [Qemu-devel] [PATCH v6 0/3] target/arm: Add a dynamic XML-description of the cp-registers to GDB
Abdallah Bouassida writes: > The previous version: > http://patchwork.ozlabs.org/project/qemu-devel/list/?series=33714 > > Abdallah Bouassida (3): > target/arm: Add "ARM_CP_NO_GDB" as a new bit field for ARMCPRegInfo > type > target/arm: Add "_S" suffix to the secure version of a sysreg > target/arm: Add the XML dynamic generation > So I got a fixed up gdb and I was testing the reading of the virtual counter: => 0xff800854a118 : mrs x0, cntvct_el0 0xff800854a11c : b 0xff800854a148 0xff800854a120 : adrpx0, 0xff800896a000 0xff800854a124 : add x0, x0, #0x5a0 0xff800854a128 :mrs x1, tpidr_el1 p/x $x0 $6 = 0xff800854a108 p/x $cntvct_el0 $7 = 0x0 stepi 0xff800854a11c 160 return arch_timer_reg_read_stable(cntvct_el0); => 0xff800854a11c : b 0xff800854a148 0xff800854a120 : adrpx0, 0xff800896a000 0xff800854a124 : add x0, x0, #0x5a0 p/x $x0 $8 = 0x7a5b32b p/x $cntvct_el0 $9 = 0x0 So I'm wondering why there is a disparity here? -- Alex Bennée
Re: [Qemu-devel] [PATCH 02/13] docs/qapi: Document optional discriminators
On 05/09/2018 11:55 AM, Max Reitz wrote:
Signed-off-by: Max Reitz
---
docs/devel/qapi-code-gen.txt | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
I might have squashed this in with patch 1, but I can live with it separate.
@@ -502,6 +502,23 @@ the enum). In the resulting generated C data types, a
flat union is
represented as a struct with the base members included directly, and
then a union of structures for each branch of the struct.
+If the discriminator points to an optional member of the base struct,
+its default value must be specified as a 'default-variant'. In the
+following example, the above BlockDriver struct is changed so it
+defaults to the 'file' driver if that field is omitted on the wire:
+
+ { 'union': 'BlockdevOptions',
+ 'base': { '*driver': 'BlockdevDriver', '*read-only': 'bool' },
+ 'discriminator': 'driver',
+ 'default-variant': 'file',
+ 'data': { 'file': 'BlockdevOptionsFile',
+ 'qcow2': 'BlockdevOptionsQcow2' } }
+
+Now the 'file' JSON object can be abbreviated to:
+
+ { "read-only": "true",
+ "filename": "/some/place/my-image" }
+
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 01/13] qapi: Add default-variant for flat unions
On 05/10/2018 08:12 AM, Eric Blake wrote:
Oh, I just had a thought:
+++ b/scripts/qapi/visit.py
@@ -40,10 +40,20 @@ def gen_visit_object_members(name, base, members,
if variants:
+ if variants.default_tag_value is None:
+ ret += mcgen('''
+ %(c_name)s = obj->%(c_name)s;
+''',
+ c_name=c_name(variants.tag_member.name))
+ else:
+ ret += mcgen('''
+ if (obj->has_%(c_name)s) {
+ %(c_name)s = obj->%(c_name)s;
+ } else {
+ %(c_name)s = %(enum_const)s;
In this branch of code, is it worth also generating:
%has_(c_name)s = true;
That way, the rest of the C code does not have to check
has_discriminator, because the process of assigning the default will
ensure that has_discriminator is always true later on. It does have the
effect that output would never omit the discriminator - but that might
be a good thing: if we ever have an output union that used to have a
mandatory discriminator and want to now make it optional, we don't want
to break older clients that expected the discriminator to be present.
It does obscure whether input relied on the default, but I don't think
that hurts.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v1 0/8] MemoryDevice: introduce and use ResourceHandler
On 10.05.2018 15:02, Igor Mammedov wrote:
> On Wed, 9 May 2018 16:13:14 +0200
> David Hildenbrand wrote:
>
>> On 03.05.2018 17:49, David Hildenbrand wrote:
>>> Hotplug handlers usually have the following tasks:
>>> 1. Allocate some resources for a new device
>>> 2. Make the new device visible for the guest
>>> 3. Notify the guest about the new device
>>>
>>> Hotplug handlers have right now one limitation: They handle their own
>>> context and only care about resources they manage.
>>>
>>> We can have devices that need certain other resources that are e.g.
>>> system resources managed by the machine. We need a clean way to assign
>>> these resources (without violating layers as brought up by Igor).
>>>
>>> One example is virtio-mem/virtio-pmem. Both device types need to be
>>> assigned some region in guest physical address space. This device memory
>>> belongs to the machine and is managed by it. However, virito devices are
>>> hotplugged using the hotplug handler their proxy device implements. So we
>>> could trigger e.g. a PCI hotplug handler for virtio-pci or a CSS/CCW
>>> hotplug handler for virtio-ccw. But definetly not the machine.
>>>
>>> So let's generalize the task of "assigning" resources and use it directly
>>> for memory devices. We now have a clean way to support any kind of memory
>>> device - independent of the underlying device type. Right now, only one
>>> resource handler per device can be supported (in addition to the existing
>>> hotplug handler).
>>>
>>> You can find more details in patch nr 2.
>>>
>>> This work is based on the already queued patch series
>>> "[PATCH v4 00/11] pc-dimm: factor out MemoryDevice"
>>>
>>> David Hildenbrand (8):
>>> memory-device: always compile support for memory devices for SOFTMMU
>>> qdev: introduce ResourceHandler as a first-stage hotplug handler
>>> machine: provide default resource handler
>>> memory-device: new functions to handle resource assignment
>>> pc-dimm: implement new memory device functions
>>> machine: introduce enforce_memory_device_align() and add it for pc
>>> memory-device: factor out pre-assign into default resource handler
>>> memory-device: factor out (un)assign into default resource handler
>>>
>>> hw/Makefile.objs | 2 +-
>>> hw/core/Makefile.objs | 1 +
>>> hw/core/machine.c | 70 +++
>>> hw/core/qdev.c | 41 +-
>>> hw/core/resource-handler.c | 57 +++
>>> hw/i386/pc.c | 31 ++-
>>> hw/mem/Makefile.objs | 2 +-
>>> hw/mem/memory-device.c | 122
>>> +
>>> hw/mem/pc-dimm.c | 53 --
>>> hw/mem/trace-events| 4 +-
>>> hw/ppc/spapr.c | 5 +-
>>> include/hw/boards.h| 17 ++
>>> include/hw/mem/memory-device.h | 17 --
>>> include/hw/mem/pc-dimm.h | 3 +-
>>> include/hw/resource-handler.h | 46
>>> stubs/Makefile.objs| 1 -
>>> stubs/qmp_memory_device.c | 13 -
>>> 17 files changed, 364 insertions(+), 121 deletions(-)
>>> create mode 100644 hw/core/resource-handler.c
>>> create mode 100644 include/hw/resource-handler.h
>>> delete mode 100644 stubs/qmp_memory_device.c
>>>
>>
>> If there are no further comments, I'll send a v2 by the end of this
>> week. Thanks!
> I couldn't convince myself that ResourceHandler is really necessary.
> My main gripe with it, is that it imposes specific ordering wrt hotplug
> handler that resources will be touched. Other issue is that it looks
> a bit over-engineered with a lot of code fragmentation. Hence,
>
> I'd suggest use simple hotplug handler chaining instead,
> which should take care of wiring up virtio-mem/virtio-pmem,
> keeping code compact at the same time.
I'll have a look tomorrow or next friday if that could work - not sure
yet about unplug vs. unplug requests. Unplug requests might be tricky.
Would be nice if it would work. Thanks!
>
> Could you try something along these lines (I'll post a patch to
> override default hotplug handler as reply here for you to pick up,
> that should make following work):
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 100dfdc..c400c0c 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -393,12 +393,30 @@ static void s390_machine_reset(void)
> s390_ipl_prepare_cpu(ipl_cpu);
> s390_cpu_set_state(S390_CPU_STATE_OPERATING, ipl_cpu);
> }
> +static void s390_machine_device_pre_plug(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> +{
> +if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_CCW)) {
> +/* do checks && set default values if it weren't set by user */
> +/* possibly pass to device's bus pre_plug handler if need */
> +}
> +}
>
> static void s3
Re: [Qemu-devel] [PATCH v2 1/2] qemu-iotests: reduce chance of races in 185
On 05/10/2018 05:05 AM, Stefan Hajnoczi wrote: Add sleep 0.5 to reduce the chance of races. This is not a real fix but appears to reduce spurious failures in practice. Cc: Vladimir Sementsov-Ogievskiy Signed-off-by: Stefan Hajnoczi --- tests/qemu-iotests/185 | 12 1 file changed, 12 insertions(+) I'm not opposed to this patch, but is there any way to write the test to take both events in either order, without logging the events as they arrive, but instead summarizing in a deterministic order which events were received after the fact? That way, no matter which way the race is won, we merely log that we got two expected events, and could avoid the extra sleep. I don't think there is a practical way of doing that without big changes to the test. It could be rewritten in Python to make filtering the QMP events easier. Hiding the race doesn't solve the deeper problem though: the test case doesn't exercise the same code path each time. The test should really cover all cancellation points in the block job lifecycle instead of just one at random. If we solve this problem then we don't need to filter the QMP event sequence. Maybe it can be done with blkdebug. If not then maybe a blockjobdbg interface is necessary to perform deterministic tests (eliminating the need for the ratelimiting trick used by this test!). So trying to restate your question - can blkdebug be used to pause I/O, or does it just cause an error? If the rate limiting is in effect, then we expect that the job will only write to the first half of a destination, so a blkdebug injected error for writes to the second half would either not trigger (the normal cancel won the race) or does trigger (the job advanced before the normal cancel, but blkdebug's injected error also serves as a means of cancelling the job, so while we'd have to filter things, we at least have a deterministic way of ending the job before it runs to completion). Except that I'm writing that without even re-reading the test in question to see how it would all fit in. It may or may not be worth pursuing. Please share ideas, but I think this is a long-term item that shouldn't block this series. I agree that any further improvements don't need to hold up this patch from making things at least more reliable, even if not perfect. So: Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v2 2/3] checkpatch: generalize xen handle matching in the list of types
On 05/10/2018 04:15 AM, Paul Durrant wrote:
All the xen stable APIs define handle types of the form:
_handle
and some define additional handle types of the form:
__handle
Maybe worth mentioning that always has a 'xen' prefix,
and/or spelling it:
xen_handle
xen__handle
Examples of these are xenforeignmemory_handle and
xenforeignmemory_resource_handle.
Both of these types will be misparsed by checkpatch if they appear as the
first token in a line since, as types defined by an external library, they
do not conform to the QEMU CODING_STYLE, which suggests CamelCase.
A previous patch (5ac067a24a8) added xendevicemodel_handle to the list
of types. This patch changes that to xen\w+_handle such that it will
match all Xen stable API handles of the forms detailed above.
Nice use of a regex.
Signed-off-by: Paul Durrant
---
Cc: Eric Blake
Cc: Paolo Bonzini
Cc: Daniel P. Berrange
v2:
- New in this version
Reviewed-by: Eric Blake
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5b8735defb..98ed799f29 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -266,7 +266,7 @@ our @typeList = (
qr{target_(?:u)?long},
qr{hwaddr},
qr{xml${Ident}},
- qr{xendevicemodel_handle},
+ qr{xen\w+_handle},
);
# This can be modified by sub possible. Since it can be empty, be careful
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 3/3] nvdimm: platform capabilities command line option
On Fri, 27 Apr 2018 15:53:14 -0600
Ross Zwisler wrote:
> Add a device command line option to allow the user to control the Platform
> Capabilities Structure in the virtualized NFIT.
>
> Signed-off-by: Ross Zwisler
> ---
> docs/nvdimm.txt | 22 ++
> hw/acpi/nvdimm.c| 29 +
> hw/mem/nvdimm.c | 28
> include/hw/mem/nvdimm.h | 6 ++
> 4 files changed, 81 insertions(+), 4 deletions(-)
>
> diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
> index e903d8bb09..13a2c15b70 100644
> --- a/docs/nvdimm.txt
> +++ b/docs/nvdimm.txt
> @@ -153,3 +153,25 @@ guest NVDIMM region mapping structure. This unarmed
> flag indicates
> guest software that this vNVDIMM device contains a region that cannot
> accept persistent writes. In result, for example, the guest Linux
> NVDIMM driver, marks such vNVDIMM device as read-only.
> +
> +Platform Capabilities
> +-
> +
> +ACPI 6.2 Errata A added support for a new Platform Capabilities Structure
> +which allows the platform to communicate what features it supports related to
> +NVDIMM data durability. Users can provide a capabilities value to a guest
> via
> +the optional "cap" device command line option:
> +
> + -device nvdimm,id=nvdimm1,memdev=mem1,cap=3
> +
> +As of ACPI 6.2 Errata A, the following values are valid for the bottom two
> +bits:
> +
> +2 - Memory Controller Flush to NVDIMM Durability on Power Loss Capable.
> +3 - CPU Cache Flush to NVDIMM Durability on Power Loss Capable.
> +
> +For a complete list of the flags available please consult the ACPI spec.
> +
> +These platform capabilities apply to the entire virtual platform, so it is
> +recommended that only one "cap" device command option be given per virtual
> +machine. This value will apply to all NVDIMMs in the virtual platform.
This looks like it should be machine property instead of per device one,
you can get rid of static variable and mismatch check and a weird nvdimm CLI
option that implies that the option is per device.
Also an extra patch to for make check that will test setting 'cap'
would be nice (an extra testcase in tests/bios-tables-test.c)
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 859b390e07..375237c96c 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -370,7 +370,7 @@ static void nvdimm_build_structure_dcr(GArray
> *structures, DeviceState *dev)
> * ACPI 6.2 Errata A: 5.2.25.9 NVDIMM Platform Capabilities Structure
> */
> static void
> -nvdimm_build_structure_caps(GArray *structures)
> +nvdimm_build_structure_caps(GArray *structures, uint32_t capabilities)
> {
> NvdimmNfitPlatformCaps *nfit_caps;
>
> @@ -378,13 +378,31 @@ nvdimm_build_structure_caps(GArray *structures)
>
> nfit_caps->type = cpu_to_le16(7 /* NVDIMM Platform Capabilities */);
> nfit_caps->length = cpu_to_le16(sizeof(*nfit_caps));
> -nfit_caps->highest_cap = 1;
> -nfit_caps->capabilities = cpu_to_le32(2 /* memory controller */);
> +nfit_caps->highest_cap = 2;
> +nfit_caps->capabilities = cpu_to_le32(capabilities);
> }
> +
> +static uint32_t nvdimm_get_capabilities(DeviceState *dev)
> +{
> +static uint32_t capabilities = 0;
> +uint32_t this_cap = object_property_get_uint(OBJECT(dev),
> +NVDIMM_CAPABILITIES_PROP, NULL);
> +
> +if (this_cap && !capabilities)
> +capabilities = this_cap;
> +
> +if (this_cap && this_cap != capabilities)
> +nvdimm_debug("WARNING: multiple capabilities (%d and %d) defined\n",
> +this_cap, capabilities);
> +
> +return capabilities;
> +}
> +
> static GArray *nvdimm_build_device_structure(void)
> {
> GSList *device_list = nvdimm_get_device_list();
> GArray *structures = g_array_new(false, true /* clear */, 1);
> +uint32_t capabilities = 0;
>
> for (; device_list; device_list = device_list->next) {
> DeviceState *dev = device_list->data;
> @@ -400,10 +418,13 @@ static GArray *nvdimm_build_device_structure(void)
>
> /* build NVDIMM Control Region Structure. */
> nvdimm_build_structure_dcr(structures, dev);
> +
> +capabilities = nvdimm_get_capabilities(dev);
> }
> g_slist_free(device_list);
>
> -nvdimm_build_structure_caps(structures);
> +if (capabilities)
> +nvdimm_build_structure_caps(structures, capabilities);
>
> return structures;
> }
> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> index 4087aca25e..923364e190 100644
> --- a/hw/mem/nvdimm.c
> +++ b/hw/mem/nvdimm.c
> @@ -87,6 +87,31 @@ static void nvdimm_set_unarmed(Object *obj, bool value,
> Error **errp)
> error_propagate(errp, local_err);
> }
>
> +static void nvdimm_get_capabilities(Object *obj, Visitor *v, const char
> *name,
> + void *opaque, Error **errp)
> +{
> +NVDIMMDevice *nvdimm = NVDIMM(obj);
> +uint32_t value
Re: [Qemu-devel] [PATCH] iscsi: Avoid potential for get_status overflow
On 08/05/2018 23:27, Eric Blake wrote:
> Detected by Coverity: Multiplying two 32-bit int and assigning
> the result to a 64-bit number is a risk of overflow. Prior to
> the conversion to byte-based interfaces, the block layer took
> care of ensuring that a status request never exceeded 2G in
> the driver; but after that conversion, the block layer expects
> drivers to deal with any size request (the driver can always
> truncate the request size back down, as long as it makes
> progress). So, in the off-chance that someone makes a large
> request, we are at the mercy of whether iscsi_get_lba_status_task()
> will cap things to at most INT_MAX / iscsilun->block_size when
> it populates lbasd->num_blocks; since I could not easily audit
> that, it's better to be safe than sorry by just forcing a 64-bit
> multiply.
>
> Fixes: 92809c36
> CC: qemu-sta...@nongnu.org
> Signed-off-by: Eric Blake
> ---
> block/iscsi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 35423ded03b..a6311b9a320 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -732,7 +732,7 @@ retry:
> goto out_unlock;
> }
>
> -*pnum = lbasd->num_blocks * iscsilun->block_size;
> +*pnum = (int64_t) lbasd->num_blocks * iscsilun->block_size;
>
> if (lbasd->provisioning == SCSI_PROVISIONING_TYPE_DEALLOCATED ||
> lbasd->provisioning == SCSI_PROVISIONING_TYPE_ANCHORED) {
>
Queued, thanks.
Paolo
Re: [Qemu-devel] [PATCH v1 0/8] MemoryDevice: introduce and use ResourceHandler
On Thu, 10 May 2018 15:20:55 +0200 David Hildenbrand wrote: > On 10.05.2018 15:02, Igor Mammedov wrote: > > On Wed, 9 May 2018 16:13:14 +0200 > > David Hildenbrand wrote: > > > >> On 03.05.2018 17:49, David Hildenbrand wrote: > >>> Hotplug handlers usually have the following tasks: > >>> 1. Allocate some resources for a new device > >>> 2. Make the new device visible for the guest > >>> 3. Notify the guest about the new device > >>> > >>> Hotplug handlers have right now one limitation: They handle their own > >>> context and only care about resources they manage. > >>> > >>> We can have devices that need certain other resources that are e.g. > >>> system resources managed by the machine. We need a clean way to assign > >>> these resources (without violating layers as brought up by Igor). > >>> > >>> One example is virtio-mem/virtio-pmem. Both device types need to be > >>> assigned some region in guest physical address space. This device memory > >>> belongs to the machine and is managed by it. However, virito devices are > >>> hotplugged using the hotplug handler their proxy device implements. So we > >>> could trigger e.g. a PCI hotplug handler for virtio-pci or a CSS/CCW > >>> hotplug handler for virtio-ccw. But definetly not the machine. > >>> > >>> So let's generalize the task of "assigning" resources and use it directly > >>> for memory devices. We now have a clean way to support any kind of memory > >>> device - independent of the underlying device type. Right now, only one > >>> resource handler per device can be supported (in addition to the existing > >>> hotplug handler). > >>> > >>> You can find more details in patch nr 2. > >>> > >>> This work is based on the already queued patch series > >>> "[PATCH v4 00/11] pc-dimm: factor out MemoryDevice" > >>> > >>> David Hildenbrand (8): > >>> memory-device: always compile support for memory devices for SOFTMMU > >>> qdev: introduce ResourceHandler as a first-stage hotplug handler > >>> machine: provide default resource handler > >>> memory-device: new functions to handle resource assignment > >>> pc-dimm: implement new memory device functions > >>> machine: introduce enforce_memory_device_align() and add it for pc > >>> memory-device: factor out pre-assign into default resource handler > >>> memory-device: factor out (un)assign into default resource handler > >>> > >>> hw/Makefile.objs | 2 +- > >>> hw/core/Makefile.objs | 1 + > >>> hw/core/machine.c | 70 +++ > >>> hw/core/qdev.c | 41 +- > >>> hw/core/resource-handler.c | 57 +++ > >>> hw/i386/pc.c | 31 ++- > >>> hw/mem/Makefile.objs | 2 +- > >>> hw/mem/memory-device.c | 122 > >>> + > >>> hw/mem/pc-dimm.c | 53 -- > >>> hw/mem/trace-events| 4 +- > >>> hw/ppc/spapr.c | 5 +- > >>> include/hw/boards.h| 17 ++ > >>> include/hw/mem/memory-device.h | 17 -- > >>> include/hw/mem/pc-dimm.h | 3 +- > >>> include/hw/resource-handler.h | 46 > >>> stubs/Makefile.objs| 1 - > >>> stubs/qmp_memory_device.c | 13 - > >>> 17 files changed, 364 insertions(+), 121 deletions(-) > >>> create mode 100644 hw/core/resource-handler.c > >>> create mode 100644 include/hw/resource-handler.h > >>> delete mode 100644 stubs/qmp_memory_device.c > >>> > >> > >> If there are no further comments, I'll send a v2 by the end of this > >> week. Thanks! > > I couldn't convince myself that ResourceHandler is really necessary. > > My main gripe with it, is that it imposes specific ordering wrt hotplug > > handler that resources will be touched. Other issue is that it looks > > a bit over-engineered with a lot of code fragmentation. Hence, > > > > I'd suggest use simple hotplug handler chaining instead, > > which should take care of wiring up virtio-mem/virtio-pmem, > > keeping code compact at the same time. > > I'll have a look tomorrow or next friday if that could work - not sure > yet about unplug vs. unplug requests. Unplug requests might be tricky. > Would be nice if it would work. Thanks! If you have issues with it, ping me, Maybe we'd figure out how to make it work together. > > > > > Could you try something along these lines (I'll post a patch to > > override default hotplug handler as reply here for you to pick up, > > that should make following work): > > > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > > index 100dfdc..c400c0c 100644 > > --- a/hw/s390x/s390-virtio-ccw.c > > +++ b/hw/s390x/s390-virtio-ccw.c > > @@ -393,12 +393,30 @@ static void s390_machine_reset(void) > > s390_ipl_prepare_cpu(ipl_cpu); > > s390_cpu_set_state(S390_CPU_STATE_OPERATING, ipl_cpu); > > } > > +static void s390_machine_device_pre_plug(Ho
Re: [Qemu-devel] [PATCH v3 0/5] refactor float-to-float and fix AHP
Peter Maydell writes: > On 10 May 2018 at 10:42, Alex Bennée wrote: >> Hi, >> >> Hi, >> >> I've not included the test case in the series but you can find it in >> my TCG fixup branch: >> >> >> https://github.com/stsquad/qemu/blob/testing/tcg-tests-revival-v4/tests/tcg/arm/fcvt.c >> >> Some of the ARMv7 versions are commented out as they where not >> supported until later revs. I do have a build that includes that but >> unfortunately the Debian compiler it too old to build it. >> >> : patch 0001/fpu softfloat int_to_float ensure r fully initial.patch needs >> review >> : patch 0004/target arm convert conversion helpers to fpst ahp.patch needs >> review >> : patch 0005/target arm squash FZ16 behaviour for conversions.patch needs >> review > > This still seems to regress the NaN conversion case I mentioned > in review of the previous series: > > (3) Here's a NaN case we get wrong now: 64 to IEEE-16 conversion, > input is 0x7ff1 (an SNaN), we produce > 0x7c00 (infinity) but should produce 0x7e00 (a QNaN). Hmm I had added the test case but due to another bug it never actually ran :-/ > > thanks > -- PMM -- Alex Bennée
Re: [Qemu-devel] [PATCH] lm32: take BQL before writing IP/IM register
On 09/05/2018 21:45, Philippe Mathieu-Daudé wrote:
> On 02/01/2018 06:09 AM, Michael Walle wrote:
>>
>> Hi Peter,
>>
>> do you apply this patch? Or do I have to send a pull request?
>
> Cc'ing Paolo.
Please send a pull request.
Paolo
>>
>> -michael
>>
>> Am 2018-01-09 18:01, schrieb Michael Walle:
>>> Writing to these registers may raise an interrupt request. Actually,
>>> this prevents the milkymist board from starting.
>>>
>>> Cc: qemu-sta...@nongnu.org
>>> Signed-off-by: Michael Walle
>
> Tested-by: Philippe Mathieu-Daudé
>
>>> ---
>>> target/lm32/op_helper.c | 4
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/target/lm32/op_helper.c b/target/lm32/op_helper.c
>>> index 30f670eee8..a766a1ece4 100644
>>> --- a/target/lm32/op_helper.c
>>> +++ b/target/lm32/op_helper.c
>>> @@ -102,12 +102,16 @@ void HELPER(wcsr_dc)(CPULM32State *env, uint32_t
>>> dc)
>>>
>>> void HELPER(wcsr_im)(CPULM32State *env, uint32_t im)
>>> {
>>> + qemu_mutex_lock_iothread();
>>> lm32_pic_set_im(env->pic_state, im);
>>> + qemu_mutex_unlock_iothread();
>>> }
>>>
>>> void HELPER(wcsr_ip)(CPULM32State *env, uint32_t im)
>>> {
>>> + qemu_mutex_lock_iothread();
>>> lm32_pic_set_ip(env->pic_state, im);
>>> + qemu_mutex_unlock_iothread();
>>> }
>>>
>>> void HELPER(wcsr_jtx)(CPULM32State *env, uint32_t jtx)
>>
Re: [Qemu-devel] [PATCH 2/3] nvdimm, acpi: add NFIT platform capabilities
On Fri, 27 Apr 2018 15:53:13 -0600
Ross Zwisler wrote:
> Add support for the NFIT Platform Capabilities Structure, newly added in
> ACPI 6.2 Errata A.
Look fine but I'd squash it into the next 3/3 patch.
> Signed-off-by: Ross Zwisler
> ---
> hw/acpi/nvdimm.c | 32
> 1 file changed, 32 insertions(+)
>
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 59d6e4254c..859b390e07 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -169,6 +169,21 @@ struct NvdimmNfitControlRegion {
> } QEMU_PACKED;
> typedef struct NvdimmNfitControlRegion NvdimmNfitControlRegion;
>
> +/*
> + * NVDIMM Platform Capabilities Structure
> + *
> + * Defined in section 5.2.25.9 of ACPI 6.2 Errata A, September 2017
> + */
> +struct NvdimmNfitPlatformCaps {
> +uint16_t type;
> +uint16_t length;
> +uint8_t highest_cap;
> +uint8_t reserved[3];
> +uint32_t capabilities;
> +uint8_t reserved2[4];
> +} QEMU_PACKED;
> +typedef struct NvdimmNfitPlatformCaps NvdimmNfitPlatformCaps;
> +
> /*
> * Module serial number is a unique number for each device. We use the
> * slot id of NVDIMM device to generate this number so that each device
> @@ -351,6 +366,21 @@ static void nvdimm_build_structure_dcr(GArray
> *structures, DeviceState *dev)
> JEDEC Annex L Release 3. */);
> }
>
> +/*
> + * ACPI 6.2 Errata A: 5.2.25.9 NVDIMM Platform Capabilities Structure
> + */
> +static void
> +nvdimm_build_structure_caps(GArray *structures)
> +{
> +NvdimmNfitPlatformCaps *nfit_caps;
> +
> +nfit_caps = acpi_data_push(structures, sizeof(*nfit_caps));
> +
> +nfit_caps->type = cpu_to_le16(7 /* NVDIMM Platform Capabilities */);
> +nfit_caps->length = cpu_to_le16(sizeof(*nfit_caps));
> +nfit_caps->highest_cap = 1;
> +nfit_caps->capabilities = cpu_to_le32(2 /* memory controller */);
> +}
> static GArray *nvdimm_build_device_structure(void)
> {
> GSList *device_list = nvdimm_get_device_list();
> @@ -373,6 +403,8 @@ static GArray *nvdimm_build_device_structure(void)
> }
> g_slist_free(device_list);
>
> +nvdimm_build_structure_caps(structures);
> +
> return structures;
> }
>
Re: [Qemu-devel] [PATCH v2 1/2] qemu-iotests: reduce chance of races in 185
08.05.2018 16:54, Stefan Hajnoczi wrote:
Commit 8565c3ab537e78f3e69977ec2c609dc9417a806e ("qemu-iotests: fix
185") identified a race condition in a sub-test.
Similar issues also affect the other sub-tests. If disk I/O completes
quickly, it races with the QMP 'quit' command. This causes spurious
test failures because QMP events are emitted in an unpredictable order.
Hmm, can you give an example? I'm looking at 8565c3ab537. and
it's not similar.
If disk I/O completes quickly, it will have large final offset, and test
will
fail. And pause of 0.5 will not help.
My case was that quit is handled before the first iteration of mirror.
This test relies on QEMU internals and there is no QMP API for getting
deterministic behavior needed to make this test 100% reliable. At the
same time, the test is useful and it would be a shame to remove it.
Add sleep 0.5 to reduce the chance of races. This is not a real fix but
appears to reduce spurious failures in practice.
Cc: Vladimir Sementsov-Ogievskiy
Signed-off-by: Stefan Hajnoczi
---
tests/qemu-iotests/185 | 12
1 file changed, 12 insertions(+)
diff --git a/tests/qemu-iotests/185 b/tests/qemu-iotests/185
index 298d88d04e..975404c99d 100755
--- a/tests/qemu-iotests/185
+++ b/tests/qemu-iotests/185
@@ -118,6 +118,9 @@ _send_qemu_cmd $h \
'speed': 65536 } }" \
"return"
+# If we don't sleep here 'quit' command races with disk I/O
+sleep 0.5
+
_send_qemu_cmd $h "{ 'execute': 'quit' }" "return"
wait=1 _cleanup_qemu
@@ -137,6 +140,9 @@ _send_qemu_cmd $h \
'speed': 65536 } }" \
"return"
+# If we don't sleep here 'quit' command races with disk I/O
+sleep 0.5
+
_send_qemu_cmd $h "{ 'execute': 'quit' }" "return"
wait=1 _cleanup_qemu
@@ -183,6 +189,9 @@ _send_qemu_cmd $h \
'speed': 65536 } }" \
"return"
+# If we don't sleep here 'quit' command races with disk I/O
+sleep 0.5
+
_send_qemu_cmd $h "{ 'execute': 'quit' }" "return"
wait=1 _cleanup_qemu
@@ -201,6 +210,9 @@ _send_qemu_cmd $h \
'speed': 65536 } }" \
"return"
+# If we don't sleep here 'quit' command races with disk I/O
+sleep 0.5
+
_send_qemu_cmd $h "{ 'execute': 'quit' }" "return"
wait=1 _cleanup_qemu
--
Best regards,
Vladimir
[Qemu-devel] [PATCH] fpu/softfloat: Don't set Invalid for float-to-int(MAXINT)
In float-to-integer conversion, if the floating point input
converts exactly to the largest or smallest integer that
fits in to the result type, this is not an overflow.
In this situation we were producing the correct result value,
but were incorrectly setting the Invalid flag.
For example for Arm A64, "FCVTAS w0, d0" on an input of
0x41dfffc0 should produce 0x7fff and set no flags.
Fix the boundary case to take the right half of the if()
statements.
This fixes a regression from 2.11 introduced by the softfloat
refactoring.
Cc: qemu-sta...@nongnu.org
Fixes: ab52f973a50
Signed-off-by: Peter Maydell
---
fpu/softfloat.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 8401b37bd4..9bcaaebe4f 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1368,14 +1368,14 @@ static int64_t round_to_int_and_pack(FloatParts in, int
rmode,
r = UINT64_MAX;
}
if (p.sign) {
-if (r < -(uint64_t) min) {
+if (r <= -(uint64_t) min) {
return -r;
} else {
s->float_exception_flags = orig_flags | float_flag_invalid;
return min;
}
} else {
-if (r < max) {
+if (r <= max) {
return r;
} else {
s->float_exception_flags = orig_flags | float_flag_invalid;
--
2.17.0
Re: [Qemu-devel] [PATCH 03/13] tests: Add QAPI optional discriminator tests
On 05/09/2018 11:55 AM, Max Reitz wrote:
There already is an optional discriminator test, although it also noted
the discriminator name itself as optional. Changing that, and adding a
default-variant field, makes that test pass instead of failing.
I'm not sure I agree with that one. I think that you should instead add
a positive test of a default variant into qapi-schema-test.json,
especially since that is the one positive test where we also ensure that
the C compiler is happy with the generated code (other positive tests
prove that the generator produced something without error, but not that
what it produced could be compiled).
17 files changed, 61 insertions(+), 4 deletions(-)
create mode 100644
tests/qapi-schema/flat-union-optional-discriminator-invalid-default.json
create mode 100644
tests/qapi-schema/flat-union-optional-discriminator-no-default.json
create mode 100644
tests/qapi-schema/flat-union-superfluous-default-variant.json
create mode 100644
tests/qapi-schema/flat-union-optional-discriminator-invalid-default.err
create mode 100644
tests/qapi-schema/flat-union-optional-discriminator-invalid-default.exit
create mode 100644
tests/qapi-schema/flat-union-optional-discriminator-invalid-default.out
create mode 100644
tests/qapi-schema/flat-union-optional-discriminator-no-default.err
create mode 100644
tests/qapi-schema/flat-union-optional-discriminator-no-default.exit
create mode 100644
tests/qapi-schema/flat-union-optional-discriminator-no-default.out
create mode 100644
tests/qapi-schema/flat-union-superfluous-default-variant.err
create mode 100644
tests/qapi-schema/flat-union-superfluous-default-variant.exit
create mode 100644
tests/qapi-schema/flat-union-superfluous-default-variant.out
Patch 1 converted 2 error messages into 6, where 2 of them look identical:
-# The value of member 'discriminator' must name a non-optional
-# member of the base struct.
+# The value of member 'discriminator' must name a member of
+# the base struct.
check_name(info, "Discriminator of flat union '%s'" % name,
discriminator)
[0] check_name() checks that 'discriminator':'*name' is rejected - this
check is unchanged
-discriminator_type = base_members.get(discriminator)
-if not discriminator_type:
-raise QAPISemError(info,
- "Discriminator '%s' is not a member of base "
- "struct '%s'"
- % (discriminator, base))
The old code ensured that 'discriminator':'name' finds 'name' as a
mandatory field in the base type; which changed into:
+if default_variant is None:
+discriminator_type = base_members.get(discriminator)
+if not discriminator_type:
+if base_members.get('*' + discriminator) is None:
+raise QAPISemError(info,
+ "Discriminator '%s' is not a member of "
+ "base struct '%s'"
+ % (discriminator, base))
[1] the discriminator type must be a member field (we didn't find either
a mandatory or an optional field)
+else:
+raise QAPISemError(info,
+ "Default variant must be specified for "
+ "optional discriminator '%s'"
+ % discriminator)
[2] without default_variant, the member field must be mandatory
+else:
+discriminator_type = base_members.get('*' + discriminator)
+if not discriminator_type:
+if base_members.get(discriminator) is None:
+raise QAPISemError(info,
+ "Discriminator '%s' is not a member of "
+ "base struct '%s'"
+ % (discriminator, base))
[3] the discriminator type must be a member field (we didn't find either
a mandatory or an optional field), identical message to [1]
+else:
+raise QAPISemError(info,
+ "Must not specify a default variant for
"
+ "non-optional discriminator '%s'"
+ % discriminator)
[4] with default_variant, the member field must be optional
+
enum_define = enum_types.get(discriminator_type)
allow_metas = ['struct']
# Do not allow string discriminator
@@ -763,6 +785,15 @@ def check_union(expr, info):
"Discriminator '%s' must be of enumeration "
"type" % discriminator)
+if default_variant is not None:
+# Must be a value of the enumeration
+if default_variant no
[Qemu-devel] [PATCH] fix fp16 tininess
In commit d81ce0ef2c4f105 we added an extra float_status field
fp_status_fp16 for Arm, but forgot to initialize it correctly
by setting it to float_tininess_before_rounding. This currently
will only cause problems for the new V8_FP16 feature, since the
float-to-float conversion code doesn't use it yet. The effect
would be that we failed to set the Underflow IEEE exception flag
in all the cases where we should.
Add the missing initialization.
Fixes: d81ce0ef2c4f105
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Maydell
---
target/arm/cpu.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index d175c5e94f..7939c6b8ae 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -324,6 +324,8 @@ static void arm_cpu_reset(CPUState *s)
&env->vfp.fp_status);
set_float_detect_tininess(float_tininess_before_rounding,
&env->vfp.standard_fp_status);
+set_float_detect_tininess(float_tininess_before_rounding,
+ &env->vfp.fp_status_f16);
#ifndef CONFIG_USER_ONLY
if (kvm_enabled()) {
kvm_arm_reset_vcpu(cpu);
--
2.17.0
Re: [Qemu-devel] [Qemu-arm] [PATCH] fix fp16 tininess
On 10 May 2018 at 15:09, Peter Maydell wrote: > In commit d81ce0ef2c4f105 we added an extra float_status field > fp_status_fp16 for Arm, but forgot to initialize it correctly > by setting it to float_tininess_before_rounding. This currently > will only cause problems for the new V8_FP16 feature, since the > float-to-float conversion code doesn't use it yet. The effect > would be that we failed to set the Underflow IEEE exception flag > in all the cases where we should. Oops, I forgot to fix the patch summary line from my quick-hack commit. Something like: target/arm: Fix tininess detection for fp16 arithmetic would be a bit clearer. thanks -- PMM
Re: [Qemu-devel] [PATCH v4] dump: add Windows dump format to dump-guest-memory
On Tue, 1 May 2018 16:20:31 +0300
Viktor Prutyanov wrote:
> This patch adds Windows crashdumping feature. Now QEMU can produce
> ELF-dump containing Windows crashdump header, which can help to
> convert to a valid WinDbg-understandable crashdump file, or
> immediately create such file. The crashdump will be obtained by
> joining physical memory dump and 8K header exposed through
> vmcoreinfo/fw_cfg device by guest driver at BSOD time. Option '-w'
> was added to dump-guest-memory command. At the moment, only x64
> configuration is supported. Suitable driver can be found at
> https://github.com/virtio-win/kvm-guest-drivers-windows/tree/master/fwcfg64
>
> Signed-off-by: Viktor Prutyanov
> Reviewed-by: Marc-André Lureau
> ---
>
> v1: documentation updated
> v2: qapi/misc.json updated with version info
> v3: qapi/misc.json codestyle fixed
> v4: make error processing more quality
>
> Makefile.target | 1 +
> dump.c | 24 ++-
> hmp-commands.hx | 13 ++--
> hmp.c | 9 ++-
> qapi/misc.json | 5 +-
> win_dump.c | 209
>
> win_dump.h | 87 +++ 7 files changed, 339
> insertions(+), 9 deletions(-) create mode 100644 win_dump.c
> create mode 100644 win_dump.h
>
> diff --git a/Makefile.target b/Makefile.target
> index d0ec77a307..6ae2609597 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -138,6 +138,7 @@ obj-y += hw/
> obj-y += memory.o
> obj-y += memory_mapping.o
> obj-y += dump.o
> +obj-y += win_dump.o
> obj-y += migration/ram.o
> LIBS := $(libs_softmmu) $(LIBS)
>
> diff --git a/dump.c b/dump.c
> index b54cd42b21..04467b353e 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -29,6 +29,10 @@
> #include "qemu/error-report.h"
> #include "hw/misc/vmcoreinfo.h"
>
> +#ifdef TARGET_X86_64
> +#include "win_dump.h"
> +#endif
> +
> #include
> #ifdef CONFIG_LZO
> #include
> @@ -1866,7 +1870,11 @@ static void dump_process(DumpState *s, Error
> **errp) Error *local_err = NULL;
> DumpQueryResult *result = NULL;
>
> -if (s->has_format && s->format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
> +if (s->has_format && s->format ==
> DUMP_GUEST_MEMORY_FORMAT_WIN_DMP) { +#ifdef TARGET_X86_64
> +create_win_dump(s, &local_err);
> +#endif
> +} else if (s->has_format && s->format !=
> DUMP_GUEST_MEMORY_FORMAT_ELF) { create_kdump_vmcore(s, &local_err);
> } else {
> create_vmcore(s, &local_err);
> @@ -1970,6 +1978,13 @@ void qmp_dump_guest_memory(bool paging, const
> char *file, }
> #endif
>
> +#ifndef TARGET_X86_64
> +if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_WIN_DMP) {
> +error_setg(errp, "Windows dump is only available for
> x86-64");
> +return;
> +}
> +#endif
> +
> #if !defined(WIN32)
> if (strstart(file, "fd:", &p)) {
> fd = monitor_get_fd(cur_mon, p, errp);
> @@ -2044,5 +2059,12 @@ DumpGuestMemoryCapability
> *qmp_query_dump_guest_memory_capability(Error **errp) item->value =
> DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY; #endif
>
> +/* Windows dump is available only if target is x86_64 */
> +#ifdef TARGET_X86_64
> +item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> +item = item->next;
> +item->value = DUMP_GUEST_MEMORY_FORMAT_WIN_DMP;
> +#endif
> +
> return cap;
> }
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 35d862a5d2..6f35e4f5d0 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1088,30 +1088,33 @@ ETEXI
>
> {
> .name = "dump-guest-memory",
> -.args_type =
> "paging:-p,detach:-d,zlib:-z,lzo:-l,snappy:-s,filename:F,begin:i?,length:i?",
> -.params = "[-p] [-d] [-z|-l|-s] filename [begin length]",
> +.args_type =
> "paging:-p,detach:-d,windmp:-w,zlib:-z,lzo:-l,snappy:-s,filename:F,begin:i?,length:i?",
> +.params = "[-p] [-d] [-z|-l|-s|-w] filename [begin
> length]", .help = "dump guest memory into file
> 'filename'.\n\t\t\t" "-p: do paging to get guest's memory
> mapping.\n\t\t\t" "-d: return immediately (do not wait for
> completion).\n\t\t\t" "-z: dump in kdump-compressed format, with zlib
> compression.\n\t\t\t" "-l: dump in kdump-compressed format, with lzo
> compression.\n\t\t\t" "-s: dump in kdump-compressed format, with
> snappy compression.\n\t\t\t"
> + "-w: dump in Windows crashdump format (can be
> used instead of ELF-dump converting),\n\t\t\t"
> + "for Windows x64 guests with vmcoreinfo
> driver only.\n\t\t\t" "begin: the starting physical address.\n\t\t\t"
>"length: the memory size, in bytes.",
> .cmd= hmp_dump_guest_memory,
> },
>
> -
> STEXI
> @item dump-guest-memory [-p] @var{filename} @var{begin} @var{length}
> -@item dump-guest-memory [-z|-l|-s] @var{filename}
> +@item dump-guest-memory [-z|-l|-s|-w] @var{filename}
> @findex dump-guest-memory
> Dump guest memory to @var{prot
Re: [Qemu-devel] [PATCH 04/13] qapi: Formalize qcow2 encryption probing
On 05/10/2018 02:58 AM, Daniel P. Berrangé wrote:
On Wed, May 09, 2018 at 06:55:21PM +0200, Max Reitz wrote:
Currently, you can give no encryption format for a qcow2 file while
still passing a key-secret. That does not conform to the schema, so
this patch changes the schema to allow it.
Signed-off-by: Max Reitz
---
qapi/block-core.json | 44
1 file changed, 40 insertions(+), 4 deletions(-)
{ 'union': 'BlockdevQcow2Encryption',
- 'base': { 'format': 'BlockdevQcow2EncryptionFormat' },
+ 'base': { '*format': 'BlockdevQcow2EncryptionFormat' },
'discriminator': 'format',
+ 'default-variant': 'from-image',
'data': { 'aes': 'QCryptoBlockOptionsQCow',
-'luks': 'QCryptoBlockOptionsLUKS'} }
+'luks': 'QCryptoBlockOptionsLUKS',
+'from-image': 'BlockdevQcow2EncryptionSecret' } }
Bike-shedding on name, how about "auto" or "probe" ?
Either of those sounds nicer to me; 'auto' might be better in the
context of creation (that way, we can state that creating a NEW image
with x-blockdev-create maps 'auto' to 'luks'; while connecting to an
EXISTING image maps 'auto' to either 'aes' or 'luks' as appropriate).
IIUC, this schema addition means the QAPI parser now allows
encrypt.format=from-image,encrypt.key-secret=sec0,...other opts...
Yes. You could, perhaps, add a special case on the command line parsing
code to reject an explicit use of format=from-image, but the QMP should
not reject an explicit discriminator.
Hmm, it plays in with my comment on 1/13 - should the QMP parser
automatically set has_discriminator to true when it supplies the
default? If it does, you lose the ability to see whether the user
supplied an explicit encrypt.format=from-image (or the equivalent when
using QMP instead of the command line), if you wanted to enforce that
the user MUST omit format when relying on the from-image variant.
I don't see a problem in allowing the user to explicitly specify the
name of the default branch, but I _do_ think the patch is incomplete for
not handling the new QCOW_CRYPT_FROM_IMAGE case and converting it as
soon as possible back into one of the other two preferred enum values.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 05/13] qapi: Formalize qcow encryption probing
On 05/09/2018 11:55 AM, Max Reitz wrote:
Currently, you can give no encryption format for a qcow file while still
passing a key-secret. That does not conform to the schema, so this
patch changes the schema to allow it.
Signed-off-by: Max Reitz
---
##
# @BlockdevQcowEncryptionFormat:
#
# @aes: AES-CBC with plain64 initialization vectors
#
+# @from-image: Determine the encryption format from the image
+# header. This only allows the use of the
+# key-secret option. (Since: 2.13)
+#
# Since: 2.10
##
{ 'enum': 'BlockdevQcowEncryptionFormat',
- 'data': [ 'aes' ] }
+ 'data': [ 'aes', 'from-image' ] }
Overkill. Why not just:
##
# @BlockdevQcowEncryption:
@@ -2728,9 +2748,11 @@
# Since: 2.10
##
{ 'union': 'BlockdevQcowEncryption',
- 'base': { 'format': 'BlockdevQcowEncryptionFormat' },
+ 'base': { '*format': 'BlockdevQcowEncryptionFormat' },
'discriminator': 'format',
- 'data': { 'aes': 'QCryptoBlockOptionsQCow' } }
+ 'default-variant': 'from-image',
'default-variant': 'aes'
+ 'data': { 'aes': 'QCryptoBlockOptionsQCow',
and call it good, because there are no other options to pick from, so
'from-image' would always resolve to 'aes' anyway.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v3 04/10] tcg: Introduce atomic helpers for integer min/max
On 8 May 2018 at 19:27, Peter Maydell wrote:
> On 8 May 2018 at 18:49, Peter Maydell wrote:
>> [weird compiler errors]
>
> This fixes them:
>
> --- a/include/qemu/atomic.h
> +++ b/include/qemu/atomic.h
> @@ -187,7 +187,7 @@
> /* Returns the eventual value, failed or not */
> #define atomic_cmpxchg__nocheck(ptr, old, new)({\
> typeof_strip_qual(*ptr) _old = (old); \
> -__atomic_compare_exchange_n(ptr, &_old, new, false, \
> +(void)__atomic_compare_exchange_n(ptr, &_old, new, false, \
>__ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); \
> _old; \
> })
>
> Seems pretty clearly a compiler bug -- rth says newer gcc don't
> do this, so presumably fixed upstream somewhere between gcc 5 and 6.
Standalone testcase, fwiw:
===begin===
/*
* Weirdly, this compiler will complain about FOO(int8_t) but not
* the others:
* $ gcc -g -Wall -Wunused-value -o zz9.o -c zz9.c
* zz9.c: In function ‘foo_int8_t’:
* zz9.c:12:5: warning: value computed is not used [-Wunused-value]
* __atomic_compare_exchange_n(p, exp, des, 0, __ATOMIC_SEQ_CST,
__ATOMIC_SEQ_CST); \
* ^
* /tmp/zz9.c:17:1: note: in expansion of macro ‘FOO’
* FOO(int8_t)
* ^
*
* This is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
*/
typedef unsigned char uint8_t;
typedef signed char int8_t;
#define FOO(TYPE) \
void foo_##TYPE(TYPE *p, TYPE *exp, TYPE *des) { \
__atomic_compare_exchange_n(p, exp, des, 0, __ATOMIC_SEQ_CST,
__ATOMIC_SEQ_CST); \
}
FOO(int)
FOO(uint8_t)
FOO(int8_t)
===endit===
thanks
-- PMM
Re: [Qemu-devel] [PATCH v7 2/2] Add QTest testcase for the Intel Hexadecimal Object File Loader.
On Thu, May 10, 2018 at 03:18:31PM +0800, Su Hang wrote: > '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. > > `pre_store` array in 'hexloader-test.c' file, stores the binary format > of 'test.hex' file, which is used to verify correctness. > > Signed-off-by: Su Hang > --- > MAINTAINERS | 6 > tests/Makefile.include | 2 ++ > tests/hex-loader-check-data/test.hex | 12 > tests/hexloader-test.c | 56 > > 4 files changed, 76 insertions(+) > create mode 100644 tests/hex-loader-check-data/test.hex > create mode 100644 tests/hexloader-test.c Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 05/13] qapi: Formalize qcow encryption probing
On Thu, May 10, 2018 at 09:24:24AM -0500, Eric Blake wrote:
> On 05/09/2018 11:55 AM, Max Reitz wrote:
> > Currently, you can give no encryption format for a qcow file while still
> > passing a key-secret. That does not conform to the schema, so this
> > patch changes the schema to allow it.
> >
> > Signed-off-by: Max Reitz
> > ---
>
> > ##
> > # @BlockdevQcowEncryptionFormat:
> > #
> > # @aes: AES-CBC with plain64 initialization vectors
> > #
> > +# @from-image: Determine the encryption format from the image
> > +# header. This only allows the use of the
> > +# key-secret option. (Since: 2.13)
> > +#
> > # Since: 2.10
> > ##
> > { 'enum': 'BlockdevQcowEncryptionFormat',
> > - 'data': [ 'aes' ] }
> > + 'data': [ 'aes', 'from-image' ] }
>
> Overkill. Why not just:
>
> > ##
> > # @BlockdevQcowEncryption:
> > @@ -2728,9 +2748,11 @@
> > # Since: 2.10
> > ##
> > { 'union': 'BlockdevQcowEncryption',
> > - 'base': { 'format': 'BlockdevQcowEncryptionFormat' },
> > + 'base': { '*format': 'BlockdevQcowEncryptionFormat' },
> > 'discriminator': 'format',
> > - 'data': { 'aes': 'QCryptoBlockOptionsQCow' } }
> > + 'default-variant': 'from-image',
>
> 'default-variant': 'aes'
>
> > + 'data': { 'aes': 'QCryptoBlockOptionsQCow',
>
> and call it good, because there are no other options to pick from, so
> 'from-image' would always resolve to 'aes' anyway.
Sounds reasonable because qcowv1 is a dead format we don't intend to
add more features to.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH] fix fp16 tininess
On 05/10/2018 07:09 AM, Peter Maydell wrote: > In commit d81ce0ef2c4f105 we added an extra float_status field > fp_status_fp16 for Arm, but forgot to initialize it correctly > by setting it to float_tininess_before_rounding. This currently > will only cause problems for the new V8_FP16 feature, since the > float-to-float conversion code doesn't use it yet. The effect > would be that we failed to set the Underflow IEEE exception flag > in all the cases where we should. > > Add the missing initialization. > > Fixes: d81ce0ef2c4f105 > Cc: qemu-sta...@nongnu.org > Signed-off-by: Peter Maydell > --- > target/arm/cpu.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Richard Henderson r~
[Qemu-devel] [PATCH] atomic.h: Work around gcc spurious "unused value" warning
Some versions of gcc produce a spurious warning if the result of
__atomic_compare_echange_n() is not used and the type involved
is a signed 8 bit value:
error: value computed is not used [-Werror=unused-value]
This has been seen on at least
gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
Work around this by using an explicit cast to void to indicate
that we don't care about the return value.
We don't currently use our atomic_cmpxchg() macro on any signed
8 bit types, but the upcoming support for the Arm v8.1-Atomics
will require it.
Signed-off-by: Peter Maydell
---
Sending out my workaround as an actual patch. If this is OK I'll
put in via target-arm.next as it's a dependency for the v8.1-atomics
series.
include/qemu/atomic.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index d73c9e14d7..9ed39effd3 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -187,7 +187,7 @@
/* Returns the eventual value, failed or not */
#define atomic_cmpxchg__nocheck(ptr, old, new)({\
typeof_strip_qual(*ptr) _old = (old); \
-__atomic_compare_exchange_n(ptr, &_old, new, false, \
+(void)__atomic_compare_exchange_n(ptr, &_old, new, false, \
__ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); \
_old; \
})
--
2.17.0
Re: [Qemu-devel] [PATCH] tcg: Optionally log FPU state in TCG -d cpu logging
On 05/10/2018 06:00 AM, Peter Maydell wrote: > Usually the logging of the CPU state produced by -d cpu is sufficient > to diagnose problems, but sometimes you want to see the state of > the floating point registers as well. We don't want to enable that > by default as it adds a lot of extra data to the log; instead, > allow it to be optionally enabled via -d fpu. > > Signed-off-by: Peter Maydell > --- > I've found this helpful while tracking down fp-emulation related bugs. > > include/qemu/log.h | 1 + > accel/tcg/cpu-exec.c | 9 ++--- > util/log.c | 2 ++ > 3 files changed, 9 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson I'll also note that only i386 and arm check this flag; something to fix for the rest... r~
Re: [Qemu-devel] [PATCH] atomic.h: Work around gcc spurious "unused value" warning
On 05/10/2018 07:36 AM, Peter Maydell wrote: > Some versions of gcc produce a spurious warning if the result of > __atomic_compare_echange_n() is not used and the type involved > is a signed 8 bit value: > error: value computed is not used [-Werror=unused-value] > This has been seen on at least > gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609 > > Work around this by using an explicit cast to void to indicate > that we don't care about the return value. > > We don't currently use our atomic_cmpxchg() macro on any signed > 8 bit types, but the upcoming support for the Arm v8.1-Atomics > will require it. > > Signed-off-by: Peter Maydell > --- > Sending out my workaround as an actual patch. If this is OK I'll > put in via target-arm.next as it's a dependency for the v8.1-atomics > series. > > include/qemu/atomic.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH] tcg: Optionally log FPU state in TCG -d cpu logging
On 10 May 2018 at 15:36, Richard Henderson wrote: > On 05/10/2018 06:00 AM, Peter Maydell wrote: >> Usually the logging of the CPU state produced by -d cpu is sufficient >> to diagnose problems, but sometimes you want to see the state of >> the floating point registers as well. We don't want to enable that >> by default as it adds a lot of extra data to the log; instead, >> allow it to be optionally enabled via -d fpu. >> >> Signed-off-by: Peter Maydell >> --- >> I've found this helpful while tracking down fp-emulation related bugs. >> >> include/qemu/log.h | 1 + >> accel/tcg/cpu-exec.c | 9 ++--- >> util/log.c | 2 ++ >> 3 files changed, 9 insertions(+), 3 deletions(-) > > Reviewed-by: Richard Henderson > > I'll also note that only i386 and arm check this flag; > something to fix for the rest... Mmm. It would also be nice not to have that TARGET_I386 special case for CPU_DUMP_CCOP -- we should either care about that for everything, or for nothing (my vote would be for not printing it unless user-requested, since it's an internal tcg target detail.) thanks -- PMM
Re: [Qemu-devel] [PATCH] atomic.h: Work around gcc spurious "unused value" warning
On 10 May 2018 at 15:37, Richard Henderson wrote: > On 05/10/2018 07:36 AM, Peter Maydell wrote: >> Some versions of gcc produce a spurious warning if the result of >> __atomic_compare_echange_n() is not used and the type involved >> is a signed 8 bit value: >> error: value computed is not used [-Werror=unused-value] >> This has been seen on at least >> gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609 >> >> Work around this by using an explicit cast to void to indicate >> that we don't care about the return value. >> >> We don't currently use our atomic_cmpxchg() macro on any signed >> 8 bit types, but the upcoming support for the Arm v8.1-Atomics >> will require it. >> >> Signed-off-by: Peter Maydell >> --- >> Sending out my workaround as an actual patch. If this is OK I'll >> put in via target-arm.next as it's a dependency for the v8.1-atomics >> series. >> >> include/qemu/atomic.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Reviewed-by: Richard Henderson Thanks. Do we care about trying to follow up on the gcc side to find out if this is a definitely-fixed bug? -- PMM
Re: [Qemu-devel] [PATCH] atomic.h: Work around gcc spurious "unused value" warning
On 05/10/2018 07:38 AM, Peter Maydell wrote: > Thanks. Do we care about trying to follow up on the gcc side > to find out if this is a definitely-fixed bug? Given I did not see the problem with gcc 6 as shipped with debian 9, I'm not overly inclined to dig further than that. r~
Re: [Qemu-devel] [PATCH v7 1/2] Implement .hex file loader
On Thu, May 10, 2018 at 03:18:30PM +0800, Su Hang wrote:
Great, this is getting close. A few more comments below...
> if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
> kernel_size = load_aarch64_image(info->kernel_filename,
> info->loader_start, &entry, as);
> is_linux = 1;
> -} else if (kernel_size < 0) {
> -/* 32-bit ARM */
> +}
> +if (kernel_size < 0) {
> +/* 32-bit ARM binary file */
Why is this change necessary? A 64-bit machine previously didn't
attempt a 32-bit kernel load. I'm not sure why you changed this
behavior.
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 5ed3fd8ae67a..ba82c9686ba3 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -28,6 +28,19 @@ 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_image_targphys_hex_as:
s/load_image_targphys_hex_as/load_targphys_hex_as/
> + * @filename: Path to the .hex file
> + * @entry: Store the entry point get from .hex file
s/get from/given by the/g
> + * @max_sz: The maximum size of the .hex file to load
This argument does not exist, please drop it.
> @@ -241,6 +254,8 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict);
> rom_add_file(_f, NULL, _a, _i, false, NULL, _as)
> #define rom_add_blob_fixed_as(_f, _b, _l, _a, _as) \
> rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, _as, true)
> +#define rom_add_hex_blob_as(_f, _b, _l, _a, _as) \
> +rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, _as, true)
This macro is equivalent to rom_add_blob_fixed_as(). Why not use
rom_add_blob_fixed_as()?
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v3 1/5] fpu/softfloat: int_to_float ensure r fully initialised
On 05/10/2018 02:42 AM, Alex Bennée wrote: > Reported by Coverity (CID1390635). We ensure this for uint_to_float > later on so we might as well mirror that. > > Signed-off-by: Alex Bennée > --- > fpu/softfloat.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH v3 3/9] qcow2: Implement copy offloading
On Thu, 05/10 10:25, Stefan Hajnoczi wrote:
> On Wed, May 09, 2018 at 10:58:09PM +0800, Fam Zheng wrote:
> > +static int qcow2_co_copy_range_from(BlockDriverState *bs,
> > +BdrvChild *src, uint64_t src_offset,
> > +BdrvChild *dst, uint64_t dst_offset,
> > +uint64_t bytes, BdrvRequestFlags flags)
> > +{
> > +BDRVQcow2State *s = bs->opaque;
> > +int offset_in_cluster;
> > +int ret;
> > +unsigned int cur_bytes; /* number of bytes in current iteration */
> > +uint64_t cluster_offset = 0;
> > +BdrvChild *child = NULL;
> > +
> > +assert(!bs->encrypted);
> > +qemu_co_mutex_lock(&s->lock);
> > +
> > +while (bytes != 0) {
> > +
> > +/* prepare next request */
> > +cur_bytes = MIN(bytes, INT_MAX);
> > +
> > +ret = qcow2_get_cluster_offset(bs, src_offset, &cur_bytes,
> > &cluster_offset);
> > +if (ret < 0) {
> > +goto out;
> > +}
> > +
> > +offset_in_cluster = offset_into_cluster(s, src_offset);
> > +
> > +switch (ret) {
> > +case QCOW2_CLUSTER_UNALLOCATED:
> > +if (bs->backing) {
> > +child = bs->backing;
> > +} else {
> > +flags |= BDRV_REQ_ZERO_WRITE;
> > +}
> > +break;
>
> Do we need a special case if the backing file is shorter than this
> image?
Yes, we should take the BDRV_REQ_ZERO_WRITE path if the range is fully beyond
EOF, otherwise clip cur_bytes at the backing image offset.
Fam
Re: [Qemu-devel] [PATCH 06/13] block: Add block-specific QDict header
On 05/09/2018 11:55 AM, Max Reitz wrote: There are numerous QDict functions that have been introduced for and are used only by the block layer. Move their declarations into an own s/an own/their own/ header file to reflect that. While qdict_extract_subqdict() is in fact used outside of the block layer (in util/qemu-config.c), it is still a function related very closely to how the block layer works with nested QDicts, namely by sometimes flattening them. Therefore, its declaration is put into this header as well and util/qemu-config.c includes it with a comment stating exactly which function it needs. Suggested-by: Markus Armbruster Signed-off-by: Max Reitz --- +++ b/include/block/qdict.h @@ -0,0 +1,35 @@ +/* + * Special QDict functions used by the block layer + * + * Copyright (c) 2018 Red Hat, Inc. You are extracting this from qdict.h which has: * Copyright (C) 2009 Red Hat Inc. Is it worth listing 2009-2018, instead of just this year? + +void qdict_copy_default(QDict *dst, QDict *src, const char *key); +void qdict_set_default_str(QDict *dst, const char *key, const char *val); These two might be useful outside of the block layer; we can move them back in a later patch if so. But for now, I agree with your choice of moving them. + +void qdict_join(QDict *dest, QDict *src, bool overwrite); This is borderline whether it would be useful outside of the block layer. + +void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start); +void qdict_array_split(QDict *src, QList **dst); +int qdict_array_entries(QDict *src, const char *subqdict); +QObject *qdict_crumple(const QDict *src, Error **errp); +void qdict_flatten(QDict *qdict); And these are definitely hacks that the block layer relies on, where hopefully someday long term we can rewrite the block layer to use QAPI types directly instead of constant reconversion between QemuOpts and QDict and QAPI types. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH] atomic.h: Work around gcc spurious "unused value" warning
On 05/10/2018 11:36 AM, Peter Maydell wrote:
> Some versions of gcc produce a spurious warning if the result of
> __atomic_compare_echange_n() is not used and the type involved
> is a signed 8 bit value:
> error: value computed is not used [-Werror=unused-value]
> This has been seen on at least
> gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
>
> Work around this by using an explicit cast to void to indicate
> that we don't care about the return value.
>
> We don't currently use our atomic_cmpxchg() macro on any signed
> 8 bit types, but the upcoming support for the Arm v8.1-Atomics
> will require it.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> Sending out my workaround as an actual patch. If this is OK I'll
> put in via target-arm.next as it's a dependency for the v8.1-atomics
> series.
>
> include/qemu/atomic.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
> index d73c9e14d7..9ed39effd3 100644
> --- a/include/qemu/atomic.h
> +++ b/include/qemu/atomic.h
> @@ -187,7 +187,7 @@
> /* Returns the eventual value, failed or not */
> #define atomic_cmpxchg__nocheck(ptr, old, new)({\
> typeof_strip_qual(*ptr) _old = (old); \
> -__atomic_compare_exchange_n(ptr, &_old, new, false, \
> +(void)__atomic_compare_exchange_n(ptr, &_old, new, false, \
>__ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); \
> _old; \
> })
>
Re: [Qemu-devel] [PATCH v3 00/10] target/arm: Implement v8.1-Atomics
On 8 May 2018 at 16:14, Richard Henderson wrote: > This implements the Atomics extension, which is mandatory for v8.1. > While testing the v8.2-SVE extension, I've run into issues with the > GCC testsuite expecting this to exist. > > Missing is the wiring up of the system registers to indicate that > the extension exists, but we have no system CPU model that would > exercise such a setting. > > Changes since v3: > * Patch 8: Do not zero-extend X[s] via the third parameter > to read_cpu_reg. > > Changes since v2: > * New patch to use a helper macro for opposite-endian > atomic_fetch_add and atomic_add_fetch, as suggested > by pm215. > * Introduce ARM_FEATURE_V8_1 and define ARM_FEATURE_V8_ATOMICS > in terms of that, reinforcing the mandatory nature of > the extension. > * Typo fix in patch 8 Applied to target-arm.next (with the atomics.h workaround first and the removal of ARM_FEATURE_V8_1), thanks. -- PMM
Re: [Qemu-devel] [PATCH] qcow2: fix preallocation with metadata on bare block device
ping for review
On Tue, May 8, 2018 at 8:27 PM Ivan Ren wrote:
> Create a qcow2 directly on bare block device with
> "-o preallocation=metadata" option. When read this qcow2, it will
> return dirty data of block device. This patch add QCOW_OFLAG_ZERO
> for all preallocated l2 entry if the underlying device is a bare
> block device.
>
> Signed-off-by: Ivan Ren
> ---
> block/qcow2-cluster.c | 5 +++--
> block/qcow2.c | 19 ---
> block/qcow2.h | 3 ++-
> 3 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 1aee726..b9e0ceb 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -919,7 +919,8 @@ fail:
> return ret;
> }
>
> -int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
> +int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
> +uint64_t flags)
> {
> BDRVQcow2State *s = bs->opaque;
> int i, j = 0, l2_index, ret;
> @@ -969,7 +970,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs,
> QCowL2Meta *m)
> }
>
> l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
> -(i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
> +(i << s->cluster_bits)) | QCOW_OFLAG_COPIED | flags);
> }
>
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 2f36e63..093735c 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2044,7 +2044,7 @@ static coroutine_fn int
> qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
> while (l2meta != NULL) {
> QCowL2Meta *next;
>
> -ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
> +ret = qcow2_alloc_cluster_link_l2(bs, l2meta, 0);
> if (ret < 0) {
> goto fail;
> }
> @@ -2534,6 +2534,7 @@ static void coroutine_fn preallocate_co(void *opaque)
> uint64_t host_offset = 0;
> unsigned int cur_bytes;
> int ret;
> +struct stat st;
> QCowL2Meta *meta;
>
> qemu_co_mutex_lock(&s->lock);
> @@ -2552,7 +2553,19 @@ static void coroutine_fn preallocate_co(void
> *opaque)
> while (meta) {
> QCowL2Meta *next = meta->next;
>
> -ret = qcow2_alloc_cluster_link_l2(bs, meta);
> +/* Check the underlying device type.
> + * If the underlying device is a block device, we add
> + * QCOW_OFLAG_ZERO for all preallocated l2 entry to ignore
> dirty
> + * data on block device.
> + * If the underlying device can't be used with stat(return <
> 0),
> + * treat it as a regular file.
> + */
> +if (stat(bs->filename, &st) < 0 || !S_ISBLK(st.st_mode)) {
> +ret = qcow2_alloc_cluster_link_l2(bs, meta, 0);
> +} else {
> +ret = qcow2_alloc_cluster_link_l2(bs, meta,
> QCOW_OFLAG_ZERO);
> +}
> +
> if (ret < 0) {
> qcow2_free_any_clusters(bs, meta->alloc_offset,
> meta->nb_clusters,
> QCOW2_DISCARD_NEVER);
> @@ -3458,7 +3471,7 @@ static int qcow2_truncate(BlockDriverState *bs,
> int64_t offset,
> };
> qemu_co_queue_init(&allocation.dependent_requests);
>
> -ret = qcow2_alloc_cluster_link_l2(bs, &allocation);
> +ret = qcow2_alloc_cluster_link_l2(bs, &allocation, 0);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Failed to update L2
> tables");
> qcow2_free_clusters(bs, host_offset,
> diff --git a/block/qcow2.h b/block/qcow2.h
> index adf5c39..9a59602 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -617,7 +617,8 @@ uint64_t
> qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
> uint64_t offset,
> int compressed_size);
>
> -int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
> +int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
> +uint64_t flags);
> int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
>uint64_t bytes, enum qcow2_discard_type type,
>bool full_discard);
> --
> 1.8.3.1
>
>
Re: [Qemu-devel] [PATCH] fix fp16 tininess
Peter Maydell writes:
> In commit d81ce0ef2c4f105 we added an extra float_status field
> fp_status_fp16 for Arm, but forgot to initialize it correctly
> by setting it to float_tininess_before_rounding. This currently
> will only cause problems for the new V8_FP16 feature, since the
> float-to-float conversion code doesn't use it yet. The effect
> would be that we failed to set the Underflow IEEE exception flag
> in all the cases where we should.
>
> Add the missing initialization.
>
> Fixes: d81ce0ef2c4f105
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Peter Maydell
Reviewed-by: Alex Bennée
> ---
> target/arm/cpu.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index d175c5e94f..7939c6b8ae 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -324,6 +324,8 @@ static void arm_cpu_reset(CPUState *s)
>&env->vfp.fp_status);
> set_float_detect_tininess(float_tininess_before_rounding,
>&env->vfp.standard_fp_status);
> +set_float_detect_tininess(float_tininess_before_rounding,
> + &env->vfp.fp_status_f16);
> #ifndef CONFIG_USER_ONLY
> if (kvm_enabled()) {
> kvm_arm_reset_vcpu(cpu);
--
Alex Bennée
Re: [Qemu-devel] [PATCH v2 00/14] sdcard: Proper implementation of CRC7
On 9 May 2018 at 04:46, Philippe Mathieu-Daudé wrote: > Hi, > > This series emerged after last Coverity scan and Peter suggestion in: > http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg05046.html > > (3) "proper" implementation of CRC, so that an sd controller > can either (a) mark the SDRequest as "no CRC" and have > sd_req_crc_validate() always pass, or (b) mark the SDRequest > as having a CRC and have sd_req_crc_validate() actually > do the check which it currently stubs out with "return 0" > > - Coverity issues fixed > - new functions documented > - qtests added > > This series would be much smaller without qtests (less refactor and code > movement), but I feel more confident having them passing :) This series isn't going in the direction I expected. We have two cases: (1) we model an SD controller which in hardware calculates its own checksums. Most of our SD controllers are like that. Since in QEMU there is no chance of data corruption between the controller and the card, there's no point in calculating a checksum by calling a function, and then checking that we get the same result a few function calls later when we call the exact same function in the SD card model. So we should just unconditionally say "no checksum provided" in the SDRequest struct the controller fills in, and then skip the check in the card model. (2) we model an SD controller which leaves the checksum calculation to guest software. The only one of these we have that I know of is milkymist-memcard. In this case, the checksum is calculated by guest software, and so we do want to check it in the SD card model. So the controller should fill in the CRC field in SDRequest, and set the flag in SDRequest to say "checksum provided". We don't need to provide a property on the device or the card objects to control this behaviour, for either case. I'm not clear why this patchset has removed the SDRequest struct in favour of a raw buffer, either: that makes it harder to just say "this request doesn't have a checksum you need to check". thanks -- PMM
Re: [Qemu-devel] [PATCH] fpu/softfloat: Don't set Invalid for float-to-int(MAXINT)
On 05/10/2018 07:01 AM, Peter Maydell wrote: > In float-to-integer conversion, if the floating point input > converts exactly to the largest or smallest integer that > fits in to the result type, this is not an overflow. > In this situation we were producing the correct result value, > but were incorrectly setting the Invalid flag. > For example for Arm A64, "FCVTAS w0, d0" on an input of > 0x41dfffc0 should produce 0x7fff and set no flags. > > Fix the boundary case to take the right half of the if() > statements. > > This fixes a regression from 2.11 introduced by the softfloat > refactoring. > > Cc: qemu-sta...@nongnu.org > Fixes: ab52f973a50 > Signed-off-by: Peter Maydell > --- > fpu/softfloat.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH] qcow2: fix preallocation with metadata on bare block device
On 05/08/2018 07:27 AM, Ivan Ren wrote:
Create a qcow2 directly on bare block device with
"-o preallocation=metadata" option. When read this qcow2, it will
return dirty data of block device.
Yes, reading garbage is expected.
This patch add QCOW_OFLAG_ZERO
for all preallocated l2 entry if the underlying device is a bare
block device.
This says what the patch does, but not why. What is the actual use case
scenario where changing semantics to have the qcow2 overwrite the
garbage to read 0 instead of any pre-existing garbage, when dealing with
portions of the disk that have not yet been written by the guest? Are
you trying to prevent a security leak of previous information that may
be resident on the block device?
Signed-off-by: Ivan Ren
---
block/qcow2-cluster.c | 5 +++--
block/qcow2.c | 19 ---
block/qcow2.h | 3 ++-
3 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 1aee726..b9e0ceb 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -919,7 +919,8 @@ fail:
return ret;
}
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
+int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
+uint64_t flags)
{
BDRVQcow2State *s = bs->opaque;
int i, j = 0, l2_index, ret;
@@ -969,7 +970,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs,
QCowL2Meta *m)
}
l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
-(i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
+(i << s->cluster_bits)) | QCOW_OFLAG_COPIED | flags);
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 2f36e63..093735c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2044,7 +2044,7 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState
*bs, uint64_t offset,
while (l2meta != NULL) {
QCowL2Meta *next;
-ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
+ret = qcow2_alloc_cluster_link_l2(bs, l2meta, 0);
if (ret < 0) {
goto fail;
}
@@ -2534,6 +2534,7 @@ static void coroutine_fn preallocate_co(void *opaque)
uint64_t host_offset = 0;
unsigned int cur_bytes;
int ret;
+struct stat st;
QCowL2Meta *meta;
qemu_co_mutex_lock(&s->lock);
@@ -2552,7 +2553,19 @@ static void coroutine_fn preallocate_co(void *opaque)
while (meta) {
QCowL2Meta *next = meta->next;
-ret = qcow2_alloc_cluster_link_l2(bs, meta);
+/* Check the underlying device type.
+ * If the underlying device is a block device, we add
+ * QCOW_OFLAG_ZERO for all preallocated l2 entry to ignore dirty
+ * data on block device.
+ * If the underlying device can't be used with stat(return < 0),
+ * treat it as a regular file.
+ */
+if (stat(bs->filename, &st) < 0 || !S_ISBLK(st.st_mode)) {
Won't work. You cannot guarantee that bs->filename is a local file; it
could be a remote protocol, such as NBD or gluster. If you need to know
whether bs->filename has a property that it reads as all zeroes when
first initialized, we already have
BlockDriverInfo::unallocated_blocks_are_zero, which is set to true for
regular files and false for block devices.
+ret = qcow2_alloc_cluster_link_l2(bs, meta, 0);
+} else {
+ret = qcow2_alloc_cluster_link_l2(bs, meta, QCOW_OFLAG_ZERO);
Why would we not want to pass QCOW_OFLAG_ZERO always, rather than
special-casing it based on what the underlying protocol might already
have in place?
+}
+
if (ret < 0) {
qcow2_free_any_clusters(bs, meta->alloc_offset,
meta->nb_clusters,
QCOW2_DISCARD_NEVER);
@@ -3458,7 +3471,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t
offset,
};
qemu_co_queue_init(&allocation.dependent_requests);
-ret = qcow2_alloc_cluster_link_l2(bs, &allocation);
+ret = qcow2_alloc_cluster_link_l2(bs, &allocation, 0);
if (ret < 0) {
error_setg_errno(errp, -ret, "Failed to update L2 tables");
qcow2_free_clusters(bs, host_offset,
diff --git a/block/qcow2.h b/block/qcow2.h
index adf5c39..9a59602 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -617,7 +617,8 @@ uint64_t
qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
uint64_t offset,
int compressed_size);
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
+int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
+uint64_t flags);
int qco
Re: [Qemu-devel] [PATCH 08/13] tests: Add qdict_stringify_for_keyval() test
On 05/09/2018 11:55 AM, Max Reitz wrote:
Signed-off-by: Max Reitz
---
tests/check-qdict.c | 54 +
1 file changed, 54 insertions(+)
+static void qdict_stringify_for_keyval_test(void)
+{
+QDict *dict = qdict_new();
+
+/*
+ * Test stringification of:
+ *
+ * {
+ * "a": "null",
+ * "b": 42,
+ * "c": -23,
+ * "d": false,
+ * "e": null,
+ * "f": "",
+ * "g": 0.5,
+ * "h": 0x,
+ * "i": true,
+ * "j": 0
Is it worth testing fun things like '-0.0'?
+g_assert(!strcmp(qdict_get_str(dict, "a"), "null"));
+g_assert(!strcmp(qdict_get_str(dict, "b"), "42"));
+g_assert(!strcmp(qdict_get_str(dict, "c"), "-23"));
+g_assert(!strcmp(qdict_get_str(dict, "d"), "off"));
+g_assert(qobject_type(qdict_get(dict, "e")) == QTYPE_QNULL);
Is it worth shortening this line to:
g_assert(qobject_to(QNull, qdict_get(dict, "e")));
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v2 01/14] target/arm: Implement vector shifted SCVF/UCVF for fp16
On 2 May 2018 at 23:15, Richard Henderson wrote: > While we have some of the scalar paths for *CVF for fp16, > we failed to decode the fp16 version of these instructions. > > Cc: qemu-sta...@nongnu.org > Signed-off-by: Richard Henderson > > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 02/14] target/arm: Implement vector shifted FCVT for fp16
On 2 May 2018 at 23:15, Richard Henderson wrote: > While we have some of the scalar paths for FCVT for fp16, > we failed to decode the fp16 version of these instructions. > > Cc: qemu-sta...@nongnu.org > Signed-off-by: Richard Henderson > > --- > v2: Use parens with (x << y) >> z. > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 05/14] target/arm: Implement FMOV (general) for fp16
On 2 May 2018 at 23:15, Richard Henderson wrote:
> Adding the fp16 moves to/from general registers.
>
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Richard Henderson
> ---
> target/arm/translate-a64.c | 22 +-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index c64c3ed99d..247a4f0cce 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -5463,6 +5463,15 @@ static void handle_fmov(DisasContext *s, int rd, int
> rn, int type, bool itof)
> tcg_gen_st_i64(tcg_rn, cpu_env, fp_reg_hi_offset(s, rd));
> clear_vec_high(s, true, rd);
> break;
> +case 3:
> +/* 16 bit */
> +tmp = tcg_temp_new_i64();
> +tcg_gen_ext16u_i64(tmp, tcg_rn);
> +write_fp_dreg(s, rd, tmp);
> +tcg_temp_free_i64(tmp);
> +break;
> +default:
> +g_assert_not_reached();
> }
> } else {
> TCGv_i64 tcg_rd = cpu_reg(s, rd);
> @@ -5480,6 +5489,12 @@ static void handle_fmov(DisasContext *s, int rd, int
> rn, int type, bool itof)
> /* 64 bits from top half */
> tcg_gen_ld_i64(tcg_rd, cpu_env, fp_reg_hi_offset(s, rn));
> break;
> +case 3:
> +/* 16 bit */
> +tcg_gen_ld16u_i64(tcg_rd, cpu_env, fp_reg_offset(s, rn, MO_16));
> +break;
> +default:
> +g_assert_not_reached();
> }
> }
> }
> @@ -5519,10 +5534,15 @@ static void disas_fp_int_conv(DisasContext *s,
> uint32_t insn)
> case 0xa: /* 64 bit */
> case 0xd: /* 64 bit to top half of quad */
> break;
> +case 0x6: /* 16-bit */
> +if (arm_dc_feature(s, ARM_FEATURE_V8_FP16)) {
> +break;
> +}
> +/* fallthru */
This is a switch on (sf << 3 | type << 1 | rmode), so this catches
the halfprec <-> 32 bit cases, but it misses the halfprec <-> 64 bit
ops, which have sf == 1. So I think you need a 'case 0xe' as well.
> default:
> /* all other sf/type/rmode combinations are invalid */
> unallocated_encoding(s);
> -break;
> +return;
This is a distinct bugfix, I think (though the only effect would
be generation of unnecessary dead code after the undef).
> }
>
> if (!fp_access_check(s)) {
> --
> 2.14.3
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 08/14] target/arm: Introduce and use read_fp_hreg
On 2 May 2018 at 23:15, Richard Henderson wrote: > Cc: qemu-sta...@nongnu.org > Signed-off-by: Richard Henderson > --- > target/arm/translate-a64.c | 30 ++ > 1 file changed, 14 insertions(+), 16 deletions(-) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v4 02/11] machine: make MemoryHotplugState accessible via the machine
On 04/05/2018 21:26, Eduardo Habkost wrote:
> On Mon, Apr 23, 2018 at 06:51:17PM +0200, David Hildenbrand wrote:
> [...]
>> +/* always allocate the device memory information */
>> +machine->device_memory = g_malloc(sizeof(*machine->device_memory));
> [...]
>> -/* initialize hotplug memory address space */
>> +/* always allocate the device memory information */
>> +machine->device_memory = g_malloc(sizeof(*machine->device_memory));
>
> This makes QEMU crash because machine->device_memory->base is initialized with
> garbage:
>
> #1 0x7fffef30a8f8 in abort () at /lib64/libc.so.6
> #2 0x7fffef302026 in __assert_fail_base () at /lib64/libc.so.6
> #3 0x7fffef3020d2 in () at /lib64/libc.so.6
> #4 0x55833483 in int128_get64 (a=) at
> .../qemu-build/include/qemu/int128.h:22
> #5 0x55837c2e in memory_region_size (a=) at
> .../qemu-build/memory.c:1735
> #6 0x55837c2e in memory_region_size (mr=) at
> .../qemu-build/memory.c:1739
> #7 0x558a2b14 in pc_memory_init (pcms=pcms@entry=0x56850050,
> system_memory=system_memory@entry=0x56851e00,
> rom_memory=rom_memory@entry=0x568b8120,
> ram_memory=ram_memory@entry=0x7fffd630)
> at .../qemu-build/hw/i386/pc.c:1440
> #8 0x558a5a73 in pc_init1 (machine=0x56850050,
> pci_type=0x55cb6fd0 "i440FX", host_type=0x55c43e41 "i440FX-pcihost")
> at .../qemu-build/hw/i386/pc_piix.c:179
> #9 0x559abbda in machine_run_board_init (machine=0x56850050) at
> .../qemu-build/hw/core/machine.c:829
> #10 0x557dc515 in main (argc=, argv=,
> envp=) at .../qemu-build/vl.c:4563
>
>
> I will squash the following fixup:
>
> From 6216fdb28476ed21c4ced4672003c9c7cb0e04d2 Mon Sep 17 00:00:00 2001
> From: David Hildenbrand
> Date: Fri, 4 May 2018 15:54:46 +0200
> Subject: [PATCH] memory-device: fix device_memory creation on pc and spapr
>
> We have to inititalize the struct to 0. Otherwise, without "maxmem",
> the content is undefined, which might result in random asserts
> striking when e.g. reading out the size of the contained memory region.
>
> Signed-off-by: David Hildenbrand
> ---
> hw/i386/pc.c | 2 +-
> hw/ppc/spapr.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index ffcd7b85d9..868893d0a1 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1372,7 +1372,7 @@ void pc_memory_init(PCMachineState *pcms,
> }
>
> /* always allocate the device memory information */
> -machine->device_memory = g_malloc(sizeof(*machine->device_memory));
> +machine->device_memory = g_malloc0(sizeof(*machine->device_memory));
>
> /* initialize device memory address space */
> if (pcmc->has_reserved_memory &&
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ef05075232..a1abcba6ad 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2637,7 +2637,7 @@ static void spapr_machine_init(MachineState *machine)
> memory_region_add_subregion(sysmem, 0, ram);
>
> /* always allocate the device memory information */
> -machine->device_memory = g_malloc(sizeof(*machine->device_memory));
> +machine->device_memory = g_malloc0(sizeof(*machine->device_memory));
g_new0 since you are at it? :)
Paolo
>
> /* initialize hotplug memory address space */
> if (machine->ram_size < machine->maxram_size) {
>
Re: [Qemu-devel] [PATCH v2 11/14] target/arm: Implement FCMP for fp16
On 2 May 2018 at 23:15, Richard Henderson wrote: > From: Alex Bennée > > These where missed out from the rest of the half-precision work. > > Cc: qemu-sta...@nongnu.org > Signed-off-by: Alex Bennée > [rth: Diagnose lack of FP16 before fp_access_check] > Signed-off-by: Richard Henderson > --- > target/arm/helper-a64.h| 2 ++ > target/arm/helper-a64.c| 10 ++ > target/arm/translate-a64.c | 88 > +- > 3 files changed, 83 insertions(+), 17 deletions(-) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 12/14] target/arm: Implement FCSEL for fp16
On 2 May 2018 at 23:15, Richard Henderson wrote: > From: Alex Bennée > > These were missed out from the rest of the half-precision work. > > Cc: qemu-sta...@nongnu.org > Signed-off-by: Alex Bennée > [rth: Fix erroneous check vs type] > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 13/14] target/arm: Implement FMOV (immediate) for fp16
On 2 May 2018 at 23:15, Richard Henderson wrote: > From: Alex Bennée > > All the hard work is already done by vfp_expand_imm, we just need to > make sure we pick up the correct size. > > Cc: qemu-sta...@nongnu.org > Signed-off-by: Alex Bennée > [rth: Merge unallocated_encoding check with TCGMemOp conversion.] > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
