Re: [Qemu-devel] [PATCH v2 3/8] target/sparc: Define an enumeration for accessing env->regwptr

[Philippe Mathieu-Daudé]

Hi Richard,

On 5/10/19 5:27 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/sparc/cpu.h | 33 +
>  1 file changed, 33 insertions(+)
> 
> diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
> index 85b9665ccc..08f7d1a3c6 100644
> --- a/target/sparc/cpu.h
> +++ b/target/sparc/cpu.h
> @@ -31,6 +31,39 @@
>  
>  /*#define EXCP_INTERRUPT 0x100*/
>  
> +/* Windowed register indexes.  */
> +enum {
> +WREG_O0,
> +WREG_O1,
> +WREG_O2,
> +WREG_O3,
> +WREG_O4,
> +WREG_O5,
> +WREG_O6,
> +WREG_O7,
> +
> +WREG_L0,
> +WREG_L1,
> +WREG_L2,
> +WREG_L3,
> +WREG_L4,
> +WREG_L5,
> +WREG_L6,
> +WREG_L7,
> +
> +WREG_I0,
> +WREG_I1,
> +WREG_I2,
> +WREG_I3,
> +WREG_I4,
> +WREG_I5,
> +WREG_I6,
> +WREG_I7,

I'd feel safer if you initialize those enums (better safe than sorry!).

> +
> +WREG_SP = WREG_O6,
> +WREG_FP = WREG_I6,
> +};
> +
>  /* trap definitions */
>  #ifndef TARGET_SPARC64
>  #define TT_TFAULT   0x01
> 

Re: [Qemu-devel] [PATCH v4 6/6] acpi: pci: use build_append_foo() API to construct MCFG

[Philippe Mathieu-Daudé]

On 5/15/19 3:10 AM, Michael S. Tsirkin wrote:
> On Fri, Apr 19, 2019 at 08:30:53AM +0800, Wei Yang wrote:
>> build_append_foo() API doesn't need explicit endianness conversions
>> which eliminates a source of errors and it makes build_mcfg() look like
>> declarative definition of MCFG table in ACPI spec, which makes it easy
>> to review.
>>
>> Signed-off-by: Wei Yang 
>> Suggested-by: Igor Mammedov 
>> Reviewed-by: Igor Mammedov 
>> Reviewed-by: Philippe Mathieu-Daudé 
> 
> Causes a regression with an invalid MCFG produced.
> Dropped.
> 
>> ---
>>  hw/acpi/pci.c   | 33 +
>>  include/hw/acpi/acpi-defs.h | 18 --
>>  2 files changed, 21 insertions(+), 30 deletions(-)
>>
>> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
>> index fa0fa30bb9..341805e786 100644
>> --- a/hw/acpi/pci.c
>> +++ b/hw/acpi/pci.c
>> @@ -30,17 +30,26 @@
>>  
>>  void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
>>  {
>> -AcpiTableMcfg *mcfg;
>> -int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
>> -
>> -mcfg = acpi_data_push(table_data, len);
>> -mcfg->allocation[0].address = cpu_to_le64(info->base);
>> -
>> -/* Only a single allocation so no need to play with segments */
>> -mcfg->allocation[0].pci_segment = cpu_to_le16(0);
>> -mcfg->allocation[0].start_bus_number = 0;
>> -mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->size - 1);
>> -
>> -build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, 
>> NULL);
>> +int mcfg_start = table_data->len;
>> +
>> +acpi_data_push(table_data, sizeof(AcpiTableHeader));

Ah, it seems we missed AcpiTableMcfg.reserved[8]...:

build_append_int_noprefix(table_data, 0, 8);

>> +
>> +/*
>> + * PCI Firmware Specification, Revision 3.0
>> + * 4.1.2 MCFG Table Description.
>> + */
>> +/* Base address, processor-relative */
>> +build_append_int_noprefix(table_data, info->base, 8);
>> +/* PCI segment group number */
>> +build_append_int_noprefix(table_data, 0, 2);
>> +/* Starting PCI Bus number */
>> +build_append_int_noprefix(table_data, 0, 1);
>> +/* Final PCI Bus number */
>> +build_append_int_noprefix(table_data, PCIE_MMCFG_BUS(info->size - 1), 
>> 1);
>> +/* Reserved */
>> +build_append_int_noprefix(table_data, 0, 4);
>> +
>> +build_header(linker, table_data, (void *)(table_data->data + 
>> mcfg_start),
>> + "MCFG", table_data->len - mcfg_start, 1, NULL, NULL);
>>  }
>>  
>> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
>> index f9aa4bd398..57a3f58b0c 100644
>> --- a/include/hw/acpi/acpi-defs.h
>> +++ b/include/hw/acpi/acpi-defs.h
>> @@ -449,24 +449,6 @@ struct AcpiSratProcessorGiccAffinity {
>>  
>>  typedef struct AcpiSratProcessorGiccAffinity AcpiSratProcessorGiccAffinity;
>>  
>> -/* PCI fw r3.0 MCFG table. */
>> -/* Subtable */
>> -struct AcpiMcfgAllocation {
>> -uint64_t address;/* Base address, processor-relative */
>> -uint16_t pci_segment;/* PCI segment group number */
>> -uint8_t start_bus_number;   /* Starting PCI Bus number */
>> -uint8_t end_bus_number; /* Final PCI Bus number */
>> -uint32_t reserved;
>> -} QEMU_PACKED;
>> -typedef struct AcpiMcfgAllocation AcpiMcfgAllocation;
>> -
>> -struct AcpiTableMcfg {
>> -ACPI_TABLE_HEADER_DEF;
>> -uint8_t reserved[8];

   ^

Thanks Michael for testing...

Wei, can you add a MCFG test in tests/bios-tables-test.c?

Regards,

Phil.

>> -AcpiMcfgAllocation allocation[0];
>> -} QEMU_PACKED;
>> -typedef struct AcpiTableMcfg AcpiTableMcfg;
>> -
>>  /*
>>   * TCPA Description Table
>>   *
>> -- 
>> 2.19.1

[Qemu-devel] [PATCH v2 2/2] iotests: Test unaligned raw images with O_DIRECT

[Max Reitz]

We already have 221 for accesses through the page cache, but it is
better to create a new file for O_DIRECT instead of integrating those
test cases into 221.  This way, we can make use of
_supported_cache_modes (and _default_cache_mode) so the test is
automatically skipped on filesystems that do not support O_DIRECT.

As part of the split, add _supported_cache_modes to 221.  With that, it
no longer fails when run with -c none or -c directsync.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 tests/qemu-iotests/221 |  4 ++
 tests/qemu-iotests/253 | 84 ++
 tests/qemu-iotests/253.out | 14 +++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 103 insertions(+)
 create mode 100755 tests/qemu-iotests/253
 create mode 100644 tests/qemu-iotests/253.out

diff --git a/tests/qemu-iotests/221 b/tests/qemu-iotests/221
index 25dd47bcfe..0e9096fec7 100755
--- a/tests/qemu-iotests/221
+++ b/tests/qemu-iotests/221
@@ -1,6 +1,7 @@
 #!/usr/bin/env bash
 #
 # Test qemu-img vs. unaligned images
+# (See also 253, which is the O_DIRECT version)
 #
 # Copyright (C) 2018-2019 Red Hat, Inc.
 #
@@ -37,6 +38,9 @@ _supported_fmt raw
 _supported_proto file
 _supported_os Linux
 
+_default_cache_mode writeback
+_supported_cache_modes writeback writethrough unsafe
+
 echo
 echo "=== Check mapping of unaligned raw image ==="
 echo
diff --git a/tests/qemu-iotests/253 b/tests/qemu-iotests/253
new file mode 100755
index 00..d88d5afa45
--- /dev/null
+++ b/tests/qemu-iotests/253
@@ -0,0 +1,84 @@
+#!/usr/bin/env bash
+#
+# Test qemu-img vs. unaligned images; O_DIRECT version
+# (Originates from 221)
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt raw
+_supported_proto file
+_supported_os Linux
+
+_default_cache_mode none
+_supported_cache_modes none directsync
+
+echo
+echo "=== Check mapping of unaligned raw image ==="
+echo
+
+# We do not know how large a physical sector is, but it is certainly
+# going to be a factor of 1 MB
+size=$((1 * 1024 * 1024 - 1))
+
+# qemu-img create rounds size up to BDRV_SECTOR_SIZE
+_make_test_img $size
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# so we resize it and check again
+truncate --size=$size "$TEST_IMG"
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# qemu-io with O_DIRECT always writes whole physical sectors.  Again,
+# we do not know how large a physical sector is, so we just start
+# writing from a 64 kB boundary, which should always be aligned.
+offset=$((1 * 1024 * 1024 - 64 * 1024))
+$QEMU_IO -c "w $offset $((size - offset))" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# Resize it and check again -- contrary to 221, we may not get partial
+# sectors here, so there should be only two areas (one zero, one
+# data).
+truncate --size=$size "$TEST_IMG"
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/253.out b/tests/qemu-iotests/253.out
new file mode 100644
index 00..607c0baa0b
--- /dev/null
+++ b/tests/qemu-iotests/253.out
@@ -0,0 +1,14 @@
+QA output created by 253
+
+=== Check mapping of unaligned raw image ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048575
+[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, 
"offset": OFFSET}]
+[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, 
"offset": OFFSET}]
+wrote 65535/65535 bytes at offset 983040
+63.999 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 983040, "depth": 0, "zero": true, "data": false, 
"offset": OFFSET},
+{ 

Re: [Qemu-devel] [PATCH v2] iotests: Filter 175's allocation information

[Max Reitz]

On 13.05.19 17:52, Max Reitz wrote:
> It is possible for an empty file to take up blocks on a filesystem.
> Make iotest 175 take this into account.
> 
> Reported-by: Thomas Huth 
> Signed-off-by: Max Reitz 
> ---
> v2: [Nir]
> - Use a function for filtering
> - s/empty_blocks/extra_blocks/
> ---
>  tests/qemu-iotests/175 | 26 ++
>  tests/qemu-iotests/175.out |  8 
>  2 files changed, 26 insertions(+), 8 deletions(-)

Thanks for the reviews, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 0/2] block/file-posix: Fix unaligned O_DIRECT block status

[Max Reitz]

The user-visible problem:
$ echo > foo
$ qemu-img map --image-opts driver=file,filename=foo,cache.direct=on
Offset  Length  Mapped to   File
qemu-img: block/io.c:2093: bdrv_co_block_status: Assertion `*pnum &&
QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset'
failed.

The internal problem: file-posix truncates status requests to the EOF.
If the EOF is not aligned at the request_alignment,
bdrv_co_block_status() won't like that.

See patch 1 for a deeper discussion (including two possible alternatives
how we could address the problem).
(As I note there, I’ve looked through all block drivers, and I didn’t
find any other which could have the same problem.  gluster uses the same
block-status code, but it doesn’t set a request_alignment.  NBD
force-aligns the server response in nbd_parse_blockstatus_payload().
qcow2... Should be fine as long as no crypto driver has a block limit
exceeding the qcow2 cluster size.  And so on.)

Patch 2 adds a test.  After writing that test, I noticed that we already
had one: 109 fails with -c none before patch 1.  Er, well, at least the
new test is more succinct and has the correct default cache mode, so it
will actually do the test if you run ./check without enforcing any cache
on a filesystem that supports O_DIRECT.


v2:
- Shortened the assert() in patch 1 [Eric]


git backport-diff against v1:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/2:[0003] [FC] 'block/file-posix: Unaligned O_DIRECT block-status'
002/2:[] [--] 'iotests: Test unaligned raw images with O_DIRECT'


Max Reitz (2):
  block/file-posix: Unaligned O_DIRECT block-status
  iotests: Test unaligned raw images with O_DIRECT

 block/file-posix.c | 16 
 tests/qemu-iotests/221 |  4 ++
 tests/qemu-iotests/253 | 84 ++
 tests/qemu-iotests/253.out | 14 +++
 tests/qemu-iotests/group   |  1 +
 5 files changed, 119 insertions(+)
 create mode 100755 tests/qemu-iotests/253
 create mode 100644 tests/qemu-iotests/253.out

-- 
2.21.0

[Qemu-devel] [PATCH v2 1/2] block/file-posix: Unaligned O_DIRECT block-status

[Max Reitz]

Currently, qemu crashes whenever someone queries the block status of an
unaligned image tail of an O_DIRECT image:
$ echo > foo
$ qemu-img map --image-opts driver=file,filename=foo,cache.direct=on
Offset  Length  Mapped to   File
qemu-img: block/io.c:2093: bdrv_co_block_status: Assertion `*pnum &&
QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset'
failed.

This is because bdrv_co_block_status() checks that the result returned
by the driver's implementation is aligned to the request_alignment, but
file-posix can fail to do so, which is actually mentioned in a comment
there: "[...] possibly including a partial sector at EOF".

Fix this by rounding up those partial sectors.

There are two possible alternative fixes:
(1) We could refuse to open unaligned image files with O_DIRECT
altogether.  That sounds reasonable until you realize that qcow2
does necessarily not fill up its metadata clusters, and that nobody
runs qemu-img create with O_DIRECT.  Therefore, unpreallocated qcow2
files usually have an unaligned image tail.

(2) bdrv_co_block_status() could ignore unaligned tails.  It actually
throws away everything past the EOF already, so that sounds
reasonable.
Unfortunately, the block layer knows file lengths only with a
granularity of BDRV_SECTOR_SIZE, so bdrv_co_block_status() usually
would have to guess whether its file length information is inexact
or whether the driver is broken.

Fixing what raw_co_block_status() returns is the safest thing to do.

There seems to be no other block driver that sets request_alignment and
does not make sure that it always returns aligned values.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 block/file-posix.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index e09e15bbf8..d018429672 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2488,6 +2488,8 @@ static int coroutine_fn 
raw_co_block_status(BlockDriverState *bs,
 off_t data = 0, hole = 0;
 int ret;
 
+assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment));
+
 ret = fd_open(bs);
 if (ret < 0) {
 return ret;
@@ -2513,6 +2515,20 @@ static int coroutine_fn 
raw_co_block_status(BlockDriverState *bs,
 /* On a data extent, compute bytes to the end of the extent,
  * possibly including a partial sector at EOF. */
 *pnum = MIN(bytes, hole - offset);
+
+/*
+ * We are not allowed to return partial sectors, though, so
+ * round up if necessary.
+ */
+if (!QEMU_IS_ALIGNED(*pnum, bs->bl.request_alignment)) {
+int64_t file_length = raw_getlength(bs);
+if (file_length > 0) {
+/* Ignore errors, this is just a safeguard */
+assert(hole == file_length);
+}
+*pnum = ROUND_UP(*pnum, bs->bl.request_alignment);
+}
+
 ret = BDRV_BLOCK_DATA;
 } else {
 /* On a hole, compute bytes to the beginning of the next extent.  */
-- 
2.21.0

Re: [Qemu-devel] [PATCH 1/2] block/file-posix: Unaligned O_DIRECT block-status

[Max Reitz]

On 14.05.19 23:50, Eric Blake wrote:
> On 5/14/19 4:42 PM, Max Reitz wrote:
>> Currently, qemu crashes whenever someone queries the block status of an
>> unaligned image tail of an O_DIRECT image:
>> $ echo > foo
>> $ qemu-img map --image-opts driver=file,filename=foo,cache.direct=on
>> Offset  Length  Mapped to   File
>> qemu-img: block/io.c:2093: bdrv_co_block_status: Assertion `*pnum &&
>> QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset'
>> failed.
>>
>> This is because bdrv_co_block_status() checks that the result returned
>> by the driver's implementation is aligned to the request_alignment, but
>> file-posix can fail to do so, which is actually mentioned in a comment
>> there: "[...] possibly including a partial sector at EOF".
>>
>> Fix this by rounding up those partial sectors.
>>
>> There are two possible alternative fixes:
>> (1) We could refuse to open unaligned image files with O_DIRECT
>> altogether.  That sounds reasonable until you realize that qcow2
>> does necessarily not fill up its metadata clusters, and that nobody
>> runs qemu-img create with O_DIRECT.  Therefore, unpreallocated qcow2
>> files usually have an unaligned image tail.
> 
> Yep, non-starter.
> 
>>
>> (2) bdrv_co_block_status() could ignore unaligned tails.  It actually
>> throws away everything past the EOF already, so that sounds
>> reasonable.
>> Unfortunately, the block layer knows file lengths only with a
>> granularity of BDRV_SECTOR_SIZE, so bdrv_co_block_status() usually
>> would have to guess whether its file length information is inexact
>> or whether the driver is broken.
> 
> Well, if I ever get around to my thread of making the block layer honor
> byte-accurate sizes, instead of rounding up, then there is no longer
> than inexactness. I think our mails crossed, and you missed another idea
> of mine of having block drivers (probably only file-posix, per your
> audit) set BDRV_BLOCK_EOF when returning an unaligned answer due to EOF,
> as the key for letting the block layer know whether the unaligned answer
> was due to size rounding.

Yes, that EOF change makes sense, I think.  Not least because right now
the EOF detection in block/io.c has to be a bit wonky considering that
it's inexact...  But to be honest, returning the EOF flag from the
drivers would have required me to modify all drivers.  I felt like maybe
that something to be left for another time. :-)

OTOH, I don’t know whether returning the EOF flag from the drivers would
still sense if we had a byte-accurate bdrv_getlength()...

>> Fixing what raw_co_block_status() returns is the safest thing to do.
> 
> Agree.
> 
>>
>> There seems to be no other block driver that sets request_alignment and
>> does not make sure that it always returns aligned values.
> 
> Thanks for auditing.
> 
>>
>> Cc: qemu-sta...@nongnu.org
>> Signed-off-by: Max Reitz 
>> ---
>>  block/file-posix.c | 17 +
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/block/file-posix.c b/block/file-posix.c
>> index e09e15bbf8..f489a5420c 100644
>> --- a/block/file-posix.c
>> +++ b/block/file-posix.c
>> @@ -2488,6 +2488,9 @@ static int coroutine_fn 
>> raw_co_block_status(BlockDriverState *bs,
>>  off_t data = 0, hole = 0;
>>  int ret;
>>  
>> +assert(QEMU_IS_ALIGNED(offset, bs->bl.request_alignment) &&
>> +   QEMU_IS_ALIGNED(bytes, bs->bl.request_alignment));
>> +
> 
> Can write in one line as:
> 
> assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment));

Ah, yeah, sure, why not.

>>  ret = fd_open(bs);
>>  if (ret < 0) {
>>  return ret;
>> @@ -2513,6 +2516,20 @@ static int coroutine_fn 
>> raw_co_block_status(BlockDriverState *bs,
>>  /* On a data extent, compute bytes to the end of the extent,
>>   * possibly including a partial sector at EOF. */
>>  *pnum = MIN(bytes, hole - offset);
>> +
>> +/*
>> + * We are not allowed to return partial sectors, though, so
>> + * round up if necessary.
>> + */
>> +if (!QEMU_IS_ALIGNED(*pnum, bs->bl.request_alignment)) {
>> +int64_t file_length = raw_getlength(bs);
>> +if (file_length > 0) {
>> +/* Ignore errors, this is just a safeguard */
>> +assert(hole == file_length);
>> +}
>> +*pnum = ROUND_UP(*pnum, bs->bl.request_alignment);
>> +}
> 
> Reviewed-by: Eric Blake 

Thanks!

I'll send a v2 with shorter assert().

Max

> bl.request_alignment is normally 1 (making this a no-op), but is
> definitely larger for O_DIRECT images (where rounding up and treating
> the post-EOF hole the same as the rest of the sector is the same thing
> that NBD chose to do).



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/6] qemu-bridge-helper: Fix misuse of isspace()

[Jason Wang]

On 2019/5/14 下午8:18, Markus Armbruster wrote:

Peter Maydell  writes:


On Mon, 13 May 2019 at 14:21, Markus Armbruster  wrote:

Perhaps I should do it just for this file while I touch it anyway.  The
question to ask: should parse_acl_file() obey the locale for whitespace
recognition?

I vote for "no".

Q: do we document the format of the ACL file anywhere ?

Support for it was added in commit bdef79a2994, v1.1.  Just code, no
documentation.

Grepping for qemu-bridge-helper finds just qemu-options.hx.  Contains
-help output and some .texi that goes into qemu-doc and the manual page.
None of it mentions how qemu-bridge-helper is run, or the ACL file
feature, let alone what its format might be.

I'm afraid all we have is the commit message.  Which doesn't really
define the file format, it merely gives a bunch of examples.

As far as I can tell, qemu-bridge-helper is for use with -netdev tap and
-netdev bridge.

Both variations of -netdev call net_bridge_run_helper() to run the
helper.  First argument is -netdev parameter "helper", default usually
"$prefix/libexec/qemu-bridge-helper".  Second argument is parameter
"br", default "br0".

If @helper contains space or tab, net_bridge_run_helper() guesses its a
full command, else it guesses its the name of the executable.  Bad
magic.

If it guesses name of executable, it execv()s this executable with
arguments "--use-vnet", "--fd=FD", "--br=@bridge".

If it guesses full command, it appends "--use-vnet --fd=FD", where FD is
the helper's half of the socketpair used to connect QEMU and the helper.
It further appends "--br=@bridge", unless @helper contains "--br=".
More bad magic.

It executes the resulting string with sh -c.  Magic cherry on top.

When the helper fails, netdev creation fails.

The helper we ship with QEMU unconditionally tries to read
"$prefix/etc/bridge.conf".  Fatal error if this file doesn't exist.
Errors in this file are fatal.  Errors in files it includes are not
fatal; instead, the remainder of the erroneous file is ignored.
*Boggle*

As far as I can tell, libvirt runs qemu-bridge-helper itself (Paolo's
commit 2d80fbb14df).  Makes sense, because running QEMU with the
necessary privileges would be unwise, and so would be letting it execute
setuid helpers.  Also bypasses the bad magic in QEMU's
net_bridge_run_helper().



I don't notice this before. Is this only for the convenience of 
development? I guess libvirt should have native support like adding port 
to bridge/OVS without the help any external command or script.





qemu-bridge-helper should have a manual page, and its handling of errors
in ACL include files needs work.  There's probably more; I just glanced
at it.  I'm not volunteering, though.  It lacks a maintainer.  Should we
add it to Jason's "Network device backends"?



Yes.




-netdev's helper parameter is seriously underdocumented.  Document or
deprecate?



I believe management should only use fd parameter of TAP. If we have 
other, it should be a duplication. So I suggest to deprecate the bridge 
helper and -netdev bridge.


Thanks

Re: [Qemu-devel] [PATCH] configure: Change capstone's default state to disabled

[Programmingkid]

> On May 12, 2019, at 9:47 AM, Thomas Huth  wrote:
> 
> On 11/05/2019 20.28, Programmingkid wrote:
>> 
>>> On May 11, 2019, at 2:05 PM, Thomas Huth  wrote:
>>> 
>>> On 11/05/2019 19.21, Programmingkid wrote:
 
> On Apr 20, 2019, at 6:40 AM, Thomas Huth  wrote:
> 
> On 19/04/2019 15.44, G 3 wrote:
> [...]
>> Thank you for replying. Capstone comes with QEMU? Every time I try to
>> compile QEMU I see an error relating to Capstone not being on my system.
>> Why do you feel that disabling Capstone by default is not a good idea?
>> 
>> Here is the error message I see when compiling QEMU:
>> 
>> CHK version_gen.h
>> make[1]: *** No rule to make target
>> `/Users/John/qemu-git/capstone/libcapstone.a'.  Stop.
>> make: *** [subdir-capstone] Error 2
> 
> I assume you're using a git checkout here, right? For git checkouts, the
> Makefile should take care of calling the scripts/git-submodule.sh script
> which should initialize the submodule in the capstone directory.
> 
> What's the content of your .git-submodule-status file? What does
> "configure" say about capstone support on your system?
> 
> Thomas
 
 Yes I use a git checkout.
 
 This is the contents of my .git-submodule-status file:
 #!/bin/sh
>>> [...]
>>> 
>>> That were the contents of scripts/git-submodule.sh. I meant the hidden
>>> file .git-submodule-status in the main directory.
>> 
>> This is it:
>> 88f18909db731a627456f26d779445f84e449536 dtc (v1.4.7)
>> f0da6726207b740f6101028b2992f918477a4b08 slirp (v4.0.0-rc0-25-gf0da672)
>> b64af41c3276f97f0e181920400ee056b9c88037 tests/fp/berkeley-softfloat-3 
>> (heads/master)
>> 5a59dcec19327396a011a17fd924aed4fec416b3 tests/fp/berkeley-testfloat-3 
>> (remotes/origin/HEAD)
>> 6b3d716e2b6472eb7189d3220552280ef3d832ce ui/keycodemapdb 
>> (heads/master-4-g6b3d716)
> 
> There should be an entry for capstone in here, too. :-/
> 
 I did a 'make clean' followed by a 'make distclean'. Then tried building 
 again using this command line:
 
 ./configure --target-list=ppc-softmmu,i386-softmmu,x86_64-softmmu
 make -j 4
>>> 
>>> That should normally populate the capstone directory. What happens if
>>> you run "make git-submodule-update" directly?
>> 
>> Here is the result:
>> $ make git-submodule-update
>> make[1]: Nothing to be done for `all'.
>> make[1]: *** No rule to make target 
>> `/Users/John/Documents/Development/Projects/Qemu/qemu-git/capstone/libcapstone.a'.
>>   Stop.
>> make: *** [subdir-capstone] Error 2
> 
> Apparently the submodule update is not working right for you. What do
> you get when you run:
> 
> git submodule update capstone
> 
> ?

Nothing appears to happen. When I try to make QEMU I still see the same error 
about no rule to make target libcapstone.a.

> 
 I took a look at the capstone folder. There is no 'make' file in this 
 folder. Should there be one?
>>> 
>>> Yes, the capstone folder should be populated automatically. Is it
>>> completely empty for you?
>> 
>> It isn't empty. All I see are two folders: obj and docs.
> 
> Maybe try to clean the folder first:
> 
> rm -r capstone
> mkdir capstone
> make git-submodule-update
> 
> If that does not help, maybe try a completely fresh git checkout?

That did it! The capstone folder is now fully populated. Thank you so much.

Re: [Qemu-devel] [PATCH v4 6/6] acpi: pci: use build_append_foo() API to construct MCFG

[Michael S. Tsirkin]

On Fri, Apr 19, 2019 at 08:30:53AM +0800, Wei Yang wrote:
> build_append_foo() API doesn't need explicit endianness conversions
> which eliminates a source of errors and it makes build_mcfg() look like
> declarative definition of MCFG table in ACPI spec, which makes it easy
> to review.
> 
> Signed-off-by: Wei Yang 
> Suggested-by: Igor Mammedov 
> Reviewed-by: Igor Mammedov 
> Reviewed-by: Philippe Mathieu-Daudé 

Causes a regression with an invalid MCFG produced.
Dropped.

> ---
>  hw/acpi/pci.c   | 33 +
>  include/hw/acpi/acpi-defs.h | 18 --
>  2 files changed, 21 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> index fa0fa30bb9..341805e786 100644
> --- a/hw/acpi/pci.c
> +++ b/hw/acpi/pci.c
> @@ -30,17 +30,26 @@
>  
>  void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
>  {
> -AcpiTableMcfg *mcfg;
> -int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
> -
> -mcfg = acpi_data_push(table_data, len);
> -mcfg->allocation[0].address = cpu_to_le64(info->base);
> -
> -/* Only a single allocation so no need to play with segments */
> -mcfg->allocation[0].pci_segment = cpu_to_le16(0);
> -mcfg->allocation[0].start_bus_number = 0;
> -mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->size - 1);
> -
> -build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, 
> NULL);
> +int mcfg_start = table_data->len;
> +
> +acpi_data_push(table_data, sizeof(AcpiTableHeader));
> +
> +/*
> + * PCI Firmware Specification, Revision 3.0
> + * 4.1.2 MCFG Table Description.
> + */
> +/* Base address, processor-relative */
> +build_append_int_noprefix(table_data, info->base, 8);
> +/* PCI segment group number */
> +build_append_int_noprefix(table_data, 0, 2);
> +/* Starting PCI Bus number */
> +build_append_int_noprefix(table_data, 0, 1);
> +/* Final PCI Bus number */
> +build_append_int_noprefix(table_data, PCIE_MMCFG_BUS(info->size - 1), 1);
> +/* Reserved */
> +build_append_int_noprefix(table_data, 0, 4);
> +
> +build_header(linker, table_data, (void *)(table_data->data + mcfg_start),
> + "MCFG", table_data->len - mcfg_start, 1, NULL, NULL);
>  }
>  
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index f9aa4bd398..57a3f58b0c 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -449,24 +449,6 @@ struct AcpiSratProcessorGiccAffinity {
>  
>  typedef struct AcpiSratProcessorGiccAffinity AcpiSratProcessorGiccAffinity;
>  
> -/* PCI fw r3.0 MCFG table. */
> -/* Subtable */
> -struct AcpiMcfgAllocation {
> -uint64_t address;/* Base address, processor-relative */
> -uint16_t pci_segment;/* PCI segment group number */
> -uint8_t start_bus_number;   /* Starting PCI Bus number */
> -uint8_t end_bus_number; /* Final PCI Bus number */
> -uint32_t reserved;
> -} QEMU_PACKED;
> -typedef struct AcpiMcfgAllocation AcpiMcfgAllocation;
> -
> -struct AcpiTableMcfg {
> -ACPI_TABLE_HEADER_DEF;
> -uint8_t reserved[8];
> -AcpiMcfgAllocation allocation[0];
> -} QEMU_PACKED;
> -typedef struct AcpiTableMcfg AcpiTableMcfg;
> -
>  /*
>   * TCPA Description Table
>   *
> -- 
> 2.19.1

Re: [Qemu-devel] [PATCH v2 1/8] linux-user: Disallow setting newsp for fork

[Richard Henderson]

On 5/9/19 8:27 PM, Richard Henderson wrote:
> Or really, just clone devolving into fork.  This should not ever happen
> in practice.  We do want to reserve calling cpu_clone_regs for the case
> in which we are actually performing a clone.
> 
> Reviewed-by: Alex Bennée 
> Signed-off-by: Richard Henderson 
> ---
>  linux-user/syscall.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 96cd4bf86d..f7d0754c8d 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5553,10 +5553,14 @@ static int do_fork(CPUArchState *env, unsigned int 
> flags, abi_ulong newsp,
>  pthread_mutex_destroy();
>  pthread_mutex_unlock(_lock);
>  } else {
> -/* if no CLONE_VM, we consider it is a fork */
> +/* If no CLONE_VM, we consider it is a fork.  */
>  if (flags & CLONE_INVALID_FORK_FLAGS) {
>  return -TARGET_EINVAL;
>  }
> +/* As a fork, setting a new sp does not make sense.  */
> +if (newsp) {
> +return -TARGET_EINVAL;
> +}

This causes failures for aarch64 and riscv.

We have to allow no-op setting of sp as well.
Other targets set newsp to 0 for in vfork.S in glibc.


r~

Re: [Qemu-devel] [PATCH v2 1/1] target/arm: Fix vector operation segfault

[Richard Henderson]

On 5/14/19 5:21 PM, Alistair Francis wrote:
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index dd053c80d6..298c262825 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -6598,13 +6598,13 @@ static int disas_neon_data_insn(DisasContext *s, 
> uint32_t insn)
>  tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),
> rn_ofs, rm_ofs, vec_size, vec_size,
> (u ? uqadd_op : sqadd_op) + size);
> -break;
> +return 0;
>  
>  case NEON_3R_VQSUB:
>  tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),
> rn_ofs, rm_ofs, vec_size, vec_size,
> (u ? uqsub_op : sqsub_op) + size);
> -break;
> +return 0;
>  
>  case NEON_3R_VMUL: /* VMUL */
>  if (u) {

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH v2 1/1] target/arm: Fix vector operation segfault

[Alistair Francis]

Commit 89e68b575 "target/arm: Use vector operations for saturation"
causes this abort() when booting QEMU ARM with a Cortex-A15:

0  0x74c2382f in raise () at /usr/lib/libc.so.6
1  0x74c0e672 in abort () at /usr/lib/libc.so.6
2  0x559c1839 in disas_neon_data_insn (insn=, 
s=) at ./target/arm/translate.c:6673
3  0x559c1839 in disas_neon_data_insn (s=, 
insn=) at ./target/arm/translate.c:6386
4  0x559cd8a4 in disas_arm_insn (insn=4081107068, s=0x7fffe59a9510) at 
./target/arm/translate.c:9289
5  0x559cd8a4 in arm_tr_translate_insn (dcbase=0x7fffe59a9510, 
cpu=) at ./target/arm/translate.c:13612
6  0x558d1d39 in translator_loop (ops=0x561cc580 
, db=0x7fffe59a9510, cpu=0x5686a2f0, tb=, max_insns=) at ./accel/tcg/translator.c:96
7  0x559d10d4 in gen_intermediate_code (cpu=cpu@entry=0x5686a2f0, 
tb=tb@entry=0x7fffd7840080 , 
max_insns=max_insns@entry=512) at ./target/arm/translate.c:13901
8  0x558d06b9 in tb_gen_code (cpu=cpu@entry=0x5686a2f0, 
pc=3067096216, cs_base=0, flags=192, cflags=-16252928, cflags@entry=524288) at 
./accel/tcg/translate-all.c:1736
9  0x558ce467 in tb_find (cf_mask=524288, tb_exit=1, 
last_tb=0x7fffd783e640 , cpu=0x1) at 
./accel/tcg/cpu-exec.c:407
10 0x558ce467 in cpu_exec (cpu=cpu@entry=0x5686a2f0) at 
./accel/tcg/cpu-exec.c:728
11 0x5588b0cf in tcg_cpu_exec (cpu=0x5686a2f0) at ./cpus.c:1431
12 0x5588d223 in qemu_tcg_cpu_thread_fn (arg=0x5686a2f0) at 
./cpus.c:1735
13 0x5588d223 in qemu_tcg_cpu_thread_fn (arg=arg@entry=0x5686a2f0) 
at ./cpus.c:1709
14 0x55d2629a in qemu_thread_start (args=) at 
./util/qemu-thread-posix.c:502
15 0x74db8a92 in start_thread () at /usr/lib/libpthread.

This patch ensures that we don't hit the abort() in the second switch
case in disas_neon_data_insn() as we will return from the first case.

Signed-off-by: Alistair Francis 
---
 target/arm/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index dd053c80d6..298c262825 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -6598,13 +6598,13 @@ static int disas_neon_data_insn(DisasContext *s, 
uint32_t insn)
 tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),
rn_ofs, rm_ofs, vec_size, vec_size,
(u ? uqadd_op : sqadd_op) + size);
-break;
+return 0;
 
 case NEON_3R_VQSUB:
 tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),
rn_ofs, rm_ofs, vec_size, vec_size,
(u ? uqsub_op : sqsub_op) + size);
-break;
+return 0;
 
 case NEON_3R_VMUL: /* VMUL */
 if (u) {
-- 
2.21.0

Re: [Qemu-devel] [PATCH v6 00/25] Add qemu_getrandom and ARMv8.5-RNG etc

[Richard Henderson]

On 5/14/19 2:43 PM, Eric Blake wrote:
>> It didn't occur to me that there was nothing in the object files for the
>> reference.  I'll have to drop the crypto-obj-y patch and come up with a
>> different solution.
> 
> Isn't there a gcc annotation for marking a simple as mandatorily
> included during link?

No.

There's stuff you can mark a single function within an object file that you can
use to avoid the function being elided...

> __attribute__((externally_visible)) sounds promising (it nullifies the
> effects of -fwhole-program, so that a function remains visible even if
> the linker would have otherwise suppressed it)
> 
> __attribute__((used)) also sounds useful (the function must be emitted
> even if it does not appear to be referenced, which may be enough for the
> linker to infer that it is used)

... and you found those.  But those do not affect the linker's behaviour with
.a files at all.

You can force a symbol reference from the ld command-line: -u sym, which can
cause the .o containing sym to be included from the .a file.  But that doesn't
work if there's no global symbol in the .o to reference.

You can force all .o from a .a file to be included, with --whole-archive.  That
is useful when you're using .a files a shorthand for lots and lots of .o files.
 But in our case that would break the use of stubs.

Anyway, see v7 now.


r~

[Qemu-devel] [Bug 1829079] [NEW] Can't build static on ARM (Raspbian)

[Dariusz Zyzański]

Public bug reported:

I am trying to build static QEMU on Raspbian, chrooted into using 
systemd-nspawn with QEMU 4.0.0.
This is how my compiling looks:
https://pastebin.com/PYZYeRCN
Just the problematic part:
https://pastebin.com/7LxWPMxA
How I do the compiling:
https://pastebin.com/pYM17A6R (I plan to share this tutorial when it will work)
It is a coincidence, or the build fails because it cannot find lp11-kit. I did 
some symlinks:
ln -s /usr/lib/arm-linux-gnueabihf/libp11-kit.so.0 /usr/lib/libp11-kit.so.0
ln -s /usr/lib/arm-linux-gnueabihf/libp11-kit.so /usr/lib/libp11-kit.so
(should I also symlink libp11.so and libp11.so.2? I think I have installed all 
required p11 packages!

Git commit hash: git rev-parse HEAD
e329ad2ab72c43b56df88b34954c2c7d839bb373

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1829079

Title:
  Can't build static on ARM (Raspbian)

Status in QEMU:
  New

Bug description:
  I am trying to build static QEMU on Raspbian, chrooted into using 
systemd-nspawn with QEMU 4.0.0.
  This is how my compiling looks:
  https://pastebin.com/PYZYeRCN
  Just the problematic part:
  https://pastebin.com/7LxWPMxA
  How I do the compiling:
  https://pastebin.com/pYM17A6R (I plan to share this tutorial when it will 
work)
  It is a coincidence, or the build fails because it cannot find lp11-kit. I 
did some symlinks:
  ln -s /usr/lib/arm-linux-gnueabihf/libp11-kit.so.0 /usr/lib/libp11-kit.so.0
  ln -s /usr/lib/arm-linux-gnueabihf/libp11-kit.so /usr/lib/libp11-kit.so
  (should I also symlink libp11.so and libp11.so.2? I think I have installed 
all required p11 packages!

  Git commit hash: git rev-parse HEAD
  e329ad2ab72c43b56df88b34954c2c7d839bb373

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1829079/+subscriptions

Re: [Qemu-devel] [PATCH 2/2] iotests: Test unaligned raw images with O_DIRECT

[Eric Blake]

On 5/14/19 4:42 PM, Max Reitz wrote:
> We already have 221 for accesses through the page cache, but it is
> better to create a new file for O_DIRECT instead of integrating those
> test cases into 221.  This way, we can make use of
> _supported_cache_modes (and _default_cache_mode) so the test is
> automatically skipped on filesystems that do not support O_DIRECT.
> 
> As part of the split, add _supported_cache_modes to 221.  With that, it
> no longer fails when run with -c none or -c directsync.
> 
> Signed-off-by: Max Reitz 
> ---
>  tests/qemu-iotests/221 |  4 ++
>  tests/qemu-iotests/253 | 84 ++
>  tests/qemu-iotests/253.out | 14 +++
>  tests/qemu-iotests/group   |  1 +
>  4 files changed, 103 insertions(+)
>  create mode 100755 tests/qemu-iotests/253
>  create mode 100644 tests/qemu-iotests/253.out

Reviewed-by: Eric Blake 

> 
> diff --git a/tests/qemu-iotests/221 b/tests/qemu-iotests/221
> index 25dd47bcfe..0e9096fec7 100755
> --- a/tests/qemu-iotests/221
> +++ b/tests/qemu-iotests/221
> @@ -1,6 +1,7 @@
>  #!/usr/bin/env bash
>  #
>  # Test qemu-img vs. unaligned images
> +# (See also 253, which is the O_DIRECT version)
>  #
>  # Copyright (C) 2018-2019 Red Hat, Inc.
>  #
> @@ -37,6 +38,9 @@ _supported_fmt raw
>  _supported_proto file
>  _supported_os Linux
>  
> +_default_cache_mode writeback
> +_supported_cache_modes writeback writethrough unsafe
> +
>  echo
>  echo "=== Check mapping of unaligned raw image ==="
>  echo
> diff --git a/tests/qemu-iotests/253 b/tests/qemu-iotests/253
> new file mode 100755
> index 00..d88d5afa45
> --- /dev/null
> +++ b/tests/qemu-iotests/253
> @@ -0,0 +1,84 @@
> +#!/usr/bin/env bash
> +#
> +# Test qemu-img vs. unaligned images; O_DIRECT version
> +# (Originates from 221)
> +#
> +# Copyright (C) 2019 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +#
> +
> +seq="$(basename $0)"
> +echo "QA output created by $seq"
> +
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> +_cleanup_test_img
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +_supported_fmt raw
> +_supported_proto file
> +_supported_os Linux
> +
> +_default_cache_mode none
> +_supported_cache_modes none directsync
> +
> +echo
> +echo "=== Check mapping of unaligned raw image ==="
> +echo
> +
> +# We do not know how large a physical sector is, but it is certainly
> +# going to be a factor of 1 MB
> +size=$((1 * 1024 * 1024 - 1))
> +
> +# qemu-img create rounds size up to BDRV_SECTOR_SIZE
> +_make_test_img $size
> +$QEMU_IMG map --output=json --image-opts \
> +
> "driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
> +| _filter_qemu_img_map
> +
> +# so we resize it and check again
> +truncate --size=$size "$TEST_IMG"
> +$QEMU_IMG map --output=json --image-opts \
> +
> "driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
> +| _filter_qemu_img_map
> +
> +# qemu-io with O_DIRECT always writes whole physical sectors.  Again,
> +# we do not know how large a physical sector is, so we just start
> +# writing from a 64 kB boundary, which should always be aligned.
> +offset=$((1 * 1024 * 1024 - 64 * 1024))
> +$QEMU_IO -c "w $offset $((size - offset))" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IMG map --output=json --image-opts \
> +
> "driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
> +| _filter_qemu_img_map
> +
> +# Resize it and check again -- contrary to 221, we may not get partial
> +# sectors here, so there should be only two areas (one zero, one
> +# data).
> +truncate --size=$size "$TEST_IMG"
> +$QEMU_IMG map --output=json --image-opts \
> +
> "driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
> +| _filter_qemu_img_map
> +
> +# success, all done
> +echo '*** done'
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/253.out b/tests/qemu-iotests/253.out
> new file mode 100644
> index 00..607c0baa0b
> --- /dev/null
> +++ b/tests/qemu-iotests/253.out
> @@ -0,0 +1,14 @@
> +QA output created by 253
> +
> +=== Check mapping of unaligned raw image ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048575
> +[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, 

Re: [Qemu-devel] [PATCH 1/2] block/file-posix: Unaligned O_DIRECT block-status

[Eric Blake]

On 5/14/19 4:42 PM, Max Reitz wrote:
> Currently, qemu crashes whenever someone queries the block status of an
> unaligned image tail of an O_DIRECT image:
> $ echo > foo
> $ qemu-img map --image-opts driver=file,filename=foo,cache.direct=on
> Offset  Length  Mapped to   File
> qemu-img: block/io.c:2093: bdrv_co_block_status: Assertion `*pnum &&
> QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset'
> failed.
> 
> This is because bdrv_co_block_status() checks that the result returned
> by the driver's implementation is aligned to the request_alignment, but
> file-posix can fail to do so, which is actually mentioned in a comment
> there: "[...] possibly including a partial sector at EOF".
> 
> Fix this by rounding up those partial sectors.
> 
> There are two possible alternative fixes:
> (1) We could refuse to open unaligned image files with O_DIRECT
> altogether.  That sounds reasonable until you realize that qcow2
> does necessarily not fill up its metadata clusters, and that nobody
> runs qemu-img create with O_DIRECT.  Therefore, unpreallocated qcow2
> files usually have an unaligned image tail.

Yep, non-starter.

> 
> (2) bdrv_co_block_status() could ignore unaligned tails.  It actually
> throws away everything past the EOF already, so that sounds
> reasonable.
> Unfortunately, the block layer knows file lengths only with a
> granularity of BDRV_SECTOR_SIZE, so bdrv_co_block_status() usually
> would have to guess whether its file length information is inexact
> or whether the driver is broken.

Well, if I ever get around to my thread of making the block layer honor
byte-accurate sizes, instead of rounding up, then there is no longer
than inexactness. I think our mails crossed, and you missed another idea
of mine of having block drivers (probably only file-posix, per your
audit) set BDRV_BLOCK_EOF when returning an unaligned answer due to EOF,
as the key for letting the block layer know whether the unaligned answer
was due to size rounding.

> 
> Fixing what raw_co_block_status() returns is the safest thing to do.

Agree.

> 
> There seems to be no other block driver that sets request_alignment and
> does not make sure that it always returns aligned values.

Thanks for auditing.

> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Max Reitz 
> ---
>  block/file-posix.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index e09e15bbf8..f489a5420c 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2488,6 +2488,9 @@ static int coroutine_fn 
> raw_co_block_status(BlockDriverState *bs,
>  off_t data = 0, hole = 0;
>  int ret;
>  
> +assert(QEMU_IS_ALIGNED(offset, bs->bl.request_alignment) &&
> +   QEMU_IS_ALIGNED(bytes, bs->bl.request_alignment));
> +

Can write in one line as:

assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment));

>  ret = fd_open(bs);
>  if (ret < 0) {
>  return ret;
> @@ -2513,6 +2516,20 @@ static int coroutine_fn 
> raw_co_block_status(BlockDriverState *bs,
>  /* On a data extent, compute bytes to the end of the extent,
>   * possibly including a partial sector at EOF. */
>  *pnum = MIN(bytes, hole - offset);
> +
> +/*
> + * We are not allowed to return partial sectors, though, so
> + * round up if necessary.
> + */
> +if (!QEMU_IS_ALIGNED(*pnum, bs->bl.request_alignment)) {
> +int64_t file_length = raw_getlength(bs);
> +if (file_length > 0) {
> +/* Ignore errors, this is just a safeguard */
> +assert(hole == file_length);
> +}
> +*pnum = ROUND_UP(*pnum, bs->bl.request_alignment);
> +}

Reviewed-by: Eric Blake 

bl.request_alignment is normally 1 (making this a no-op), but is
definitely larger for O_DIRECT images (where rounding up and treating
the post-EOF hole the same as the rest of the sector is the same thing
that NBD chose to do).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1746394] Re: No provider of glEGLImageTargetTexture2DOES found with NVIDIA proprietary driver

[Chris Sharp]

I'm hitting this issue on Fedora 30 after an in-place upgrade.  Using
gnome-boxes, I click on a virtual machine to open the console viewer and
it crashes after a hang.  Terminal output looks like this:

[chris@gereon ~]$ gnome-boxes

(gnome-boxes:15640): Gtk-WARNING **: 17:21:17.105: GtkFlowBox with a
model will ignore sort and filter functions

(gnome-boxes:15640): Gtk-WARNING **: 17:21:17.107: GtkListBox with a model will 
ignore sort and filter functions
Memory pressure relief: Total: res = 11759616/11722752/-36864, res+swap = 
7540736/7540736/0
No provider of glEGLImageTargetTexture2DOES found.  Requires one of:
GL extension "GL_OES_EGL_image"
Aborted (core dumped)

nvidia driver version: 418.56

01:00.0 VGA compatible controller: NVIDIA Corporation GM206 [GeForce GTX
960] (rev a1)

Web searches lead me to the closed libepoxy bug posted by the OP.  I'm
happy to provide more details about my system.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1746394

Title:
  No provider of glEGLImageTargetTexture2DOES found with NVIDIA
  proprietary driver

Status in QEMU:
  New

Bug description:
  https://github.com/anholt/libepoxy/issues/148

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1746394/+subscriptions

Re: [Qemu-devel] [PATCH v6 00/25] Add qemu_getrandom and ARMv8.5-RNG etc

[Eric Blake]

On 5/14/19 12:46 PM, Richard Henderson wrote:
> On 5/14/19 9:50 AM, Daniel P. Berrangé wrote:
>> On Tue, May 14, 2019 at 09:14:57AM -0700, Richard Henderson wrote:
>>> Yes, that would do it.  We would need something in the test that forces the
>>> objects into the link.  Without having yet looked at the test cases, any 
>>> ideas?
>>
>> I don't think this is only the test suite. I think it will affect all the
>> binaries we build
> 
> You're right, it does.
> 
> $ nm aarch64-softmmu/qemu-system-aarch64  \
>   | grep qcrypto_tls_creds_x509_register_types
> 
> comes up empty.
> 
> It didn't occur to me that there was nothing in the object files for the
> reference.  I'll have to drop the crypto-obj-y patch and come up with a
> different solution.

Isn't there a gcc annotation for marking a simple as mandatorily
included during link?

/me goes looking...

__attribute__((externally_visible)) sounds promising (it nullifies the
effects of -fwhole-program, so that a function remains visible even if
the linker would have otherwise suppressed it)

__attribute__((used)) also sounds useful (the function must be emitted
even if it does not appear to be referenced, which may be enough for the
linker to infer that it is used)

There may be other tricks, although I didn't go searching very hard.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 2/2] iotests: Test unaligned raw images with O_DIRECT

[Max Reitz]

We already have 221 for accesses through the page cache, but it is
better to create a new file for O_DIRECT instead of integrating those
test cases into 221.  This way, we can make use of
_supported_cache_modes (and _default_cache_mode) so the test is
automatically skipped on filesystems that do not support O_DIRECT.

As part of the split, add _supported_cache_modes to 221.  With that, it
no longer fails when run with -c none or -c directsync.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/221 |  4 ++
 tests/qemu-iotests/253 | 84 ++
 tests/qemu-iotests/253.out | 14 +++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 103 insertions(+)
 create mode 100755 tests/qemu-iotests/253
 create mode 100644 tests/qemu-iotests/253.out

diff --git a/tests/qemu-iotests/221 b/tests/qemu-iotests/221
index 25dd47bcfe..0e9096fec7 100755
--- a/tests/qemu-iotests/221
+++ b/tests/qemu-iotests/221
@@ -1,6 +1,7 @@
 #!/usr/bin/env bash
 #
 # Test qemu-img vs. unaligned images
+# (See also 253, which is the O_DIRECT version)
 #
 # Copyright (C) 2018-2019 Red Hat, Inc.
 #
@@ -37,6 +38,9 @@ _supported_fmt raw
 _supported_proto file
 _supported_os Linux
 
+_default_cache_mode writeback
+_supported_cache_modes writeback writethrough unsafe
+
 echo
 echo "=== Check mapping of unaligned raw image ==="
 echo
diff --git a/tests/qemu-iotests/253 b/tests/qemu-iotests/253
new file mode 100755
index 00..d88d5afa45
--- /dev/null
+++ b/tests/qemu-iotests/253
@@ -0,0 +1,84 @@
+#!/usr/bin/env bash
+#
+# Test qemu-img vs. unaligned images; O_DIRECT version
+# (Originates from 221)
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt raw
+_supported_proto file
+_supported_os Linux
+
+_default_cache_mode none
+_supported_cache_modes none directsync
+
+echo
+echo "=== Check mapping of unaligned raw image ==="
+echo
+
+# We do not know how large a physical sector is, but it is certainly
+# going to be a factor of 1 MB
+size=$((1 * 1024 * 1024 - 1))
+
+# qemu-img create rounds size up to BDRV_SECTOR_SIZE
+_make_test_img $size
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# so we resize it and check again
+truncate --size=$size "$TEST_IMG"
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# qemu-io with O_DIRECT always writes whole physical sectors.  Again,
+# we do not know how large a physical sector is, so we just start
+# writing from a 64 kB boundary, which should always be aligned.
+offset=$((1 * 1024 * 1024 - 64 * 1024))
+$QEMU_IO -c "w $offset $((size - offset))" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# Resize it and check again -- contrary to 221, we may not get partial
+# sectors here, so there should be only two areas (one zero, one
+# data).
+truncate --size=$size "$TEST_IMG"
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/253.out b/tests/qemu-iotests/253.out
new file mode 100644
index 00..607c0baa0b
--- /dev/null
+++ b/tests/qemu-iotests/253.out
@@ -0,0 +1,14 @@
+QA output created by 253
+
+=== Check mapping of unaligned raw image ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048575
+[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, 
"offset": OFFSET}]
+[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, 
"offset": OFFSET}]
+wrote 65535/65535 bytes at offset 983040
+63.999 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 983040, "depth": 0, "zero": true, "data": false, 
"offset": OFFSET},
+{ "start": 983040, "length": 

[Qemu-devel] [PATCH 0/2] block/file-posix: Fix unaligned O_DIRECT block status

[Max Reitz]

The user-visible problem:
$ echo > foo
$ qemu-img map --image-opts driver=file,filename=foo,cache.direct=on
Offset  Length  Mapped to   File
qemu-img: block/io.c:2093: bdrv_co_block_status: Assertion `*pnum &&
QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset'
failed.

The internal problem: file-posix truncates status requests to the EOF.
If the EOF is not aligned at the request_alignment,
bdrv_co_block_status() won't like that.

See patch 1 for a deeper discussion (including two possible alternatives
how we could address the problem).
(As I note there, I’ve looked through all block drivers, and I didn’t
find any other which could have the same problem.  gluster uses the same
block-status code, but it doesn’t set a request_alignment.  NBD
force-aligns the server response in nbd_parse_blockstatus_payload().
qcow2... Should be fine as long as no crypto driver has a block limit
exceeding the qcow2 cluster size.  And so on.)

Patch 2 adds a test.  After writing that test, I noticed that we already
had one: 109 fails with -c none before patch 1.  Er, well, at least the
new test is more succinct and has the correct default cache mode, so it
will actually do the test if you run ./check without enforcing any cache
on a filesystem that supports O_DIRECT.


Max Reitz (2):
  block/file-posix: Unaligned O_DIRECT block-status
  iotests: Test unaligned raw images with O_DIRECT

 block/file-posix.c | 17 
 tests/qemu-iotests/221 |  4 ++
 tests/qemu-iotests/253 | 84 ++
 tests/qemu-iotests/253.out | 14 +++
 tests/qemu-iotests/group   |  1 +
 5 files changed, 120 insertions(+)
 create mode 100755 tests/qemu-iotests/253
 create mode 100644 tests/qemu-iotests/253.out

-- 
2.21.0

[Qemu-devel] [PATCH 1/2] block/file-posix: Unaligned O_DIRECT block-status

[Max Reitz]

Currently, qemu crashes whenever someone queries the block status of an
unaligned image tail of an O_DIRECT image:
$ echo > foo
$ qemu-img map --image-opts driver=file,filename=foo,cache.direct=on
Offset  Length  Mapped to   File
qemu-img: block/io.c:2093: bdrv_co_block_status: Assertion `*pnum &&
QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset'
failed.

This is because bdrv_co_block_status() checks that the result returned
by the driver's implementation is aligned to the request_alignment, but
file-posix can fail to do so, which is actually mentioned in a comment
there: "[...] possibly including a partial sector at EOF".

Fix this by rounding up those partial sectors.

There are two possible alternative fixes:
(1) We could refuse to open unaligned image files with O_DIRECT
altogether.  That sounds reasonable until you realize that qcow2
does necessarily not fill up its metadata clusters, and that nobody
runs qemu-img create with O_DIRECT.  Therefore, unpreallocated qcow2
files usually have an unaligned image tail.

(2) bdrv_co_block_status() could ignore unaligned tails.  It actually
throws away everything past the EOF already, so that sounds
reasonable.
Unfortunately, the block layer knows file lengths only with a
granularity of BDRV_SECTOR_SIZE, so bdrv_co_block_status() usually
would have to guess whether its file length information is inexact
or whether the driver is broken.

Fixing what raw_co_block_status() returns is the safest thing to do.

There seems to be no other block driver that sets request_alignment and
does not make sure that it always returns aligned values.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Reitz 
---
 block/file-posix.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index e09e15bbf8..f489a5420c 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2488,6 +2488,9 @@ static int coroutine_fn 
raw_co_block_status(BlockDriverState *bs,
 off_t data = 0, hole = 0;
 int ret;
 
+assert(QEMU_IS_ALIGNED(offset, bs->bl.request_alignment) &&
+   QEMU_IS_ALIGNED(bytes, bs->bl.request_alignment));
+
 ret = fd_open(bs);
 if (ret < 0) {
 return ret;
@@ -2513,6 +2516,20 @@ static int coroutine_fn 
raw_co_block_status(BlockDriverState *bs,
 /* On a data extent, compute bytes to the end of the extent,
  * possibly including a partial sector at EOF. */
 *pnum = MIN(bytes, hole - offset);
+
+/*
+ * We are not allowed to return partial sectors, though, so
+ * round up if necessary.
+ */
+if (!QEMU_IS_ALIGNED(*pnum, bs->bl.request_alignment)) {
+int64_t file_length = raw_getlength(bs);
+if (file_length > 0) {
+/* Ignore errors, this is just a safeguard */
+assert(hole == file_length);
+}
+*pnum = ROUND_UP(*pnum, bs->bl.request_alignment);
+}
+
 ret = BDRV_BLOCK_DATA;
 } else {
 /* On a hole, compute bytes to the beginning of the next extent.  */
-- 
2.21.0

Re: [Qemu-devel] Unaligned images with O_DIRECT

[Eric Blake]

On 5/14/19 12:28 PM, Max Reitz wrote:

>>>
>>> The tail of an unaligned file is generally inaccessible to O_DIRECT,
>>
>> Especially with this.
>>
>>> where it is easier to use ftruncate() up to an aligned boundary if you
>>> really must play with that region of the file, and then ftruncate() back
>>> to the intended size after I/O. But that sounds hairy.  We could also
>>> round down and silently ignore the tail of the file, but that is at odds
>>> with our practice of rounding size up.  So for the short term, I'd be
>>> happy with a patch that just rejects any attempt to use cache.direct=on
>>> (O_DIRECT) with a file that is not already a multiple of the alignment
>>> required thereby. (For reference, that's what qemu as NBD client
>>> recently did when talking to a server that advertises a size
>>> inconsistent with forced minimum block access: commit 3add3ab7)
>>
>> OK, I’ll send a patch.  Thanks for you explanation!
> Well, or maybe not.
> 
> $ ./qemu-img create -f qcow2 foo.qcow2 64M
> $ ./qemu-img map --image-opts \
> driver=qcow2,file.filename=foo.qcow2,cache.direct=on
> qemu-img: Could not open
> 'driver=qcow2,file.filename=foo.qcow2,cache.direct=on': File length
> (196616 bytes) is not a multiple of the O_DIRECT alignment (512 bytes)
> Try cache.direct=off, or increasing the file size to match the alignment
> 
> That may be considered a bug in qcow2.  Maybe it should always fill all
> clusters.  But even if we did so and fixed it now, we can’t disallow
> qemu from opening such images.
> 
> Also, well, the tail is accessible, we just need to access it with the
> proper alignment (and then we get a short read).  This seems to be
> handled just fine.

Oh. Yeah, short reads with O_DIRECT are possible (short writes not so
much; for those, you have to write a full buffer then ftruncate back
down). But we DO want to support short reads because of pre-existing
images, whether or not we also improve qcow2 to always create aligned
image sizes. The qcow2 spec allows unaligned images, even if we quit
creating new ones.

> 
> So I think file-posix should just return a rounded result.  Well, or
> bdrv_co_Block_status() could ignore it for the EOF, because it throws
> away everything past the EOF anyway with:
> 
> if (*pnum > bytes) {
> *pnum = bytes;
> }
> 
> On one hand, I agree that file-posix should return an aligned result.
> On the other, it doesn’t make a difference, so I don’t think we need to
> enforce it (at EOF).

My thoughts:

Right now, only io.c sets (or even reads) BDRV_BLOCK_EOF, and it is
documented as an internal flag for optimizations.  But it would be very
easy to amend the contract of driver's .bdrv_co_block_status to state
that a driver may set BDRV_BLOCK_EOF at the end of a file, and MUST set
that flag if the end of the file also happens to be unaligned with
respect to the driver's request_alignment. (Most drivers won't need to
care, but file-posix.c under O_DIRECT would have to start caring).  Then
fix io.c to relax the assertion - the result must either be aligned
(current condition) OR the driver must have reported BDRV_BLOCK_EOF (new
condition). At that point, the block layer can take care of rounding out
the block status for the unaligned tail beyond EOF up to the alignment
boundary (similar to the rounding I have proposed in my other patches).
If you don't get to that first, then it looks like I'll have to fold
that in to my v2 patches when I get back to addressing those block
status alignment problems.

Thanks again for testing, and forcing me to think about the issue.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC v3] s390/css: handle CCW_FLAG_SKIP

[Eric Farman]

On 5/8/19 5:12 AM, Cornelia Huck wrote:

If a ccw has CCW_FLAG_SKIP set, and the command is of type
read, read backwards, or sense, no data should be written
to the guest for that command.

Signed-off-by: Cornelia Huck 
---

v2 -> v3: fixed checks even more [Pierre]
v1 -> v2: fixed checks for command type [Eric]

---
  hw/s390x/css.c | 22 ++
  include/hw/s390x/css.h |  1 +
  2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 8fc9e35ba5d3..0fbaa233ffb5 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -830,8 +830,12 @@ static int ccw_dstream_rw_noflags(CcwDataStream *cds, void 
*buff, int len,
  if (op == CDS_OP_A) {
  goto incr;
  }
-ret = address_space_rw(_space_memory, cds->cda,
-   MEMTXATTRS_UNSPECIFIED, buff, len, op);
+if (!cds->do_skip) {
+ret = address_space_rw(_space_memory, cds->cda,
+   MEMTXATTRS_UNSPECIFIED, buff, len, op);
+} else {
+ret = 0;


Maybe set this to MEMTX_OK (defined as zero), just so it's clear based 
on the existing check below?



+}
  if (ret != MEMTX_OK) {
  cds->flags |= CDS_F_STREAM_BROKEN;
  return -EINVAL;
@@ -928,8 +932,13 @@ static int ccw_dstream_rw_ida(CcwDataStream *cds, void 
*buff, int len,
  do {
  iter_len = MIN(len, cont_left);
  if (op != CDS_OP_A) {
-ret = address_space_rw(_space_memory, cds->cda,
-   MEMTXATTRS_UNSPECIFIED, buff, iter_len, op);
+if (!cds->do_skip) {
+ret = address_space_rw(_space_memory, cds->cda,
+   MEMTXATTRS_UNSPECIFIED, buff, iter_len,
+   op);
+} else {
+ret = 0;


(here too)

Either way, this seems reasonable to me.

Reviewed-by: Eric Farman 


+}
  if (ret != MEMTX_OK) {
  /* assume inaccessible address */
  ret = -EINVAL; /* channel program check */
@@ -968,6 +977,11 @@ void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, 
ORB const *orb)
  
  cds->count = ccw->count;

  cds->cda_orig = ccw->cda;
+/* skip is only effective for read, read backwards, or sense commands */
+cds->do_skip = (ccw->flags & CCW_FLAG_SKIP) &&
+((ccw->cmd_code & 0x0f) == CCW_CMD_BASIC_SENSE ||
+ (ccw->cmd_code & 0x03) == 0x02 /* read */ ||
+ (ccw->cmd_code & 0x0f) == 0x0c /* read backwards */);
  ccw_dstream_rewind(cds);
  if (!(cds->flags & CDS_F_IDA)) {
  cds->op_handler = ccw_dstream_rw_noflags;
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index aae19c427229..7cc183ef4366 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -97,6 +97,7 @@ typedef struct CcwDataStream {
  int (*op_handler)(struct CcwDataStream *cds, void *buff, int len,
CcwDataStreamOp op);
  hwaddr cda;
+bool do_skip;
  } CcwDataStream;
  
  /*

[Qemu-devel] [PATCH 0/9] target/xtensa: implement options for modern cores

[Max Filippov]

Hello,

this series implements options used by the modern xtensa cores: memory
protection unit, block prefetch and exclusive access and adds special
register definitions and IRQ types for a few other options not fully
implemented: ECC/parity, scatter/gather and IDMA.

Max Filippov (9):
  target/xtensa: get rid of centralized SR properties
  target/xtensa: make internal MMU functions static
  target/xtensa: define IDMA and gather/scatter IRQ types
  target/xtensa: add parity/ECC option SRs
  target/xtensa: implement MPU option
  target/xtensa: implement DIWBUI.P opcode
  target/xtensa: implement block prefetch option opcodes
  target/xtensa: update list of exception causes
  target/xtensa: implement exclusive access option

 target/xtensa/cpu.c   |2 +-
 target/xtensa/cpu.h   |   58 +-
 target/xtensa/helper.c|1 +
 target/xtensa/helper.h|6 +
 target/xtensa/mmu_helper.c|  532 ++-
 target/xtensa/op_helper.c |   42 +
 target/xtensa/overlay_tool.h  |   43 +-
 target/xtensa/translate.c | 2951 -
 tests/tcg/xtensa/test_exclusive.S |   48 +
 9 files changed, 2574 insertions(+), 1109 deletions(-)
 create mode 100644 tests/tcg/xtensa/test_exclusive.S

-- 
2.11.0

[Qemu-devel] [PATCH 2/9] target/xtensa: make internal MMU functions static

[Max Filippov]

Remove declarations of the internal mmu_helper functions from the cpu.h,
make these functions static and shuffle them.

Signed-off-by: Max Filippov 
---
 target/xtensa/cpu.h|  19 --
 target/xtensa/mmu_helper.c | 163 -
 2 files changed, 87 insertions(+), 95 deletions(-)

diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index 539033fccb61..502d41688365 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -658,17 +658,6 @@ static inline int xtensa_get_cring(const CPUXtensaState 
*env)
 }
 
 #ifndef CONFIG_USER_ONLY
-uint32_t xtensa_tlb_get_addr_mask(const CPUXtensaState *env,
-  bool dtlb, uint32_t way);
-void split_tlb_entry_spec_way(const CPUXtensaState *env, uint32_t v, bool dtlb,
-uint32_t *vpn, uint32_t wi, uint32_t *ei);
-int xtensa_tlb_lookup(const CPUXtensaState *env, uint32_t addr, bool dtlb,
-uint32_t *pwi, uint32_t *pei, uint8_t *pring);
-void xtensa_tlb_set_entry_mmu(const CPUXtensaState *env,
-xtensa_tlb_entry *entry, bool dtlb,
-unsigned wi, unsigned ei, uint32_t vpn, uint32_t pte);
-void xtensa_tlb_set_entry(CPUXtensaState *env, bool dtlb,
-unsigned wi, unsigned ei, uint32_t vpn, uint32_t pte);
 int xtensa_get_physical_addr(CPUXtensaState *env, bool update_tlb,
 uint32_t vaddr, int is_write, int mmu_idx,
 uint32_t *paddr, uint32_t *page_size, unsigned *access);
@@ -679,14 +668,6 @@ static inline MemoryRegion 
*xtensa_get_er_region(CPUXtensaState *env)
 {
 return env->system_er;
 }
-
-static inline xtensa_tlb_entry *xtensa_tlb_get_entry(CPUXtensaState *env,
-bool dtlb, unsigned wi, unsigned ei)
-{
-return dtlb ?
-env->dtlb[wi] + ei :
-env->itlb[wi] + ei;
-}
 #endif
 
 static inline uint32_t xtensa_replicate_windowstart(CPUXtensaState *env)
diff --git a/target/xtensa/mmu_helper.c b/target/xtensa/mmu_helper.c
index 79a10da2310e..465cfbf61359 100644
--- a/target/xtensa/mmu_helper.c
+++ b/target/xtensa/mmu_helper.c
@@ -78,8 +78,8 @@ static uint32_t get_page_size(const CPUXtensaState *env,
 /*!
  * Get bit mask for the virtual address bits translated by the TLB way
  */
-uint32_t xtensa_tlb_get_addr_mask(const CPUXtensaState *env,
-  bool dtlb, uint32_t way)
+static uint32_t xtensa_tlb_get_addr_mask(const CPUXtensaState *env,
+ bool dtlb, uint32_t way)
 {
 if (xtensa_option_enabled(env->config, XTENSA_OPTION_MMU)) {
 bool varway56 = dtlb ?
@@ -145,8 +145,9 @@ static uint32_t get_vpn_mask(const CPUXtensaState *env, 
bool dtlb, uint32_t way)
  * Split virtual address into VPN (with index) and entry index
  * for the given TLB way
  */
-void split_tlb_entry_spec_way(const CPUXtensaState *env, uint32_t v, bool dtlb,
-uint32_t *vpn, uint32_t wi, uint32_t *ei)
+static void split_tlb_entry_spec_way(const CPUXtensaState *env, uint32_t v,
+ bool dtlb, uint32_t *vpn,
+ uint32_t wi, uint32_t *ei)
 {
 bool varway56 = dtlb ?
 env->config->dtlb.varway56 :
@@ -213,6 +214,14 @@ static void split_tlb_entry_spec(CPUXtensaState *env, 
uint32_t v, bool dtlb,
 }
 }
 
+static xtensa_tlb_entry *xtensa_tlb_get_entry(CPUXtensaState *env, bool dtlb,
+  unsigned wi, unsigned ei)
+{
+return dtlb ?
+env->dtlb[wi] + ei :
+env->itlb[wi] + ei;
+}
+
 static xtensa_tlb_entry *get_tlb_entry(CPUXtensaState *env,
 uint32_t v, bool dtlb, uint32_t *pwi)
 {
@@ -227,65 +236,10 @@ static xtensa_tlb_entry *get_tlb_entry(CPUXtensaState 
*env,
 return xtensa_tlb_get_entry(env, dtlb, wi, ei);
 }
 
-uint32_t HELPER(rtlb0)(CPUXtensaState *env, uint32_t v, uint32_t dtlb)
-{
-if (xtensa_option_enabled(env->config, XTENSA_OPTION_MMU)) {
-uint32_t wi;
-const xtensa_tlb_entry *entry = get_tlb_entry(env, v, dtlb, );
-return (entry->vaddr & get_vpn_mask(env, dtlb, wi)) | entry->asid;
-} else {
-return v & REGION_PAGE_MASK;
-}
-}
-
-uint32_t HELPER(rtlb1)(CPUXtensaState *env, uint32_t v, uint32_t dtlb)
-{
-const xtensa_tlb_entry *entry = get_tlb_entry(env, v, dtlb, NULL);
-return entry->paddr | entry->attr;
-}
-
-void HELPER(itlb)(CPUXtensaState *env, uint32_t v, uint32_t dtlb)
-{
-if (xtensa_option_enabled(env->config, XTENSA_OPTION_MMU)) {
-uint32_t wi;
-xtensa_tlb_entry *entry = get_tlb_entry(env, v, dtlb, );
-if (entry->variable && entry->asid) {
-tlb_flush_page(CPU(xtensa_env_get_cpu(env)), entry->vaddr);
-entry->asid = 0;
-}
-}
-}
-
-uint32_t HELPER(ptlb)(CPUXtensaState *env, uint32_t v, uint32_t dtlb)
-{
-if (xtensa_option_enabled(env->config, XTENSA_OPTION_MMU)) {
-uint32_t wi;
-uint32_t ei;
-uint8_t ring;
-int res = xtensa_tlb_lookup(env, v, dtlb, , , );
-
- 

[Qemu-devel] [PATCH 7/9] target/xtensa: implement block prefetch option opcodes

[Max Filippov]

Block prefetch option adds a bunch of non-privileged opcodes that may be
implemented as nops since QEMU doesn't model caches.

Signed-off-by: Max Filippov 
---
 target/xtensa/translate.c | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 24eb70d619d5..356eb9948701 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -3078,6 +3078,9 @@ static const XtensaOpcodeOps core_ops[] = {
 .translate = translate_dcache,
 .op_flags = XTENSA_OP_PRIVILEGED,
 }, {
+.name = "dhi.b",
+.translate = translate_nop,
+}, {
 .name = "dhu",
 .translate = translate_dcache,
 .op_flags = XTENSA_OP_PRIVILEGED,
@@ -3085,9 +3088,15 @@ static const XtensaOpcodeOps core_ops[] = {
 .name = "dhwb",
 .translate = translate_dcache,
 }, {
+.name = "dhwb.b",
+.translate = translate_nop,
+}, {
 .name = "dhwbi",
 .translate = translate_dcache,
 }, {
+.name = "dhwbi.b",
+.translate = translate_nop,
+}, {
 .name = "dii",
 .translate = translate_nop,
 .op_flags = XTENSA_OP_PRIVILEGED,
@@ -3112,15 +3121,33 @@ static const XtensaOpcodeOps core_ops[] = {
 .translate = translate_dcache,
 .op_flags = XTENSA_OP_PRIVILEGED,
 }, {
+.name = "dpfm.b",
+.translate = translate_nop,
+}, {
+.name = "dpfm.bf",
+.translate = translate_nop,
+}, {
 .name = "dpfr",
 .translate = translate_nop,
 }, {
+.name = "dpfr.b",
+.translate = translate_nop,
+}, {
+.name = "dpfr.bf",
+.translate = translate_nop,
+}, {
 .name = "dpfro",
 .translate = translate_nop,
 }, {
 .name = "dpfw",
 .translate = translate_nop,
 }, {
+.name = "dpfw.b",
+.translate = translate_nop,
+}, {
+.name = "dpfw.bf",
+.translate = translate_nop,
+}, {
 .name = "dpfwo",
 .translate = translate_nop,
 }, {
@@ -3628,6 +3655,21 @@ static const XtensaOpcodeOps core_ops[] = {
 .par = (const uint32_t[]){true},
 .op_flags = XTENSA_OP_PRIVILEGED,
 }, {
+.name = "pfend.a",
+.translate = translate_nop,
+}, {
+.name = "pfend.o",
+.translate = translate_nop,
+}, {
+.name = "pfnxt.f",
+.translate = translate_nop,
+}, {
+.name = "pfwait.a",
+.translate = translate_nop,
+}, {
+.name = "pfwait.o",
+.translate = translate_nop,
+}, {
 .name = "pitlb",
 .translate = translate_ptlb,
 .par = (const uint32_t[]){false},
-- 
2.11.0

[Qemu-devel] [PATCH 3/9] target/xtensa: define IDMA and gather/scatter IRQ types

[Max Filippov]

IDMA and scatter/gather features introduced new IRQ types that
overlay_tool.h need to initialize Xtensa configuration.

Signed-off-by: Max Filippov 
---
 target/xtensa/cpu.h  | 3 +++
 target/xtensa/overlay_tool.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index 502d41688365..d4258fcc6199 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -298,6 +298,9 @@ typedef enum {
 INTTYPE_DEBUG,
 INTTYPE_WRITE_ERR,
 INTTYPE_PROFILING,
+INTTYPE_IDMA_DONE,
+INTTYPE_IDMA_ERR,
+INTTYPE_GS_ERR,
 INTTYPE_MAX
 } interrupt_type;
 
diff --git a/target/xtensa/overlay_tool.h b/target/xtensa/overlay_tool.h
index ea07576bc921..8b380ce5e329 100644
--- a/target/xtensa/overlay_tool.h
+++ b/target/xtensa/overlay_tool.h
@@ -200,6 +200,9 @@
 #define XTHAL_INTTYPE_TBD2 INTTYPE_WRITE_ERR
 #define XTHAL_INTTYPE_WRITE_ERROR INTTYPE_WRITE_ERR
 #define XTHAL_INTTYPE_PROFILING INTTYPE_PROFILING
+#define XTHAL_INTTYPE_IDMA_DONE INTTYPE_IDMA_DONE
+#define XTHAL_INTTYPE_IDMA_ERR INTTYPE_IDMA_ERR
+#define XTHAL_INTTYPE_GS_ERR INTTYPE_GS_ERR
 
 
 #define INTERRUPT(i) { \
-- 
2.11.0

[Qemu-devel] [PATCH 6/9] target/xtensa: implement DIWBUI.P opcode

[Max Filippov]

This is a recent addition to the set of data cache opcodes.

Signed-off-by: Max Filippov 
---
 target/xtensa/cpu.h  |  1 +
 target/xtensa/overlay_tool.h |  1 +
 target/xtensa/translate.c| 10 ++
 3 files changed, 12 insertions(+)

diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index d6e6bf6ca183..ba4ef2b6a729 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -466,6 +466,7 @@ struct XtensaConfig {
 
 unsigned icache_ways;
 unsigned dcache_ways;
+unsigned dcache_line_bytes;
 uint32_t memctl_mask;
 
 XtensaMemory instrom;
diff --git a/target/xtensa/overlay_tool.h b/target/xtensa/overlay_tool.h
index b61c92539861..4925b21f0edf 100644
--- a/target/xtensa/overlay_tool.h
+++ b/target/xtensa/overlay_tool.h
@@ -425,6 +425,7 @@
 #define CACHE_SECTION \
 .icache_ways = XCHAL_ICACHE_WAYS, \
 .dcache_ways = XCHAL_DCACHE_WAYS, \
+.dcache_line_bytes = XCHAL_DCACHE_LINESIZE, \
 .memctl_mask = \
 (XCHAL_ICACHE_SIZE ? MEMCTL_IUSEWAYS_MASK : 0) | \
 (XCHAL_DCACHE_SIZE ? \
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 782f2ec62099..24eb70d619d5 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -1620,6 +1620,12 @@ static void translate_depbits(DisasContext *dc, const 
OpcodeArg arg[],
 arg[2].imm, arg[3].imm);
 }
 
+static void translate_diwbuip(DisasContext *dc, const OpcodeArg arg[],
+  const uint32_t par[])
+{
+tcg_gen_addi_i32(arg[0].out, arg[0].in, dc->config->dcache_line_bytes);
+}
+
 static bool test_ill_entry(DisasContext *dc, const OpcodeArg arg[],
const uint32_t par[])
 {
@@ -3098,6 +3104,10 @@ static const XtensaOpcodeOps core_ops[] = {
 .translate = translate_nop,
 .op_flags = XTENSA_OP_PRIVILEGED,
 }, {
+.name = "diwbui.p",
+.translate = translate_diwbuip,
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
 .name = "dpfl",
 .translate = translate_dcache,
 .op_flags = XTENSA_OP_PRIVILEGED,
-- 
2.11.0

[Qemu-devel] [PATCH 4/9] target/xtensa: add parity/ECC option SRs

[Max Filippov]

Add SRs and rsr/wsr/xsr opcodes defined by the parity/ECC xtensa option.
The implementation is trivial since we don't emulate parity/ECC yet.

Signed-off-by: Max Filippov 
---
 target/xtensa/cpu.h  |   6 ++
 target/xtensa/overlay_tool.h |   2 +
 target/xtensa/translate.c| 162 +++
 3 files changed, 170 insertions(+)

diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index d4258fcc6199..74ee7d125360 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -144,6 +144,12 @@ enum {
 CACHEATTR = 98,
 ATOMCTL = 99,
 DDR = 104,
+MEPC = 106,
+MEPS = 107,
+MESAVE = 108,
+MESR = 109,
+MECR = 110,
+MEVADDR = 111,
 IBREAKA = 128,
 DBREAKA = 144,
 DBREAKC = 160,
diff --git a/target/xtensa/overlay_tool.h b/target/xtensa/overlay_tool.h
index 8b380ce5e329..ffaab4b094cc 100644
--- a/target/xtensa/overlay_tool.h
+++ b/target/xtensa/overlay_tool.h
@@ -112,6 +112,8 @@
 XCHAL_OPTION(XCHAL_DCACHE_LINE_LOCKABLE, \
 XTENSA_OPTION_DCACHE_INDEX_LOCK) | \
 XCHAL_OPTION(XCHAL_UNALIGNED_LOAD_HW, XTENSA_OPTION_HW_ALIGNMENT) | \
+XCHAL_OPTION(XCHAL_HAVE_MEM_ECC_PARITY, \
+ XTENSA_OPTION_MEMORY_ECC_PARITY) | \
 /* Memory protection and translation */ \
 XCHAL_OPTION(XCHAL_HAVE_MIMIC_CACHEATTR, \
 XTENSA_OPTION_REGION_PROTECTION) | \
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 100d6e126590..63a90fdd17dc 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -4216,6 +4216,60 @@ static const XtensaOpcodeOps core_ops[] = {
 .par = (const uint32_t[]){MEMCTL},
 .op_flags = XTENSA_OP_PRIVILEGED,
 }, {
+.name = "rsr.mecr",
+.translate = translate_rsr,
+.test_ill = test_ill_sr,
+.par = (const uint32_t[]){
+MECR,
+XTENSA_OPTION_MEMORY_ECC_PARITY,
+},
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
+.name = "rsr.mepc",
+.translate = translate_rsr,
+.test_ill = test_ill_sr,
+.par = (const uint32_t[]){
+MEPC,
+XTENSA_OPTION_MEMORY_ECC_PARITY,
+},
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
+.name = "rsr.meps",
+.translate = translate_rsr,
+.test_ill = test_ill_sr,
+.par = (const uint32_t[]){
+MEPS,
+XTENSA_OPTION_MEMORY_ECC_PARITY,
+},
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
+.name = "rsr.mesave",
+.translate = translate_rsr,
+.test_ill = test_ill_sr,
+.par = (const uint32_t[]){
+MESAVE,
+XTENSA_OPTION_MEMORY_ECC_PARITY,
+},
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
+.name = "rsr.mesr",
+.translate = translate_rsr,
+.test_ill = test_ill_sr,
+.par = (const uint32_t[]){
+MESR,
+XTENSA_OPTION_MEMORY_ECC_PARITY,
+},
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
+.name = "rsr.mevaddr",
+.translate = translate_rsr,
+.test_ill = test_ill_sr,
+.par = (const uint32_t[]){
+MESR,
+XTENSA_OPTION_MEMORY_ECC_PARITY,
+},
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
 .name = "rsr.misc0",
 .translate = translate_rsr,
 .test_ill = test_ill_sr,
@@ -5036,6 +5090,60 @@ static const XtensaOpcodeOps core_ops[] = {
 .par = (const uint32_t[]){MEMCTL},
 .op_flags = XTENSA_OP_PRIVILEGED,
 }, {
+.name = "wsr.mecr",
+.translate = translate_wsr,
+.test_ill = test_ill_sr,
+.par = (const uint32_t[]){
+MECR,
+XTENSA_OPTION_MEMORY_ECC_PARITY,
+},
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
+.name = "wsr.mepc",
+.translate = translate_wsr,
+.test_ill = test_ill_sr,
+.par = (const uint32_t[]){
+MEPC,
+XTENSA_OPTION_MEMORY_ECC_PARITY,
+},
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
+.name = "wsr.meps",
+.translate = translate_wsr,
+.test_ill = test_ill_sr,
+.par = (const uint32_t[]){
+MEPS,
+XTENSA_OPTION_MEMORY_ECC_PARITY,
+},
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
+.name = "wsr.mesave",
+.translate = translate_wsr,
+.test_ill = test_ill_sr,
+.par = (const uint32_t[]){
+MESAVE,
+XTENSA_OPTION_MEMORY_ECC_PARITY,
+},
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
+.name = "wsr.mesr",
+.translate = translate_wsr,
+.test_ill = test_ill_sr,
+.par = (const uint32_t[]){
+MESR,
+XTENSA_OPTION_MEMORY_ECC_PARITY,
+},
+.op_flags = XTENSA_OP_PRIVILEGED,
+}, {
+.name = "wsr.mevaddr",
+.translate = translate_wsr,
+ 

[Qemu-devel] [PATCH 5/9] target/xtensa: implement MPU option

[Max Filippov]

The Memory Protection Unit Option (MPU) is a combined instruction and
data memory protection unit with more protection flexibility than the
Region Protection Option or the Region Translation Option but without
any translation capability. It does no demand paging and does not
reference a memory-based page table.

Add memory protection unit option, internal state, SRs and opcodes.
Implement MPU entries dumping in dump_mmu.

Signed-off-by: Max Filippov 
---
 target/xtensa/cpu.c  |   1 -
 target/xtensa/cpu.h  |  17 ++
 target/xtensa/helper.h   |   5 +
 target/xtensa/mmu_helper.c   | 369 +++
 target/xtensa/overlay_tool.h |  29 
 target/xtensa/translate.c| 146 +
 6 files changed, 566 insertions(+), 1 deletion(-)

diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index a54dbe42602d..4215a1881ec7 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -78,7 +78,6 @@ static void xtensa_cpu_reset(CPUState *s)
 env->sregs[VECBASE] = env->config->vecbase;
 env->sregs[IBREAKENABLE] = 0;
 env->sregs[MEMCTL] = MEMCTL_IL0EN & env->config->memctl_mask;
-env->sregs[CACHEATTR] = 0x;
 env->sregs[ATOMCTL] = xtensa_option_enabled(env->config,
 XTENSA_OPTION_ATOMCTL) ? 0x28 : 0x15;
 env->sregs[CONFIGID0] = env->config->configid[0];
diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index 74ee7d125360..d6e6bf6ca183 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -99,6 +99,7 @@ enum {
 /* Memory protection and translation */
 XTENSA_OPTION_REGION_PROTECTION,
 XTENSA_OPTION_REGION_TRANSLATION,
+XTENSA_OPTION_MPU,
 XTENSA_OPTION_MMU,
 XTENSA_OPTION_CACHEATTR,
 
@@ -137,11 +138,15 @@ enum {
 PTEVADDR = 83,
 MMID = 89,
 RASID = 90,
+MPUENB = 90,
 ITLBCFG = 91,
 DTLBCFG = 92,
+MPUCFG = 92,
+ERACCESS = 95,
 IBREAKENABLE = 96,
 MEMCTL = 97,
 CACHEATTR = 98,
+CACHEADRDIS = 98,
 ATOMCTL = 99,
 DDR = 104,
 MEPC = 106,
@@ -234,6 +239,7 @@ enum {
 #define MAX_TLB_WAY_SIZE 8
 #define MAX_NDBREAK 2
 #define MAX_NMEMORY 4
+#define MAX_MPU_FOREGROUND_SEGMENTS 32
 
 #define REGION_PAGE_MASK 0xe000
 
@@ -327,6 +333,11 @@ typedef struct xtensa_tlb {
 unsigned nrefillentries;
 } xtensa_tlb;
 
+typedef struct xtensa_mpu_entry {
+uint32_t vaddr;
+uint32_t attr;
+} xtensa_mpu_entry;
+
 typedef struct XtensaGdbReg {
 int targno;
 unsigned flags;
@@ -477,6 +488,11 @@ struct XtensaConfig {
 
 xtensa_tlb itlb;
 xtensa_tlb dtlb;
+
+uint32_t mpu_align;
+unsigned n_mpu_fg_segments;
+unsigned n_mpu_bg_segments;
+const xtensa_mpu_entry *mpu_bg;
 };
 
 typedef struct XtensaConfigList {
@@ -513,6 +529,7 @@ typedef struct CPUXtensaState {
 #ifndef CONFIG_USER_ONLY
 xtensa_tlb_entry itlb[7][MAX_TLB_WAY_SIZE];
 xtensa_tlb_entry dtlb[10][MAX_TLB_WAY_SIZE];
+xtensa_mpu_entry mpu_fg[MAX_MPU_FOREGROUND_SEGMENTS];
 unsigned autorefill_idx;
 bool runstall;
 AddressSpace *address_space_er;
diff --git a/target/xtensa/helper.h b/target/xtensa/helper.h
index 0b9ec670c86e..9216bee57e9a 100644
--- a/target/xtensa/helper.h
+++ b/target/xtensa/helper.h
@@ -33,6 +33,11 @@ DEF_HELPER_FLAGS_3(rtlb1, TCG_CALL_NO_RWG_SE, i32, env, i32, 
i32)
 DEF_HELPER_3(itlb, void, env, i32, i32)
 DEF_HELPER_3(ptlb, i32, env, i32, i32)
 DEF_HELPER_4(wtlb, void, env, i32, i32, i32)
+DEF_HELPER_2(wsr_mpuenb, void, env, i32)
+DEF_HELPER_3(wptlb, void, env, i32, i32)
+DEF_HELPER_FLAGS_2(rptlb0, TCG_CALL_NO_RWG_SE, i32, env, i32)
+DEF_HELPER_FLAGS_2(rptlb1, TCG_CALL_NO_RWG_SE, i32, env, i32)
+DEF_HELPER_2(pptlb, i32, env, i32)
 
 DEF_HELPER_2(wsr_ibreakenable, void, env, i32)
 DEF_HELPER_3(wsr_ibreaka, void, env, i32, i32)
diff --git a/target/xtensa/mmu_helper.c b/target/xtensa/mmu_helper.c
index 465cfbf61359..cab39f687a21 100644
--- a/target/xtensa/mmu_helper.c
+++ b/target/xtensa/mmu_helper.c
@@ -35,6 +35,31 @@
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
 
+#define XTENSA_MPU_SEGMENT_MASK 0x001f
+#define XTENSA_MPU_ACC_RIGHTS_MASK 0x0f00
+#define XTENSA_MPU_ACC_RIGHTS_SHIFT 8
+#define XTENSA_MPU_MEM_TYPE_MASK 0x001ff000
+#define XTENSA_MPU_MEM_TYPE_SHIFT 12
+#define XTENSA_MPU_ATTR_MASK 0x001fff00
+
+#define XTENSA_MPU_PROBE_B 0x4000
+#define XTENSA_MPU_PROBE_V 0x8000
+
+#define XTENSA_MPU_SYSTEM_TYPE_DEVICE 0x0001
+#define XTENSA_MPU_SYSTEM_TYPE_NC 0x0002
+#define XTENSA_MPU_SYSTEM_TYPE_C  0x0003
+#define XTENSA_MPU_SYSTEM_TYPE_MASK   0x0003
+
+#define XTENSA_MPU_TYPE_SYS_C 0x0010
+#define XTENSA_MPU_TYPE_SYS_W 0x0020
+#define XTENSA_MPU_TYPE_SYS_R 0x0040
+#define XTENSA_MPU_TYPE_CPU_C 0x0100
+#define XTENSA_MPU_TYPE_CPU_W 0x0200
+#define XTENSA_MPU_TYPE_CPU_R 0x0400
+#define XTENSA_MPU_TYPE_CPU_CACHE 0x0800
+#define XTENSA_MPU_TYPE_B 0x1000
+#define XTENSA_MPU_TYPE_INT   0x2000
+
 void HELPER(itlb_hit_test)(CPUXtensaState 

[Qemu-devel] [PATCH 9/9] target/xtensa: implement exclusive access option

[Max Filippov]

The Exclusive Instructions provide a general-purpose mechanism for
atomic updates of memory-based synchronization variables that can be
used for exclusion algorithms.

Use cmpxchg-based implementation that is sufficient for the typical use
of exclusive access in atomic operations.

Signed-off-by: Max Filippov 
---
 target/xtensa/cpu.c   |   1 +
 target/xtensa/cpu.h   |   2 +
 target/xtensa/helper.h|   1 +
 target/xtensa/op_helper.c |  42 
 target/xtensa/overlay_tool.h  |   8 ++-
 target/xtensa/translate.c | 100 ++
 tests/tcg/xtensa/test_exclusive.S |  48 ++
 7 files changed, 200 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/xtensa/test_exclusive.S

diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 4215a1881ec7..54c834228a91 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -82,6 +82,7 @@ static void xtensa_cpu_reset(CPUState *s)
 XTENSA_OPTION_ATOMCTL) ? 0x28 : 0x15;
 env->sregs[CONFIGID0] = env->config->configid[0];
 env->sregs[CONFIGID1] = env->config->configid[1];
+env->exclusive_addr = -1;
 
 #ifndef CONFIG_USER_ONLY
 reset_mmu(env);
diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index 8301923e4c4a..28a6fb4d796d 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -527,6 +527,8 @@ typedef struct CPUXtensaState {
 } fregs[16];
 float_status fp_status;
 uint32_t windowbase_next;
+uint32_t exclusive_addr;
+uint32_t exclusive_val;
 
 #ifndef CONFIG_USER_ONLY
 xtensa_tlb_entry itlb[7][MAX_TLB_WAY_SIZE];
diff --git a/target/xtensa/helper.h b/target/xtensa/helper.h
index 9216bee57e9a..8532de0b35f5 100644
--- a/target/xtensa/helper.h
+++ b/target/xtensa/helper.h
@@ -24,6 +24,7 @@ DEF_HELPER_1(check_interrupts, void, env)
 DEF_HELPER_2(intset, void, env, i32)
 DEF_HELPER_2(intclear, void, env, i32)
 DEF_HELPER_3(check_atomctl, void, env, i32, i32)
+DEF_HELPER_4(check_exclusive, void, env, i32, i32, i32)
 DEF_HELPER_2(wsr_memctl, void, env, i32)
 
 DEF_HELPER_2(itlb_hit_test, void, env, i32)
diff --git a/target/xtensa/op_helper.c b/target/xtensa/op_helper.c
index 04971b044fac..09f4962d008a 100644
--- a/target/xtensa/op_helper.c
+++ b/target/xtensa/op_helper.c
@@ -130,6 +130,48 @@ void HELPER(check_atomctl)(CPUXtensaState *env, uint32_t 
pc, uint32_t vaddr)
 }
 }
 
+void HELPER(check_exclusive)(CPUXtensaState *env, uint32_t pc, uint32_t vaddr,
+ uint32_t is_write)
+{
+uint32_t paddr, page_size, access;
+uint32_t atomctl = env->sregs[ATOMCTL];
+int rc = xtensa_get_physical_addr(env, true, vaddr, is_write,
+  xtensa_get_cring(env), ,
+  _size, );
+
+if (rc) {
+HELPER(exception_cause_vaddr)(env, pc, rc, vaddr);
+}
+
+/* When data cache is not configured use ATOMCTL bypass field. */
+if (!xtensa_option_enabled(env->config, XTENSA_OPTION_DCACHE)) {
+access = PAGE_CACHE_BYPASS;
+}
+
+switch (access & PAGE_CACHE_MASK) {
+case PAGE_CACHE_WB:
+atomctl >>= 2;
+/* fall through */
+case PAGE_CACHE_WT:
+atomctl >>= 2;
+/* fall through */
+case PAGE_CACHE_BYPASS:
+if ((atomctl & 0x3) == 0) {
+HELPER(exception_cause_vaddr)(env, pc,
+  EXCLUSIVE_ERROR_CAUSE, vaddr);
+}
+break;
+
+case PAGE_CACHE_ISOLATE:
+HELPER(exception_cause_vaddr)(env, pc,
+LOAD_STORE_ERROR_CAUSE, vaddr);
+break;
+
+default:
+break;
+}
+}
+
 void HELPER(wsr_memctl)(CPUXtensaState *env, uint32_t v)
 {
 if (xtensa_option_enabled(env->config, XTENSA_OPTION_ICACHE)) {
diff --git a/target/xtensa/overlay_tool.h b/target/xtensa/overlay_tool.h
index 4925b21f0edf..f0cc33adfe05 100644
--- a/target/xtensa/overlay_tool.h
+++ b/target/xtensa/overlay_tool.h
@@ -76,6 +76,10 @@
 #define XCHAL_HAVE_MPU 0
 #endif
 
+#ifndef XCHAL_HAVE_EXCLUSIVE
+#define XCHAL_HAVE_EXCLUSIVE 0
+#endif
+
 #define XCHAL_OPTION(xchal, qemu) ((xchal) ? XTENSA_OPTION_BIT(qemu) : 0)
 
 #define XTENSA_OPTIONS ( \
@@ -96,8 +100,8 @@
 XCHAL_OPTION(XCHAL_HAVE_FP, XTENSA_OPTION_FP_COPROCESSOR) | \
 XCHAL_OPTION(XCHAL_HAVE_RELEASE_SYNC, XTENSA_OPTION_MP_SYNCHRO) | \
 XCHAL_OPTION(XCHAL_HAVE_S32C1I, XTENSA_OPTION_CONDITIONAL_STORE) | \
-XCHAL_OPTION(XCHAL_HAVE_S32C1I && XCHAL_HW_MIN_VERSION >= 23, \
-XTENSA_OPTION_ATOMCTL) | \
+XCHAL_OPTION(((XCHAL_HAVE_S32C1I && XCHAL_HW_MIN_VERSION >= 23) || \
+  XCHAL_HAVE_EXCLUSIVE), XTENSA_OPTION_ATOMCTL) | \
 XCHAL_OPTION(XCHAL_HAVE_DEPBITS, XTENSA_OPTION_DEPBITS) | \
 /* Interrupts and exceptions */ \
 XCHAL_OPTION(XCHAL_HAVE_EXCEPTIONS, XTENSA_OPTION_EXCEPTION) | \
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 

[Qemu-devel] [PATCH 1/9] target/xtensa: get rid of centralized SR properties

[Max Filippov]

SR numbers are not unique: different Xtensa options may reuse SR number
for different purposes. Introduce generic rsr/wsr functions and xsr
template and use them instead of centralized SR access functions. Change
prototypes of specific rsr/wsr functions to match XtensaOpcodeOp and use
them instead of centralized SR access functions. Put xtensa option that
introduces SR into the second opcode description parameter and use it to
test for rsr/wsr/xsr opcode validity. Extract SR and UR names for the
xtensa_cpu_dump_state from libisa. Merge SRs and URs in the dump.
Register names of used SR/UR in init_libisa and use these names for TCG
globals referencing these SR/UR.

Signed-off-by: Max Filippov 
---
 target/xtensa/cpu.h   |1 +
 target/xtensa/helper.c|1 +
 target/xtensa/translate.c | 2503 +++--
 3 files changed, 1492 insertions(+), 1013 deletions(-)

diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index 5d23e1345be5..539033fccb61 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -589,6 +589,7 @@ void xtensa_cpu_do_unaligned_access(CPUState *cpu, vaddr 
addr,
 #define XTENSA_DEFAULT_CPU_NOMMU_TYPE \
 XTENSA_CPU_TYPE_NAME(XTENSA_DEFAULT_CPU_NOMMU_MODEL)
 
+void xtensa_collect_sr_names(const XtensaConfig *config);
 void xtensa_translate_init(void);
 void **xtensa_get_regfile_by_name(const char *name);
 void xtensa_breakpoint_handler(CPUState *cs);
diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
index 5f37f378a311..ed0108a81209 100644
--- a/target/xtensa/helper.c
+++ b/target/xtensa/helper.c
@@ -141,6 +141,7 @@ static void init_libisa(XtensaConfig *config)
 }
 #endif
 }
+xtensa_collect_sr_names(config);
 }
 
 static void xtensa_finalize_config(XtensaConfig *config)
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 301c8e31613c..100d6e126590 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -92,128 +92,40 @@ static GHashTable *xtensa_regfile_table;
 
 #include "exec/gen-icount.h"
 
-typedef struct XtensaReg {
-const char *name;
-uint64_t opt_bits;
-enum {
-SR_R = 1,
-SR_W = 2,
-SR_X = 4,
-SR_RW = 3,
-SR_RWX = 7,
-} access;
-} XtensaReg;
-
-#define XTENSA_REG_ACCESS(regname, opt, acc) { \
-.name = (regname), \
-.opt_bits = XTENSA_OPTION_BIT(opt), \
-.access = (acc), \
-}
+static char *sr_name[256];
+static char *ur_name[256];
 
-#define XTENSA_REG(regname, opt) XTENSA_REG_ACCESS(regname, opt, SR_RWX)
+void xtensa_collect_sr_names(const XtensaConfig *config)
+{
+xtensa_isa isa = config->isa;
+int n = xtensa_isa_num_sysregs(isa);
+int i;
 
-#define XTENSA_REG_BITS_ACCESS(regname, opt, acc) { \
-.name = (regname), \
-.opt_bits = (opt), \
-.access = (acc), \
+for (i = 0; i < n; ++i) {
+int sr = xtensa_sysreg_number(isa, i);
+
+if (sr >= 0 && sr < 256) {
+const char *name = xtensa_sysreg_name(isa, i);
+char **pname =
+(xtensa_sysreg_is_user(isa, i) ? ur_name : sr_name) + sr;
+
+if (*pname) {
+if (strstr(*pname, name) == NULL) {
+char *new_name =
+malloc(strlen(*pname) + strlen(name) + 2);
+
+strcpy(new_name, *pname);
+strcat(new_name, "/");
+strcat(new_name, name);
+free(*pname);
+*pname = new_name;
+}
+} else {
+*pname = strdup(name);
+}
+}
 }
-
-#define XTENSA_REG_BITS(regname, opt) \
-XTENSA_REG_BITS_ACCESS(regname, opt, SR_RWX)
-
-static const XtensaReg sregnames[256] = {
-[LBEG] = XTENSA_REG("LBEG", XTENSA_OPTION_LOOP),
-[LEND] = XTENSA_REG("LEND", XTENSA_OPTION_LOOP),
-[LCOUNT] = XTENSA_REG("LCOUNT", XTENSA_OPTION_LOOP),
-[SAR] = XTENSA_REG_BITS("SAR", XTENSA_OPTION_ALL),
-[BR] = XTENSA_REG("BR", XTENSA_OPTION_BOOLEAN),
-[LITBASE] = XTENSA_REG("LITBASE", XTENSA_OPTION_EXTENDED_L32R),
-[SCOMPARE1] = XTENSA_REG("SCOMPARE1", XTENSA_OPTION_CONDITIONAL_STORE),
-[ACCLO] = XTENSA_REG("ACCLO", XTENSA_OPTION_MAC16),
-[ACCHI] = XTENSA_REG("ACCHI", XTENSA_OPTION_MAC16),
-[MR] = XTENSA_REG("MR0", XTENSA_OPTION_MAC16),
-[MR + 1] = XTENSA_REG("MR1", XTENSA_OPTION_MAC16),
-[MR + 2] = XTENSA_REG("MR2", XTENSA_OPTION_MAC16),
-[MR + 3] = XTENSA_REG("MR3", XTENSA_OPTION_MAC16),
-[PREFCTL] = XTENSA_REG_BITS("PREFCTL", XTENSA_OPTION_ALL),
-[WINDOW_BASE] = XTENSA_REG("WINDOW_BASE", XTENSA_OPTION_WINDOWED_REGISTER),
-[WINDOW_START] = XTENSA_REG("WINDOW_START",
-XTENSA_OPTION_WINDOWED_REGISTER),
-[PTEVADDR] = XTENSA_REG("PTEVADDR", XTENSA_OPTION_MMU),
-[MMID] = XTENSA_REG_BITS("MMID", XTENSA_OPTION_ALL),
-[RASID] = XTENSA_REG("RASID", XTENSA_OPTION_MMU),
-[ITLBCFG] = 

[Qemu-devel] [PATCH 8/9] target/xtensa: update list of exception causes

[Max Filippov]

Add XEA2 exception cause codes defined in recent Xtensa ISA releases.

Signed-off-by: Max Filippov 
---
 target/xtensa/cpu.h | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index ba4ef2b6a729..8301923e4c4a 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -280,14 +280,15 @@ enum {
 LEVEL1_INTERRUPT_CAUSE,
 ALLOCA_CAUSE,
 INTEGER_DIVIDE_BY_ZERO_CAUSE,
-PRIVILEGED_CAUSE = 8,
+PC_VALUE_ERROR_CAUSE,
+PRIVILEGED_CAUSE,
 LOAD_STORE_ALIGNMENT_CAUSE,
-
-INSTR_PIF_DATA_ERROR_CAUSE = 12,
+EXTERNAL_REG_PRIVILEGE_CAUSE,
+EXCLUSIVE_ERROR_CAUSE,
+INSTR_PIF_DATA_ERROR_CAUSE,
 LOAD_STORE_PIF_DATA_ERROR_CAUSE,
 INSTR_PIF_ADDR_ERROR_CAUSE,
 LOAD_STORE_PIF_ADDR_ERROR_CAUSE,
-
 INST_TLB_MISS_CAUSE,
 INST_TLB_MULTI_HIT_CAUSE,
 INST_FETCH_PRIVILEGE_CAUSE,
-- 
2.11.0

Re: [Qemu-devel] [PATCH for-4.0.1] q35: Revert to kernel irqchip

[Alex Williamson]

On Tue, 14 May 2019 13:03:31 -0600
Alex Williamson  wrote:

> Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
> the default for the pc-q35-4.0 machine type to use split irqchip, which
> turned out to have disasterous effects on vfio-pci INTx support.  KVM
> resampling irqfds are registered for handling these interrupts, but
> these are non-functional in split irqchip mode.  We can't simply test
> for split irqchip in QEMU as userspace handling of this interrupt is a
> significant performance regression versus KVM handling (GeForce GPUs
> assigned to Windows VMs are non-functional without forcing MSI mode or
> re-enabling kernel irqchip).
> 
> The resolution is to revert the change in default irqchip mode with a
> new pc-q35-4.0.1 machine type for qemu-stable while the development
> branch makes the same change in the pc-q35-4.1 machine type.  The
> qemu-q35-4.0 machine type should not be used in vfio-pci configurations
> for devices requiring legacy INTx support without explicitly modifying
> the VM configuration to use KVM irqchip.  This new 4.0.1 machine type
> makes this change automatically.
> 
> Link: https://bugs.launchpad.net/qemu/+bug/1826422
> Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03305.html

This link is superseded by a v2 of the mainline patch:

Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03338.html

I believe this patch is still the proper stable backport though.  Also
to clarify, this patch should be gated on mainline acceptance of the
link above, but clearly there's no clean cherry-pick between mainline
and stable for this, so I'm proposing them in parallel.  Thanks,

Alex

> Fixes: b2fc91db8447 ("q35: set split kernel irqchip as default")
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Alex Williamson 
> ---
> 
> Do we want new stable versions for other archs too or only as needed?
> 
>  hw/core/machine.c|3 +++
>  hw/i386/pc.c |3 +++
>  hw/i386/pc_q35.c |   16 ++--
>  include/hw/boards.h  |3 +++
>  include/hw/i386/pc.h |3 +++
>  5 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 743fef28982c..5d046a43e3d2 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -24,6 +24,9 @@
>  #include "hw/pci/pci.h"
>  #include "hw/mem/nvdimm.h"
>  
> +GlobalProperty hw_compat_4_0[] = {};
> +const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
> +
>  GlobalProperty hw_compat_3_1[] = {
>  { "pcie-root-port", "x-speed", "2_5" },
>  { "pcie-root-port", "x-width", "1" },
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f2c15bf1f2c3..d98b737b8f3b 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -115,6 +115,9 @@ struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
>  /* Physical Address of PVH entry point read from kernel ELF NOTE */
>  static size_t pvh_start_addr;
>  
> +GlobalProperty pc_compat_4_0[] = {};
> +const size_t pc_compat_4_0_len = G_N_ELEMENTS(pc_compat_4_0);
> +
>  GlobalProperty pc_compat_3_1[] = {
>  { "intel-iommu", "dma-drain", "off" },
>  { "Opteron_G3" "-" TYPE_X86_CPU, "rdtscp", "off" },
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 372c6b73bebd..45cc29d1adb7 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -357,7 +357,7 @@ static void pc_q35_machine_options(MachineClass *m)
>  m->units_per_default_bus = 1;
>  m->default_machine_opts = "firmware=bios-256k.bin";
>  m->default_display = "std";
> -m->default_kernel_irqchip_split = true;
> +m->default_kernel_irqchip_split = false;
>  m->no_floppy = 1;
>  machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
>  machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
> @@ -365,12 +365,24 @@ static void pc_q35_machine_options(MachineClass *m)
>  m->max_cpus = 288;
>  }
>  
> -static void pc_q35_4_0_machine_options(MachineClass *m)
> +static void pc_q35_4_0_1_machine_options(MachineClass *m)
>  {
>  pc_q35_machine_options(m);
>  m->alias = "q35";
>  }
>  
> +DEFINE_Q35_MACHINE(v4_0_1, "pc-q35-4.0.1", NULL,
> +   pc_q35_4_0_1_machine_options);
> +
> +static void pc_q35_4_0_machine_options(MachineClass *m)
> +{
> +pc_q35_4_0_1_machine_options(m);
> +m->default_kernel_irqchip_split = true;
> +m->alias = NULL;
> +compat_props_add(m->compat_props, hw_compat_4_0, hw_compat_4_0_len);
> +compat_props_add(m->compat_props, pc_compat_4_0, pc_compat_4_0_len);
> +}
> +
>  DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
> pc_q35_4_0_machine_options);
>  
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index e231860666a1..fe1885cbffa0 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -293,6 +293,9 @@ struct MachineState {
>  } \
>  type_init(machine_initfn##_register_types)
>  
> +extern GlobalProperty hw_compat_4_0[];
> +extern const size_t hw_compat_4_0_len;
> +
>  

[Qemu-devel] [PATCH v2] migration/dirty-bitmaps: change bitmap enumeration method

[John Snow]

Shift from looking at every root BDS to *every* BDS. This will migrate
bitmaps that are attached to blockdev created nodes instead of just ones
attached to emulated storage devices.

Note that this will not migrate anonymous or internal-use bitmaps, as
those are defined as having no name.

This will also fix the Coverity issues Peter Maydell has been asking
about for the past several releases, as well as fixing a real bug.

Reported-by: Peter Maydell 
Reported-by: Coverity 
Reported-by: aihua liang 
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1652490
Fixes: Coverity CID 1390625
CC: Stefan Hajnoczi 
Signed-off-by: John Snow 
---
 migration/block-dirty-bitmap.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index d1bb863cb6..4a896a09eb 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -273,7 +273,6 @@ static int init_dirty_bitmap_migration(void)
 BlockDriverState *bs;
 BdrvDirtyBitmap *bitmap;
 DirtyBitmapMigBitmapState *dbms;
-BdrvNextIterator it;
 Error *local_err = NULL;
 
 dirty_bitmap_mig_state.bulk_completed = false;
@@ -281,13 +280,8 @@ static int init_dirty_bitmap_migration(void)
 dirty_bitmap_mig_state.prev_bitmap = NULL;
 dirty_bitmap_mig_state.no_bitmaps = false;
 
-for (bs = bdrv_first(); bs; bs = bdrv_next()) {
-const char *drive_name = bdrv_get_device_or_node_name(bs);
-
-/* skip automatically inserted nodes */
-while (bs && bs->drv && bs->implicit) {
-bs = backing_bs(bs);
-}
+for (bs = bdrv_next_all_states(NULL); bs; bs = bdrv_next_all_states(bs)) {
+const char *name = bdrv_get_device_or_node_name(bs);
 
 for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
  bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
@@ -296,7 +290,7 @@ static int init_dirty_bitmap_migration(void)
 continue;
 }
 
-if (drive_name == NULL) {
+if (!name || strcmp(name, "") == 0) {
 error_report("Found bitmap '%s' in unnamed node %p. It can't "
  "be migrated", bdrv_dirty_bitmap_name(bitmap), 
bs);
 goto fail;
@@ -313,7 +307,7 @@ static int init_dirty_bitmap_migration(void)
 
 dbms = g_new0(DirtyBitmapMigBitmapState, 1);
 dbms->bs = bs;
-dbms->node_name = drive_name;
+dbms->node_name = name;
 dbms->bitmap = bitmap;
 dbms->total_sectors = bdrv_nb_sectors(bs);
 dbms->sectors_per_chunk = CHUNK_SIZE * 8 *
-- 
2.20.1

Re: [Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests

[Richard Henderson]

On 5/14/19 9:03 AM, Andrea Bolognani wrote:
> On Tue, 2019-05-14 at 14:53 +0200, Andrew Jones wrote:
>> We already have sve-max-vq, so I'm not sure we want to rename it.
> 
> Oh, I didn't realize that was the case. And of course it already
> takes a number of quadwords as argument, I suppose? That's pretty
> unfortunate :(
> 
> Perhaps we could consider deprecating it in favor of a user-friendly
> variant that's actually suitable for regular humans, like the one I
> suggest above?

Why is =4 less user-friendly than =512?

I don't actually see "total bits in vector" as more user-friendly than "number
of quadwords" when it comes to non-powers-of-2 like =7 vs =896 or =13 vs =1664.


r~

[Qemu-devel] [PATCH for-4.1 v2] q35: Revert to kernel irqchip

[Alex Williamson]

Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
the default for the pc-q35-4.0 machine type to use split irqchip, which
turned out to have disasterous effects on vfio-pci INTx support.  KVM
resampling irqfds are registered for handling these interrupts, but
these are non-functional in split irqchip mode.  We can't simply test
for split irqchip in QEMU as userspace handling of this interrupt is a
significant performance regression versus KVM handling (GeForce GPUs
assigned to Windows VMs are non-functional without forcing MSI mode or
re-enabling kernel irqchip).

The resolution is to revert the change in default irqchip mode in the
pc-q35-4.1 machine and create a pc-q35-4.0.1 machine for the 4.0-stable
branch.  The qemu-q35-4.0 machine type should not be used in vfio-pci
configurations for devices requiring legacy INTx support without
explicitly modifying the VM configuration to use kernel irqchip.

Link: https://bugs.launchpad.net/qemu/+bug/1826422
Fixes: b2fc91db8447 ("q35: set split kernel irqchip as default")
Signed-off-by: Alex Williamson 
---
 hw/core/machine.c|3 +++
 hw/i386/pc.c |3 +++
 hw/i386/pc_q35.c |   16 ++--
 include/hw/boards.h  |3 +++
 include/hw/i386/pc.h |3 +++
 5 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 5d046a43e3d2..e41e6698ac9f 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -24,6 +24,9 @@
 #include "hw/pci/pci.h"
 #include "hw/mem/nvdimm.h"
 
+GlobalProperty hw_compat_4_0_1[] = {};
+const size_t hw_compat_4_0_1_len = G_N_ELEMENTS(hw_compat_4_0_1);
+
 GlobalProperty hw_compat_4_0[] = {};
 const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d98b737b8f3b..b5311e7e2bd5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -115,6 +115,9 @@ struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 /* Physical Address of PVH entry point read from kernel ELF NOTE */
 static size_t pvh_start_addr;
 
+GlobalProperty pc_compat_4_0_1[] = {};
+const size_t pc_compat_4_0_1_len = G_N_ELEMENTS(pc_compat_4_0_1);
+
 GlobalProperty pc_compat_4_0[] = {};
 const size_t pc_compat_4_0_len = G_N_ELEMENTS(pc_compat_4_0);
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 37dd350511a9..dcddc6466200 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -357,7 +357,7 @@ static void pc_q35_machine_options(MachineClass *m)
 m->units_per_default_bus = 1;
 m->default_machine_opts = "firmware=bios-256k.bin";
 m->default_display = "std";
-m->default_kernel_irqchip_split = true;
+m->default_kernel_irqchip_split = false;
 m->no_floppy = 1;
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
@@ -374,10 +374,22 @@ static void pc_q35_4_1_machine_options(MachineClass *m)
 DEFINE_Q35_MACHINE(v4_1, "pc-q35-4.1", NULL,
pc_q35_4_1_machine_options);
 
-static void pc_q35_4_0_machine_options(MachineClass *m)
+static void pc_q35_4_0_1_machine_options(MachineClass *m)
 {
 pc_q35_4_1_machine_options(m);
 m->alias = NULL;
+compat_props_add(m->compat_props, hw_compat_4_0_1, hw_compat_4_0_1_len);
+compat_props_add(m->compat_props, pc_compat_4_0_1, pc_compat_4_0_1_len);
+}
+
+DEFINE_Q35_MACHINE(v4_0_1, "pc-q35-4.0.1", NULL,
+   pc_q35_4_0_1_machine_options);
+
+static void pc_q35_4_0_machine_options(MachineClass *m)
+{
+pc_q35_4_0_1_machine_options(m);
+m->default_kernel_irqchip_split = true;
+m->alias = NULL;
 compat_props_add(m->compat_props, hw_compat_4_0, hw_compat_4_0_len);
 compat_props_add(m->compat_props, pc_compat_4_0, pc_compat_4_0_len);
 }
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 6f7916f88f02..6ff02bf3e472 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -292,6 +292,9 @@ struct MachineState {
 } \
 type_init(machine_initfn##_register_types)
 
+extern GlobalProperty hw_compat_4_0_1[];
+extern const size_t hw_compat_4_0_1_len;
+
 extern GlobalProperty hw_compat_4_0[];
 extern const size_t hw_compat_4_0_len;
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 43df7230a22b..5d5636241e34 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -293,6 +293,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
+extern GlobalProperty pc_compat_4_0_1[];
+extern const size_t pc_compat_4_0_1_len;
+
 extern GlobalProperty pc_compat_4_0[];
 extern const size_t pc_compat_4_0_len;
 

[Qemu-devel] [PATCH v7 22/24] target/ppc: Use qemu_guest_getrandom for DARN

[Richard Henderson]

We now have an interface for guest visible random numbers.

Acked-by: David Gibson 
Reviewed-by: Laurent Vivier 
Signed-off-by: Richard Henderson 
---
 target/ppc/int_helper.c | 39 +++
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index f6a088ac08..9af779ad38 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -23,6 +23,8 @@
 #include "exec/helper-proto.h"
 #include "crypto/aes.h"
 #include "fpu/softfloat.h"
+#include "qapi/error.h"
+#include "qemu/guest-random.h"
 
 #include "helper_regs.h"
 /*/
@@ -158,25 +160,38 @@ uint32_t helper_cmpeqb(target_ulong ra, target_ulong rb)
 #undef hasvalue
 
 /*
- * Return invalid random number.
- *
- * FIXME: Add rng backend or other mechanism to get cryptographically suitable
- * random number
+ * Return a random number.
  */
-target_ulong helper_darn32(void)
+uint64_t helper_darn32(void)
 {
-return -1;
+Error *err = NULL;
+uint32_t ret;
+
+if (qemu_guest_getrandom(, sizeof(ret), ) < 0) {
+qemu_log_mask(LOG_UNIMP, "darn: Crypto failure: %s",
+  error_get_pretty(err));
+error_free(err);
+return -1;
+}
+
+return ret;
 }
 
-target_ulong helper_darn64(void)
+uint64_t helper_darn64(void)
 {
-return -1;
+Error *err = NULL;
+uint64_t ret;
+
+if (qemu_guest_getrandom(, sizeof(ret), ) < 0) {
+qemu_log_mask(LOG_UNIMP, "darn: Crypto failure: %s",
+  error_get_pretty(err));
+error_free(err);
+return -1;
+}
+
+return ret;
 }
 
-#endif
-
-#if defined(TARGET_PPC64)
-
 uint64_t helper_bpermd(uint64_t rs, uint64_t rb)
 {
 int i;
-- 
2.17.1

[Qemu-devel] [PATCH v7 20/24] target/arm: Put all PAC keys into a structure

[Richard Henderson]

This allows us to use a single syscall to initialize them all.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 target/arm/cpu.h  | 12 +++-
 linux-user/aarch64/cpu_loop.c |  6 +-
 linux-user/syscall.c  | 10 +-
 target/arm/helper.c   | 20 ++--
 target/arm/pauth_helper.c | 18 +-
 5 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 733b840a71..892f9a4ad2 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -636,11 +636,13 @@ typedef struct CPUARMState {
 } iwmmxt;
 
 #ifdef TARGET_AARCH64
-ARMPACKey apia_key;
-ARMPACKey apib_key;
-ARMPACKey apda_key;
-ARMPACKey apdb_key;
-ARMPACKey apga_key;
+struct {
+ARMPACKey apia;
+ARMPACKey apib;
+ARMPACKey apda;
+ARMPACKey apdb;
+ARMPACKey apga;
+} keys;
 #endif
 
 #if defined(CONFIG_USER_ONLY)
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index cedad39ca0..2f2f63e3e8 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -175,11 +175,7 @@ void target_cpu_copy_regs(CPUArchState *env, struct 
target_pt_regs *regs)
 #endif
 
 if (cpu_isar_feature(aa64_pauth, cpu)) {
-qemu_guest_getrandom_nofail(>apia_key, sizeof(ARMPACKey));
-qemu_guest_getrandom_nofail(>apib_key, sizeof(ARMPACKey));
-qemu_guest_getrandom_nofail(>apda_key, sizeof(ARMPACKey));
-qemu_guest_getrandom_nofail(>apdb_key, sizeof(ARMPACKey));
-qemu_guest_getrandom_nofail(>apga_key, sizeof(ARMPACKey));
+qemu_guest_getrandom_nofail(>keys, sizeof(env->keys));
 }
 
 ts->stack_base = info->start_stack;
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8c17b14d51..394b956b4a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9775,23 +9775,23 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 return -TARGET_EINVAL;
 }
 if (arg2 & TARGET_PR_PAC_APIAKEY) {
-ret |= qemu_guest_getrandom(>apia_key,
+ret |= qemu_guest_getrandom(>keys.apia,
 sizeof(ARMPACKey), );
 }
 if (arg2 & TARGET_PR_PAC_APIBKEY) {
-ret |= qemu_guest_getrandom(>apib_key,
+ret |= qemu_guest_getrandom(>keys.apib,
 sizeof(ARMPACKey), );
 }
 if (arg2 & TARGET_PR_PAC_APDAKEY) {
-ret |= qemu_guest_getrandom(>apda_key,
+ret |= qemu_guest_getrandom(>keys.apda,
 sizeof(ARMPACKey), );
 }
 if (arg2 & TARGET_PR_PAC_APDBKEY) {
-ret |= qemu_guest_getrandom(>apdb_key,
+ret |= qemu_guest_getrandom(>keys.apdb,
 sizeof(ARMPACKey), );
 }
 if (arg2 & TARGET_PR_PAC_APGAKEY) {
-ret |= qemu_guest_getrandom(>apga_key,
+ret |= qemu_guest_getrandom(>keys.apga,
 sizeof(ARMPACKey), );
 }
 if (ret != 0) {
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 1e6eb0d0f3..7e88b2cadd 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5707,43 +5707,43 @@ static const ARMCPRegInfo pauth_reginfo[] = {
 { .name = "APDAKEYLO_EL1", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 2, .opc2 = 0,
   .access = PL1_RW, .accessfn = access_pauth,
-  .fieldoffset = offsetof(CPUARMState, apda_key.lo) },
+  .fieldoffset = offsetof(CPUARMState, keys.apda.lo) },
 { .name = "APDAKEYHI_EL1", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 2, .opc2 = 1,
   .access = PL1_RW, .accessfn = access_pauth,
-  .fieldoffset = offsetof(CPUARMState, apda_key.hi) },
+  .fieldoffset = offsetof(CPUARMState, keys.apda.hi) },
 { .name = "APDBKEYLO_EL1", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 2, .opc2 = 2,
   .access = PL1_RW, .accessfn = access_pauth,
-  .fieldoffset = offsetof(CPUARMState, apdb_key.lo) },
+  .fieldoffset = offsetof(CPUARMState, keys.apdb.lo) },
 { .name = "APDBKEYHI_EL1", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 2, .opc2 = 3,
   .access = PL1_RW, .accessfn = access_pauth,
-  .fieldoffset = offsetof(CPUARMState, apdb_key.hi) },
+  .fieldoffset = offsetof(CPUARMState, keys.apdb.hi) },
 { .name = "APGAKEYLO_EL1", .state = 

Re: [Qemu-devel] [PATCH for-4.0.1] q35: Revert to kernel irqchip

[Alex Williamson]

On Tue, 14 May 2019 20:22:32 +0100
Daniel P. Berrangé  wrote:

> On Tue, May 14, 2019 at 01:03:31PM -0600, Alex Williamson wrote:
> > Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
> > the default for the pc-q35-4.0 machine type to use split irqchip, which
> > turned out to have disasterous effects on vfio-pci INTx support.  KVM
> > resampling irqfds are registered for handling these interrupts, but
> > these are non-functional in split irqchip mode.  We can't simply test
> > for split irqchip in QEMU as userspace handling of this interrupt is a
> > significant performance regression versus KVM handling (GeForce GPUs
> > assigned to Windows VMs are non-functional without forcing MSI mode or
> > re-enabling kernel irqchip).
> > 
> > The resolution is to revert the change in default irqchip mode with a
> > new pc-q35-4.0.1 machine type for qemu-stable while the development
> > branch makes the same change in the pc-q35-4.1 machine type.  The
> > qemu-q35-4.0 machine type should not be used in vfio-pci configurations
> > for devices requiring legacy INTx support without explicitly modifying
> > the VM configuration to use KVM irqchip.  This new 4.0.1 machine type
> > makes this change automatically.  
> 
> If we introduce a pc-q35-4.0.1 machine type in -stable, then VMs
> created in stable won't be migratable to future 4.1 unless we also
> create this same machine type in master.

Yes, I overlooked 4.0.1 on 4.1, I'm working on that now.  Reposting a
v2 of the 4.1 version shortly.

> If we really want to create a new 4.0.1 machine type, then this
> patch needs to go to git master before stable IMHO to guarantee
> no regression to master.

Yes, I'm doing both in parallel, I expect the 4.0.1 support to be
contingent on be the master branch change, thus the second link below.
Thanks,

Alex

> > Link: https://bugs.launchpad.net/qemu/+bug/1826422
> > Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03305.html
> > Fixes: b2fc91db8447 ("q35: set split kernel irqchip as default")
> > Cc: qemu-sta...@nongnu.org
> > Signed-off-by: Alex Williamson 
> > ---
> > 
> > Do we want new stable versions for other archs too or only as needed?  
> 
> 
> 
> Regards,
> Daniel

Re: [Qemu-devel] [PATCH for-4.1] q35: Revert to kernel irqchip

[Alex Williamson]

On Tue, 14 May 2019 12:46:47 -0600
Alex Williamson  wrote:

> Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
> the default for the pc-q35-4.0 machine type to use split irqchip, which
> turned out to have disasterous effects on vfio-pci INTx support.  KVM
> resampling irqfds are registered for handling these interrupts, but
> these are non-functional in split irqchip mode.  We can't simply test
> for split irqchip in QEMU as userspace handling of this interrupt is a
> significant performance regression versus KVM handling (GeForce GPUs
> assigned to Windows VMs are non-functional without forcing MSI mode or
> re-enabling kernel irqchip).
> 
> The resolution is to revert the change in default irqchip mode in the
> pc-q35-4.1 machine type.  The qemu-q35-4.0 machine type should not be
> used in vfio-pci configurations for devices requiring legacy INTx
> support without explicitly modifying the VM configuration to use KVM
> irqchip.  A new pc-q35-4.0.1 machine type is submitted to resolve this
> in the stable branch.
> 
> Link: https://bugs.launchpad.net/qemu/+bug/1826422
> Fixes: b2fc91db8447 ("q35: set split kernel irqchip as default")
> Signed-off-by: Alex Williamson 
> ---
>  hw/i386/pc_q35.c |3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 37dd350511a9..9f90dc72a53f 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -357,7 +357,7 @@ static void pc_q35_machine_options(MachineClass *m)
>  m->units_per_default_bus = 1;
>  m->default_machine_opts = "firmware=bios-256k.bin";
>  m->default_display = "std";
> -m->default_kernel_irqchip_split = true;
> +m->default_kernel_irqchip_split = false;
>  m->no_floppy = 1;
>  machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
>  machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
> @@ -377,6 +377,7 @@ DEFINE_Q35_MACHINE(v4_1, "pc-q35-4.1", NULL,
>  static void pc_q35_4_0_machine_options(MachineClass *m)
>  {
>  pc_q35_4_1_machine_options(m);
> +m->default_kernel_irqchip_split = true;
>  m->alias = NULL;
>  compat_props_add(m->compat_props, hw_compat_4_0, hw_compat_4_0_len);
>  compat_props_add(m->compat_props, pc_compat_4_0, pc_compat_4_0_len);
> 

Hmm, I need to make 4.0.1 here too, v2 coming...

[Qemu-devel] [PATCH v7 19/24] hw/misc/exynos4210_rng: Use qemu_guest_getrandom

[Richard Henderson]

The random number is intended for use by the guest.  As such, we should
honor the -seed argument for reproducibility.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 hw/misc/exynos4210_rng.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/misc/exynos4210_rng.c b/hw/misc/exynos4210_rng.c
index 4ecbebd2d7..0e70ffb404 100644
--- a/hw/misc/exynos4210_rng.c
+++ b/hw/misc/exynos4210_rng.c
@@ -18,10 +18,10 @@
  */
 
 #include "qemu/osdep.h"
-#include "crypto/random.h"
 #include "hw/sysbus.h"
 #include "qapi/error.h"
 #include "qemu/log.h"
+#include "qemu/guest-random.h"
 
 #define DEBUG_EXYNOS_RNG 0
 
@@ -109,7 +109,6 @@ static void exynos4210_rng_set_seed(Exynos4210RngState *s, 
unsigned int i,
 static void exynos4210_rng_run_engine(Exynos4210RngState *s)
 {
 Error *err = NULL;
-int ret;
 
 /* Seed set? */
 if ((s->reg_status & EXYNOS4210_RNG_STATUS_SEED_SETTING_DONE) == 0) {
@@ -127,13 +126,11 @@ static void exynos4210_rng_run_engine(Exynos4210RngState 
*s)
 }
 
 /* Get randoms */
-ret = qcrypto_random_bytes((uint8_t *)s->randr_value,
-   sizeof(s->randr_value), );
-if (!ret) {
+if (qemu_guest_getrandom(s->randr_value, sizeof(s->randr_value), )) {
+error_report_err(err);
+} else {
 /* Notify that PRNG is ready */
 s->reg_status |= EXYNOS4210_RNG_STATUS_PRNG_DONE;
-} else {
-error_report_err(err);
 }
 
 out:
-- 
2.17.1

[Qemu-devel] [PATCH v7 24/24] target/i386: Implement CPUID_EXT_RDRAND

[Richard Henderson]

We now have an interface for guest visible random numbers.

Reviewed-by: Eduardo Habkost 
Signed-off-by: Richard Henderson 
---
 target/i386/helper.h |  2 ++
 target/i386/cpu.c|  5 ++--
 target/i386/int_helper.c | 21 ++
 target/i386/translate.c  | 62 ++--
 4 files changed, 73 insertions(+), 17 deletions(-)

diff --git a/target/i386/helper.h b/target/i386/helper.h
index 6fb8fb9b74..8f9e1905c3 100644
--- a/target/i386/helper.h
+++ b/target/i386/helper.h
@@ -226,3 +226,5 @@ DEF_HELPER_3(rcrl, tl, env, tl, tl)
 DEF_HELPER_3(rclq, tl, env, tl, tl)
 DEF_HELPER_3(rcrq, tl, env, tl, tl)
 #endif
+
+DEF_HELPER_1(rdrand, tl, env)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 722c5514d4..1386814957 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -730,13 +730,14 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t 
vendor1,
   CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 | CPUID_EXT_CX16 | \
   CPUID_EXT_SSE41 | CPUID_EXT_SSE42 | CPUID_EXT_POPCNT | \
   CPUID_EXT_XSAVE | /* CPUID_EXT_OSXSAVE is dynamic */   \
-  CPUID_EXT_MOVBE | CPUID_EXT_AES | CPUID_EXT_HYPERVISOR)
+  CPUID_EXT_MOVBE | CPUID_EXT_AES | CPUID_EXT_HYPERVISOR | \
+  CPUID_EXT_RDRAND)
   /* missing:
   CPUID_EXT_DTES64, CPUID_EXT_DSCPL, CPUID_EXT_VMX, CPUID_EXT_SMX,
   CPUID_EXT_EST, CPUID_EXT_TM2, CPUID_EXT_CID, CPUID_EXT_FMA,
   CPUID_EXT_XTPR, CPUID_EXT_PDCM, CPUID_EXT_PCID, CPUID_EXT_DCA,
   CPUID_EXT_X2APIC, CPUID_EXT_TSC_DEADLINE_TIMER, CPUID_EXT_AVX,
-  CPUID_EXT_F16C, CPUID_EXT_RDRAND */
+  CPUID_EXT_F16C */
 
 #ifdef TARGET_X86_64
 #define TCG_EXT2_X86_64_FEATURES (CPUID_EXT2_SYSCALL | CPUID_EXT2_LM)
diff --git a/target/i386/int_helper.c b/target/i386/int_helper.c
index 4dc5c65991..334469ca8c 100644
--- a/target/i386/int_helper.c
+++ b/target/i386/int_helper.c
@@ -22,6 +22,8 @@
 #include "exec/exec-all.h"
 #include "qemu/host-utils.h"
 #include "exec/helper-proto.h"
+#include "qapi/error.h"
+#include "qemu/guest-random.h"
 
 //#define DEBUG_MULDIV
 
@@ -470,3 +472,22 @@ void helper_cr4_testbit(CPUX86State *env, uint32_t bit)
 raise_exception_ra(env, EXCP06_ILLOP, GETPC());
 }
 }
+
+target_ulong HELPER(rdrand)(CPUX86State *env)
+{
+Error *err = NULL;
+target_ulong ret;
+
+if (qemu_guest_getrandom(, sizeof(ret), ) < 0) {
+qemu_log_mask(LOG_UNIMP, "rdrand: Crypto failure: %s",
+  error_get_pretty(err));
+error_free(err);
+/* Failure clears CF and all other flags, and returns 0.  */
+env->cc_src = 0;
+return 0;
+}
+
+/* Success sets CF and clears all others.  */
+env->cc_src = CC_C;
+return ret;
+}
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 77d6b73e42..03150a86e2 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -5332,31 +5332,63 @@ static target_ulong disas_insn(DisasContext *s, 
CPUState *cpu)
 case 0x1c7: /* cmpxchg8b */
 modrm = x86_ldub_code(env, s);
 mod = (modrm >> 6) & 3;
-if ((mod == 3) || ((modrm & 0x38) != 0x8))
-goto illegal_op;
-#ifdef TARGET_X86_64
-if (dflag == MO_64) {
-if (!(s->cpuid_ext_features & CPUID_EXT_CX16))
+switch ((modrm >> 3) & 7) {
+case 1: /* CMPXCHG8, CMPXCHG16 */
+if (mod == 3) {
 goto illegal_op;
-gen_lea_modrm(env, s, modrm);
-if ((s->prefix & PREFIX_LOCK) && (tb_cflags(s->base.tb) & 
CF_PARALLEL)) {
-gen_helper_cmpxchg16b(cpu_env, s->A0);
-} else {
-gen_helper_cmpxchg16b_unlocked(cpu_env, s->A0);
 }
-} else
+#ifdef TARGET_X86_64
+if (dflag == MO_64) {
+if (!(s->cpuid_ext_features & CPUID_EXT_CX16)) {
+goto illegal_op;
+}
+gen_lea_modrm(env, s, modrm);
+if ((s->prefix & PREFIX_LOCK) &&
+(tb_cflags(s->base.tb) & CF_PARALLEL)) {
+gen_helper_cmpxchg16b(cpu_env, s->A0);
+} else {
+gen_helper_cmpxchg16b_unlocked(cpu_env, s->A0);
+}
+set_cc_op(s, CC_OP_EFLAGS);
+break;
+}
 #endif
-{
-if (!(s->cpuid_features & CPUID_CX8))
+if (!(s->cpuid_features & CPUID_CX8)) {
 goto illegal_op;
+}
 gen_lea_modrm(env, s, modrm);
-if ((s->prefix & PREFIX_LOCK) && (tb_cflags(s->base.tb) & 
CF_PARALLEL)) {
+if ((s->prefix & PREFIX_LOCK) &&
+(tb_cflags(s->base.tb) & CF_PARALLEL)) {
 gen_helper_cmpxchg8b(cpu_env, s->A0);
 } else {
 gen_helper_cmpxchg8b_unlocked(cpu_env, s->A0);
 }
+set_cc_op(s, CC_OP_EFLAGS);
+   

[Qemu-devel] [PATCH v7 18/24] hw/misc/bcm2835_rng: Use qemu_guest_getrandom_nofail

[Richard Henderson]

The random number is intended for use by the guest.  As such, we should
honor the -seed argument for reproducibility.  Use the *_nofail routine
instead of rolling our own error handling locally.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 hw/misc/bcm2835_rng.c | 32 ++--
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/hw/misc/bcm2835_rng.c b/hw/misc/bcm2835_rng.c
index 4d62143b24..fe59c868f5 100644
--- a/hw/misc/bcm2835_rng.c
+++ b/hw/misc/bcm2835_rng.c
@@ -9,30 +9,26 @@
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
-#include "qapi/error.h"
-#include "crypto/random.h"
+#include "qemu/guest-random.h"
 #include "hw/misc/bcm2835_rng.h"
 
 static uint32_t get_random_bytes(void)
 {
 uint32_t res;
-Error *err = NULL;
 
-if (qcrypto_random_bytes((uint8_t *), sizeof(res), ) < 0) {
-/* On failure we don't want to return the guest a non-random
- * value in case they're really using it for cryptographic
- * purposes, so the best we can do is die here.
- * This shouldn't happen unless something's broken.
- * In theory we could implement this device's full FIFO
- * and interrupt semantics and then just stop filling the
- * FIFO. That's a lot of work, though, so we assume any
- * errors are systematic problems and trust that if we didn't
- * fail as the guest inited then we won't fail later on
- * mid-run.
- */
-error_report_err(err);
-exit(1);
-}
+/*
+ * On failure we don't want to return the guest a non-random
+ * value in case they're really using it for cryptographic
+ * purposes, so the best we can do is die here.
+ * This shouldn't happen unless something's broken.
+ * In theory we could implement this device's full FIFO
+ * and interrupt semantics and then just stop filling the
+ * FIFO. That's a lot of work, though, so we assume any
+ * errors are systematic problems and trust that if we didn't
+ * fail as the guest inited then we won't fail later on
+ * mid-run.
+ */
+qemu_guest_getrandom_nofail(, sizeof(res));
 return res;
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH v7 21/24] target/arm: Implement ARMv8.5-RNG

[Richard Henderson]

Use the newly introduced infrastructure for guest random numbers.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target/arm/cpu.h|  5 +
 target/arm/cpu64.c  |  1 +
 target/arm/helper.c | 44 
 3 files changed, 50 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 892f9a4ad2..c34207611b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3521,6 +3521,11 @@ static inline bool isar_feature_aa64_condm_5(const 
ARMISARegisters *id)
 return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TS) >= 2;
 }
 
+static inline bool isar_feature_aa64_rndr(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, RNDR) != 0;
+}
+
 static inline bool isar_feature_aa64_jscvt(const ARMISARegisters *id)
 {
 return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, JSCVT) != 0;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 228906f267..835f73cceb 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -310,6 +310,7 @@ static void aarch64_max_initfn(Object *obj)
 t = FIELD_DP64(t, ID_AA64ISAR0, DP, 1);
 t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 1);
 t = FIELD_DP64(t, ID_AA64ISAR0, TS, 2); /* v8.5-CondM */
+t = FIELD_DP64(t, ID_AA64ISAR0, RNDR, 1);
 cpu->isar.id_aa64isar0 = t;
 
 t = cpu->isar.id_aa64isar1;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7e88b2cadd..1e90f4d722 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -22,6 +22,8 @@
 #include "fpu/softfloat.h"
 #include "qemu/range.h"
 #include "qapi/qapi-commands-target.h"
+#include "qapi/error.h"
+#include "qemu/guest-random.h"
 
 #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
 
@@ -5746,6 +5748,45 @@ static const ARMCPRegInfo pauth_reginfo[] = {
   .fieldoffset = offsetof(CPUARMState, keys.apib.hi) },
 REGINFO_SENTINEL
 };
+
+static uint64_t rndr_readfn(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+Error *err = NULL;
+uint64_t ret;
+
+/* Success sets NZCV = .  */
+env->NF = env->CF = env->VF = 0, env->ZF = 1;
+
+if (qemu_guest_getrandom(, sizeof(ret), ) < 0) {
+/*
+ * ??? Failed, for unknown reasons in the crypto subsystem.
+ * The best we can do is log the reason and return the
+ * timed-out indication to the guest.  There is no reason
+ * we know to expect this failure to be transitory, so the
+ * guest may well hang retrying the operation.
+ */
+qemu_log_mask(LOG_UNIMP, "%s: Crypto failure: %s",
+  ri->name, error_get_pretty(err));
+error_free(err);
+
+env->ZF = 0; /* NZCF = 0100 */
+return 0;
+}
+return ret;
+}
+
+/* We do not support re-seeding, so the two registers operate the same.  */
+static const ARMCPRegInfo rndr_reginfo[] = {
+{ .name = "RNDR", .state = ARM_CP_STATE_AA64,
+  .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END | ARM_CP_IO,
+  .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 0,
+  .access = PL0_R, .readfn = rndr_readfn },
+{ .name = "RNDRRS", .state = ARM_CP_STATE_AA64,
+  .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END | ARM_CP_IO,
+  .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 1,
+  .access = PL0_R, .readfn = rndr_readfn },
+REGINFO_SENTINEL
+};
 #endif
 
 static CPAccessResult access_predinv(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -6690,6 +6731,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
 if (cpu_isar_feature(aa64_pauth, cpu)) {
 define_arm_cp_regs(cpu, pauth_reginfo);
 }
+if (cpu_isar_feature(aa64_rndr, cpu)) {
+define_arm_cp_regs(cpu, rndr_reginfo);
+}
 #endif
 
 /*
-- 
2.17.1

[Qemu-devel] [PATCH v7 16/24] aspeed/scu: Use qemu_guest_getrandom_nofail

[Richard Henderson]

The random number is intended for use by the guest.  As such, we should
honor the -seed argument for reproducibility.  Use the *_nofail routine
instead of rolling our own error handling locally.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cédric Le Goater 
Reviewed-by: Joel Stanley 
Signed-off-by: Richard Henderson 
---
 hw/misc/aspeed_scu.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index c8217740ef..ab1e18ed4b 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -16,7 +16,7 @@
 #include "qapi/visitor.h"
 #include "qemu/bitops.h"
 #include "qemu/log.h"
-#include "crypto/random.h"
+#include "qemu/guest-random.h"
 #include "trace.h"
 
 #define TO_REG(offset) ((offset) >> 2)
@@ -157,14 +157,8 @@ static const uint32_t 
ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
 
 static uint32_t aspeed_scu_get_random(void)
 {
-Error *err = NULL;
 uint32_t num;
-
-if (qcrypto_random_bytes((uint8_t *), sizeof(num), )) {
-error_report_err(err);
-exit(1);
-}
-
+qemu_guest_getrandom_nofail(, sizeof(num));
 return num;
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH v7 15/24] linux-user: Remove srand call

[Richard Henderson]

We no longer use rand() within linux-user.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 linux-user/main.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 4c72f07555..5792c40ace 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -623,8 +623,6 @@ int main(int argc, char **argv, char **envp)
 
 cpu_model = NULL;
 
-srand(time(NULL));
-
 qemu_add_opts(_trace_opts);
 
 optind = parse_args(argc, argv);
@@ -692,15 +690,6 @@ int main(int argc, char **argv, char **envp)
 {
 Error *err = NULL;
 if (seed_optarg != NULL) {
-unsigned long long seed;
-
-/* This will go away with the last user of rand(). */
-if (parse_uint_full(seed_optarg, , 0) != 0) {
-fprintf(stderr, "Invalid seed number: %s\n", seed_optarg);
-exit(EXIT_FAILURE);
-}
-srand(seed);
-
 qemu_guest_random_seed_main(seed_optarg, );
 } else {
 qcrypto_random_init();
-- 
2.17.1

[Qemu-devel] [PATCH v7 17/24] hw/misc/nrf51_rng: Use qemu_guest_getrandom_nofail

[Richard Henderson]

The random number is intended for use by the guest.  As such, we should
honor the -seed argument for reproducibility.  Use the *_nofail routine
instead of error_abort directly.

Reviewed-by: Laurent Vivier 
Reviewed-by: Joel Stanley 
Signed-off-by: Richard Henderson 
---
 hw/misc/nrf51_rng.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/nrf51_rng.c b/hw/misc/nrf51_rng.c
index d188f044f4..3400e90a9b 100644
--- a/hw/misc/nrf51_rng.c
+++ b/hw/misc/nrf51_rng.c
@@ -14,7 +14,7 @@
 #include "qapi/error.h"
 #include "hw/arm/nrf51.h"
 #include "hw/misc/nrf51_rng.h"
-#include "crypto/random.h"
+#include "qemu/guest-random.h"
 
 static void update_irq(NRF51RNGState *s)
 {
@@ -145,7 +145,7 @@ static void nrf51_rng_timer_expire(void *opaque)
 {
 NRF51RNGState *s = NRF51_RNG(opaque);
 
-qcrypto_random_bytes(>value, 1, _abort);
+qemu_guest_getrandom_nofail(>value, 1);
 
 s->event_valrdy = 1;
 qemu_set_irq(s->eep_valrdy, 1);
-- 
2.17.1

[Qemu-devel] [PATCH v7 14/24] linux-user/aarch64: Use qemu_guest_getrandom for PAUTH keys

[Richard Henderson]

Use a better interface for random numbers than rand() * 3.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 linux-user/aarch64/target_syscall.h |  2 --
 linux-user/aarch64/cpu_loop.c   | 29 ++-
 linux-user/syscall.c| 31 -
 3 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/linux-user/aarch64/target_syscall.h 
b/linux-user/aarch64/target_syscall.h
index b595e5da82..995e475c73 100644
--- a/linux-user/aarch64/target_syscall.h
+++ b/linux-user/aarch64/target_syscall.h
@@ -29,6 +29,4 @@ struct target_pt_regs {
 # define TARGET_PR_PAC_APDBKEY   (1 << 3)
 # define TARGET_PR_PAC_APGAKEY   (1 << 4)
 
-void arm_init_pauth_key(ARMPACKey *key);
-
 #endif /* AARCH64_TARGET_SYSCALL_H */
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index d75fd9d3e2..cedad39ca0 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "qemu.h"
 #include "cpu_loop-common.h"
+#include "qemu/guest-random.h"
 
 #define get_user_code_u32(x, gaddr, env)\
 ({ abi_long __r = get_user_u32((x), (gaddr));   \
@@ -147,24 +148,6 @@ void cpu_loop(CPUARMState *env)
 }
 }
 
-static uint64_t arm_rand64(void)
-{
-int shift = 64 - clz64(RAND_MAX);
-int i, n = 64 / shift + (64 % shift != 0);
-uint64_t ret = 0;
-
-for (i = 0; i < n; i++) {
-ret = (ret << shift) | rand();
-}
-return ret;
-}
-
-void arm_init_pauth_key(ARMPACKey *key)
-{
-key->lo = arm_rand64();
-key->hi = arm_rand64();
-}
-
 void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
 {
 ARMCPU *cpu = arm_env_get_cpu(env);
@@ -192,11 +175,11 @@ void target_cpu_copy_regs(CPUArchState *env, struct 
target_pt_regs *regs)
 #endif
 
 if (cpu_isar_feature(aa64_pauth, cpu)) {
-arm_init_pauth_key(>apia_key);
-arm_init_pauth_key(>apib_key);
-arm_init_pauth_key(>apda_key);
-arm_init_pauth_key(>apdb_key);
-arm_init_pauth_key(>apga_key);
+qemu_guest_getrandom_nofail(>apia_key, sizeof(ARMPACKey));
+qemu_guest_getrandom_nofail(>apib_key, sizeof(ARMPACKey));
+qemu_guest_getrandom_nofail(>apda_key, sizeof(ARMPACKey));
+qemu_guest_getrandom_nofail(>apdb_key, sizeof(ARMPACKey));
+qemu_guest_getrandom_nofail(>apga_key, sizeof(ARMPACKey));
 }
 
 ts->stack_base = info->start_stack;
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 96f20886ce..8c17b14d51 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -108,6 +108,7 @@
 
 #include "qemu.h"
 #include "qemu/guest-random.h"
+#include "qapi/error.h"
 #include "fd-trans.h"
 
 #ifndef CLONE_IO
@@ -9765,25 +9766,45 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 int all = (TARGET_PR_PAC_APIAKEY | TARGET_PR_PAC_APIBKEY |
TARGET_PR_PAC_APDAKEY | TARGET_PR_PAC_APDBKEY |
TARGET_PR_PAC_APGAKEY);
+int ret = 0;
+Error *err = NULL;
+
 if (arg2 == 0) {
 arg2 = all;
 } else if (arg2 & ~all) {
 return -TARGET_EINVAL;
 }
 if (arg2 & TARGET_PR_PAC_APIAKEY) {
-arm_init_pauth_key(>apia_key);
+ret |= qemu_guest_getrandom(>apia_key,
+sizeof(ARMPACKey), );
 }
 if (arg2 & TARGET_PR_PAC_APIBKEY) {
-arm_init_pauth_key(>apib_key);
+ret |= qemu_guest_getrandom(>apib_key,
+sizeof(ARMPACKey), );
 }
 if (arg2 & TARGET_PR_PAC_APDAKEY) {
-arm_init_pauth_key(>apda_key);
+ret |= qemu_guest_getrandom(>apda_key,
+sizeof(ARMPACKey), );
 }
 if (arg2 & TARGET_PR_PAC_APDBKEY) {
-arm_init_pauth_key(>apdb_key);
+ret |= qemu_guest_getrandom(>apdb_key,
+sizeof(ARMPACKey), );
 }
 if (arg2 & TARGET_PR_PAC_APGAKEY) {
-arm_init_pauth_key(>apga_key);
+ret |= qemu_guest_getrandom(>apga_key,
+sizeof(ARMPACKey), );
+}
+if (ret != 0) {
+/*
+ * Some unknown failure in the crypto.  The best
+ * we can do is log it and fail the syscall.
+  

[Qemu-devel] [PATCH v7 13/24] linux-user: Use qemu_guest_getrandom_nofail for AT_RANDOM

[Richard Henderson]

Use a better interface for random numbers than rand * 16.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 linux-user/elfload.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index ef42e02d82..1e06b908b7 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -7,6 +7,7 @@
 #include "qemu.h"
 #include "disas/disas.h"
 #include "qemu/path.h"
+#include "qemu/guest-random.h"
 
 #ifdef _ARCH_PPC64
 #undef ARCH_DLINFO
@@ -1883,12 +1884,9 @@ static abi_ulong create_elf_tables(abi_ulong p, int 
argc, int envc,
 }
 
 /*
- * Generate 16 random bytes for userspace PRNG seeding (not
- * cryptically secure but it's not the aim of QEMU).
+ * Generate 16 random bytes for userspace PRNG seeding.
  */
-for (i = 0; i < 16; i++) {
-k_rand_bytes[i] = rand();
-}
+qemu_guest_getrandom_nofail(k_rand_bytes, sizeof(k_rand_bytes));
 if (STACK_GROWS_DOWN) {
 sp -= 16;
 u_rand_bytes = sp;
-- 
2.17.1

[Qemu-devel] [PATCH v7 09/24] util: Add qemu_guest_getrandom and associated routines

[Richard Henderson]

This routine is intended to produce high-quality random numbers to the
guest.  Normally, such numbers are crypto quality from the host, but a
command-line option can force the use of a fully deterministic sequence
for use while debugging.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 include/qemu/guest-random.h | 68 +++
 util/guest-random.c | 93 +
 util/Makefile.objs  |  1 +
 3 files changed, 162 insertions(+)
 create mode 100644 include/qemu/guest-random.h
 create mode 100644 util/guest-random.c

diff --git a/include/qemu/guest-random.h b/include/qemu/guest-random.h
new file mode 100644
index 00..09ff9c2236
--- /dev/null
+++ b/include/qemu/guest-random.h
@@ -0,0 +1,68 @@
+/*
+ * QEMU guest-visible random functions
+ *
+ * Copyright 2019 Linaro, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#ifndef QEMU_GUEST_RANDOM_H
+#define QEMU_GUEST_RANDOM_H
+
+/**
+ * qemu_guest_random_seed_main(const char *optarg, Error **errp)
+ * @optarg: a non-NULL pointer to a C string
+ * @errp: an error indicator
+ *
+ * The @optarg value is that which accompanies the -seed argument.
+ * This forces qemu_guest_getrandom into deterministic mode.
+ *
+ * Returns 0 on success, < 0 on failure while setting *errp.
+ */
+int qemu_guest_random_seed_main(const char *optarg, Error **errp);
+
+/**
+ * qemu_guest_random_seed_thread_part1(void)
+ *
+ * If qemu_getrandom is in deterministic mode, returns an
+ * independent seed for the new thread.  Otherwise returns 0.
+ */
+uint64_t qemu_guest_random_seed_thread_part1(void);
+
+/**
+ * qemu_guest_random_seed_thread_part2(uint64_t seed)
+ * @seed: a value for the new thread.
+ *
+ * If qemu_guest_getrandom is in deterministic mode, this stores an
+ * independent seed for the new thread.  Otherwise a no-op.
+ */
+void qemu_guest_random_seed_thread_part2(uint64_t seed);
+
+/**
+ * qemu_guest_getrandom(void *buf, size_t len, Error **errp)
+ * @buf: a buffer of bytes to be written
+ * @len: the number of bytes in @buf
+ * @errp: an error indicator
+ *
+ * Fills len bytes in buf with random data.  This should only be used
+ * for data presented to the guest.  Host-side crypto services should
+ * use qcrypto_random_bytes.
+ *
+ * Returns 0 on success, < 0 on failure while setting *errp.
+ */
+int qemu_guest_getrandom(void *buf, size_t len, Error **errp);
+
+/**
+ * qemu_guest_getrandom_nofail(void *buf, size_t len)
+ * @buf: a buffer of bytes to be written
+ * @len: the number of bytes in @buf
+ *
+ * Like qemu_guest_getrandom, but will assert for failure.
+ * Use this when there is no reasonable recovery.
+ */
+void qemu_guest_getrandom_nofail(void *buf, size_t len);
+
+#endif /* QEMU_GUEST_RANDOM_H */
diff --git a/util/guest-random.c b/util/guest-random.c
new file mode 100644
index 00..e8124a3cad
--- /dev/null
+++ b/util/guest-random.c
@@ -0,0 +1,93 @@
+/*
+ * QEMU guest-visible random functions
+ *
+ * Copyright 2019 Linaro, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "qemu/guest-random.h"
+#include "crypto/random.h"
+
+
+static __thread GRand *thread_rand;
+static bool deterministic;
+
+
+static int glib_random_bytes(void *buf, size_t len)
+{
+GRand *rand = thread_rand;
+size_t i;
+uint32_t x;
+
+if (unlikely(rand == NULL)) {
+/* Thread not initialized for a cpu, or main w/o -seed.  */
+thread_rand = rand = g_rand_new();
+}
+
+for (i = 0; i + 4 <= len; i += 4) {
+x = g_rand_int(rand);
+__builtin_memcpy(buf + i, , 4);
+}
+if (i < len) {
+x = g_rand_int(rand);
+__builtin_memcpy(buf + i, , i - len);
+}
+return 0;
+}
+
+int qemu_guest_getrandom(void *buf, size_t len, Error **errp)
+{
+if (unlikely(deterministic)) {
+/* Deterministic implementation using Glib's Mersenne Twister.  */
+return glib_random_bytes(buf, len);
+} else {
+/* Non-deterministic implementation using crypto routines.  */
+return qcrypto_random_bytes(buf, len, errp);
+}
+}
+
+void qemu_guest_getrandom_nofail(void *buf, size_t len)
+{
+qemu_guest_getrandom(buf, len, _fatal);
+}
+
+uint64_t qemu_guest_random_seed_thread_part1(void)
+{
+if (deterministic) {
+uint64_t ret;
+glib_random_bytes(, sizeof(ret));
+return ret;
+}
+

Re: [Qemu-devel] [PATCH for-4.0.1] q35: Revert to kernel irqchip

[Daniel P . Berrangé]

On Tue, May 14, 2019 at 01:03:31PM -0600, Alex Williamson wrote:
> Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
> the default for the pc-q35-4.0 machine type to use split irqchip, which
> turned out to have disasterous effects on vfio-pci INTx support.  KVM
> resampling irqfds are registered for handling these interrupts, but
> these are non-functional in split irqchip mode.  We can't simply test
> for split irqchip in QEMU as userspace handling of this interrupt is a
> significant performance regression versus KVM handling (GeForce GPUs
> assigned to Windows VMs are non-functional without forcing MSI mode or
> re-enabling kernel irqchip).
> 
> The resolution is to revert the change in default irqchip mode with a
> new pc-q35-4.0.1 machine type for qemu-stable while the development
> branch makes the same change in the pc-q35-4.1 machine type.  The
> qemu-q35-4.0 machine type should not be used in vfio-pci configurations
> for devices requiring legacy INTx support without explicitly modifying
> the VM configuration to use KVM irqchip.  This new 4.0.1 machine type
> makes this change automatically.

If we introduce a pc-q35-4.0.1 machine type in -stable, then VMs
created in stable won't be migratable to future 4.1 unless we also
create this same machine type in master.

If we really want to create a new 4.0.1 machine type, then this
patch needs to go to git master before stable IMHO to guarantee
no regression to master.

> Link: https://bugs.launchpad.net/qemu/+bug/1826422
> Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03305.html
> Fixes: b2fc91db8447 ("q35: set split kernel irqchip as default")
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Alex Williamson 
> ---
> 
> Do we want new stable versions for other archs too or only as needed?



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 :|

[Qemu-devel] [PATCH v7 10/24] cpus: Initialize pseudo-random seeds for all guest cpus

[Richard Henderson]

When the -seed option is given, call qemu_guest_random_seed_main,
putting the subsystem into deterministic mode.  Pass derived seeds
to each cpu created; which is a no-op unless the subsystem is in
deterministic mode.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 include/qom/cpu.h |  1 +
 cpus.c|  9 +
 vl.c  |  4 
 qemu-options.hx   | 10 ++
 4 files changed, 24 insertions(+)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 08abcbd3fe..9793ec39bc 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -369,6 +369,7 @@ struct CPUState {
 int singlestep_enabled;
 int64_t icount_budget;
 int64_t icount_extra;
+uint64_t random_seed;
 sigjmp_buf jmp_env;
 
 QemuMutex work_mutex;
diff --git a/cpus.c b/cpus.c
index e58e7ab0f6..ffc57119ca 100644
--- a/cpus.c
+++ b/cpus.c
@@ -50,6 +50,7 @@
 #include "qemu/option.h"
 #include "qemu/bitmap.h"
 #include "qemu/seqlock.h"
+#include "qemu/guest-random.h"
 #include "tcg.h"
 #include "hw/nmi.h"
 #include "sysemu/replay.h"
@@ -1276,6 +1277,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 /* signal CPU creation */
 cpu->created = true;
 qemu_cond_signal(_cpu_cond);
+qemu_guest_random_seed_thread_part2(cpu->random_seed);
 
 do {
 if (cpu_can_run(cpu)) {
@@ -1319,6 +1321,7 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
 /* signal CPU creation */
 cpu->created = true;
 qemu_cond_signal(_cpu_cond);
+qemu_guest_random_seed_thread_part2(cpu->random_seed);
 
 do {
 qemu_mutex_unlock_iothread();
@@ -1478,6 +1481,7 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
 cpu->created = true;
 cpu->can_do_io = 1;
 qemu_cond_signal(_cpu_cond);
+qemu_guest_random_seed_thread_part2(cpu->random_seed);
 
 /* wait for initial kick-off after machine start */
 while (first_cpu->stopped) {
@@ -1592,6 +1596,7 @@ static void *qemu_hax_cpu_thread_fn(void *arg)
 
 hax_init_vcpu(cpu);
 qemu_cond_signal(_cpu_cond);
+qemu_guest_random_seed_thread_part2(cpu->random_seed);
 
 do {
 if (cpu_can_run(cpu)) {
@@ -1631,6 +1636,7 @@ static void *qemu_hvf_cpu_thread_fn(void *arg)
 /* signal CPU creation */
 cpu->created = true;
 qemu_cond_signal(_cpu_cond);
+qemu_guest_random_seed_thread_part2(cpu->random_seed);
 
 do {
 if (cpu_can_run(cpu)) {
@@ -1671,6 +1677,7 @@ static void *qemu_whpx_cpu_thread_fn(void *arg)
 /* signal CPU creation */
 cpu->created = true;
 qemu_cond_signal(_cpu_cond);
+qemu_guest_random_seed_thread_part2(cpu->random_seed);
 
 do {
 if (cpu_can_run(cpu)) {
@@ -1724,6 +1731,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 cpu->can_do_io = 1;
 current_cpu = cpu;
 qemu_cond_signal(_cpu_cond);
+qemu_guest_random_seed_thread_part2(cpu->random_seed);
 
 /* process any pending work */
 cpu->exit_request = 1;
@@ -2071,6 +2079,7 @@ void qemu_init_vcpu(CPUState *cpu)
 cpu->nr_cores = smp_cores;
 cpu->nr_threads = smp_threads;
 cpu->stopped = true;
+cpu->random_seed = qemu_guest_random_seed_thread_part1();
 
 if (!cpu->as) {
 /* If the target cpu hasn't set up any address spaces itself,
diff --git a/vl.c b/vl.c
index b6709514c1..e1d75a047f 100644
--- a/vl.c
+++ b/vl.c
@@ -128,6 +128,7 @@ int main(int argc, char **argv)
 #include "qapi/qapi-commands-ui.h"
 #include "qapi/qmp/qerror.h"
 #include "sysemu/iothread.h"
+#include "qemu/guest-random.h"
 
 #define MAX_VIRTIO_CONSOLES 1
 
@@ -3347,6 +3348,9 @@ int main(int argc, char **argv, char **envp)
 case QEMU_OPTION_DFILTER:
 qemu_set_dfilter_ranges(optarg, _fatal);
 break;
+case QEMU_OPTION_seed:
+qemu_guest_random_seed_main(optarg, _fatal);
+break;
 case QEMU_OPTION_s:
 add_device_config(DEV_GDB, "tcp::" DEFAULT_GDBSTUB_PORT);
 break;
diff --git a/qemu-options.hx b/qemu-options.hx
index 51802cbb26..0191ef8b1e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3601,6 +3601,16 @@ the 0x200 sized block starting at 0xffc8 and 
another 0x1000 sized
 block starting at 0xffc5f000.
 ETEXI
 
+DEF("seed", HAS_ARG, QEMU_OPTION_seed, \
+"-seed number   seed the pseudo-random number generator\n",
+QEMU_ARCH_ALL)
+STEXI
+@item -seed @var{number}
+@findex -seed
+Force the guest to use a deterministic pseudo-random number generator, seeded
+with @var{number}.  This does not affect crypto routines within the host.
+ETEXI
+
 DEF("L", HAS_ARG, QEMU_OPTION_L, \
 "-L path set the directory for the BIOS, VGA BIOS and keymaps\n",
 QEMU_ARCH_ALL)
-- 
2.17.1

[Qemu-devel] [PATCH v7 04/24] crypto: Use O_CLOEXEC in qcrypto_random_init

[Richard Henderson]

Avoids leaking the /dev/urandom fd into any child processes.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 crypto/random-platform.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index 260b64564d..6df40744c7 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -42,9 +42,9 @@ int qcrypto_random_init(Error **errp)
 #else
 /* TBD perhaps also add support for BSD getentropy / Linux
  * getrandom syscalls directly */
-fd = open("/dev/urandom", O_RDONLY);
+fd = open("/dev/urandom", O_RDONLY | O_CLOEXEC);
 if (fd == -1 && errno == ENOENT) {
-fd = open("/dev/random", O_RDONLY);
+fd = open("/dev/random", O_RDONLY | O_CLOEXEC);
 }
 
 if (fd < 0) {
-- 
2.17.1

[Qemu-devel] [PATCH v7 23/24] target/ppc: Use gen_io_start/end around DARN

[Richard Henderson]

Generating a random number counts as I/O, as it cannot be
replayed and produce the same results.

Acked-by: David Gibson 
Reviewed-by: Laurent Vivier 
Suggested-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target/ppc/translate.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b5217f632f..4a5de28036 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1847,13 +1847,22 @@ static void gen_darn(DisasContext *ctx)
 {
 int l = L(ctx->opcode);
 
-if (l == 0) {
-gen_helper_darn32(cpu_gpr[rD(ctx->opcode)]);
-} else if (l <= 2) {
-/* Return 64-bit random for both CRN and RRN */
-gen_helper_darn64(cpu_gpr[rD(ctx->opcode)]);
-} else {
+if (l > 2) {
 tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], -1);
+} else {
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+if (l == 0) {
+gen_helper_darn32(cpu_gpr[rD(ctx->opcode)]);
+} else {
+/* Return 64-bit random for both CRN and RRN */
+gen_helper_darn64(cpu_gpr[rD(ctx->opcode)]);
+}
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_end();
+gen_stop_exception(ctx);
+}
 }
 }
 #endif
-- 
2.17.1

[Qemu-devel] [PATCH v7 12/24] linux-user: Call qcrypto_random_init if not using -seed

[Richard Henderson]

When not using -seed, we will use the random part of the crypto
subsystem.  For softmmu, we initialize the full qcrypto_init,
because we use crypto for reasons other than random numbers.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 linux-user/main.c | 28 +++-
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 7e704845c0..4c72f07555 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -39,6 +39,7 @@
 #include "trace/control.h"
 #include "target_elf.h"
 #include "cpu_loop-common.h"
+#include "crypto/random.h"
 
 char *exec_path;
 
@@ -688,17 +689,26 @@ int main(int argc, char **argv, char **envp)
 if (seed_optarg == NULL) {
 seed_optarg = getenv("QEMU_RAND_SEED");
 }
-if (seed_optarg != NULL) {
-unsigned long long seed;
+{
+Error *err = NULL;
+if (seed_optarg != NULL) {
+unsigned long long seed;
 
-/* This will go away with the last user of rand(). */
-if (parse_uint_full(seed_optarg, , 0) != 0) {
-fprintf(stderr, "Invalid seed number: %s\n", seed_optarg);
-exit(EXIT_FAILURE);
+/* This will go away with the last user of rand(). */
+if (parse_uint_full(seed_optarg, , 0) != 0) {
+fprintf(stderr, "Invalid seed number: %s\n", seed_optarg);
+exit(EXIT_FAILURE);
+}
+srand(seed);
+
+qemu_guest_random_seed_main(seed_optarg, );
+} else {
+qcrypto_random_init();
+}
+if (err) {
+error_reportf_err(err, "cannot initialize crypto: ");
+exit(1);
 }
-srand(seed);
-
-qemu_guest_random_seed_main(seed_optarg, _fatal);
 }
 
 target_environ = envlist_to_environ(envlist, NULL);
-- 
2.17.1

[Qemu-devel] [PATCH v7 07/24] ui/vnc: Split out authentication_failed

[Richard Henderson]

There were 3 copies of this code, one of which used the wrong
data size for the failure indicator.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 ui/vnc.c | 37 +++--
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 1871422e1d..785edf3af1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2535,6 +2535,18 @@ void start_client_init(VncState *vs)
 vnc_read_when(vs, protocol_client_init, 1);
 }
 
+static void authentication_failed(VncState *vs)
+{
+vnc_write_u32(vs, 1); /* Reject auth */
+if (vs->minor >= 8) {
+static const char err[] = "Authentication failed";
+vnc_write_u32(vs, sizeof(err));
+vnc_write(vs, err, sizeof(err));
+}
+vnc_flush(vs);
+vnc_client_error(vs);
+}
+
 static void make_challenge(VncState *vs)
 {
 int i;
@@ -2609,14 +2621,7 @@ static int protocol_client_auth_vnc(VncState *vs, 
uint8_t *data, size_t len)
 return 0;
 
 reject:
-vnc_write_u32(vs, 1); /* Reject auth */
-if (vs->minor >= 8) {
-static const char err[] = "Authentication failed";
-vnc_write_u32(vs, sizeof(err));
-vnc_write(vs, err, sizeof(err));
-}
-vnc_flush(vs);
-vnc_client_error(vs);
+authentication_failed(vs);
 qcrypto_cipher_free(cipher);
 return 0;
 }
@@ -2638,13 +2643,7 @@ static int protocol_client_auth(VncState *vs, uint8_t 
*data, size_t len)
  * must pick the one we sent. Verify this */
 if (data[0] != vs->auth) { /* Reject auth */
trace_vnc_auth_reject(vs, vs->auth, (int)data[0]);
-   vnc_write_u32(vs, 1);
-   if (vs->minor >= 8) {
-   static const char err[] = "Authentication failed";
-   vnc_write_u32(vs, sizeof(err));
-   vnc_write(vs, err, sizeof(err));
-   }
-   vnc_client_error(vs);
+   authentication_failed(vs);
 } else { /* Accept requested auth */
trace_vnc_auth_start(vs, vs->auth);
switch (vs->auth) {
@@ -2673,13 +2672,7 @@ static int protocol_client_auth(VncState *vs, uint8_t 
*data, size_t len)
 
default: /* Should not be possible, but just in case */
trace_vnc_auth_fail(vs, vs->auth, "Unhandled auth method", "");
-   vnc_write_u8(vs, 1);
-   if (vs->minor >= 8) {
-   static const char err[] = "Authentication failed";
-   vnc_write_u32(vs, sizeof(err));
-   vnc_write(vs, err, sizeof(err));
-   }
-   vnc_client_error(vs);
+   authentication_failed(vs);
}
 }
 return 0;
-- 
2.17.1

[Qemu-devel] [PATCH v7 02/24] crypto: Reverse code blocks in random-platform.c

[Richard Henderson]

Use #ifdef _WIN32 instead of #ifndef _WIN32.
This will make other tests easier to sequence.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 crypto/random-platform.c | 35 +--
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index 7541b4cae7..f995fc0ef1 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -32,7 +32,14 @@ static int fd; /* a file handle to either /dev/urandom or 
/dev/random */
 
 int qcrypto_random_init(Error **errp)
 {
-#ifndef _WIN32
+#ifdef _WIN32
+if (!CryptAcquireContext(, NULL, NULL, PROV_RSA_FULL,
+ CRYPT_SILENT | CRYPT_VERIFYCONTEXT)) {
+error_setg_win32(errp, GetLastError(),
+ "Unable to create cryptographic provider");
+return -1;
+}
+#else
 /* TBD perhaps also add support for BSD getentropy / Linux
  * getrandom syscalls directly */
 fd = open("/dev/urandom", O_RDONLY);
@@ -44,15 +51,7 @@ int qcrypto_random_init(Error **errp)
 error_setg(errp, "No /dev/urandom or /dev/random found");
 return -1;
 }
-#else
-if (!CryptAcquireContext(, NULL, NULL, PROV_RSA_FULL,
- CRYPT_SILENT | CRYPT_VERIFYCONTEXT)) {
-error_setg_win32(errp, GetLastError(),
- "Unable to create cryptographic provider");
-return -1;
-}
 #endif
-
 return 0;
 }
 
@@ -60,7 +59,15 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
  size_t buflen G_GNUC_UNUSED,
  Error **errp)
 {
-#ifndef _WIN32
+#ifdef _WIN32
+if (!CryptGenRandom(hCryptProv, buflen, buf)) {
+error_setg_win32(errp, GetLastError(),
+ "Unable to read random bytes");
+return -1;
+}
+
+return 0;
+#else
 int ret = -1;
 int got;
 
@@ -82,13 +89,5 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
 ret = 0;
  cleanup:
 return ret;
-#else
-if (!CryptGenRandom(hCryptProv, buflen, buf)) {
-error_setg_win32(errp, GetLastError(),
- "Unable to read random bytes");
-return -1;
-}
-
-return 0;
 #endif
 }
-- 
2.17.1

[Qemu-devel] [PATCH v7 06/24] crypto: Change the qcrypto_random_bytes buffer type to void*

[Richard Henderson]

Using uint8_t* merely requires useless casts for use with
other types to be filled with randomness.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 include/crypto/random.h  | 2 +-
 crypto/random-gcrypt.c   | 2 +-
 crypto/random-gnutls.c   | 2 +-
 crypto/random-platform.c | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/crypto/random.h b/include/crypto/random.h
index 8764ca0562..fde592904e 100644
--- a/include/crypto/random.h
+++ b/include/crypto/random.h
@@ -34,7 +34,7 @@
  *
  * Returns 0 on success, -1 on error
  */
-int qcrypto_random_bytes(uint8_t *buf,
+int qcrypto_random_bytes(void *buf,
  size_t buflen,
  Error **errp);
 
diff --git a/crypto/random-gcrypt.c b/crypto/random-gcrypt.c
index 9f1c9ee60e..7aea4ac81f 100644
--- a/crypto/random-gcrypt.c
+++ b/crypto/random-gcrypt.c
@@ -24,7 +24,7 @@
 
 #include 
 
-int qcrypto_random_bytes(uint8_t *buf,
+int qcrypto_random_bytes(void *buf,
  size_t buflen,
  Error **errp G_GNUC_UNUSED)
 {
diff --git a/crypto/random-gnutls.c b/crypto/random-gnutls.c
index 445fd6a30b..ed6c9ca12f 100644
--- a/crypto/random-gnutls.c
+++ b/crypto/random-gnutls.c
@@ -26,7 +26,7 @@
 #include 
 #include 
 
-int qcrypto_random_bytes(uint8_t *buf,
+int qcrypto_random_bytes(void *buf,
  size_t buflen,
  Error **errp)
 {
diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index cb3ca1bc09..66624106fe 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -64,8 +64,8 @@ int qcrypto_random_init(Error **errp)
 return 0;
 }
 
-int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
- size_t buflen G_GNUC_UNUSED,
+int qcrypto_random_bytes(void *buf,
+ size_t buflen,
  Error **errp)
 {
 #ifdef _WIN32
-- 
2.17.1

[Qemu-devel] [PATCH v7 11/24] linux-user: Initialize pseudo-random seeds for all guest cpus

[Richard Henderson]

When the -seed option is given, call qemu_guest_random_seed_main,
putting the subsystem into deterministic mode.  Pass derived seeds
to each cpu created during clone; which is a no-op unless the
subsystem is in deterministic mode.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 linux-user/main.c| 30 +++---
 linux-user/syscall.c |  3 +++
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 3d2230320b..7e704845c0 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -34,6 +34,7 @@
 #include "tcg.h"
 #include "qemu/timer.h"
 #include "qemu/envlist.h"
+#include "qemu/guest-random.h"
 #include "elf.h"
 #include "trace/control.h"
 #include "target_elf.h"
@@ -48,6 +49,7 @@ static int gdbstub_port;
 static envlist_t *envlist;
 static const char *cpu_model;
 static const char *cpu_type;
+static const char *seed_optarg;
 unsigned long mmap_min_addr;
 unsigned long guest_base;
 int have_guest_base;
@@ -290,15 +292,9 @@ static void handle_arg_pagesize(const char *arg)
 }
 }
 
-static void handle_arg_randseed(const char *arg)
+static void handle_arg_seed(const char *arg)
 {
-unsigned long long seed;
-
-if (parse_uint_full(arg, , 0) != 0 || seed > UINT_MAX) {
-fprintf(stderr, "Invalid seed number: %s\n", arg);
-exit(EXIT_FAILURE);
-}
-srand(seed);
+seed_optarg = arg;
 }
 
 static void handle_arg_gdb(const char *arg)
@@ -433,7 +429,7 @@ static const struct qemu_argument arg_table[] = {
  "",   "run in singlestep mode"},
 {"strace", "QEMU_STRACE",  false, handle_arg_strace,
  "",   "log system calls"},
-{"seed",   "QEMU_RAND_SEED",   true,  handle_arg_randseed,
+{"seed",   "QEMU_RAND_SEED",   true,  handle_arg_seed,
  "",   "Seed for pseudo-random number generator"},
 {"trace",  "QEMU_TRACE",   true,  handle_arg_trace,
  "",   "[[enable=]][,events=][,file=]"},
@@ -689,8 +685,20 @@ int main(int argc, char **argv, char **envp)
 do_strace = 1;
 }
 
-if (getenv("QEMU_RAND_SEED")) {
-handle_arg_randseed(getenv("QEMU_RAND_SEED"));
+if (seed_optarg == NULL) {
+seed_optarg = getenv("QEMU_RAND_SEED");
+}
+if (seed_optarg != NULL) {
+unsigned long long seed;
+
+/* This will go away with the last user of rand(). */
+if (parse_uint_full(seed_optarg, , 0) != 0) {
+fprintf(stderr, "Invalid seed number: %s\n", seed_optarg);
+exit(EXIT_FAILURE);
+}
+srand(seed);
+
+qemu_guest_random_seed_main(seed_optarg, _fatal);
 }
 
 target_environ = envlist_to_environ(envlist, NULL);
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f5ff6f5dc8..96f20886ce 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -107,6 +107,7 @@
 #include "uname.h"
 
 #include "qemu.h"
+#include "qemu/guest-random.h"
 #include "fd-trans.h"
 
 #ifndef CLONE_IO
@@ -5482,6 +5483,7 @@ static void *clone_func(void *arg)
 put_user_u32(info->tid, info->child_tidptr);
 if (info->parent_tidptr)
 put_user_u32(info->tid, info->parent_tidptr);
+qemu_guest_random_seed_thread_part2(cpu->random_seed);
 /* Enable signals.  */
 sigprocmask(SIG_SETMASK, >sigmask, NULL);
 /* Signal to the parent that we're ready.  */
@@ -5568,6 +5570,7 @@ static int do_fork(CPUArchState *env, unsigned int flags, 
abi_ulong newsp,
initializing, so temporarily block all signals.  */
 sigfillset();
 sigprocmask(SIG_BLOCK, , );
+cpu->random_seed = qemu_guest_random_seed_thread_part1();
 
 /* If this is our first additional thread, we need to ensure we
  * generate code for parallel execution and flush old translations.
-- 
2.17.1

[Qemu-devel] [PATCH v7 05/24] crypto: Use getrandom for qcrypto_random_bytes

[Richard Henderson]

Prefer it to direct use of /dev/urandom.

Reviewed-by: Laurent Vivier 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 crypto/random-platform.c | 37 -
 configure| 18 +-
 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index 6df40744c7..cb3ca1bc09 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -27,7 +27,11 @@
 #include 
 static HCRYPTPROV hCryptProv;
 #else
-static int fd; /* a file handle to either /dev/urandom or /dev/random */
+# ifdef CONFIG_GETRANDOM
+#  include 
+# endif
+/* This is -1 for getrandom(), or a file handle for /dev/{u,}random.  */
+static int fd;
 #endif
 
 int qcrypto_random_init(Error **errp)
@@ -40,15 +44,20 @@ int qcrypto_random_init(Error **errp)
 return -1;
 }
 #else
-/* TBD perhaps also add support for BSD getentropy / Linux
- * getrandom syscalls directly */
+# ifdef CONFIG_GETRANDOM
+if (getrandom(NULL, 0, 0) == 0) {
+/* Use getrandom() */
+fd = -1;
+return 0;
+}
+/* Fall through to /dev/urandom case.  */
+# endif
 fd = open("/dev/urandom", O_RDONLY | O_CLOEXEC);
 if (fd == -1 && errno == ENOENT) {
 fd = open("/dev/random", O_RDONLY | O_CLOEXEC);
 }
-
 if (fd < 0) {
-error_setg(errp, "No /dev/urandom or /dev/random found");
+error_setg_errno(errp, errno, "No /dev/urandom or /dev/random");
 return -1;
 }
 #endif
@@ -66,6 +75,24 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
 return -1;
 }
 #else
+# ifdef CONFIG_GETRANDOM
+if (likely(fd < 0)) {
+while (1) {
+ssize_t got = getrandom(buf, buflen, 0);
+if (likely(got == buflen)) {
+return 0;
+}
+if (got >= 0) {
+buflen -= got;
+buf += got;
+} else if (errno != EINTR) {
+error_setg_errno(errp, errno, "getrandom");
+return -1;
+}
+}
+}
+/* Fall through to /dev/urandom case.  */
+# endif
 while (1) {
 ssize_t got = read(fd, buf, buflen);
 if (likely(got == buflen)) {
diff --git a/configure b/configure
index 8999698bc2..9ecec9c494 100755
--- a/configure
+++ b/configure
@@ -5802,6 +5802,20 @@ if compile_prog "" "" ; then
 have_utmpx=yes
 fi
 
+##
+# check for getrandom()
+
+have_getrandom=no
+cat > $TMPC << EOF
+#include 
+int main(void) {
+return getrandom(0, 0, GRND_NONBLOCK);
+}
+EOF
+if compile_prog "" "" ; then
+have_getrandom=yes
+fi
+
 ##
 # checks for sanitizers
 
@@ -7189,7 +7203,9 @@ fi
 if test "$have_utmpx" = "yes" ; then
   echo "HAVE_UTMPX=y" >> $config_host_mak
 fi
-
+if test "$have_getrandom" = "yes" ; then
+  echo "CONFIG_GETRANDOM=y" >> $config_host_mak
+fi
 if test "$ivshmem" = "yes" ; then
   echo "CONFIG_IVSHMEM=y" >> $config_host_mak
 fi
-- 
2.17.1

[Qemu-devel] [PATCH v7 01/24] build: Link user-only with crypto-rng-obj-y

[Richard Henderson]

For user-only, we require only the random number bits of the
crypto subsystem.

We need to preserve --static linking, which for many recent Linux
distributions precludes using GnuTLS or GCrypt.  Instead, use our
random-platform module unconditionally.

Signed-off-by: Richard Henderson 
---
 Makefile | 6 --
 Makefile.objs| 1 +
 Makefile.target  | 3 ++-
 crypto/Makefile.objs | 1 +
 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 66d5c65156..524f2f8a57 100644
--- a/Makefile
+++ b/Makefile
@@ -411,6 +411,7 @@ dummy := $(call unnest-vars,, \
 block-obj-m \
 crypto-obj-y \
 crypto-aes-obj-y \
+crypto-rng-obj-y \
 qom-obj-y \
 io-obj-y \
 common-obj-y \
@@ -482,8 +483,9 @@ subdir-capstone: .git-submodule-status
 subdir-slirp: .git-submodule-status
$(call quiet-command,$(MAKE) -C $(SRC_PATH)/slirp 
BUILD_DIR="$(BUILD_DIR)/slirp" CC="$(CC)" AR="$(AR)" LD="$(LD)" 
RANLIB="$(RANLIB)" CFLAGS="$(QEMU_CFLAGS) $(CFLAGS)" LDFLAGS="$(LDFLAGS)")
 
-$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) \
-   $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
+$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) $(qom-obj-y) \
+   $(crypto-aes-obj-$(CONFIG_USER_ONLY)) \
+   $(crypto-rng-obj-$(CONFIG_USER_ONLY))
 
 ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
 # Only keep -O and -g cflags
diff --git a/Makefile.objs b/Makefile.objs
index cf065de5ed..0c13ff47ea 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -26,6 +26,7 @@ block-obj-m = block/
 
 crypto-obj-y = crypto/
 crypto-aes-obj-y = crypto/
+crypto-rng-obj-y = crypto/
 
 ###
 # qom-obj-y is code used by both qemu system emulation and qemu-img
diff --git a/Makefile.target b/Makefile.target
index ae02495951..4e579a0a84 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -181,6 +181,7 @@ dummy := $(call unnest-vars,.., \
chardev-obj-y \
crypto-obj-y \
crypto-aes-obj-y \
+   crypto-rng-obj-y \
qom-obj-y \
io-obj-y \
common-obj-y \
@@ -189,7 +190,7 @@ all-obj-y += $(common-obj-y)
 all-obj-y += $(qom-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(authz-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y) $(chardev-obj-y)
-all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y)
+all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y) $(crypto-rng-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)
 
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 256c9aca1f..ee7e628ca6 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -37,5 +37,6 @@ crypto-obj-y += block-luks.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
+crypto-rng-obj-y = random-platform.o
 
 stub-obj-y += pbkdf-stub.o
-- 
2.17.1

[Qemu-devel] [PATCH v7 03/24] crypto: Do not fail for EINTR during qcrypto_random_bytes

[Richard Henderson]

We can always get EINTR for read; /dev/urandom is no exception.

Rearrange the order of tests for likelihood; allow degenerate buflen==0
case to perform a no-op zero-length read.  This means that the normal
success path is a straight line with a single test for success.

Reviewed-by: Laurent Vivier 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 crypto/random-platform.c | 36 +++-
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index f995fc0ef1..260b64564d 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -65,29 +65,23 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
  "Unable to read random bytes");
 return -1;
 }
-
-return 0;
 #else
-int ret = -1;
-int got;
-
-while (buflen > 0) {
-got = read(fd, buf, buflen);
-if (got < 0) {
-error_setg_errno(errp, errno,
- "Unable to read random bytes");
-goto cleanup;
-} else if (!got) {
-error_setg(errp,
-   "Unexpected EOF reading random bytes");
-goto cleanup;
+while (1) {
+ssize_t got = read(fd, buf, buflen);
+if (likely(got == buflen)) {
+return 0;
+}
+if (got > 0) {
+buflen -= got;
+buf += got;
+} else if (got == 0) {
+error_setg(errp, "Unexpected EOF reading random bytes");
+return -1;
+} else if (errno != EINTR) {
+error_setg_errno(errp, errno, "Unable to read random bytes");
+return -1;
 }
-buflen -= got;
-buf += got;
 }
-
-ret = 0;
- cleanup:
-return ret;
 #endif
+return 0;
 }
-- 
2.17.1

[Qemu-devel] [PATCH v7 08/24] ui/vnc: Use gcrypto_random_bytes for start_auth_vnc

[Richard Henderson]

Use a better interface for random numbers than rand().
Fail gracefully if for some reason we cannot use the crypto system.

Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 ui/vnc.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 785edf3af1..d83f4a6ff9 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -43,6 +43,7 @@
 #include "crypto/hash.h"
 #include "crypto/tlscredsanon.h"
 #include "crypto/tlscredsx509.h"
+#include "crypto/random.h"
 #include "qom/object_interfaces.h"
 #include "qemu/cutils.h"
 #include "io/dns-resolver.h"
@@ -2547,16 +2548,6 @@ static void authentication_failed(VncState *vs)
 vnc_client_error(vs);
 }
 
-static void make_challenge(VncState *vs)
-{
-int i;
-
-srand(time(NULL)+getpid()+getpid()*987654+rand());
-
-for (i = 0 ; i < sizeof(vs->challenge) ; i++)
-vs->challenge[i] = (int) (256.0*rand()/(RAND_MAX+1.0));
-}
-
 static int protocol_client_auth_vnc(VncState *vs, uint8_t *data, size_t len)
 {
 unsigned char response[VNC_AUTH_CHALLENGE_SIZE];
@@ -2628,7 +2619,16 @@ reject:
 
 void start_auth_vnc(VncState *vs)
 {
-make_challenge(vs);
+Error *err = NULL;
+
+if (qcrypto_random_bytes(vs->challenge, sizeof(vs->challenge), )) {
+trace_vnc_auth_fail(vs, vs->auth, "cannot get random bytes",
+error_get_pretty(err));
+error_free(err);
+authentication_failed(vs);
+return;
+}
+
 /* Send client a 'random' challenge */
 vnc_write(vs, vs->challenge, sizeof(vs->challenge));
 vnc_flush(vs);
-- 
2.17.1

[Qemu-devel] [PATCH v7 00/24] Add qemu_getrandom and ARMv8.5-RNG etc

[Richard Henderson]

Changes since v6:
  * Drop the crypto-obj-y, crypto-aes-obj-y, and configure changes.
This fixes the regression visible in make check-unit, due to
objects not being pulled in from libqemuutil.a.
  * Add a crypto-rng-obj-y, and force it to be random-platform.o.
This avoids attempting to link the crypto libraries into the
user-only binaries.  Which in turn means we can drop all of
the configure changes to cope with the crypto libraries not
having the static libraries packaged by recent distros.

Changes since v5:
  * Merge crypto-obj-y into util-obj-y (patch 2).
  * Fix leftover crypto-obj-aes-y reference (patch 2).
  * Add ARM_CP_IO to the RNG registers (patch 22).
  * Issue gen_io_start/end around ppc DARN (new patch 24).
  * Issue gen_io_start/end around x86 rdrand (patch 25).

Changes since v4:
  * Do not autoenable nettle or gcrypt if linking is broken.
Fixes --static on fedora 30.
  * Delay removal of srand() for -seed.
  * Do not loop for -1 result for ppc64 DARN.

Changes since v3:
  * Do not autoenable gnutls if linking is broken.
Fixes --static on ubuntu 18.04.

Changes since v2:
  * Changes from review.
- getrandom is not exclusive of /dev/urandom fallback.
- vnc fails gracefully on crypto failure.
- a great renaming.
  * Drop the "nonblock" argument, as it's not deliverable from the backend.
  * Propagate Error back through qemu_guest_getrandom.
  * Add qemu_guest_getrandom_nofail to centralize "Argh! Death!".
  * Convert hw/misc/
  * Implement ppc darn.
  * Implement x86 rdrand.

Changes since v1:
  * Build crypto-obj-y for linux-user as well.
  * Several patches to tidy crypto/random-platform.c.
  * Use getrandom(2) in crypto/random-platform.c.
  * Use qcrypto_random_bytes in ui/vnc.c.
  * In qemu_getrandom:
- Use g_rand_int instead of srand48.
- Use qcrypto_random_bytes instead of getrandom directly.


r~


Richard Henderson (24):
  build: Link user-only with crypto-rng-obj-y
  crypto: Reverse code blocks in random-platform.c
  crypto: Do not fail for EINTR during qcrypto_random_bytes
  crypto: Use O_CLOEXEC in qcrypto_random_init
  crypto: Use getrandom for qcrypto_random_bytes
  crypto: Change the qcrypto_random_bytes buffer type to void*
  ui/vnc: Split out authentication_failed
  ui/vnc: Use gcrypto_random_bytes for start_auth_vnc
  util: Add qemu_guest_getrandom and associated routines
  cpus: Initialize pseudo-random seeds for all guest cpus
  linux-user: Initialize pseudo-random seeds for all guest cpus
  linux-user: Call qcrypto_random_init if not using -seed
  linux-user: Use qemu_guest_getrandom_nofail for AT_RANDOM
  linux-user/aarch64: Use qemu_guest_getrandom for PAUTH keys
  linux-user: Remove srand call
  aspeed/scu: Use qemu_guest_getrandom_nofail
  hw/misc/nrf51_rng: Use qemu_guest_getrandom_nofail
  hw/misc/bcm2835_rng: Use qemu_guest_getrandom_nofail
  hw/misc/exynos4210_rng: Use qemu_guest_getrandom
  target/arm: Put all PAC keys into a structure
  target/arm: Implement ARMv8.5-RNG
  target/ppc: Use qemu_guest_getrandom for DARN
  target/ppc: Use gen_io_start/end around DARN
  target/i386: Implement CPUID_EXT_RDRAND

 Makefile|   6 +-
 Makefile.objs   |   1 +
 Makefile.target |   3 +-
 include/crypto/random.h |   2 +-
 include/qemu/guest-random.h |  68 ++
 include/qom/cpu.h   |   1 +
 linux-user/aarch64/target_syscall.h |   2 -
 target/arm/cpu.h|  17 +++--
 target/i386/helper.h|   2 +
 cpus.c  |   9 +++
 crypto/random-gcrypt.c  |   2 +-
 crypto/random-gnutls.c  |   2 +-
 crypto/random-platform.c| 104 +---
 hw/misc/aspeed_scu.c|  10 +--
 hw/misc/bcm2835_rng.c   |  32 -
 hw/misc/exynos4210_rng.c|  11 ++-
 hw/misc/nrf51_rng.c |   4 +-
 linux-user/aarch64/cpu_loop.c   |  25 +--
 linux-user/elfload.c|   8 +--
 linux-user/main.c   |  33 +
 linux-user/syscall.c|  34 +++--
 target/arm/cpu64.c  |   1 +
 target/arm/helper.c |  64 ++---
 target/arm/pauth_helper.c   |  18 ++---
 target/i386/cpu.c   |   5 +-
 target/i386/int_helper.c|  21 ++
 target/i386/translate.c |  62 +
 target/ppc/int_helper.c |  39 +++
 target/ppc/translate.c  |  21 --
 ui/vnc.c|  53 ++
 util/guest-random.c |  93 +
 vl.c|   4 ++
 configure   |  18 -
 crypto/Makefile.objs|   1 +
 qemu-options.hx |  10 +++
 util/Makefile.objs  |   1 +
 36 files changed, 566 

Re: [Qemu-devel] [PATCH] configure: Disable slirp if --disable-system

[Aleksandar Markovic]

On May 13, 2019 11:14 PM, "Richard Henderson" 
wrote:
>
> On 5/11/19 5:47 AM, Aleksandar Markovic wrote:
> >
> > On May 10, 2019 10:36 PM, "Richard Henderson" <
richard.hender...@linaro.org
> > > wrote:
> >>
> >> For linux-user, there is no need to add slirp to the set of
> >> git modules checked out, nor build it.
> >>
> >> This also avoids a makefile bug wrt insufficient dependencies
> >> on subdir-slirp.  If slirp/ is not initially present, the
> >> dependencies that check it out are associated with softmmu,
> >> which then generates a build error on slirp/ not present.
> >>
> >
> > Hi,
> >
> > Does this work if only user mode targets are specified via
ˊ--target-listˊ
> > switch?
>
> Yes.  There is a bit of code that converts such a target list to the same
> result as --disable-system, which is $softmmu = no.
>
> > If no, the patch shoud be amended. If yes, the commit message should be
> > extended.
>
> Like what?  I think it's pretty clear as is.
>

Richard, no. In this case, there is a glaring discrepancy between the title
and the functionality that the change provides. Much better title would be
“configure: Disable slirp if no system mode target is selected”.

I leave it to you to find out what can be improved in the commit message.

How well did you test your change? Did you try some corner cases?

I don't have concerns about the wording of the commit message only. I agree
with Thomas that combination of “no system mode target is selected” and
“--enable-slirp is used” must have some special handing. We can't just
leave the rest of the script to do whatever the current code happens to do.
The patch code should be completed.

Thanks,
Aleksandar

>
> r~

[Qemu-devel] [PATCH for-4.0.1] q35: Revert to kernel irqchip

[Alex Williamson]

Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
the default for the pc-q35-4.0 machine type to use split irqchip, which
turned out to have disasterous effects on vfio-pci INTx support.  KVM
resampling irqfds are registered for handling these interrupts, but
these are non-functional in split irqchip mode.  We can't simply test
for split irqchip in QEMU as userspace handling of this interrupt is a
significant performance regression versus KVM handling (GeForce GPUs
assigned to Windows VMs are non-functional without forcing MSI mode or
re-enabling kernel irqchip).

The resolution is to revert the change in default irqchip mode with a
new pc-q35-4.0.1 machine type for qemu-stable while the development
branch makes the same change in the pc-q35-4.1 machine type.  The
qemu-q35-4.0 machine type should not be used in vfio-pci configurations
for devices requiring legacy INTx support without explicitly modifying
the VM configuration to use KVM irqchip.  This new 4.0.1 machine type
makes this change automatically.

Link: https://bugs.launchpad.net/qemu/+bug/1826422
Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03305.html
Fixes: b2fc91db8447 ("q35: set split kernel irqchip as default")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Alex Williamson 
---

Do we want new stable versions for other archs too or only as needed?

 hw/core/machine.c|3 +++
 hw/i386/pc.c |3 +++
 hw/i386/pc_q35.c |   16 ++--
 include/hw/boards.h  |3 +++
 include/hw/i386/pc.h |3 +++
 5 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 743fef28982c..5d046a43e3d2 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -24,6 +24,9 @@
 #include "hw/pci/pci.h"
 #include "hw/mem/nvdimm.h"
 
+GlobalProperty hw_compat_4_0[] = {};
+const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
+
 GlobalProperty hw_compat_3_1[] = {
 { "pcie-root-port", "x-speed", "2_5" },
 { "pcie-root-port", "x-width", "1" },
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f2c15bf1f2c3..d98b737b8f3b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -115,6 +115,9 @@ struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 /* Physical Address of PVH entry point read from kernel ELF NOTE */
 static size_t pvh_start_addr;
 
+GlobalProperty pc_compat_4_0[] = {};
+const size_t pc_compat_4_0_len = G_N_ELEMENTS(pc_compat_4_0);
+
 GlobalProperty pc_compat_3_1[] = {
 { "intel-iommu", "dma-drain", "off" },
 { "Opteron_G3" "-" TYPE_X86_CPU, "rdtscp", "off" },
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 372c6b73bebd..45cc29d1adb7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -357,7 +357,7 @@ static void pc_q35_machine_options(MachineClass *m)
 m->units_per_default_bus = 1;
 m->default_machine_opts = "firmware=bios-256k.bin";
 m->default_display = "std";
-m->default_kernel_irqchip_split = true;
+m->default_kernel_irqchip_split = false;
 m->no_floppy = 1;
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
@@ -365,12 +365,24 @@ static void pc_q35_machine_options(MachineClass *m)
 m->max_cpus = 288;
 }
 
-static void pc_q35_4_0_machine_options(MachineClass *m)
+static void pc_q35_4_0_1_machine_options(MachineClass *m)
 {
 pc_q35_machine_options(m);
 m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v4_0_1, "pc-q35-4.0.1", NULL,
+   pc_q35_4_0_1_machine_options);
+
+static void pc_q35_4_0_machine_options(MachineClass *m)
+{
+pc_q35_4_0_1_machine_options(m);
+m->default_kernel_irqchip_split = true;
+m->alias = NULL;
+compat_props_add(m->compat_props, hw_compat_4_0, hw_compat_4_0_len);
+compat_props_add(m->compat_props, pc_compat_4_0, pc_compat_4_0_len);
+}
+
 DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
pc_q35_4_0_machine_options);
 
diff --git a/include/hw/boards.h b/include/hw/boards.h
index e231860666a1..fe1885cbffa0 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -293,6 +293,9 @@ struct MachineState {
 } \
 type_init(machine_initfn##_register_types)
 
+extern GlobalProperty hw_compat_4_0[];
+extern const size_t hw_compat_4_0_len;
+
 extern GlobalProperty hw_compat_3_1[];
 extern const size_t hw_compat_3_1_len;
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ca65ef18afb4..43df7230a22b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -293,6 +293,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
+extern GlobalProperty pc_compat_4_0[];
+extern const size_t pc_compat_4_0_len;
+
 extern GlobalProperty pc_compat_3_1[];
 extern const size_t pc_compat_3_1_len;
 

Re: [Qemu-devel] [PATCH v9 02/27] gdbstub: Implement deatch (D pkt) with new infra

[Alex Bennée]

Jon Doron  writes:

> Signed-off-by: Jon Doron 
> ---
>  gdbstub.c | 90 ++-
>  1 file changed, 50 insertions(+), 40 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index d5e0f3878a..621d689868 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1418,11 +1418,6 @@ static inline int startswith(const char *string, const 
> char *pattern)
>return !strncmp(string, pattern, strlen(pattern));
>  }
>
> -static int process_string_cmd(
> -GDBState *s, void *user_ctx, const char *data,
> -const GdbCmdParseEntry *cmds, int num_cmds)
> -__attribute__((unused));
> -
>  static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
>const GdbCmdParseEntry *cmds, int num_cmds)
>  {
> @@ -1468,6 +1463,41 @@ static int process_string_cmd(GDBState *s, void 
> *user_ctx, const char *data,
>  return -1;
>  }
>
> +static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +GDBProcess *process;
> +GDBState *s = gdb_ctx->s;
> +uint32_t pid = 1;
> +
> +if (s->multiprocess) {
> +if (!gdb_ctx->num_params) {
> +put_packet(s, "E22");
> +return;
> +}
> +
> +pid = gdb_ctx->params[0].val_ul;
> +}
> +
> +process = gdb_get_process(s, pid);
> +gdb_process_breakpoint_remove_all(s, process);
> +process->attached = false;
> +
> +if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
> +s->c_cpu = gdb_first_attached_cpu(s);
> +}
> +
> +if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
> +s->g_cpu = gdb_first_attached_cpu(s);
> +}
> +
> +if (!s->c_cpu) {
> +/* No more process attached */
> +gdb_syscall_mode = GDB_SYS_DISABLED;
> +gdb_continue(s);
> +}
> +put_packet(s, "OK");
> +}
> +
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>  CPUState *cpu;
> @@ -1482,6 +1512,7 @@ static int gdb_handle_packet(GDBState *s, const char 
> *line_buf)
>  uint8_t *registers;
>  target_ulong addr, len;
>  GDBThreadIdKind thread_kind;
> +const GdbCmdParseEntry *cmd_parser = NULL;
>
>  trace_gdbstub_io_command(line_buf);
>
> @@ -1582,42 +1613,15 @@ static int gdb_handle_packet(GDBState *s, const char 
> *line_buf)
>  error_report("QEMU: Terminated via GDBstub");
>  exit(0);
>  case 'D':
> -/* Detach packet */
> -pid = 1;
> -
> -if (s->multiprocess) {
> -unsigned long lpid;
> -if (*p != ';') {
> -put_packet(s, "E22");
> -break;
> -}
> -
> -if (qemu_strtoul(p + 1, , 16, )) {
> -put_packet(s, "E22");
> -break;
> -}
> -
> -pid = lpid;
> -}
> -
> -process = gdb_get_process(s, pid);
> -gdb_process_breakpoint_remove_all(s, process);
> -process->attached = false;
> -
> -if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
> -s->c_cpu = gdb_first_attached_cpu(s);
> -}
> -
> -if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
> -s->g_cpu = gdb_first_attached_cpu(s);
> -}
> -
> -if (s->c_cpu == NULL) {
> -/* No more process attached */
> -gdb_syscall_mode = GDB_SYS_DISABLED;
> -gdb_continue(s);
> +{
> +static const GdbCmdParseEntry detach_cmd_desc = {
> +.handler = handle_detach,
> +.cmd = "D",
> +.cmd_startswith = 1,
> +.schema = "?.l0"
> +};
> +cmd_parser = _cmd_desc;
>  }
> -put_packet(s, "OK");
>  break;
>  case 's':
>  if (*p != '\0') {
> @@ -1990,6 +1994,12 @@ static int gdb_handle_packet(GDBState *s, const char 
> *line_buf)
>  put_packet(s, buf);
>  break;
>  }
> +
> +if (cmd_parser &&
> +process_string_cmd(s, NULL, line_buf, cmd_parser, 1)) {
> +put_packet(s, "");

Why this null put_packet at the end? You've passed the handling of the
OK reply back to your handler so this seems superfluous.

--
Alex Bennée

Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: Allow machine to dump dtb after SLOF update

[Daniel Henrique Barboza]

On 5/6/19 5:09 AM, Greg Kurz wrote:

Now that SLOF can update QEMU's device tree at runtime, it makes sense
to be able to dump the resulting dtb, pretty much like it is already
possible to dump the initial dtb with the dumpdtb machine option.

Add a new dumpdtb-slof property to the pseries machine with the same
semantics as dumpdtb, except that the dtb is dumped at the first call
to h_update_dt() and QEMU exits right after that.

The dtb size sanity check is skipped on purpose so that one has a chance
to peek into the dump file and see what's wrong. If the size is big enough
to cause g_malloc0() to fail then QEMU will abort though. This is likely
not ever to happen, and anyway, we don't really care because dumpdtb-slof
is for developpers, not production, and they should try to debug at the


typo: developers



SLOF level in this case.

Even if 3.1 and older machine types don't support device tree updates, it
doesn't hurt to let them dump the dtb and exit anyway, and it seems better
to ensure a consistent behaviour for this feature.

Signed-off-by: Greg Kurz 
---


LGTM

Reviewed-by: Daniel Henrique Barboza 


  hw/ppc/spapr.c |   19 +++
  hw/ppc/spapr_hcall.c   |   22 ++
  include/hw/ppc/spapr.h |1 +
  3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ce84806ee3d5..18de51d03bd1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3322,6 +3322,19 @@ static void spapr_set_host_serial(Object *obj, const 
char *value, Error **errp)
  spapr->host_serial = g_strdup(value);
  }
  
+static char *spapr_get_dumpdtb_slof(Object *obj, Error **errp)

+{
+return g_strdup(SPAPR_MACHINE(obj)->dumpdtb_slof);
+}
+
+static void spapr_set_dumpdtb_slof(Object *obj, const char *value, Error 
**errp)
+{
+SpaprMachineState *spapr = SPAPR_MACHINE(obj);
+
+g_free(spapr->dumpdtb_slof);
+spapr->dumpdtb_slof = g_strdup(value);
+}
+
  static void spapr_instance_init(Object *obj)
  {
  SpaprMachineState *spapr = SPAPR_MACHINE(obj);
@@ -3378,6 +3391,12 @@ static void spapr_instance_init(Object *obj)
  _abort);
  object_property_set_description(obj, "host-serial",
  "Host serial number to advertise in guest device tree", _abort);
+
+object_property_add_str(obj, "dumpdtb-slof", spapr_get_dumpdtb_slof,
+spapr_set_dumpdtb_slof, _abort);
+object_property_set_description(obj, "dumpdtb-slof",
+"Dump SLOF dtb to a file and quit",
+_abort);
  }
  
  static void spapr_machine_finalizefn(Object *obj)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 6c16d2b12040..30a3880cf1d6 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1766,20 +1766,26 @@ static target_ulong h_update_dt(PowerPCCPU *cpu, 
SpaprMachineState *spapr,
  cpu_physical_memory_read(dt, , sizeof(hdr));
  cb = fdt32_to_cpu(hdr.totalsize);
  
-if (!smc->update_dt_enabled) {

-return H_SUCCESS;
-}
+if (!spapr->dumpdtb_slof) {
+if (!smc->update_dt_enabled) {
+return H_SUCCESS;
+}
  
-/* Check that the fdt did not grow out of proportion */

-if (cb > spapr->fdt_initial_size * 2) {
-trace_spapr_update_dt_failed_size(spapr->fdt_initial_size, cb,
-  fdt32_to_cpu(hdr.magic));
-return H_PARAMETER;
+/* Check that the fdt did not grow out of proportion */
+if (cb > spapr->fdt_initial_size * 2) {
+trace_spapr_update_dt_failed_size(spapr->fdt_initial_size, cb,
+  fdt32_to_cpu(hdr.magic));
+return H_PARAMETER;
+}
  }
  
  fdt = g_malloc0(cb);

  cpu_physical_memory_read(dt, fdt, cb);
  
+if (spapr->dumpdtb_slof) {

+exit(g_file_set_contents(spapr->dumpdtb_slof, fdt, cb, NULL) ? 0 : 1);
+}
+
  /* Check the fdt consistency */
  if (fdt_check_full(fdt, cb)) {
  trace_spapr_update_dt_failed_check(spapr->fdt_initial_size, cb,
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 7e32f309c2be..72a5ff7bfee9 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -191,6 +191,7 @@ struct SpaprMachineState {
  char *kvm_type;
  char *host_model;
  char *host_serial;
+char *dumpdtb_slof;
  
  int32_t irq_map_nr;

  unsigned long *irq_map;

[Qemu-devel] [PATCH for-4.1] q35: Revert to kernel irqchip

[Alex Williamson]

Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
the default for the pc-q35-4.0 machine type to use split irqchip, which
turned out to have disasterous effects on vfio-pci INTx support.  KVM
resampling irqfds are registered for handling these interrupts, but
these are non-functional in split irqchip mode.  We can't simply test
for split irqchip in QEMU as userspace handling of this interrupt is a
significant performance regression versus KVM handling (GeForce GPUs
assigned to Windows VMs are non-functional without forcing MSI mode or
re-enabling kernel irqchip).

The resolution is to revert the change in default irqchip mode in the
pc-q35-4.1 machine type.  The qemu-q35-4.0 machine type should not be
used in vfio-pci configurations for devices requiring legacy INTx
support without explicitly modifying the VM configuration to use KVM
irqchip.  A new pc-q35-4.0.1 machine type is submitted to resolve this
in the stable branch.

Link: https://bugs.launchpad.net/qemu/+bug/1826422
Fixes: b2fc91db8447 ("q35: set split kernel irqchip as default")
Signed-off-by: Alex Williamson 
---
 hw/i386/pc_q35.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 37dd350511a9..9f90dc72a53f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -357,7 +357,7 @@ static void pc_q35_machine_options(MachineClass *m)
 m->units_per_default_bus = 1;
 m->default_machine_opts = "firmware=bios-256k.bin";
 m->default_display = "std";
-m->default_kernel_irqchip_split = true;
+m->default_kernel_irqchip_split = false;
 m->no_floppy = 1;
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
@@ -377,6 +377,7 @@ DEFINE_Q35_MACHINE(v4_1, "pc-q35-4.1", NULL,
 static void pc_q35_4_0_machine_options(MachineClass *m)
 {
 pc_q35_4_1_machine_options(m);
+m->default_kernel_irqchip_split = true;
 m->alias = NULL;
 compat_props_add(m->compat_props, hw_compat_4_0, hw_compat_4_0_len);
 compat_props_add(m->compat_props, pc_compat_4_0, pc_compat_4_0_len);

[Qemu-devel] [PULL 14/16] migration: Fix use-after-free during process exit

[Dr. David Alan Gilbert (git)]

From: Yury Kotov 

It fixes heap-use-after-free which was found by clang's ASAN.

Control flow of this use-after-free:
main_thread:
* Got SIGTERM and completes main loop
* Calls migration_shutdown
  - migrate_fd_cancel (so, migration_thread begins to complete)
  - object_unref(OBJECT(current_migration));

migration_thread:
* migration_iteration_finish -> schedule cleanup bh
* object_unref(OBJECT(s)); (Now, current_migration is freed)
* exits

main_thread:
* Calls vm_shutdown -> drain bdrvs -> main loop
  -> cleanup_bh -> use after free

If you want to reproduce, these couple of sleeps will help:
vl.c:4613:
 migration_shutdown();
+sleep(2);
migration.c:3269:
+sleep(1);
 trace_migration_thread_after_loop();
 migration_iteration_finish(s);

Original output:
qemu-system-x86_64: terminating on signal 15 from pid 31980 ()
=
==31958==ERROR: AddressSanitizer: heap-use-after-free on address 0x6191d210
  at pc 0x58a535ca bp 0x7fffb190 sp 0x7fffb188
READ of size 8 at 0x6191d210 thread T0 (qemu-vm-0)
#0 0x58a535c9 in migrate_fd_cleanup migration/migration.c:1502:23
#1 0x594fde0a in aio_bh_call util/async.c:90:5
#2 0x594fe522 in aio_bh_poll util/async.c:118:13
#3 0x59524783 in aio_poll util/aio-posix.c:725:17
#4 0x59504fb3 in aio_wait_bh_oneshot util/aio-wait.c:71:5
#5 0x573bddf6 in virtio_blk_data_plane_stop
  hw/block/dataplane/virtio-blk.c:282:5
#6 0x589d5c09 in virtio_bus_stop_ioeventfd hw/virtio/virtio-bus.c:246:9
#7 0x589e9917 in virtio_pci_stop_ioeventfd hw/virtio/virtio-pci.c:287:5
#8 0x589e22bf in virtio_pci_vmstate_change hw/virtio/virtio-pci.c:1072:9
#9 0x57628931 in virtio_vmstate_change hw/virtio/virtio.c:2257:9
#10 0x57c36713 in vm_state_notify vl.c:1605:9
#11 0x5716ef53 in do_vm_stop cpus.c:1074:9
#12 0x5716eeff in vm_shutdown cpus.c:1092:12
#13 0x57c4283e in main vl.c:4617:5
#14 0x7fffdfdb482f in __libc_start_main
  (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#15 0x56ecb118 in _start (x86_64-softmmu/qemu-system-x86_64+0x1977118)

0x6191d210 is located 144 bytes inside of 952-byte region
  [0x6191d180,0x6191d538)
freed by thread T6 (live_migration) here:
#0 0x56f76782 in __interceptor_free
  
/tmp/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
#1 0x58d5fa94 in object_finalize qom/object.c:618:9
#2 0x58d57651 in object_unref qom/object.c:1068:9
#3 0x58a55588 in migration_thread migration/migration.c:3272:5
#4 0x595393f2 in qemu_thread_start util/qemu-thread-posix.c:502:9
#5 0x7fffe057f6b9 in start_thread 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

previously allocated by thread T0 (qemu-vm-0) here:
#0 0x56f76b03 in __interceptor_malloc
  
/tmp/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
#1 0x76ee37b8 in g_malloc 
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4f7b8)
#2 0x58d58031 in object_new qom/object.c:640:12
#3 0x58a31f21 in migration_object_init migration/migration.c:139:25
#4 0x57c41398 in main vl.c:4320:5
#5 0x7fffdfdb482f in __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

Thread T6 (live_migration) created by T0 (qemu-vm-0) here:
#0 0x56f5f0dd in pthread_create
  
/tmp/final/llvm.src/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
#1 0x59538cf9 in qemu_thread_create util/qemu-thread-posix.c:539:11
#2 0x58a53304 in migrate_fd_connect migration/migration.c:3332:5
#3 0x58a72bd8 in migration_channel_connect migration/channel.c:92:5
#4 0x58a6ef87 in exec_start_outgoing_migration migration/exec.c:42:5
#5 0x58a4f3c2 in qmp_migrate migration/migration.c:1922:9
#6 0x58bb4f6a in qmp_marshal_migrate 
qapi/qapi-commands-migration.c:607:5
#7 0x59363738 in do_qmp_dispatch qapi/qmp-dispatch.c:131:5
#8 0x59362a15 in qmp_dispatch qapi/qmp-dispatch.c:174:11
#9 0x571bac15 in monitor_qmp_dispatch monitor.c:4124:11
#10 0x5719a22d in monitor_qmp_bh_dispatcher monitor.c:4207:9
#11 0x594fde0a in aio_bh_call util/async.c:90:5
#12 0x594fe522 in aio_bh_poll util/async.c:118:13
#13 0x595201e0 in aio_dispatch util/aio-posix.c:460:5
#14 0x59503553 in aio_ctx_dispatch util/async.c:261:5
#15 0x76ede196 in g_main_context_dispatch
  (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4a196)

SUMMARY: AddressSanitizer: heap-use-after-free migration/migration.c:1502:23
  in migrate_fd_cleanup
Shadow bytes around the buggy address:
  0x0c327fffb9f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffba00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffba10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffba20: fa fa fa fa fa fa 

[Qemu-devel] [PULL 16/16] monitor: Call mon_get_cpu() only once at hmp_gva2gpa()

[Dr. David Alan Gilbert (git)]

From: Eduardo Habkost 

hmp_gva2gpa() calls mon_get_cpu() twice, which is unnecessary.
Not an actual bug, but this is reported as a defect by Coverity
Scan (CID 1401346).

Signed-off-by: Eduardo Habkost 
Message-Id: <20190510185620.15757-1-ehabk...@redhat.com>
Reviewed-by: Richard Henderson 
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 monitor.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index bb48997913..6428eb3b7e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1685,8 +1685,7 @@ static void hmp_gva2gpa(Monitor *mon, const QDict *qdict)
 return;
 }
 
-gpa  = cpu_get_phys_page_attrs_debug(mon_get_cpu(),
- addr & TARGET_PAGE_MASK, );
+gpa  = cpu_get_phys_page_attrs_debug(cs, addr & TARGET_PAGE_MASK, );
 if (gpa == -1) {
 monitor_printf(mon, "Unmapped\n");
 } else {
-- 
2.21.0

[Qemu-devel] [PULL 11/16] migration/savevm: remove duplicate check of migration_is_blocked

[Dr. David Alan Gilbert (git)]

From: Wei Yang 

Current call flow of save_snapshot is:

  save_snapshot
migration_is_blocked
  qemu_savevm_state
migration_is_blocked

Since qemu_savevm_state is only called in save_snapshot, this means
migration_is_blocked has been already checked.

Signed-off-by: Wei Yang 
Message-Id: <20190424004700.12766-2-richardw.y...@linux.intel.com>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/savevm.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 81a9a2ef30..6e8912af49 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1418,10 +1418,6 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
 return -EINVAL;
 }
 
-if (migration_is_blocked(errp)) {
-return -EINVAL;
-}
-
 if (migrate_use_block()) {
 error_setg(errp, "Block migration and snapshots are incompatible");
 return -EINVAL;
-- 
2.21.0

Re: [Qemu-devel] [PATCH v2 2/6] tests/vhost-user-bridge: Fix misuse of isdigit()

[Thomas Huth]

On 14/05/2019 20.03, Markus Armbruster wrote:
> vubr_set_host() passes char values to isdigit().  Undefined behavior
> when the value is negative.
> 
> Fix by using qemu_isdigit() instead.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/vhost-user-bridge.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
> index 0033b61f2e..d70b107ebc 100644
> --- a/tests/vhost-user-bridge.c
> +++ b/tests/vhost-user-bridge.c
> @@ -645,7 +645,7 @@ vubr_host_notifier_setup(VubrDev *dev)
>  static void
>  vubr_set_host(struct sockaddr_in *saddr, const char *host)
>  {
> -if (isdigit(host[0])) {
> +if (qemu_isdigit(host[0])) {
>  if (!inet_aton(host, >sin_addr)) {
>  fprintf(stderr, "inet_aton() failed.\n");
>  exit(1);

Reviewed-by: Thomas Huth 

[Qemu-devel] [PULL 13/16] migration/savevm: wrap into qemu_loadvm_state_header()

[Dr. David Alan Gilbert (git)]

From: Wei Yang 

On source side, we have qemu_savevm_state_header() to send related data,
while on the receiving side those steps are scattered in
qemu_loadvm_state().

This patch wrap those related steps into qemu_loadvm_state_header() to
make it friendly to read.

Signed-off-by: Wei Yang 
Message-Id: <20190424004700.12766-5-richardw.y...@linux.intel.com>
Reviewed-by: Daniel Henrique Barboza 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/savevm.c | 69 +++---
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 4c7b8379e8..c0e557b4c2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2262,6 +2262,43 @@ qemu_loadvm_section_part_end(QEMUFile *f, 
MigrationIncomingState *mis)
 return 0;
 }
 
+static int qemu_loadvm_state_header(QEMUFile *f)
+{
+unsigned int v;
+int ret;
+
+v = qemu_get_be32(f);
+if (v != QEMU_VM_FILE_MAGIC) {
+error_report("Not a migration stream");
+return -EINVAL;
+}
+
+v = qemu_get_be32(f);
+if (v == QEMU_VM_FILE_VERSION_COMPAT) {
+error_report("SaveVM v2 format is obsolete and don't work anymore");
+return -ENOTSUP;
+}
+if (v != QEMU_VM_FILE_VERSION) {
+error_report("Unsupported migration stream version");
+return -ENOTSUP;
+}
+
+if (migrate_get_current()->send_configuration) {
+if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) {
+error_report("Configuration section missing");
+qemu_loadvm_state_cleanup();
+return -EINVAL;
+}
+ret = vmstate_load_state(f, _configuration, _state, 0);
+
+if (ret) {
+qemu_loadvm_state_cleanup();
+return ret;
+}
+}
+return 0;
+}
+
 static int qemu_loadvm_state_setup(QEMUFile *f)
 {
 SaveStateEntry *se;
@@ -2410,7 +2447,6 @@ int qemu_loadvm_state(QEMUFile *f)
 {
 MigrationIncomingState *mis = migration_incoming_get_current();
 Error *local_err = NULL;
-unsigned int v;
 int ret;
 
 if (qemu_savevm_state_blocked(_err)) {
@@ -2418,34 +2454,9 @@ int qemu_loadvm_state(QEMUFile *f)
 return -EINVAL;
 }
 
-v = qemu_get_be32(f);
-if (v != QEMU_VM_FILE_MAGIC) {
-error_report("Not a migration stream");
-return -EINVAL;
-}
-
-v = qemu_get_be32(f);
-if (v == QEMU_VM_FILE_VERSION_COMPAT) {
-error_report("SaveVM v2 format is obsolete and don't work anymore");
-return -ENOTSUP;
-}
-if (v != QEMU_VM_FILE_VERSION) {
-error_report("Unsupported migration stream version");
-return -ENOTSUP;
-}
-
-if (migrate_get_current()->send_configuration) {
-if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) {
-error_report("Configuration section missing");
-qemu_loadvm_state_cleanup();
-return -EINVAL;
-}
-ret = vmstate_load_state(f, _configuration, _state, 0);
-
-if (ret) {
-qemu_loadvm_state_cleanup();
-return ret;
-}
+ret = qemu_loadvm_state_header(f);
+if (ret) {
+return ret;
 }
 
 if (qemu_loadvm_state_setup(f) != 0) {
-- 
2.21.0

Re: [Qemu-devel] [PATCH v2 5/6] pc-bios/s390-ccw: Clean up harmless misuse of isdigit()

[Thomas Huth]

On 14/05/2019 20.04, Christian Borntraeger wrote:
> 
> 
> On 14.05.19 20:03, Markus Armbruster wrote:
>> atoui() and get_index() pass char values to isdigit().  With a
>> standard isdigit(), we'd get undefined behavior when the value is
>> negative.  Can't happen as char is unsigned on s390x.  Even if it
>> could, we're actually using isdigit() from pc-bios/s390-ccw/libc.h
>> here, which works fine for negative values.  Clean up anyway, just
>> to avoid setting a bad example.
>>
>> Cc: Christian Borntraeger 
> Acked-by: Christian Borntraeger 

FYI, this patch is already queued in Cornelia's s390-next tree (and it
should also go via her tree, since you need to rebuild the bios binary
afterwards).

 Thomas

[Qemu-devel] [PULL 06/16] migration/colo.c: Remove redundant input parameter

[Dr. David Alan Gilbert (git)]

From: Zhang Chen 

The colo_do_failover no need the input parameter.

Signed-off-by: Zhang Chen 
Message-Id: <20190426090730.2691-2-chen.zh...@intel.com>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 include/migration/colo.h  | 2 +-
 migration/colo-failover.c | 2 +-
 migration/colo.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/migration/colo.h b/include/migration/colo.h
index 99ce17aca7..ddebe0ad27 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -37,7 +37,7 @@ bool migration_incoming_in_colo_state(void);
 COLOMode get_colo_mode(void);
 
 /* failover */
-void colo_do_failover(MigrationState *s);
+void colo_do_failover(void);
 
 void colo_checkpoint_notify(void *opaque);
 #endif
diff --git a/migration/colo-failover.c b/migration/colo-failover.c
index 4854a96c92..e9ca0b4774 100644
--- a/migration/colo-failover.c
+++ b/migration/colo-failover.c
@@ -39,7 +39,7 @@ static void colo_failover_bh(void *opaque)
 return;
 }
 
-colo_do_failover(NULL);
+colo_do_failover();
 }
 
 void failover_request_active(Error **errp)
diff --git a/migration/colo.c b/migration/colo.c
index 238a6d62c7..8c1644091f 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -193,7 +193,7 @@ COLOMode get_colo_mode(void)
 }
 }
 
-void colo_do_failover(MigrationState *s)
+void colo_do_failover(void)
 {
 /* Make sure VM stopped while failover happened. */
 if (!colo_runstate_is_stopped()) {
-- 
2.21.0

[Qemu-devel] [PULL 15/16] migration/ram.c: fix typos in comments

[Dr. David Alan Gilbert (git)]

From: Wei Yang 

Signed-off-by: Wei Yang 
Message-Id: <20190510233729.15554-1-richardw.y...@linux.intel.com>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index e1fe45311d..4c60869226 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -917,7 +917,7 @@ struct {
  *- to make easier to know what to free at the end of migration
  *
  * This way we always know who is the owner of each "pages" struct,
- * and we don't need any loocking.  It belongs to the migration thread
+ * and we don't need any locking.  It belongs to the migration thread
  * or to the channel thread.  Switching is safe because the migration
  * thread is using the channel mutex when changing it, and the channel
  * have to had finish with its own, otherwise pending_job can't be
@@ -1630,7 +1630,7 @@ static int save_xbzrle_page(RAMState *rs, uint8_t 
**current_data,
 /**
  * migration_bitmap_find_dirty: find the next dirty page from start
  *
- * Returns the byte offset within memory region of the start of a dirty page
+ * Returns the page offset within memory region of the start of a dirty page
  *
  * @rs: current RAM state
  * @rb: RAMBlock where to search for dirty pages
@@ -2144,7 +2144,7 @@ retry:
  * find_dirty_block: find the next dirty page and update any state
  * associated with the search process.
  *
- * Returns if a page is found
+ * Returns true if a page is found
  *
  * @rs: current RAM state
  * @pss: data about the state of the current dirty page scan
@@ -2240,7 +2240,7 @@ static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t 
*offset)
  *
  * Skips pages that are already sent (!dirty)
  *
- * Returns if a queued page is found
+ * Returns true if a queued page is found
  *
  * @rs: current RAM state
  * @pss: data about the state of the current dirty page scan
@@ -3447,7 +3447,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
 
 /* we want to check in the 1st loop, just in case it was the 1st time
and we had to sync the dirty bitmap.
-   qemu_get_clock_ns() is a bit expensive, so we only check each some
+   qemu_clock_get_ns() is a bit expensive, so we only check each some
iterations
 */
 if ((i & 63) == 0) {
-- 
2.21.0

[Qemu-devel] [PULL 05/16] migration: savevm: fix error code with migration blockers

[Dr. David Alan Gilbert (git)]

From: Cole Robinson 

The only caller that checks the error code is looking for != 0,
so returning false is incorrect.

Fixes: 5aaac467938 "migration: savevm: consult migration blockers"

Signed-off-by: Cole Robinson 
Message-Id: 

Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/savevm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 587fec8ce2..81a9a2ef30 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2542,7 +2542,7 @@ int save_snapshot(const char *name, Error **errp)
 AioContext *aio_context;
 
 if (migration_is_blocked(errp)) {
-return false;
+return ret;
 }
 
 if (!replay_can_snapshot()) {
-- 
2.21.0

[Qemu-devel] [PULL 10/16] migration: update comments of migration bitmap

[Dr. David Alan Gilbert (git)]

From: Yi Wang 

Since the ram bitmap and the unsent bitmap are split by RAMBlock
in commit 6b6712e, it's better to update the comments about them.

Signed-off-by: Yi Wang 
Message-Id: <1555311089-18610-1-git-send-email-wang.y...@zte.com.cn>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index ec11161d58..e1fe45311d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1630,8 +1630,6 @@ static int save_xbzrle_page(RAMState *rs, uint8_t 
**current_data,
 /**
  * migration_bitmap_find_dirty: find the next dirty page from start
  *
- * Called with rcu_read_lock() to protect migration_bitmap
- *
  * Returns the byte offset within memory region of the start of a dirty page
  *
  * @rs: current RAM state
@@ -2681,7 +2679,7 @@ static void ram_save_cleanup(void *opaque)
 RAMBlock *block;
 
 /* caller have hold iothread lock or is in a bh, so there is
- * no writing race against this migration_bitmap
+ * no writing race against the migration bitmap
  */
 memory_global_dirty_log_stop();
 
-- 
2.21.0

[Qemu-devel] [PULL 09/16] migration/ram.c: start of migration_bitmap_sync_range is always 0

[Dr. David Alan Gilbert (git)]

From: Wei Yang 

We can eliminate to pass 0.

Signed-off-by: Wei Yang 
Message-Id: <20190430034412.12935-2-richardw.y...@linux.intel.com>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 1ca9ba77b6..ec11161d58 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1681,10 +1681,10 @@ static inline bool 
migration_bitmap_clear_dirty(RAMState *rs,
 }
 
 static void migration_bitmap_sync_range(RAMState *rs, RAMBlock *rb,
-ram_addr_t start, ram_addr_t length)
+ram_addr_t length)
 {
 rs->migration_dirty_pages +=
-cpu_physical_memory_sync_dirty_bitmap(rb, start, length,
+cpu_physical_memory_sync_dirty_bitmap(rb, 0, length,
   >num_dirty_pages_period);
 }
 
@@ -1773,7 +1773,7 @@ static void migration_bitmap_sync(RAMState *rs)
 qemu_mutex_lock(>bitmap_mutex);
 rcu_read_lock();
 RAMBLOCK_FOREACH_NOT_IGNORED(block) {
-migration_bitmap_sync_range(rs, block, 0, block->used_length);
+migration_bitmap_sync_range(rs, block, block->used_length);
 }
 ram_counters.remaining = ram_bytes_remaining();
 rcu_read_unlock();
@@ -4196,7 +4196,7 @@ static void colo_flush_ram_cache(void)
 memory_global_dirty_log_sync();
 rcu_read_lock();
 RAMBLOCK_FOREACH_NOT_IGNORED(block) {
-migration_bitmap_sync_range(ram_state, block, 0, block->used_length);
+migration_bitmap_sync_range(ram_state, block, block->used_length);
 }
 rcu_read_unlock();
 
-- 
2.21.0

[Qemu-devel] [PULL 12/16] migration/savevm: load_header before load_setup

[Dr. David Alan Gilbert (git)]

From: Wei Yang 

In migration_thread() and qemu_savevm_state(), we savevm_state in
following sequence:

qemu_savevm_state_header(f);
qemu_savevm_state_setup(f);

Then it would be more proper to loadvm_state in the save sequence.

Signed-off-by: Wei Yang 
Message-Id: <20190424004700.12766-4-richardw.y...@linux.intel.com>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/savevm.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 6e8912af49..4c7b8379e8 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2434,10 +2434,6 @@ int qemu_loadvm_state(QEMUFile *f)
 return -ENOTSUP;
 }
 
-if (qemu_loadvm_state_setup(f) != 0) {
-return -EINVAL;
-}
-
 if (migrate_get_current()->send_configuration) {
 if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) {
 error_report("Configuration section missing");
@@ -2452,6 +2448,10 @@ int qemu_loadvm_state(QEMUFile *f)
 }
 }
 
+if (qemu_loadvm_state_setup(f) != 0) {
+return -EINVAL;
+}
+
 cpu_synchronize_all_pre_loadvm();
 
 ret = qemu_loadvm_state_main(f, mis);
-- 
2.21.0

[Qemu-devel] [PULL 08/16] qemu-option.hx: Update missed parameter for colo-compare

[Dr. David Alan Gilbert (git)]

From: Zhang Chen 

We missed the iothread related args in this file.
This patch is used to fix this issue.

Signed-off-by: Zhang Chen 
Message-Id: <20190426090730.2691-4-chen.zh...@intel.com>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 qemu-options.hx | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 51802cbb26..3faa935929 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4425,13 +4425,15 @@ Dump the network traffic on netdev @var{dev} to the 
file specified by
 The file format is libpcap, so it can be analyzed with tools such as tcpdump
 or Wireshark.
 
-@item -object 
colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},outdev=@var{chardevid}[,vnet_hdr_support]
+@item -object 
colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},outdev=@var{chardevid},iothread=@var{id}[,vnet_hdr_support]
 
 Colo-compare gets packet from primary_in@var{chardevid} and 
secondary_in@var{chardevid}, than compare primary packet with
 secondary packet. If the packets are same, we will output primary
 packet to outdev@var{chardevid}, else we will notify colo-frame
 do checkpoint and send primary packet to outdev@var{chardevid}.
-if it has the vnet_hdr_support flag, colo compare will send/recv packet with 
vnet_hdr_len.
+In order to improve efficiency, we need to put the task of comparison
+in another thread. If it has the vnet_hdr_support flag, colo compare
+will send/recv packet with vnet_hdr_len.
 
 we must use it with the help of filter-mirror and filter-redirector.
 
@@ -4446,10 +4448,11 @@ primary:
 -chardev socket,id=compare0-0,host=3.3.3.3,port=9001
 -chardev socket,id=compare_out,host=3.3.3.3,port=9005,server,nowait
 -chardev socket,id=compare_out0,host=3.3.3.3,port=9005
+-object iothread,id=iothread1
 -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0
 -object filter-redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out
 -object filter-redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0
--object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0
+-object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,iothread=iothread1
 
 secondary:
 -netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down script=/etc/qemu-ifdown
-- 
2.21.0

[Qemu-devel] [PULL 07/16] migration/colo.h: Remove obsolete codes

[Dr. David Alan Gilbert (git)]

From: Zhang Chen 

Signed-off-by: Zhang Chen 
Message-Id: <20190426090730.2691-3-chen.zh...@intel.com>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 include/migration/colo.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/migration/colo.h b/include/migration/colo.h
index ddebe0ad27..f6fbe23ec9 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -22,8 +22,6 @@ enum colo_event {
 COLO_EVENT_FAILOVER,
 };
 
-void colo_info_init(void);
-
 void migrate_start_colo_process(MigrationState *s);
 bool migration_in_colo_state(void);
 
-- 
2.21.0

[Qemu-devel] [PULL 04/16] vmstate: check subsection_found is enough

[Dr. David Alan Gilbert (git)]

From: Wei Yang 

subsection_found is true implies vmdesc is not NULL.

This patch remove the additional check on vmdesc and rename
subsection_found to vmdesc_has_subsections to make it more self-explain.

Signed-off-by: Wei Yang 

Message-Id: <20190403011016.12549-1-richardw.y...@linux.intel.com>
Acked-by: Stefano Garzarella 
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/vmstate.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/vmstate.c b/migration/vmstate.c
index e2bbb7b5f7..1305d1a528 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -496,7 +496,7 @@ static int vmstate_subsection_save(QEMUFile *f, const 
VMStateDescription *vmsd,
void *opaque, QJSON *vmdesc)
 {
 const VMStateDescription **sub = vmsd->subsections;
-bool subsection_found = false;
+bool vmdesc_has_subsections = false;
 int ret = 0;
 
 trace_vmstate_subsection_save_top(vmsd->name);
@@ -508,9 +508,9 @@ static int vmstate_subsection_save(QEMUFile *f, const 
VMStateDescription *vmsd,
 trace_vmstate_subsection_save_loop(vmsd->name, vmsdsub->name);
 if (vmdesc) {
 /* Only create subsection array when we have any */
-if (!subsection_found) {
+if (!vmdesc_has_subsections) {
 json_start_array(vmdesc, "subsections");
-subsection_found = true;
+vmdesc_has_subsections = true;
 }
 
 json_start_object(vmdesc, NULL);
@@ -533,7 +533,7 @@ static int vmstate_subsection_save(QEMUFile *f, const 
VMStateDescription *vmsd,
 sub++;
 }
 
-if (vmdesc && subsection_found) {
+if (vmdesc_has_subsections) {
 json_end_array(vmdesc);
 }
 
-- 
2.21.0

[Qemu-devel] [PULL 03/16] migration: remove not used field xfer_limit

[Dr. David Alan Gilbert (git)]

From: Wei Yang 

MigrationState->xfer_limit is only set to 0 in migrate_init().

Remove this unnecessary field.

Signed-off-by: Wei Yang 
Message-Id: <20190326055726.10539-1-richardw.y...@linux.intel.com>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/migration.c | 1 -
 migration/migration.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 609e0df5d0..4844ad438b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1681,7 +1681,6 @@ void migrate_init(MigrationState *s)
  * locks.
  */
 s->bytes_xfer = 0;
-s->xfer_limit = 0;
 s->cleanup_bh = 0;
 s->to_dst_file = NULL;
 s->state = MIGRATION_STATUS_NONE;
diff --git a/migration/migration.h b/migration/migration.h
index 438f17edad..780a096857 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -117,7 +117,6 @@ struct MigrationState
 
 /*< public >*/
 size_t bytes_xfer;
-size_t xfer_limit;
 QemuThread thread;
 QEMUBH *cleanup_bh;
 QEMUFile *to_dst_file;
-- 
2.21.0

[Qemu-devel] [PULL 02/16] migration: not necessary to check ops again

[Dr. David Alan Gilbert (git)]

From: Wei Yang 

During each iteration, se->ops is checked before each loop. So it is not
necessary to check it again and simplify the following check a little.

Signed-off-by: Wei Yang 
Message-Id: <20190327013130.26259-1-richardw.y...@linux.intel.com>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/savevm.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 34bcad3807..587fec8ce2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1157,15 +1157,13 @@ int qemu_savevm_state_iterate(QEMUFile *f, bool 
postcopy)
 if (!se->ops || !se->ops->save_live_iterate) {
 continue;
 }
-if (se->ops && se->ops->is_active) {
-if (!se->ops->is_active(se->opaque)) {
-continue;
-}
+if (se->ops->is_active &&
+!se->ops->is_active(se->opaque)) {
+continue;
 }
-if (se->ops && se->ops->is_active_iterate) {
-if (!se->ops->is_active_iterate(se->opaque)) {
-continue;
-}
+if (se->ops->is_active_iterate &&
+!se->ops->is_active_iterate(se->opaque)) {
+continue;
 }
 /*
  * In the postcopy phase, any device that doesn't know how to
-- 
2.21.0

[Qemu-devel] [PULL 01/16] migration: comment VMSTATE_UNUSED*() properly

[Dr. David Alan Gilbert (git)]

From: Peter Xu 

It is error prone to use VMSTATE_UNUSED*() sometimes especially when
the size of the migration stream of the field is not the same as the
size of the structure (boolean is one example).  Comment it well so
people will be aware of this when people want to use it.

Signed-off-by: Peter Xu 
Message-Id: <20190329095713.14177-1-pet...@redhat.com>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 include/migration/vmstate.h | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index a668ec75b8..9224370ed5 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1035,6 +1035,20 @@ extern const VMStateInfo vmstate_info_qtailq;
 #define VMSTATE_BUFFER_UNSAFE(_field, _state, _version, _size)\
 VMSTATE_BUFFER_UNSAFE_INFO(_field, _state, _version, vmstate_info_buffer, 
_size)
 
+/*
+ * These VMSTATE_UNUSED*() macros can be used to fill in the holes
+ * when some of the vmstate fields are obsolete to be compatible with
+ * migrations between new/old binaries.
+ *
+ * CAUTION: when using any of the VMSTATE_UNUSED*() macros please be
+ * sure that the size passed in is the size that was actually *sent*
+ * rather than the size of the *structure*.  One example is the
+ * boolean type - the size of the structure can vary depending on the
+ * definition of boolean, however the size we actually sent is always
+ * 1 byte (please refer to implementation of VMSTATE_BOOL_V and
+ * vmstate_info_bool).  So here we should always pass in size==1
+ * rather than size==sizeof(bool).
+ */
 #define VMSTATE_UNUSED_V(_v, _size)   \
 VMSTATE_UNUSED_BUFFER(NULL, _v, _size)
 
-- 
2.21.0

[Qemu-devel] [PULL 00/16] migration queue

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

The following changes since commit e329ad2ab72c43b56df88b34954c2c7d839bb373:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20190513' into 
staging (2019-05-14 10:08:47 +0100)

are available in the Git repository at:

  git://github.com/dagrh/qemu.git tags/pull-migration-20190514b

for you to fetch changes up to 9d3250d5ba8c4c5389530b861686e22e77fddcc7:

  monitor: Call mon_get_cpu() only once at hmp_gva2gpa() (2019-05-14 19:00:04 
+0100)


Migration pull 2019-05-14

Small fixes/cleanups
One HMP/monitor fix


Cole Robinson (1):
  migration: savevm: fix error code with migration blockers

Eduardo Habkost (1):
  monitor: Call mon_get_cpu() only once at hmp_gva2gpa()

Peter Xu (1):
  migration: comment VMSTATE_UNUSED*() properly

Wei Yang (8):
  migration: not necessary to check ops again
  migration: remove not used field xfer_limit
  vmstate: check subsection_found is enough
  migration/ram.c: start of migration_bitmap_sync_range is always 0
  migration/savevm: remove duplicate check of migration_is_blocked
  migration/savevm: load_header before load_setup
  migration/savevm: wrap into qemu_loadvm_state_header()
  migration/ram.c: fix typos in comments

Yi Wang (1):
  migration: update comments of migration bitmap

Yury Kotov (1):
  migration: Fix use-after-free during process exit

Zhang Chen (3):
  migration/colo.c: Remove redundant input parameter
  migration/colo.h: Remove obsolete codes
  qemu-option.hx: Update missed parameter for colo-compare

 include/migration/colo.h|  4 +-
 include/migration/vmstate.h | 14 +++
 migration/colo-failover.c   |  2 +-
 migration/colo.c|  2 +-
 migration/migration.c   | 26 ++---
 migration/migration.h   |  1 -
 migration/ram.c | 22 +--
 migration/savevm.c  | 89 -
 migration/vmstate.c |  8 ++--
 monitor.c   |  3 +-
 qemu-options.hx |  9 +++--
 11 files changed, 105 insertions(+), 75 deletions(-)

Re: [Qemu-devel] [PATCH v9 01/27] gdbstub: Add infrastructure to parse cmd packets

[Alex Bennée]

Jon Doron  writes:

> Signed-off-by: Jon Doron 
> ---

> +
> +/*
> + * cmd_startswith -> cmd is compared using startswith
> + *
> + *
> + * schema definitions:
> + * Each schema parameter entry consists of 2 chars,
> + * the first char represents the parameter type handling
> + * the second char represents the delimiter for the next parameter
> + *
> + * Currently supported schema types:
> + * 'l' -> unsigned long (stored in .val_ul)
> + * 'L' -> unsigned long long (stored in .val_ull)
> + * 's' -> string (stored in .data)
> + * 'o' -> single char (stored in .opcode)
> + * 't' -> thread id (stored in .thread_id)
> + * '?' -> skip according to delimiter
> + *
> + * Currently supported delimiters:
> + * '?' -> Stop at any delimiter (",;:=\0")
> + * '0' -> Stop at "\0"
> + * '.' -> Skip 1 char unless reached "\0"
> + * Any other value is treated as the delimiter value itself
> + */
> +typedef struct GdbCmdParseEntry {
> +GdbCmdHandler handler;
> +const char *cmd;
> +union {
> +int flags;
> +struct {
> +int cmd_startswith:1;
> +};
> +};

This union seems a little over the top given flags isn't used AFAICT.
Why not just have a bool cmd_startswith for now? You can always expand
the structure later if you need to.

Otherwise:

Reviewed-by: Alex Bennée 

--
Alex Bennée

Re: [Qemu-devel] [PATCH v2 2/6] tests/vhost-user-bridge: Fix misuse of isdigit()

[Philippe Mathieu-Daudé]

On 5/14/19 8:03 PM, Markus Armbruster wrote:
> vubr_set_host() passes char values to isdigit().  Undefined behavior

"happens"?

> when the value is negative.
> 
> Fix by using qemu_isdigit() instead.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/vhost-user-bridge.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
> index 0033b61f2e..d70b107ebc 100644
> --- a/tests/vhost-user-bridge.c
> +++ b/tests/vhost-user-bridge.c
> @@ -645,7 +645,7 @@ vubr_host_notifier_setup(VubrDev *dev)
>  static void
>  vubr_set_host(struct sockaddr_in *saddr, const char *host)
>  {
> -if (isdigit(host[0])) {
> +if (qemu_isdigit(host[0])) {
>  if (!inet_aton(host, >sin_addr)) {
>  fprintf(stderr, "inet_aton() failed.\n");
>  exit(1);
> 

Reviewed-by: Philippe Mathieu-Daudé 

[Qemu-devel] [PATCH v2 6/6] cutils: Simplify how parse_uint() checks for whitespace

[Markus Armbruster]

Use qemu_isspace() so we don't have to cast to unsigned char.

Signed-off-by: Markus Armbruster 
---
 util/cutils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/cutils.c b/util/cutils.c
index d682c90901..9aacc422ca 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -683,7 +683,7 @@ int parse_uint(const char *s, unsigned long long *value, 
char **endptr,
 }
 
 /* make sure we reject negative numbers: */
-while (isspace((unsigned char)*s)) {
+while (qemu_isspace(*s)) {
 s++;
 }
 if (*s == '-') {
-- 
2.17.2

[Qemu-devel] [PATCH v2 5/6] pc-bios/s390-ccw: Clean up harmless misuse of isdigit()

[Markus Armbruster]

atoui() and get_index() pass char values to isdigit().  With a
standard isdigit(), we'd get undefined behavior when the value is
negative.  Can't happen as char is unsigned on s390x.  Even if it
could, we're actually using isdigit() from pc-bios/s390-ccw/libc.h
here, which works fine for negative values.  Clean up anyway, just
to avoid setting a bad example.

Cc: Christian Borntraeger 
Cc: Thomas Huth 
Cc: Cornelia Huck 
Cc: qemu-s3...@nongnu.org
Signed-off-by: Markus Armbruster 
---
 pc-bios/s390-ccw/libc.c | 2 +-
 pc-bios/s390-ccw/menu.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/pc-bios/s390-ccw/libc.c b/pc-bios/s390-ccw/libc.c
index a786566c4c..3187923950 100644
--- a/pc-bios/s390-ccw/libc.c
+++ b/pc-bios/s390-ccw/libc.c
@@ -38,7 +38,7 @@ uint64_t atoui(const char *str)
 }
 
 while (*str) {
-if (!isdigit(*str)) {
+if (!isdigit(*(unsigned char *)str)) {
 break;
 }
 val = val * 10 + *str - '0';
diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 82a4ae6315..ce3815b201 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -134,7 +134,7 @@ static int get_index(void)
 
 /* Check for erroneous input */
 for (i = 0; i < len; i++) {
-if (!isdigit(buf[i])) {
+if (!isdigit((unsigned char)buf[i])) {
 return -1;
 }
 }
-- 
2.17.2

Re: [Qemu-devel] [PATCH v2 6/6] cutils: Simplify how parse_uint() checks for whitespace

[Philippe Mathieu-Daudé]

On 5/14/19 8:03 PM, Markus Armbruster wrote:
> Use qemu_isspace() so we don't have to cast to unsigned char.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  util/cutils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/cutils.c b/util/cutils.c
> index d682c90901..9aacc422ca 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -683,7 +683,7 @@ int parse_uint(const char *s, unsigned long long *value, 
> char **endptr,
>  }
>  
>  /* make sure we reject negative numbers: */
> -while (isspace((unsigned char)*s)) {
> +while (qemu_isspace(*s)) {
>  s++;
>  }
>  if (*s == '-') {
> 

Reviewed-by: Philippe Mathieu-Daudé 

[Qemu-devel] [PATCH v2 4/6] gdbstub: Fix misuse of isxdigit()

[Markus Armbruster]

gdb_read_byte() passes its @ch argument to isxdigit().  Undefined
behavior when the value is negative.  Two callers:

* gdb_chr_receive() passes an uint8_t value.  Safe.

* gdb_handlesig() a char value.  Unsafe.  Not a security issue,
  because the characters come from the gdb client, which is trusted.

The obvious fix would be casting @ch to unsigned char.  But note that
gdb_read_byte() already casts @ch to uint8_t in many places.  Uses of
@ch without such a cast:

(1) Compare to a character constant with == or !=

(2) s->linesum += ch

(3) Store ch or ch ^ 0x20 into s->line_buf[]

(4) Check for invalid RLE count:
ch < ' ' || ch == '#' || ch == '$' || ch > 126

(5) Pass to isxdigit()

(6) Pass to fromhex()

Change the parameter type from int to uint8_t, and drop the now
redundant casts.  Affects the above uses as follows:

(1) No change: the character constants are all non-negative.

(2) Effectively no change: we only ever use s->linesum & 0xff, and
s->linesum is int.

(3) No change: s->line_buf[] is char[].

(4) No change.

(5) Avoid undefined behavior.

(6) No change: only reached when isxdigit(ch)

Signed-off-by: Markus Armbruster 
---
 gdbstub.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index c41eb1de07..b129df4e59 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1987,7 +1987,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const 
char *fmt, ...)
 va_end(va);
 }
 
-static void gdb_read_byte(GDBState *s, int ch)
+static void gdb_read_byte(GDBState *s, uint8_t ch)
 {
 uint8_t reply;
 
@@ -2001,7 +2001,7 @@ static void gdb_read_byte(GDBState *s, int ch)
 } else if (ch == '+') {
 trace_gdbstub_io_got_ack();
 } else {
-trace_gdbstub_io_got_unexpected((uint8_t)ch);
+trace_gdbstub_io_got_unexpected(ch);
 }
 
 if (ch == '+' || ch == '$')
@@ -2024,7 +2024,7 @@ static void gdb_read_byte(GDBState *s, int ch)
 s->line_sum = 0;
 s->state = RS_GETLINE;
 } else {
-trace_gdbstub_err_garbage((uint8_t)ch);
+trace_gdbstub_err_garbage(ch);
 }
 break;
 case RS_GETLINE:
@@ -2070,11 +2070,11 @@ static void gdb_read_byte(GDBState *s, int ch)
  */
 if (ch < ' ' || ch == '#' || ch == '$' || ch > 126) {
 /* invalid RLE count encoding */
-trace_gdbstub_err_invalid_repeat((uint8_t)ch);
+trace_gdbstub_err_invalid_repeat(ch);
 s->state = RS_GETLINE;
 } else {
 /* decode repeat length */
-int repeat = (unsigned char)ch - ' ' + 3;
+int repeat = ch - ' ' + 3;
 if (s->line_buf_index + repeat >= sizeof(s->line_buf) - 1) {
 /* that many repeats would overrun the command buffer */
 trace_gdbstub_err_overrun();
@@ -2096,7 +2096,7 @@ static void gdb_read_byte(GDBState *s, int ch)
 case RS_CHKSUM1:
 /* get high hex digit of checksum */
 if (!isxdigit(ch)) {
-trace_gdbstub_err_checksum_invalid((uint8_t)ch);
+trace_gdbstub_err_checksum_invalid(ch);
 s->state = RS_GETLINE;
 break;
 }
@@ -2107,7 +2107,7 @@ static void gdb_read_byte(GDBState *s, int ch)
 case RS_CHKSUM2:
 /* get low hex digit of checksum */
 if (!isxdigit(ch)) {
-trace_gdbstub_err_checksum_invalid((uint8_t)ch);
+trace_gdbstub_err_checksum_invalid(ch);
 s->state = RS_GETLINE;
 break;
 }
-- 
2.17.2

[Qemu-devel] [PATCH v2 3/6] gdbstub: Reject invalid RLE repeat counts

[Markus Armbruster]

"Debugging with GDB / Appendix E GDB Remote Serial Protocol /
Overview" specifies "The printable characters '#' and '$' or with a
numeric value greater than 126 must not be used."  gdb_read_byte()
only rejects values < 32.  This is wrong.  Impact depends on the caller:

* gdb_handlesig() passes a char.  Incorrectly accepts '#', '$' and
  '\127'.

* gdb_chr_receive() passes an uint8_t.  Additionally accepts
  characters with the most-significant bit set.

Correct the validity check to match the specification.

Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdbstub.c b/gdbstub.c
index d54abd17cc..c41eb1de07 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2064,7 +2064,11 @@ static void gdb_read_byte(GDBState *s, int ch)
 }
 break;
 case RS_GETLINE_RLE:
-if (ch < ' ') {
+/*
+ * Run-length encoding is explained in "Debugging with GDB /
+ * Appendix E GDB Remote Serial Protocol / Overview".
+ */
+if (ch < ' ' || ch == '#' || ch == '$' || ch > 126) {
 /* invalid RLE count encoding */
 trace_gdbstub_err_invalid_repeat((uint8_t)ch);
 s->state = RS_GETLINE;
-- 
2.17.2

Re: [Qemu-devel] [PATCH v2 5/6] pc-bios/s390-ccw: Clean up harmless misuse of isdigit()

[Christian Borntraeger]

On 14.05.19 20:03, Markus Armbruster wrote:
> atoui() and get_index() pass char values to isdigit().  With a
> standard isdigit(), we'd get undefined behavior when the value is
> negative.  Can't happen as char is unsigned on s390x.  Even if it
> could, we're actually using isdigit() from pc-bios/s390-ccw/libc.h
> here, which works fine for negative values.  Clean up anyway, just
> to avoid setting a bad example.
> 
> Cc: Christian Borntraeger 
Acked-by: Christian Borntraeger 

> Cc: Thomas Huth 
> Cc: Cornelia Huck 
> Cc: qemu-s3...@nongnu.org
> Signed-off-by: Markus Armbruster 
> ---
>  pc-bios/s390-ccw/libc.c | 2 +-
>  pc-bios/s390-ccw/menu.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/libc.c b/pc-bios/s390-ccw/libc.c
> index a786566c4c..3187923950 100644
> --- a/pc-bios/s390-ccw/libc.c
> +++ b/pc-bios/s390-ccw/libc.c
> @@ -38,7 +38,7 @@ uint64_t atoui(const char *str)
>  }
>  
>  while (*str) {
> -if (!isdigit(*str)) {
> +if (!isdigit(*(unsigned char *)str)) {
>  break;
>  }
>  val = val * 10 + *str - '0';
> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
> index 82a4ae6315..ce3815b201 100644
> --- a/pc-bios/s390-ccw/menu.c
> +++ b/pc-bios/s390-ccw/menu.c
> @@ -134,7 +134,7 @@ static int get_index(void)
>  
>  /* Check for erroneous input */
>  for (i = 0; i < len; i++) {
> -if (!isdigit(buf[i])) {
> +if (!isdigit((unsigned char)buf[i])) {
>  return -1;
>  }
>  }
> 

[Qemu-devel] [PATCH v2 0/6] Fix misuse of ctype.h functions

[Markus Armbruster]

v2:
* PATCH 1: Use g_ascii_isspace(), adapt commit message [Philippe]
* PATCH 3: Add comment pointing to the GDB manual [Philippe]
* PATCH 5: Improve commit message [Thomas]

Markus Armbruster (6):
  qemu-bridge-helper: Fix misuse of isspace()
  tests/vhost-user-bridge: Fix misuse of isdigit()
  gdbstub: Reject invalid RLE repeat counts
  gdbstub: Fix misuse of isxdigit()
  pc-bios/s390-ccw: Clean up harmless misuse of isdigit()
  cutils: Simplify how parse_uint() checks for whitespace

 gdbstub.c | 20 
 pc-bios/s390-ccw/libc.c   |  2 +-
 pc-bios/s390-ccw/menu.c   |  2 +-
 qemu-bridge-helper.c  |  6 +++---
 tests/vhost-user-bridge.c |  2 +-
 util/cutils.c |  2 +-
 6 files changed, 19 insertions(+), 15 deletions(-)

-- 
2.17.2

[Qemu-devel] [PATCH v2 1/6] qemu-bridge-helper: Fix misuse of isspace()

[Markus Armbruster]

parse_acl_file() passes char values to isspace().  Undefined behavior
when the value is negative.  Not a security issue, because the
characters come from trusted $prefix/etc/qemu/bridge.conf and the
files it includes.

Furthermore, isspace()'s locale-dependence means qemu-bridge-helper
uses the user's locale for parsing $prefix/etc/bridge.conf.  Feels
wrong.

Use g_ascii_isspace() instead.  This fixes the undefined behavior, and
makes parsing of $prefix/etc/bridge.conf locale-independent.

Signed-off-by: Markus Armbruster 
---
 qemu-bridge-helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 5396fbfbb6..f9940deefd 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -75,7 +75,7 @@ static int parse_acl_file(const char *filename, ACLList 
*acl_list)
 char *ptr = line;
 char *cmd, *arg, *argend;
 
-while (isspace(*ptr)) {
+while (g_ascii_isspace(*ptr)) {
 ptr++;
 }
 
@@ -99,12 +99,12 @@ static int parse_acl_file(const char *filename, ACLList 
*acl_list)
 
 *arg = 0;
 arg++;
-while (isspace(*arg)) {
+while (g_ascii_isspace(*arg)) {
 arg++;
 }
 
 argend = arg + strlen(arg);
-while (arg != argend && isspace(*(argend - 1))) {
+while (arg != argend && g_ascii_isspace(*(argend - 1))) {
 argend--;
 }
 *argend = 0;
-- 
2.17.2