Re: [PATCH v10 06/10] qcow2_format.py: pass cluster size to substructures

[Andrey Shinkevich]

On 16.07.2020 12:26, Vladimir Sementsov-Ogievskiy wrote:

14.07.2020 00:36, Andrey Shinkevich wrote:

The cluster size of an image is the QcowHeader class member and may be
obtained by dependent extension structures such as Qcow2BitmapExt for
further bitmap table details print.

Signed-off-by: Andrey Shinkevich 
---
  tests/qemu-iotests/qcow2_format.py | 18 +++---
  1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py



...

    def read_bitmap_directory(self, fd):
  fd.seek(self.bitmap_directory_offset)
  self.bitmap_directory = \
-    [Qcow2BitmapDirEntry(fd) for _ in range(self.nb_bitmaps)]
+    [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size)
+ for _ in range(self.nb_bitmaps)]


Better to inline the bitmap directory loading code into __init__:

Benefits:
 1. Less code. read_bitmap_directory() is very small, used only in 
__init__ and just not needed as a separate method. __init__ is very 
small and simple too, so it's not a problem.
 2. no need of extra self.cluster_size variable (you can use 
cluster_size parameter directly)

 3. keep all fd.seek logic in one method

but it's not about this patch.



The idea behind the read_bitmap_directory() method was an encapsulation 
of reading the optional parameter. So, we can be flexible in future. 
Sure, there are prones and cons for that in the current implementation.



Andrey



    def dump(self):
  super().dump()
@@ -162,8 +164,9 @@ class Qcow2BitmapDirEntry(Qcow2Struct):


...

@@ -244,7 +249,6 @@ class QcowHeaderExtension(Qcow2Struct):
  data_str = ''
  self.data_str = data_str
  -


Unrelated style-fixing chunk, please don't do so.

with it dropped:
Reviewed-by: Vladimir Sementsov-Ogievskiy 


  def dump(self):
  super().dump()
  @@ -316,7 +320,7 @@ class QcowHeader(Qcow2Struct):
  end = self.cluster_size
    while fd.tell() < end:
-    ext = QcowHeaderExtension(fd=fd)
+    ext = QcowHeaderExtension(fd=fd, 
cluster_size=self.cluster_size)

  if ext.magic == 0:
  break
  else:

[PATCH] scripts/oss-fuzz: Limit target list to i386-softmmu

[Thomas Huth]

The build.sh script only copies qemu-fuzz-i386 to the destination folder,
so we can speed up the compilation step quite a bit by not compiling the
other targets here.

Signed-off-by: Thomas Huth 
---
 scripts/oss-fuzz/build.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
index f5cee3d67e..a07b3022e8 100755
--- a/scripts/oss-fuzz/build.sh
+++ b/scripts/oss-fuzz/build.sh
@@ -68,7 +68,7 @@ mkdir -p "$DEST_DIR/lib/"  # Copy the shared libraries here
 
 # Build once to get the list of dynamic lib paths, and copy them over
 ../configure --disable-werror --cc="$CC" --cxx="$CXX" \
---extra-cflags="$EXTRA_CFLAGS"
+--extra-cflags="$EXTRA_CFLAGS" --target-list="i386-softmmu"
 
 if ! make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" "-j$(nproc)" \
 i386-softmmu/fuzz; then
-- 
2.18.1

[RFC PATCH-for-5.1] hw/ide: Cancel pending DMA requests before setting as inactive

[Philippe Mathieu-Daudé]

libFuzzer found a case where requests are queued for later in the
AIO context, but a command set the bus inactive, then when finally
the requests are processed by the DMA it aborts because it is
inactive:

 include/hw/ide/pci.h:59: IDEState *bmdma_active_if(BMDMAState *): Assertion 
`bmdma->bus->retry_unit != (uint8_t)-1' failed.

Reproducer available on the BugLink.

Fix by draining the pending DMA requests before inactivating the bus.

BugLink: https://bugs.launchpad.net/qemu/+bug/1887303
Signed-off-by: Philippe Mathieu-Daudé 
---
RFC because I don't have much clue about block drive and IDE,
so block-team please be very careful while reviewing this bug.
---
 hw/ide/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index d997a78e47..b21d28f99c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -804,11 +804,11 @@ void dma_buf_commit(IDEState *s, uint32_t tx_bytes)
 
 void ide_set_inactive(IDEState *s, bool more)
 {
-s->bus->dma->aiocb = NULL;
-ide_clear_retry(s);
 if (s->bus->dma->ops->set_inactive) {
 s->bus->dma->ops->set_inactive(s->bus->dma, more);
 }
+ide_cancel_dma_sync(s);
+ide_clear_retry(s);
 ide_cmd_done(s);
 }
 
-- 
2.21.3

Re: [PATCH v5 10/11] hw/arm: Wire up BMC boot flash for npcm750-evb and quanta-gsj

[Philippe Mathieu-Daudé]

+Thomas

On 7/16/20 10:56 PM, Havard Skinnemoen wrote:
> On Wed, Jul 15, 2020 at 1:54 PM Havard Skinnemoen
>  wrote:
>>
>> On Wed, Jul 15, 2020 at 3:57 AM Philippe Mathieu-Daudé  
>> wrote:
>>>
>>> On 7/15/20 11:00 AM, Markus Armbruster wrote:
 Now my point.  Why first make up user configuration, then use that to
 create a BlockBackend, when you could just go ahead and create the
 BlockBackend?
>>>
>>> CLI issue mostly.
>>>
>>> We can solve it similarly to the recent "sdcard: Do not allow invalid SD
>>> card sizes" patch:
>>>
>>>  if (!dinfo) {
>>>  error_setg(errp, "Missing SPI flash drive");
>>>  error_append_hint(errp, "You can use a dummy drive using:\n");
>>>  error_append_hint(errp, "-drive if=mtd,driver=null-co,"
>>>  "read-ones=on,size=64M\n);
>>>  return;
>>>  }
>>>
>>> having npcm7xx_connect_flash() taking an Error* argument,
>>> and MachineClass::init() call it with &error_fatal.
>>
>> Erroring out if the user specifies a configuration that can't possibly
>> boot sounds good to me. Better than trying to come up with defaults
>> that are still not going to result in a bootable system.
>>
>> For testing recovery paths, I think it makes sense to explicitly
>> specify a null device as you suggest.
> 
> Hmm, one problem. qom-test fails with
> 
> qemu-system-aarch64: Missing SPI flash drive
> You can add a dummy drive using:
> -drive if=mtd,driver=null-co,read-zeroes=on,size=32M
> Broken pipe
> /usr/local/google/home/hskinnemoen/qemu/for-upstream/tests/qtest/libqtest.c:166:
> kill_qemu() tried to terminate QEMU process but encountered exit
> status 1 (expected 0)
> ERROR qom-test - too few tests run (expected 68, got 7)
> 
> So it looks like we might need a different solution to this, unless we
> want to make generic tests more machine-aware...
> 

Re: [PATCH] usb: only build hcd-dwc2 host controller for RASPI target

[Philippe Mathieu-Daudé]

On 7/17/20 3:05 AM, Paul Zimmerman wrote:
> The hcd-dwc2 host controller is currently built for all targets.
> Since for now hcd-dwc2 is only implemented on RASPI, restrict its
> build to that target only.
> 
> Signed-off-by: Paul Zimmerman 
> ---
> 
> Hi Gerd,
> 
> Do we want to apply this before the 5.1.0 release? It seems a waste
> to build this code for every target when it's only used on one.
> Sorry I didn't realize this earlier.

Not a big deal ;)

Reviewed-by: Philippe Mathieu-Daudé 

> 
> Thanks,
> Paul
> 
>  hw/arm/Kconfig | 1 +
>  hw/usb/Kconfig | 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 4a224a6351..bc3a423940 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -315,6 +315,7 @@ config RASPI
>  select FRAMEBUFFER
>  select PL011 # UART
>  select SDHCI
> +select USB_DWC2
>  
>  config STM32F205_SOC
>  bool
> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
> index d4d8c37c28..5e63dc75f8 100644
> --- a/hw/usb/Kconfig
> +++ b/hw/usb/Kconfig
> @@ -48,7 +48,6 @@ config USB_MUSB
>  
>  config USB_DWC2
>  bool
> -default y
>  select USB
>  
>  config TUSB6010
> 

Re: [PATCH] gitlab-ci.yml: Add oss-fuzz build tests

[Thomas Huth]

On 17/07/2020 07.40, Thomas Huth wrote:
> On 16/07/2020 18.33, Alexander Bulekov wrote:
>> This tries to build and run the fuzzers with the same build-script used
>> by oss-fuzz. This doesn't guarantee that the builds on oss-fuzz will
>> also succeed, since oss-fuzz provides its own compiler and fuzzer vars,
>> but it can catch changes that are not compatible with the the
>> ./scripts/oss-fuzz/build.sh script.
>> The strange way of finding fuzzer binaries stems from the method used by
>> oss-fuzz:
>> https://github.com/google/oss-fuzz/blob/master/infra/base-images/base-runner/targets_list
>>
>> Signed-off-by: Alexander Bulekov 
>> ---
>>
>> Similar to Thomas' patch:
>>
>>> Note: This patch needs two other patches merged first to work correctly:
>>
>>> - 'fuzz: Expect the cmdline in a freeable GString' from Alexander
>>
>>> - 'qom: Plug memory leak in "info qom-tree"' from Markus
>>
>> Otherwise the test will fail due to detected memory leaks.
>>
>> Fair warning: I haven't been able to trigger this new job yet. I tried
>> to run the pipeline with these changes on my forked repo on gitlab, but
>> did not reach the build-oss-fuzz. I think this is due to some failures
>> in the Containers-layer-2 stage:

I think I've got it basically working like this:

build-oss-fuzz:
  <<: *native_build_job_definition
  variables:
IMAGE: fedora
  script:
- mkdir build-oss-fuzz
- CC="clang" CXX="clang++" CFLAGS="-fsanitize=address"
  ./scripts/oss-fuzz/build.sh
- for fuzzer in $(find build-oss-fuzz/DEST_DIR/ -executable -type f
  | grep -v slirp); do
grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 ||
continue ;
echo Testing ${fuzzer} ... ;
"${fuzzer}" -runs=1000 || exit 1 ;
  done

However, it still triggered a memory leak and thus the pipeline failed:

 https://gitlab.com/huth/qemu/-/jobs/643472695

... and this was with all the other "leak fix" patches already applied.
Is there an easy way to debug that issue? That information from the
LeakSanitizer looks pretty sparse...

 Thomas

Re: [RFC PATCH-for-5.1] hw/ide: Cancel pending DMA requests before setting as inactive

[Philippe Mathieu-Daudé]

On 7/17/20 9:47 AM, Philippe Mathieu-Daudé wrote:
> libFuzzer found a case where requests are queued for later in the
> AIO context, but a command set the bus inactive, then when finally
> the requests are processed by the DMA it aborts because it is
> inactive:
> 
>  include/hw/ide/pci.h:59: IDEState *bmdma_active_if(BMDMAState *): Assertion 
> `bmdma->bus->retry_unit != (uint8_t)-1' failed.
> 
> Reproducer available on the BugLink.
> 
> Fix by draining the pending DMA requests before inactivating the bus.
> 
> BugLink: https://bugs.launchpad.net/qemu/+bug/1887303
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> RFC because I don't have much clue about block drive and IDE,
> so block-team please be very careful while reviewing this bug.
> ---
>  hw/ide/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index d997a78e47..b21d28f99c 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -804,11 +804,11 @@ void dma_buf_commit(IDEState *s, uint32_t tx_bytes)
>  
>  void ide_set_inactive(IDEState *s, bool more)
>  {
> -s->bus->dma->aiocb = NULL;
> -ide_clear_retry(s);
>  if (s->bus->dma->ops->set_inactive) {
>  s->bus->dma->ops->set_inactive(s->bus->dma, more);
>  }
> +ide_cancel_dma_sync(s);
> +ide_clear_retry(s);

Oops this is not the one I wanted to send. I'll send v2 shortly =)

>  ide_cmd_done(s);
>  }
>  
> 

[RFC PATCH-for-5.1 v2] hw/ide: Cancel pending DMA requests before setting as inactive

[Philippe Mathieu-Daudé]

libFuzzer found a case where requests are queued for later in the
AIO context, but a command set the bus inactive, then when finally
the requests are processed by the DMA it aborts because it is
inactive:

 include/hw/ide/pci.h:59: IDEState *bmdma_active_if(BMDMAState *): Assertion 
`bmdma->bus->retry_unit != (uint8_t)-1' failed.

Reproducer available on the BugLink.

Fix by draining the pending DMA requests before inactivating the bus.

BugLink: https://bugs.launchpad.net/qemu/+bug/1887303
Signed-off-by: Philippe Mathieu-Daudé 
---
RFC because I don't have much clue about block drive and IDE,
so block-team please be very careful while reviewing this bug.
---
 hw/ide/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index d997a78e47..f7affafb0c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -804,7 +804,7 @@ void dma_buf_commit(IDEState *s, uint32_t tx_bytes)
 
 void ide_set_inactive(IDEState *s, bool more)
 {
-s->bus->dma->aiocb = NULL;
+ide_cancel_dma_sync(s);
 ide_clear_retry(s);
 if (s->bus->dma->ops->set_inactive) {
 s->bus->dma->ops->set_inactive(s->bus->dma, more);
-- 
2.21.3

Re: [PATCH] scripts/oss-fuzz: Limit target list to i386-softmmu

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200717073335.25534-1-th...@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TESTcheck-unit: tests/test-crypto-secret
  TESTcheck-unit: tests/test-char
**
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should 
not be NULL
ERROR test-char - Bail out! 
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should 
not be NULL
make: *** [check-unit] Error 1
make: *** Waiting for unfinished jobs
  TESTiotest-qcow2: 022
  TESTiotest-qcow2: 024
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=f0f3b1f6d8f84ce0abf1ccec50045205', '-u', 
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-ravzxa0a/src/docker-src.2020-07-17-03.39.35.21926:/var/tmp/qemu:z,ro',
 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=f0f3b1f6d8f84ce0abf1ccec50045205
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-ravzxa0a/src'
make: *** [docker-run-test-quick@centos7] Error 2

real15m40.323s
user0m8.919s


The full log is available at
http://patchew.org/logs/20200717073335.25534-1-th...@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH v6 01/13] hw/misc: Add NPCM7xx System Global Control Registers device model

[Philippe Mathieu-Daudé]

On 7/17/20 8:02 AM, Havard Skinnemoen wrote:
> Implement a device model for the System Global Control Registers in the
> NPCM730 and NPCM750 BMC SoCs.
> 
> This is primarily used to enable SMP boot (the boot ROM spins reading
> the SCRPAD register) and DDR memory initialization; other registers are
> best effort for now.
> 
> The reset values of the MDLR and PWRON registers are determined by the
> SoC variant (730 vs 750) and board straps respectively.
> 
> Reviewed-by: Joel Stanley 
> Reviewed-by: Cédric Le Goater 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Havard Skinnemoen 
> ---
>  include/hw/misc/npcm7xx_gcr.h |  76 
>  hw/misc/npcm7xx_gcr.c | 227 ++
>  MAINTAINERS   |   8 ++
>  hw/arm/Kconfig|   3 +
>  hw/misc/Makefile.objs |   1 +
>  hw/misc/trace-events  |   4 +
>  6 files changed, 319 insertions(+)
>  create mode 100644 include/hw/misc/npcm7xx_gcr.h
>  create mode 100644 hw/misc/npcm7xx_gcr.c
...

> +static void npcm7xx_gcr_realize(DeviceState *dev, Error **errp)
> +{
> +ERRP_GUARD();
> +NPCM7xxGCRState *s = NPCM7XX_GCR(dev);
> +uint64_t dram_size;
> +Object *obj;
> +
> +obj = object_property_get_link(OBJECT(dev), "dram-mr", errp);
> +if (!obj) {
> +error_prepend(errp, "%s: required dram-mr link not found: ", 
> __func__);
> +return;
> +}
> +dram_size = memory_region_size(MEMORY_REGION(obj));
> +if (!is_power_of_2(dram_size) ||
> +dram_size < NPCM7XX_GCR_MIN_DRAM_SIZE ||
> +dram_size > NPCM7XX_GCR_MAX_DRAM_SIZE) {
> +error_setg(errp, "%s: unsupported DRAM size %" PRIu64,
> +   __func__, dram_size);

Nitpicking if you ever have to respin, please use size_to_str() here,

> +error_append_hint(errp,
> +  "DRAM size must be a power of two between %" PRIu64
> +  " and %" PRIu64 " MiB, inclusive.\n",
> +  NPCM7XX_GCR_MIN_DRAM_SIZE / MiB,
> +  NPCM7XX_GCR_MAX_DRAM_SIZE / MiB);

and here.

> +return;
> +}
> +
> +/* Power-on reset value */
> +s->reset_intcr3 = 0x1002;
> +
> +/*
> + * The GMMAP (Graphics Memory Map) field is used by u-boot to detect the
> + * DRAM size, and is normally initialized by the boot block as part of 
> DRAM
> + * training. However, since we don't have a complete emulation of the
> + * memory controller and try to make it look like it has already been
> + * initialized, the boot block will skip this initialization, and we need
> + * to make sure this field is set correctly up front.
> + *
> + * WARNING: some versions of u-boot only looks at bits 8 and 9, so 2 GiB 
> of
> + * DRAM will be interpreted as 128 MiB.
> + *
> + * 
> https://github.com/Nuvoton-Israel/u-boot/blob/2aef993bd2aafeb5408dbaad0f3ce099ee40c4aa/board/nuvoton/poleg/poleg.c#L244
> + */
> +s->reset_intcr3 |= ctz64(dram_size / NPCM7XX_GCR_MIN_DRAM_SIZE) << 8;

Nice :)

> +}
[...]

Re: [PATCH v5 10/11] hw/arm: Wire up BMC boot flash for npcm750-evb and quanta-gsj

[Thomas Huth]

On 17/07/2020 09.48, Philippe Mathieu-Daudé wrote:
> +Thomas

> On 7/16/20 10:56 PM, Havard Skinnemoen wrote:
>> On Wed, Jul 15, 2020 at 1:54 PM Havard Skinnemoen
>>  wrote:
>>>
>>> On Wed, Jul 15, 2020 at 3:57 AM Philippe Mathieu-Daudé  
>>> wrote:

 On 7/15/20 11:00 AM, Markus Armbruster wrote:
> Now my point.  Why first make up user configuration, then use that to
> create a BlockBackend, when you could just go ahead and create the
> BlockBackend?

 CLI issue mostly.

 We can solve it similarly to the recent "sdcard: Do not allow invalid SD
 card sizes" patch:

  if (!dinfo) {
  error_setg(errp, "Missing SPI flash drive");
  error_append_hint(errp, "You can use a dummy drive using:\n");
  error_append_hint(errp, "-drive if=mtd,driver=null-co,"
  "read-ones=on,size=64M\n);
  return;
  }

 having npcm7xx_connect_flash() taking an Error* argument,
 and MachineClass::init() call it with &error_fatal.
>>>
>>> Erroring out if the user specifies a configuration that can't possibly
>>> boot sounds good to me. Better than trying to come up with defaults
>>> that are still not going to result in a bootable system.
>>>
>>> For testing recovery paths, I think it makes sense to explicitly
>>> specify a null device as you suggest.
>>
>> Hmm, one problem. qom-test fails with
>>
>> qemu-system-aarch64: Missing SPI flash drive
>> You can add a dummy drive using:
>> -drive if=mtd,driver=null-co,read-zeroes=on,size=32M
>> Broken pipe
>> /usr/local/google/home/hskinnemoen/qemu/for-upstream/tests/qtest/libqtest.c:166:
>> kill_qemu() tried to terminate QEMU process but encountered exit
>> status 1 (expected 0)
>> ERROR qom-test - too few tests run (expected 68, got 7)
>>
>> So it looks like we might need a different solution to this, unless we
>> want to make generic tests more machine-aware...

I didn't follow the other mails in this thread, but what we usually do
in such a case: Add a "if (qtest_enabled())" check to the device or the
machine to ignore the error if it is running in qtest mode.

 Thomas

Re: [PATCH v6 03/13] hw/timer: Add NPCM7xx Timer device model

[Philippe Mathieu-Daudé]

On 7/17/20 8:02 AM, Havard Skinnemoen wrote:
> The NPCM730 and NPCM750 SoCs have three timer modules each holding five
> timers and some shared registers (e.g. interrupt status).
> 
> Each timer runs at 25 MHz divided by a prescaler, and counts down from a
> configurable initial value to zero. When zero is reached, the interrupt
> flag for the timer is set, and the timer is disabled (one-shot mode) or
> reloaded from its initial value (periodic mode).
> 
> This implementation is sufficient to boot a Linux kernel configured for
> NPCM750. Note that the kernel does not seem to actually turn on the
> interrupts.
> 
> Reviewed-by: Tyrone Ting 
> Reviewed-by: Joel Stanley 
> Signed-off-by: Havard Skinnemoen 
> ---
>  include/hw/timer/npcm7xx_timer.h |  96 ++
>  hw/timer/npcm7xx_timer.c | 489 +++
>  hw/timer/Makefile.objs   |   1 +
>  hw/timer/trace-events|   5 +
>  4 files changed, 591 insertions(+)
>  create mode 100644 include/hw/timer/npcm7xx_timer.h
>  create mode 100644 hw/timer/npcm7xx_timer.c
...

> +static hwaddr npcm7xx_tcsr_index(hwaddr reg)
> +{
> +switch (reg) {
> +case NPCM7XX_TIMER_TCSR0:
> +return 0;
> +case NPCM7XX_TIMER_TCSR1:
> +return 1;
> +case NPCM7XX_TIMER_TCSR2:
> +return 2;
> +case NPCM7XX_TIMER_TCSR3:
> +return 3;
> +case NPCM7XX_TIMER_TCSR4:
> +return 4;
> +default:
> +g_assert_not_reached();
> +}
> +}
> +
> +static hwaddr npcm7xx_ticr_index(hwaddr reg)
> +{
> +switch (reg) {
> +case NPCM7XX_TIMER_TICR0:
> +return 0;
> +case NPCM7XX_TIMER_TICR1:
> +return 1;
> +case NPCM7XX_TIMER_TICR2:
> +return 2;
> +case NPCM7XX_TIMER_TICR3:
> +return 3;
> +case NPCM7XX_TIMER_TICR4:
> +return 4;
> +default:
> +g_assert_not_reached();
> +}
> +}
> +
> +static hwaddr npcm7xx_tdr_index(hwaddr reg)
> +{
> +switch (reg) {
> +case NPCM7XX_TIMER_TDR0:
> +return 0;
> +case NPCM7XX_TIMER_TDR1:
> +return 1;
> +case NPCM7XX_TIMER_TDR2:
> +return 2;
> +case NPCM7XX_TIMER_TDR3:
> +return 3;
> +case NPCM7XX_TIMER_TDR4:
> +return 4;
> +default:
> +g_assert_not_reached();
> +}
> +}
> +
> +static uint64_t npcm7xx_timer_read(void *opaque, hwaddr offset, unsigned 
> size)
> +{
> +NPCM7xxTimerCtrlState *s = opaque;
> +uint64_t value = 0;
> +hwaddr reg;
> +
> +reg = offset / sizeof(uint32_t);
> +switch (reg) {
> +case NPCM7XX_TIMER_TCSR0:
> +case NPCM7XX_TIMER_TCSR1:
> +case NPCM7XX_TIMER_TCSR2:
> +case NPCM7XX_TIMER_TCSR3:
> +case NPCM7XX_TIMER_TCSR4:
> +value = s->timer[npcm7xx_tcsr_index(reg)].tcsr;
> +break;
> +
> +case NPCM7XX_TIMER_TICR0:
> +case NPCM7XX_TIMER_TICR1:
> +case NPCM7XX_TIMER_TICR2:
> +case NPCM7XX_TIMER_TICR3:
> +case NPCM7XX_TIMER_TICR4:
> +value = s->timer[npcm7xx_ticr_index(reg)].ticr;
> +break;
> +
> +case NPCM7XX_TIMER_TDR0:
> +case NPCM7XX_TIMER_TDR1:
> +case NPCM7XX_TIMER_TDR2:
> +case NPCM7XX_TIMER_TDR3:
> +case NPCM7XX_TIMER_TDR4:
> +value = npcm7xx_timer_read_tdr(&s->timer[npcm7xx_tdr_index(reg)]);
> +break;

Thanks for adding the register index getters, I'm not sure this is a
matter of taste, but I find it easier to review.

> +
> +case NPCM7XX_TIMER_TISR:
> +value = s->tisr;
> +break;
> +
> +case NPCM7XX_TIMER_WTCR:
> +value = s->wtcr;
> +break;
> +
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "%s: invalid offset 0x%04" HWADDR_PRIx "\n",
> +  __func__, offset);
> +break;
> +}
> +
> +trace_npcm7xx_timer_read(DEVICE(s)->canonical_path, offset, value);
> +
> +return value;
> +}
...

Re: [PATCH v6 03/13] hw/timer: Add NPCM7xx Timer device model

[Philippe Mathieu-Daudé]

On 7/17/20 8:02 AM, Havard Skinnemoen wrote:
> The NPCM730 and NPCM750 SoCs have three timer modules each holding five
> timers and some shared registers (e.g. interrupt status).
> 
> Each timer runs at 25 MHz divided by a prescaler, and counts down from a
> configurable initial value to zero. When zero is reached, the interrupt
> flag for the timer is set, and the timer is disabled (one-shot mode) or
> reloaded from its initial value (periodic mode).
> 
> This implementation is sufficient to boot a Linux kernel configured for
> NPCM750. Note that the kernel does not seem to actually turn on the
> interrupts.
> 
> Reviewed-by: Tyrone Ting 
> Reviewed-by: Joel Stanley 
> Signed-off-by: Havard Skinnemoen 
> ---
>  include/hw/timer/npcm7xx_timer.h |  96 ++
>  hw/timer/npcm7xx_timer.c | 489 +++
>  hw/timer/Makefile.objs   |   1 +
>  hw/timer/trace-events|   5 +
>  4 files changed, 591 insertions(+)
>  create mode 100644 include/hw/timer/npcm7xx_timer.h
>  create mode 100644 hw/timer/npcm7xx_timer.c
...

> +/*
> + * Raise the interrupt line if there's a pending interrupt and interrupts are
> + * enabled for this timer. If not, lower it.
> + */
> +static void npcm7xx_timer_check_interrupt(NPCM7xxTimer *t)
> +{
> +NPCM7xxTimerCtrlState *tc = t->ctrl;
> +int index = npcm7xx_timer_index(tc, t);
> +
> +if ((t->tcsr & NPCM7XX_TCSR_IE) && (tc->tisr & BIT(index))) {
> +qemu_irq_raise(t->irq);
> +trace_npcm7xx_timer_irq(DEVICE(tc)->canonical_path, index, 1);
> +} else {
> +qemu_irq_lower(t->irq);
> +trace_npcm7xx_timer_irq(DEVICE(tc)->canonical_path, index, 0);
> +}

Maybe simpler:

  bool pending = (t->tcsr & NPCM7XX_TCSR_IE) && (tc->tisr & BIT(index));

  trace_npcm7xx_timer_irq(DEVICE(tc)->canonical_path, index, pending);
  qemu_set_irq(t->irq, pending);

Anyway,
Reviewed-by: Philippe Mathieu-Daudé 

[PATCH v11 10/11] qcow2_format.py: introduce Qcow2HeaderExtensionsDoc class

[Andrey Shinkevich]

Per original script design, QcowHeader class may dump the QCOW2 header
info separately from the QCOW2 extensions info. To implement the
to_dict() method for dumping extensions, let us introduce the class
Qcow2HeaderExtensionsDoc.

Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index 19d29b8..d2a8659 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -248,6 +248,15 @@ class Qcow2BitmapTable:
 return dict(entries=self.entries)
 
 
+class Qcow2HeaderExtensionsDoc:
+
+def __init__(self, extensions):
+self.extensions = extensions
+
+def to_dict(self):
+return dict(Header_extensions=self.extensions)
+
+
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
 
 
-- 
1.8.3.1

[PATCH v11 05/11] qcow2_format.py: Dump bitmap directory information

[Andrey Shinkevich]

Read and dump entries from the bitmap directory of QCOW2 image.

Header extension:
magic 0x23852875 (Bitmaps)
...
Bitmap name   bitmap-1
bitmap_table_offset   0xf
bitmap_table_size 1
flags 0x2 (['auto'])
type  1
granularity_bits  16
name_size 8
extra_data_size   0

Suggested-by: Kevin Wolf 
Signed-off-by: Andrey Shinkevich 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/qcow2_format.py | 47 ++
 1 file changed, 47 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index b447344..05a8aa9 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -134,6 +134,53 @@ class Qcow2BitmapExt(Qcow2Struct):
 tail = struct.calcsize(self.fmt) % 8
 if tail:
 fd.seek(8 - tail, 1)
+position = fd.tell()
+self.read_bitmap_directory(fd)
+fd.seek(position)
+
+def read_bitmap_directory(self, fd):
+fd.seek(self.bitmap_directory_offset)
+self.bitmap_directory = \
+[Qcow2BitmapDirEntry(fd) for _ in range(self.nb_bitmaps)]
+
+def dump(self):
+super().dump()
+for entry in self.bitmap_directory:
+print()
+entry.dump()
+
+
+class Qcow2BitmapDirEntry(Qcow2Struct):
+
+fields = (
+('u64', '{:#x}', 'bitmap_table_offset'),
+('u32', '{}', 'bitmap_table_size'),
+('u32', BitmapFlags, 'flags'),
+('u8',  '{}', 'type'),
+('u8',  '{}', 'granularity_bits'),
+('u16', '{}', 'name_size'),
+('u32', '{}', 'extra_data_size')
+)
+
+def __init__(self, fd):
+super().__init__(fd=fd)
+# Seek relative to the current position in the file
+fd.seek(self.extra_data_size, 1)
+bitmap_name = fd.read(self.name_size)
+self.name = bitmap_name.decode('ascii')
+# Move position to the end of the entry in the directory
+entry_raw_size = self.bitmap_dir_entry_raw_size()
+padding = ((entry_raw_size + 7) & ~7) - entry_raw_size
+fd.seek(padding, 1)
+
+def bitmap_dir_entry_raw_size(self):
+return struct.calcsize(self.fmt) + self.name_size + \
+self.extra_data_size
+
+def dump(self):
+print(f'{"Bitmap name":<25} {self.name}')
+super(Qcow2BitmapDirEntry, self).dump()
+
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
 
-- 
1.8.3.1

[PATCH v11 00/11] iotests: Dump QCOW2 dirty bitmaps metadata

[Andrey Shinkevich]

Add dirty bitmap information to QCOW2 metadata dump in the qcow2_format.py.

v10:
  01: Fixing of issues in QCOW2 extension classes noted by Vladimir.
  02: Reading bitmap tables was moved into Qcow2BitmapTable class.
  03: Handling '-j' key was moved into "if __name__" section.
  04: Making copy of __dict__ was replaced with the method to_dict().
  05: Qcow2HeaderExtensionsDoc is introduced in the separate patch.

Andrey Shinkevich (11):
  qcow2: Fix capitalization of header extension constant.
  qcow2_format.py: make printable data an extension class member
  qcow2_format.py: change Qcow2BitmapExt initialization method
  qcow2_format.py: dump bitmap flags in human readable way.
  qcow2_format.py: Dump bitmap directory information
  qcow2_format.py: pass cluster size to substructures
  qcow2_format.py: Dump bitmap table serialized entries
  qcow2.py: Introduce '-j' key to dump in JSON format
  qcow2_format.py: collect fields to dump in JSON format
  qcow2_format.py: introduce Qcow2HeaderExtensionsDoc class
  qcow2_format.py: support dumping metadata in JSON format

 block/qcow2.c  |   2 +-
 docs/interop/qcow2.txt |   2 +-
 tests/qemu-iotests/qcow2.py|  18 ++-
 tests/qemu-iotests/qcow2_format.py | 221 ++---
 4 files changed, 220 insertions(+), 23 deletions(-)

-- 
1.8.3.1

[PATCH v11 08/11] qcow2.py: Introduce '-j' key to dump in JSON format

[Andrey Shinkevich]

Add the command key to the qcow2.py arguments list to dump QCOW2
metadata in JSON format. Here is the suggested way to do that. The
implementation of the dump in JSON format is in the patch that follows.

Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2.py| 18 ++
 tests/qemu-iotests/qcow2_format.py |  5 +++--
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 0910e6a..77ca59c 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -26,16 +26,19 @@ from qcow2_format import (
 )
 
 
+is_json = False
+
+
 def cmd_dump_header(fd):
 h = QcowHeader(fd)
-h.dump()
+h.dump(is_json)
 print()
-h.dump_extensions()
+h.dump_extensions(is_json)
 
 
 def cmd_dump_header_exts(fd):
 h = QcowHeader(fd)
-h.dump_extensions()
+h.dump_extensions(is_json)
 
 
 def cmd_set_header(fd, name, value):
@@ -151,11 +154,14 @@ def main(filename, cmd, args):
 
 
 def usage():
-print("Usage: %s   [, ...]" % sys.argv[0])
+print("Usage: %s   [, ...] [, ...]" % sys.argv[0])
 print("")
 print("Supported commands:")
 for name, handler, num_args, desc in cmds:
 print("%-20s - %s" % (name, desc))
+print("")
+print("Supported keys:")
+print("%-20s - %s" % ('-j', 'Dump in JSON format'))
 
 
 if __name__ == '__main__':
@@ -163,4 +169,8 @@ if __name__ == '__main__':
 usage()
 sys.exit(1)
 
+is_json = '-j' in sys.argv
+if is_json:
+sys.argv.remove('-j')
+
 main(sys.argv[1], sys.argv[2], sys.argv[3:])
diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index ad1918c..2921a27 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -109,7 +109,7 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
 self.__dict__ = dict((field[2], values[i])
  for i, field in enumerate(self.fields))
 
-def dump(self):
+def dump(self, is_json=False):
 for f in self.fields:
 value = self.__dict__[f[2]]
 if isinstance(f[1], str):
@@ -147,6 +147,7 @@ class Qcow2BitmapExt(Qcow2Struct):
 
 def dump(self):
 super().dump()
+
 for entry in self.bitmap_directory:
 print()
 entry.dump()
@@ -399,7 +400,7 @@ class QcowHeader(Qcow2Struct):
 buf = buf[0:header_bytes-1]
 fd.write(buf)
 
-def dump_extensions(self):
+def dump_extensions(self, is_json=False):
 for ex in self.extensions:
 print('Header extension:')
 ex.dump()
-- 
1.8.3.1

[PATCH v11 09/11] qcow2_format.py: collect fields to dump in JSON format

[Andrey Shinkevich]

As __dict__ is being extended with class members we do not want to
print, add the to_dict() method to classes that returns a dictionary
with desired fields and their values. Extend it in subclass when
necessary to print the final dictionary in the JSON output which
follows.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index 2921a27..19d29b8 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -119,6 +119,9 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
 
 print('{:<25} {}'.format(f[2], value_str))
 
+def to_dict(self):
+return dict((f[2], self.__dict__[f[2]]) for f in self.fields)
+
 
 class Qcow2BitmapExt(Qcow2Struct):
 
@@ -152,6 +155,11 @@ class Qcow2BitmapExt(Qcow2Struct):
 print()
 entry.dump()
 
+def to_dict(self):
+fields_dict = super().to_dict()
+fields_dict.update(bitmap_directory=self.bitmap_directory)
+return fields_dict
+
 
 class Qcow2BitmapDirEntry(Qcow2Struct):
 
@@ -190,6 +198,13 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
 super(Qcow2BitmapDirEntry, self).dump()
 self.bitmap_table.dump()
 
+def to_dict(self):
+fields_dict = super().to_dict()
+fields_dict.update(bitmap_table=self.bitmap_table)
+bmp_name = dict(name=self.name)
+bme_dict = {**bmp_name, **fields_dict}
+return bme_dict
+
 
 class Qcow2BitmapTableEntry:
 
@@ -205,6 +220,9 @@ class Qcow2BitmapTableEntry:
 else:
 self.type = 'all-zeroes'
 
+def to_dict(self):
+return dict(type=self.type, offset=self.offset)
+
 
 class Qcow2BitmapTable:
 
@@ -226,6 +244,9 @@ class Qcow2BitmapTable:
 for i, entry in bitmap_table:
 print(f'{i:<14} {entry.type:<15} {entry.offset:<24} {size}')
 
+def to_dict(self):
+return dict(entries=self.entries)
+
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
 
@@ -241,6 +262,9 @@ class QcowHeaderExtension(Qcow2Struct):
 0x44415441: 'Data file'
 }
 
+def to_dict(self):
+return self.mapping.get(self.value, "")
+
 fields = (
 ('u32', Magic, 'magic'),
 ('u32', '{}', 'length')
@@ -303,6 +327,17 @@ class QcowHeaderExtension(Qcow2Struct):
 else:
 self.obj.dump()
 
+def to_dict(self):
+fields_dict = super().to_dict()
+ext_name = dict(name=self.Magic(self.magic))
+he_dict = {**ext_name, **fields_dict}
+if self.obj is not None:
+he_dict.update(data=self.obj)
+else:
+he_dict.update(data_str=self.data_str)
+
+return he_dict
+
 @classmethod
 def create(cls, magic, data):
 return QcowHeaderExtension(magic, len(data), data)
@@ -405,3 +440,6 @@ class QcowHeader(Qcow2Struct):
 print('Header extension:')
 ex.dump()
 print()
+
+def to_dict(self):
+return super().to_dict()
-- 
1.8.3.1

[PATCH v11 01/11] qcow2: Fix capitalization of header extension constant.

[Andrey Shinkevich]

Make the capitalization of the hexadecimal numbers consistent for the
QCOW2 header extension constants in docs/interop/qcow2.txt.

Suggested-by: Eric Blake 
Signed-off-by: Andrey Shinkevich 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2.c  | 2 +-
 docs/interop/qcow2.txt | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index fadf342..6ad6bdc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -66,7 +66,7 @@ typedef struct {
 } QEMU_PACKED QCowExtension;
 
 #define  QCOW2_EXT_MAGIC_END 0
-#define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
+#define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xe2792aca
 #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
 #define  QCOW2_EXT_MAGIC_CRYPTO_HEADER 0x0537be77
 #define  QCOW2_EXT_MAGIC_BITMAPS 0x23852875
diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index cb72346..f072e27 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -231,7 +231,7 @@ be stored. Each extension has a structure like the 
following:
 
 Byte  0 -  3:   Header extension type:
 0x - End of the header extension area
-0xE2792ACA - Backing file format name string
+0xe2792aca - Backing file format name string
 0x6803f857 - Feature name table
 0x23852875 - Bitmaps extension
 0x0537be77 - Full disk encryption header pointer
-- 
1.8.3.1

[PATCH v11 02/11] qcow2_format.py: make printable data an extension class member

[Andrey Shinkevich]

Let us differ binary data type from string one for the extension data
variable and keep the string as the QcowHeaderExtension class member.

Signed-off-by: Andrey Shinkevich 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/qcow2_format.py | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index cc432e7..2f3681b 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -165,6 +165,13 @@ class QcowHeaderExtension(Qcow2Struct):
 self.data = fd.read(padded)
 assert self.data is not None
 
+data_str = self.data[:self.length]
+if all(c in string.printable.encode('ascii') for c in data_str):
+data_str = f"'{ data_str.decode('ascii') }'"
+else:
+data_str = ''
+self.data_str = data_str
+
 if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
 self.obj = Qcow2BitmapExt(data=self.data)
 else:
@@ -174,12 +181,7 @@ class QcowHeaderExtension(Qcow2Struct):
 super().dump()
 
 if self.obj is None:
-data = self.data[:self.length]
-if all(c in string.printable.encode('ascii') for c in data):
-data = f"'{ data.decode('ascii') }'"
-else:
-data = ''
-print(f'{"data":<25} {data}')
+print(f'{"data":<25} {self.data_str}')
 else:
 self.obj.dump()
 
-- 
1.8.3.1

[PATCH v11 06/11] qcow2_format.py: pass cluster size to substructures

[Andrey Shinkevich]

The cluster size of an image is the QcowHeader class member and may be
obtained by dependent extension structures such as Qcow2BitmapExt for
further bitmap table details print.

Signed-off-by: Andrey Shinkevich 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/qcow2_format.py | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index 05a8aa9..ca0d350 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -129,19 +129,21 @@ class Qcow2BitmapExt(Qcow2Struct):
 ('u64', '{:#x}', 'bitmap_directory_offset')
 )
 
-def __init__(self, fd):
+def __init__(self, fd, cluster_size):
 super().__init__(fd=fd)
 tail = struct.calcsize(self.fmt) % 8
 if tail:
 fd.seek(8 - tail, 1)
 position = fd.tell()
+self.cluster_size = cluster_size
 self.read_bitmap_directory(fd)
 fd.seek(position)
 
 def read_bitmap_directory(self, fd):
 fd.seek(self.bitmap_directory_offset)
 self.bitmap_directory = \
-[Qcow2BitmapDirEntry(fd) for _ in range(self.nb_bitmaps)]
+[Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size)
+ for _ in range(self.nb_bitmaps)]
 
 def dump(self):
 super().dump()
@@ -162,8 +164,9 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
 ('u32', '{}', 'extra_data_size')
 )
 
-def __init__(self, fd):
+def __init__(self, fd, cluster_size):
 super().__init__(fd=fd)
+self.cluster_size = cluster_size
 # Seek relative to the current position in the file
 fd.seek(self.extra_data_size, 1)
 bitmap_name = fd.read(self.name_size)
@@ -203,11 +206,13 @@ class QcowHeaderExtension(Qcow2Struct):
 # then padding to next multiply of 8
 )
 
-def __init__(self, magic=None, length=None, data=None, fd=None):
+def __init__(self, magic=None, length=None, data=None, fd=None,
+ cluster_size=None):
 """
 Support both loading from fd and creation from user data.
 For fd-based creation current position in a file will be used to read
 the data.
+The cluster_size value may be obtained by dependent structures.
 
 This should be somehow refactored and functionality should be moved to
 superclass (to allow creation of any qcow2 struct), but then, fields
@@ -230,7 +235,7 @@ class QcowHeaderExtension(Qcow2Struct):
 assert all(v is None for v in (magic, length, data))
 super().__init__(fd=fd)
 if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
-self.obj = Qcow2BitmapExt(fd=fd)
+self.obj = Qcow2BitmapExt(fd=fd, cluster_size=cluster_size)
 self.data = None
 else:
 padded = (self.length + 7) & ~7
@@ -319,7 +324,7 @@ class QcowHeader(Qcow2Struct):
 end = self.cluster_size
 
 while fd.tell() < end:
-ext = QcowHeaderExtension(fd=fd)
+ext = QcowHeaderExtension(fd=fd, cluster_size=self.cluster_size)
 if ext.magic == 0:
 break
 else:
-- 
1.8.3.1

[PATCH v11 07/11] qcow2_format.py: Dump bitmap table serialized entries

[Andrey Shinkevich]

Add bitmap table information to the QCOW2 metadata dump.

Bitmap name   bitmap-1
...
Bitmap table   typeoffset   size
0  serialized  4718592  65536
1  serialized  4294967296   65536
2  serialized  5348033147437056 65536
3  serialized  1379227385882214465536
4  serialized  4718592  65536
5  serialized  4294967296   65536
6  serialized  4503608217305088 65536
7  serialized  1407374883553280065536

Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 41 ++
 1 file changed, 41 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index ca0d350..ad1918c 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -175,6 +175,10 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
 entry_raw_size = self.bitmap_dir_entry_raw_size()
 padding = ((entry_raw_size + 7) & ~7) - entry_raw_size
 fd.seek(padding, 1)
+self.bitmap_table = Qcow2BitmapTable(fd=fd,
+ offset=self.bitmap_table_offset,
+ size=self.bitmap_table_size,
+ cluster_size=self.cluster_size)
 
 def bitmap_dir_entry_raw_size(self):
 return struct.calcsize(self.fmt) + self.name_size + \
@@ -183,6 +187,43 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
 def dump(self):
 print(f'{"Bitmap name":<25} {self.name}')
 super(Qcow2BitmapDirEntry, self).dump()
+self.bitmap_table.dump()
+
+
+class Qcow2BitmapTableEntry:
+
+BME_TABLE_ENTRY_OFFSET_MASK = 0x00fffe00
+BME_TABLE_ENTRY_FLAG_ALL_ONES = 1
+
+def __init__(self, entry):
+self.offset = entry & self.BME_TABLE_ENTRY_OFFSET_MASK
+if self.offset:
+self.type = 'serialized'
+elif entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES:
+self.type = 'all-ones'
+else:
+self.type = 'all-zeroes'
+
+
+class Qcow2BitmapTable:
+
+def __init__(self, fd, offset, size, cluster_size):
+self.entries = []
+self.cluster_size = cluster_size
+table_size = size * 8
+position = fd.tell()
+fd.seek(offset)
+table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))]
+fd.seek(position)
+for entry in table:
+self.entries.append(Qcow2BitmapTableEntry(entry))
+
+def dump(self):
+size = self.cluster_size
+bitmap_table = enumerate(self.entries)
+print(f'{"Bitmap table":<14} {"type":<15} {"offset":<24} {"size"}')
+for i, entry in bitmap_table:
+print(f'{i:<14} {entry.type:<15} {entry.offset:<24} {size}')
 
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
-- 
1.8.3.1

[PATCH v11 11/11] qcow2_format.py: support dumping metadata in JSON format

[Andrey Shinkevich]

Implementation of dumping QCOW2 image metadata.
The sample output:
{
"Header_extensions": [
{
"name": "Feature table",
"magic": 1745090647,
"length": 192,
"data_str": ""
},
{
"name": "Bitmaps",
"magic": 595929205,
"length": 24,
"data": {
"nb_bitmaps": 2,
"reserved32": 0,
"bitmap_directory_size": 64,
"bitmap_directory_offset": 1048576,
"bitmap_directory": [
{
"name": "bitmap-1",
"bitmap_table_offset": 589824,
"bitmap_table_size": 1,
"flags": 2,
"type": 1,
"granularity_bits": 15,
"name_size": 8,
"extra_data_size": 0,
"bitmap_table": {
"entries": [
{
"type": "serialized",
"offset": 655360
},
...

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 20 
 1 file changed, 20 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index d2a8659..d40eb49 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -19,6 +19,15 @@
 
 import struct
 import string
+import json
+
+
+class ComplexEncoder(json.JSONEncoder):
+def default(self, obj):
+if hasattr(obj, 'to_dict'):
+return obj.to_dict()
+else:
+return json.JSONEncoder.default(self, obj)
 
 
 class Qcow2Field:
@@ -110,6 +119,11 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
  for i, field in enumerate(self.fields))
 
 def dump(self, is_json=False):
+if is_json:
+print(json.dumps(self.to_dict(), indent=4,
+ cls=ComplexEncoder))
+return
+
 for f in self.fields:
 value = self.__dict__[f[2]]
 if isinstance(f[1], str):
@@ -445,6 +459,12 @@ class QcowHeader(Qcow2Struct):
 fd.write(buf)
 
 def dump_extensions(self, is_json=False):
+if is_json:
+ext_doc = Qcow2HeaderExtensionsDoc(self.extensions)
+print(json.dumps(ext_doc.to_dict(), indent=4,
+ cls=ComplexEncoder))
+return
+
 for ex in self.extensions:
 print('Header extension:')
 ex.dump()
-- 
1.8.3.1

[PATCH v11 04/11] qcow2_format.py: dump bitmap flags in human readable way.

[Andrey Shinkevich]

Introduce the class BitmapFlags that parses a bitmap flags mask.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Andrey Shinkevich 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/qcow2_format.py | 16 
 1 file changed, 16 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index d4a9974..b447344 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -40,6 +40,22 @@ class Flags64(Qcow2Field):
 return str(bits)
 
 
+class BitmapFlags(Qcow2Field):
+
+flags = {
+0x1: 'in-use',
+0x2: 'auto'
+}
+
+def __str__(self):
+bits = []
+for bit in range(64):
+flag = self.value & (1 << bit)
+if flag:
+bits.append(self.flags.get(flag, f'bit-{bit}'))
+return f'{self.value:#x} ({bits})'
+
+
 class Enum(Qcow2Field):
 
 def __str__(self):
-- 
1.8.3.1

[PATCH v11 03/11] qcow2_format.py: change Qcow2BitmapExt initialization method

[Andrey Shinkevich]

There are two ways to initialize a class derived from Qcow2Struct:
1. Pass a block of binary data to the constructor.
2. Pass the file descriptor to allow reading the file from constructor.
Let's change the Qcow2BitmapExt initialization method from 1 to 2 to
support a scattered reading in the initialization chain.
The implementation comes with the patch that follows.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 36 ++--
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index 2f3681b..d4a9974 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -113,6 +113,11 @@ class Qcow2BitmapExt(Qcow2Struct):
 ('u64', '{:#x}', 'bitmap_directory_offset')
 )
 
+def __init__(self, fd):
+super().__init__(fd=fd)
+tail = struct.calcsize(self.fmt) % 8
+if tail:
+fd.seek(8 - tail, 1)
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
 
@@ -161,21 +166,24 @@ class QcowHeaderExtension(Qcow2Struct):
 else:
 assert all(v is None for v in (magic, length, data))
 super().__init__(fd=fd)
-padded = (self.length + 7) & ~7
-self.data = fd.read(padded)
-assert self.data is not None
-
-data_str = self.data[:self.length]
-if all(c in string.printable.encode('ascii') for c in data_str):
-data_str = f"'{ data_str.decode('ascii') }'"
-else:
-data_str = ''
-self.data_str = data_str
+if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
+self.obj = Qcow2BitmapExt(fd=fd)
+self.data = None
+else:
+padded = (self.length + 7) & ~7
+self.data = fd.read(padded)
+assert self.data is not None
+self.obj = None
+
+if self.data is not None:
+data_str = self.data[:self.length]
+if all(c in string.printable.encode(
+'ascii') for c in data_str):
+data_str = f"'{ data_str.decode('ascii') }'"
+else:
+data_str = ''
+self.data_str = data_str
 
-if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
-self.obj = Qcow2BitmapExt(data=self.data)
-else:
-self.obj = None
 
 def dump(self):
 super().dump()
-- 
1.8.3.1

Re: [PATCH v5 10/11] hw/arm: Wire up BMC boot flash for npcm750-evb and quanta-gsj

[Philippe Mathieu-Daudé]

On 7/17/20 10:03 AM, Thomas Huth wrote:
> On 17/07/2020 09.48, Philippe Mathieu-Daudé wrote:
>> +Thomas
> 
>> On 7/16/20 10:56 PM, Havard Skinnemoen wrote:
>>> On Wed, Jul 15, 2020 at 1:54 PM Havard Skinnemoen
>>>  wrote:

 On Wed, Jul 15, 2020 at 3:57 AM Philippe Mathieu-Daudé  
 wrote:
>
> On 7/15/20 11:00 AM, Markus Armbruster wrote:
>> Now my point.  Why first make up user configuration, then use that to
>> create a BlockBackend, when you could just go ahead and create the
>> BlockBackend?
>
> CLI issue mostly.
>
> We can solve it similarly to the recent "sdcard: Do not allow invalid SD
> card sizes" patch:
>
>  if (!dinfo) {
>  error_setg(errp, "Missing SPI flash drive");
>  error_append_hint(errp, "You can use a dummy drive using:\n");
>  error_append_hint(errp, "-drive if=mtd,driver=null-co,"
>  "read-ones=on,size=64M\n);
>  return;
>  }
>
> having npcm7xx_connect_flash() taking an Error* argument,
> and MachineClass::init() call it with &error_fatal.

 Erroring out if the user specifies a configuration that can't possibly
 boot sounds good to me. Better than trying to come up with defaults
 that are still not going to result in a bootable system.

 For testing recovery paths, I think it makes sense to explicitly
 specify a null device as you suggest.
>>>
>>> Hmm, one problem. qom-test fails with
>>>
>>> qemu-system-aarch64: Missing SPI flash drive
>>> You can add a dummy drive using:
>>> -drive if=mtd,driver=null-co,read-zeroes=on,size=32M
>>> Broken pipe
>>> /usr/local/google/home/hskinnemoen/qemu/for-upstream/tests/qtest/libqtest.c:166:
>>> kill_qemu() tried to terminate QEMU process but encountered exit
>>> status 1 (expected 0)
>>> ERROR qom-test - too few tests run (expected 68, got 7)
>>>
>>> So it looks like we might need a different solution to this, unless we
>>> want to make generic tests more machine-aware...
> 
> I didn't follow the other mails in this thread, but what we usually do
> in such a case: Add a "if (qtest_enabled())" check to the device or the
> machine to ignore the error if it is running in qtest mode.

Hmm I'm not sure it works in this case. We could do:

  if (!dinfo) {
 if (qtest) {
/* create null drive for qtest */
opts = ...;
dinfo = drive_new(opts, IF_MTD, &error_abort);
 } else {
/* teach user to use proper CLI */
error_setg(errp, "Missing SPI flash drive");
error_append_hint(errp, "You can use a dummy drive using:\n");
error_append_hint(errp, "-drive if=mtd,driver=null-co,"
"read-ones=on,size=64M\n);
 }
  }

But I'm not sure Markus will enjoy it :)

Markus, any better idea about how to handle that with automatic qtests?

Thanks :)

Phil.

> 
>  Thomas
> 
> 

Re: [PATCH] gitlab-ci.yml: Add oss-fuzz build tests

[Alex Bennée]

Thomas Huth  writes:

> On 16/07/2020 18.33, Alexander Bulekov wrote:
>> This tries to build and run the fuzzers with the same build-script used
>> by oss-fuzz. This doesn't guarantee that the builds on oss-fuzz will
>> also succeed, since oss-fuzz provides its own compiler and fuzzer vars,
>> but it can catch changes that are not compatible with the the
>> ./scripts/oss-fuzz/build.sh script.
>> The strange way of finding fuzzer binaries stems from the method used by
>> oss-fuzz:
>> https://github.com/google/oss-fuzz/blob/master/infra/base-images/base-runner/targets_list
>> 
>> Signed-off-by: Alexander Bulekov 
>> ---
>> 
>> Similar to Thomas' patch:
>> 
>>> Note: This patch needs two other patches merged first to work correctly:
>> 
>>> - 'fuzz: Expect the cmdline in a freeable GString' from Alexander
>> 
>>> - 'qom: Plug memory leak in "info qom-tree"' from Markus
>> 
>> Otherwise the test will fail due to detected memory leaks.
>> 
>> Fair warning: I haven't been able to trigger this new job yet. I tried
>> to run the pipeline with these changes on my forked repo on gitlab, but
>> did not reach the build-oss-fuzz. I think this is due to some failures
>> in the Containers-layer-2 stage:
>> 
>> ...
>> Error response from daemon: manifest for
>> registry.gitlab.com/a1xndr/qemu/qemu/debian-all-test-cross:latest not
>> found: manifest unknown: manifest unknown
>> #2 [internal] load .dockerignore
>> #2 transferring context:
>> #2 transferring context: 2B 0.1s done
>> #2 DONE 0.1s
>> #1 [internal] load build definition from tmpg8j4xoop.docker
>> #1 transferring dockerfile: 2.21kB 0.1s done
>> #1 DONE 0.2s
>> #3 [internal] load metadata for docker.io/qemu/debian10:latest
>> #3 ERROR: pull access denied, repository does not exist or may require
>> authorization: server message: insufficient_scope: authorization failed
>
> These look like the problems that we've seen with the main repo until
> two days ago, too, e.g.:
>
>  https://gitlab.com/qemu-project/qemu/-/jobs/640410842
>
> Maybe Alex (Bennée) can comment on how to resolve them?

It all should be working now the qemu-project container repository has
been properly seeded:

  https://gitlab.com/qemu-project/qemu/container_registry

>
>> 
>>  .gitlab-ci.yml | 14 ++
>>  1 file changed, 14 insertions(+)
>> 
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index e96f8794b9..a50df420c9 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -182,6 +182,20 @@ build-fuzzer:
>>  || exit 1 ;
>>done
>
> As mentioned in my other mail, I think you can replace my build-fuzzer
> job once this is working.
>
>> +build-oss-fuzz:
>> +  <<: *native_build_job_definition
>> +  variables:
>> +IMAGE: fedora
>> +  script:
>> +- OUT_DIR="./build" CC=clang-9 CXX=clang++-9 CFLAGS="-fsanitize=address"
>> +  LIB_FUZZING_ENGINE="-fsanitize=fuzzer" CFL
>
> That "CFL" at the end seems to be a typo (leftover from "CFLAGS")?
>
> Also the fedora container does not have clang-9 :
>
>  https://gitlab.com/huth/qemu/-/jobs/643383032#L28
>
> I think it is at clang 10 already, so maybe just use CC=clang (without
> version number)?

I think all the clang-10 fixes are in now so yes.

>
>> +  ./scripts/oss-fuzz/build.sh
>> +- for fuzzer in $(find ./build-oss-fuzz/DEST_DIR/ -executable -type f); 
>> do
>> +grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 || 
>> continue ;
>> +echo Testing ${fuzzer} ... ;
>> +"${fuzzer}" -runs=1000 || exit 1 ;
>> +  done
>
> Should we exclude the virtio-net tests, since they could leak network
> traffic to the host?
>
>  Thomas


-- 
Alex Bennée

[Bug 1887303] Re: Assertion failure in *bmdma_active_if `bmdma->bus->retry_unit != (uint8_t)-1' failed.

[Philippe Mathieu-Daudé]

Proposed fix:
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg05408.html

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

Title:
  Assertion failure in *bmdma_active_if `bmdma->bus->retry_unit !=
  (uint8_t)-1' failed.

Status in QEMU:
  New

Bug description:
  Hello,
  Here is a QTest Reproducer:

  cat << EOF | ./i386-softmmu/qemu-system-i386 -M pc,accel=qtest\
   -qtest null -nographic -vga qxl -qtest stdio -nodefaults\
   -drive if=none,id=drive0,file=null-co://,file.read-zeroes=on,format=raw\
   -drive if=none,id=drive1,file=null-co://,file.read-zeroes=on,format=raw\
   -device ide-cd,drive=drive0 -device ide-hd,drive=drive1
  outw 0x176 0x3538
  outw 0x376 0x6007
  outw 0x376 0x6b6b
  outw 0x176 0x985c
  outl 0xcf8 0x8903
  outl 0xcfc 0x2f2931
  outl 0xcf8 0x8920
  outb 0xcfc 0x6b
  outb 0x68 0x7
  outw 0x176 0x2530
  EOF

  Here is the call-stack:

  #8 0x7f00e0443091 in __assert_fail 
/build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
  #9 0x55e163f5a1af in bmdma_active_if 
/home/alxndr/Development/qemu/include/hw/ide/pci.h:59:5
  #10 0x55e163f5a1af in bmdma_prepare_buf 
/home/alxndr/Development/qemu/hw/ide/pci.c:132:19
  #11 0x55e163f4f00d in ide_dma_cb 
/home/alxndr/Development/qemu/hw/ide/core.c:898:17
  #12 0x55e163de86ad in dma_complete 
/home/alxndr/Development/qemu/dma-helpers.c:120:9
  #13 0x55e163de86ad in dma_blk_cb 
/home/alxndr/Development/qemu/dma-helpers.c:138:9
  #14 0x55e1642ade85 in blk_aio_complete 
/home/alxndr/Development/qemu/block/block-backend.c:1402:9
  #15 0x55e1642ade85 in blk_aio_complete_bh 
/home/alxndr/Development/qemu/block/block-backend.c:1412:5
  #16 0x55e16443556f in aio_bh_call 
/home/alxndr/Development/qemu/util/async.c:136:5
  #17 0x55e16443556f in aio_bh_poll 
/home/alxndr/Development/qemu/util/async.c:164:13
  #18 0x55e16440fac3 in aio_dispatch 
/home/alxndr/Development/qemu/util/aio-posix.c:380:5
  #19 0x55e164436dac in aio_ctx_dispatch 
/home/alxndr/Development/qemu/util/async.c:306:5
  #20 0x7f00e16e29ed in g_main_context_dispatch 
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4e9ed)
  #21 0x55e164442f2b in glib_pollfds_poll 
/home/alxndr/Development/qemu/util/main-loop.c:219:9
  #22 0x55e164442f2b in os_host_main_loop_wait 
/home/alxndr/Development/qemu/util/main-loop.c:242:5
  #23 0x55e164442f2b in main_loop_wait 
/home/alxndr/Development/qemu/util/main-loop.c:518:11
  #24 0x55e164376953 in flush_events 
/home/alxndr/Development/qemu/tests/qtest/fuzz/fuzz.c:47:9
  #25 0x55e16437b8fa in general_fuzz 
/home/alxndr/Development/qemu/tests/qtest/fuzz/general_fuzz.c:544:17

  =

  Here is the same assertion failure but triggered through a different
  call-stack:

  cat << EOF | ./i386-softmmu/qemu-system-i386 -M pc,accel=qtest\
   -qtest null -nographic -vga qxl -qtest stdio -nodefaults\
   -drive if=none,id=drive0,file=null-co://,file.read-zeroes=on,format=raw\
   -drive if=none,id=drive1,file=null-co://,file.read-zeroes=on,format=raw\
   -device ide-cd,drive=drive0 -device ide-hd,drive=drive1
  outw 0x171 0x2fe9
  outb 0x177 0xa0
  outl 0x170 0x928
  outl 0x170 0x2b923b31
  outl 0x170 0x800a24d7
  outl 0xcf8 0x8903
  outl 0xcfc 0x842700
  outl 0xcf8 0x8920
  outb 0xcfc 0x5e
  outb 0x58 0x7
  outb 0x376 0x5
  outw 0x376 0x11
  outw 0x176 0x3538
  EOF

  Call-stack:
  #8 0x7f00e0443091 in __assert_fail 
/build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
  #9 0x55e163f5a622 in bmdma_active_if 
/home/alxndr/Development/qemu/include/hw/ide/pci.h:59:5
  #10 0x55e163f5a622 in bmdma_rw_buf 
/home/alxndr/Development/qemu/hw/ide/pci.c:187:19
  #11 0x55e163f57577 in ide_atapi_cmd_read_dma_cb 
/home/alxndr/Development/qemu/hw/ide/atapi.c:375:13
  #12 0x55e163f44c55 in ide_buffered_readv_cb 
/home/alxndr/Development/qemu/hw/ide/core.c:650:9
  #13 0x55e1642ade85 in blk_aio_complete 
/home/alxndr/Development/qemu/block/block-backend.c:1402:9
  #14 0x55e1642ade85 in blk_aio_complete_bh 
/home/alxndr/Development/qemu/block/block-backend.c:1412:5
  #15 0x55e16443556f in aio_bh_call 
/home/alxndr/Development/qemu/util/async.c:136:5
  #16 0x55e16443556f in aio_bh_poll 
/home/alxndr/Development/qemu/util/async.c:164:13
  #17 0x55e16440fac3 in aio_dispatch 
/home/alxndr/Development/qemu/util/aio-posix.c:380:5
  #18 0x55e164436dac in aio_ctx_dispatch 
/home/alxndr/Development/qemu/util/async.c:306:5
  #19 0x7f00e16e29ed in g_main_context_dispatch 
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4e9ed)
  #20 0x55e164442f2b in glib_pollfds_poll 
/home/alxndr/Development/qemu/util/main-loop.c:219:9
  #21 0x55e164442f2b in os_host_main_loop_wait 
/home/alxndr/Development/qemu/util/main-loop.c:242:5
  #22 0x55e164442f2b in main_loop_wait 
/home/alxndr/Development/qemu/util/main-loop.c:518:11

Re: Inter-VM device emulation (call on Mon 20th July 2020)

[Nikos Dragazis]

On 15/7/20 7:44 μ.μ., Alex Bennée wrote:


Stefan Hajnoczi  writes:


On Wed, Jul 15, 2020 at 01:28:07PM +0200, Jan Kiszka wrote:

On 15.07.20 13:23, Stefan Hajnoczi wrote:

Let's have a call to figure out:

1. What is unique about these approaches and how do they overlap?
2. Can we focus development and code review efforts to get something
merged sooner?

Jan and Nikos: do you have time to join on Monday, 20th of July at 15:00
UTC?
https://www.timeanddate.com/worldclock/fixedtime.html?iso=20200720T1500


Not at that slot, but one hour earlier or later would work for me (so far).

Nikos: Please let us know which of Jan's timeslots works best for you.

I'm in - the earlier slot would be preferential for me to avoid clashing with
family time.



I'm OK with all timeslots.

Re: [PATCH v5 10/11] hw/arm: Wire up BMC boot flash for npcm750-evb and quanta-gsj

[Philippe Mathieu-Daudé]

On 7/17/20 10:27 AM, Philippe Mathieu-Daudé wrote:
> On 7/17/20 10:03 AM, Thomas Huth wrote:
>> On 17/07/2020 09.48, Philippe Mathieu-Daudé wrote:
>>> +Thomas
>>
>>> On 7/16/20 10:56 PM, Havard Skinnemoen wrote:
 On Wed, Jul 15, 2020 at 1:54 PM Havard Skinnemoen
  wrote:
>
> On Wed, Jul 15, 2020 at 3:57 AM Philippe Mathieu-Daudé  
> wrote:
>>
>> On 7/15/20 11:00 AM, Markus Armbruster wrote:
>>> Now my point.  Why first make up user configuration, then use that to
>>> create a BlockBackend, when you could just go ahead and create the
>>> BlockBackend?
>>
>> CLI issue mostly.
>>
>> We can solve it similarly to the recent "sdcard: Do not allow invalid SD
>> card sizes" patch:
>>
>>  if (!dinfo) {
>>  error_setg(errp, "Missing SPI flash drive");
>>  error_append_hint(errp, "You can use a dummy drive using:\n");
>>  error_append_hint(errp, "-drive if=mtd,driver=null-co,"
>>  "read-ones=on,size=64M\n);
>>  return;
>>  }
>>
>> having npcm7xx_connect_flash() taking an Error* argument,
>> and MachineClass::init() call it with &error_fatal.
>
> Erroring out if the user specifies a configuration that can't possibly
> boot sounds good to me. Better than trying to come up with defaults
> that are still not going to result in a bootable system.
>
> For testing recovery paths, I think it makes sense to explicitly
> specify a null device as you suggest.

 Hmm, one problem. qom-test fails with

 qemu-system-aarch64: Missing SPI flash drive
 You can add a dummy drive using:
 -drive if=mtd,driver=null-co,read-zeroes=on,size=32M
 Broken pipe
 /usr/local/google/home/hskinnemoen/qemu/for-upstream/tests/qtest/libqtest.c:166:
 kill_qemu() tried to terminate QEMU process but encountered exit
 status 1 (expected 0)
 ERROR qom-test - too few tests run (expected 68, got 7)

 So it looks like we might need a different solution to this, unless we
 want to make generic tests more machine-aware...
>>
>> I didn't follow the other mails in this thread, but what we usually do
>> in such a case: Add a "if (qtest_enabled())" check to the device or the
>> machine to ignore the error if it is running in qtest mode.
> 
> Hmm I'm not sure it works in this case. We could do:
> 
>   if (!dinfo) {
>  if (qtest) {
> /* create null drive for qtest */
> opts = ...;
> dinfo = drive_new(opts, IF_MTD, &error_abort);
>  } else {
> /* teach user to use proper CLI */
> error_setg(errp, "Missing SPI flash drive");
> error_append_hint(errp, "You can use a dummy drive using:\n");
> error_append_hint(errp, "-drive if=mtd,driver=null-co,"
> "read-ones=on,size=64M\n);
>  }
>   }
> 
> But I'm not sure Markus will enjoy it :)
> 
> Markus, any better idea about how to handle that with automatic qtests?

FWIW IDE device has a concept of "Anonymous BlockBackend for an empty
drive":

static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
{
IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
IDEState *s = bus->ifs + dev->unit;
int ret;

if (!dev->conf.blk) {
if (kind != IDE_CD) {
error_setg(errp, "No drive specified");
return;
} else {
/* Anonymous BlockBackend for an empty drive */
dev->conf.blk = blk_new(qemu_get_aio_context(), 0,
BLK_PERM_ALL);
ret = blk_attach_dev(dev->conf.blk, &dev->qdev);
assert(ret == 0);
}
}

Re: [PATCH] net: check payload length limit for all frames

[P J P]

+-- On Fri, 17 Jul 2020, Jason Wang wrote --+
| Thanks but I don't see a direct relation between 64K limit and this 
| calltrace. Maybe you can elaborate more on this?

The use-after-free is not function of the size per say; The reproducer given 
sends large(>64k) packets via loopback interface with gso_type=none(0). The 
proposed patch helps to fix it. The large size & payload_len may result in 
other oob kind of access issues too I think.

@Alex, would it be possible to share the reproduces on the upstream bug 
LP#1886362?

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D

Re: [PATCH] virtiofsd: Remove "norace" from cmdline help

[Stefano Garzarella]

On Thu, Jul 16, 2020 at 12:14:42PM +0200, Sergio Lopez wrote:
> Commit 93bb3d8d4cda ("virtiofsd: remove symlink fallbacks") removed
> the implementation of the "norace" option, so remove it from the
> cmdline help too.
> 
> Signed-off-by: Sergio Lopez 
> ---
>  tools/virtiofsd/helper.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
> index 3105b6c23a..7bc5d7dc5a 100644
> --- a/tools/virtiofsd/helper.c
> +++ b/tools/virtiofsd/helper.c
> @@ -159,8 +159,6 @@ void fuse_cmdline_help(void)
> "-o max_idle_threadsthe maximum number of idle worker 
> "
> "threads\n"
> "   allowed (default: 10)\n"
> -   "-o norace  disable racy fallback\n"
> -   "   default: false\n"
> "-o posix_lock|no_posix_lock\n"
> "   enable/disable remote posix 
> lock\n"
> "   default: posix_lock\n"
> -- 
> 2.26.2
> 
> 

I noticed that 'norace' is also described in docs/tools/virtiofsd.rst,
so I think we need to remove it there too:

diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
index 824e713491..58666a4495 100644
--- a/docs/tools/virtiofsd.rst
+++ b/docs/tools/virtiofsd.rst
@@ -63,9 +63,6 @@ Options
 Print only log messages matching LEVEL or more severe.  LEVEL is one of
 ``err``, ``warn``, ``info``, or ``debug``.  The default is ``info``.

-  * norace -
-Disable racy fallback.  The default is false.
-
   * posix_lock|no_posix_lock -
 Enable/disable remote POSIX locks.  The default is ``posix_lock``.


With that fixed:
Reviewed-by: Stefano Garzarella 

Thanks,
Stefano

[PATCH v1] migration: tls: unref creds after used

[Zhenyu Ye]

We add the reference of creds in migration_tls_get_creds(),
but there was no place to unref it.  So the OBJECT(creds) will
never be freed and result in memory leak.

Unref the creds after creating the tls-channel server/client.

Signed-off-by: Zhenyu Ye 
---
 migration/tls.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/migration/tls.c b/migration/tls.c
index 5171afc6c4..0740d02976 100644
--- a/migration/tls.c
+++ b/migration/tls.c
@@ -97,7 +97,7 @@ void migration_tls_channel_process_incoming(MigrationState *s,
 s->parameters.tls_authz,
 errp);
 if (!tioc) {
-return;
+goto cleanup;
 }
 
 trace_migration_tls_incoming_handshake_start();
@@ -107,6 +107,9 @@ void migration_tls_channel_process_incoming(MigrationState 
*s,
   NULL,
   NULL,
   NULL);
+
+cleanup:
+object_unref(OBJECT(creds));
 }
 
 
@@ -146,13 +149,13 @@ void migration_tls_channel_connect(MigrationState *s,
 }
 if (!hostname) {
 error_setg(errp, "No hostname available for TLS");
-return;
+goto cleanup;
 }
 
 tioc = qio_channel_tls_new_client(
 ioc, creds, hostname, errp);
 if (!tioc) {
-return;
+goto cleanup;
 }
 
 trace_migration_tls_outgoing_handshake_start(hostname);
@@ -162,4 +165,7 @@ void migration_tls_channel_connect(MigrationState *s,
   s,
   NULL,
   NULL);
+
+cleanup:
+object_unref(OBJECT(creds));
 }
-- 
2.19.1

Re: [PATCH] target/i386: floatx80: avoid compound literals in static initializers

[Laszlo Ersek]

On 07/16/20 17:09, Philippe Mathieu-Daudé wrote:
> On 7/16/20 4:42 PM, Laszlo Ersek wrote:
>> Quoting ISO C99 6.7.8p4, "All the expressions in an initializer for an
>> object that has static storage duration shall be constant expressions or
>> string literals".
>>
>> The compound literal produced by the make_floatx80() macro is not such a
>> constant expression, per 6.6p7-9. (An implementation may accept it,
>> according to 6.6p10, but is not required to.)
>>
>> Therefore using "floatx80_zero" and make_floatx80() for initializing
>> "f2xm1_table" and "fpatan_table" is not portable. And gcc-4.8 in RHEL-7.6
>> actually chokes on them:
>>
>>> target/i386/fpu_helper.c:871:5: error: initializer element is not constant
>>>  { make_floatx80(0xbfff, 0x8000ULL),
>>>  ^
> 
> This reminds me of:
> 
> commit 6fa9ba09dbf4eb8b52bcb47d6820957f1b77ee0b
> Author: Kamil Rytarowski 
> Date:   Mon Sep 4 23:23:06 2017 +0200
> 
> target/m68k: Switch fpu_rom from make_floatx80() to make_floatx80_init()
> 
> GCC 4.7.2 on SunOS reports that the values assigned to array members
> are not
> real constants:
> 
> target/m68k/fpu_helper.c:32:5: error: initializer element is not
> constant
> target/m68k/fpu_helper.c:32:5: error: (near initialization for
> 'fpu_rom[0]')
> rules.mak:66: recipe for target 'target/m68k/fpu_helper.o' failed
> 
> Convert the array to make_floatx80_init() to fix it.
> Replace floatx80_pi-like constants with make_floatx80_init() as they are
> defined as make_floatx80().
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> 
>>
>> We've had the make_floatx80_init() macro for this purpose since commit
>> 3bf7e40ab914 ("softfloat: fix for C99", 2012-03-17), so let's use that
>> macro again.
>>
>> Fixes: eca30647fc07
>> Fixes: ff57bb7b6326
>> Link: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06566.html
>> Link: https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04714.html
>> Cc: Alex Bennée 
>> Cc: Aurelien Jarno 
>> Cc: Eduardo Habkost 
>> Cc: Joseph Myers 
>> Cc: Paolo Bonzini 
>> Cc: Peter Maydell 
>> Cc: Richard Henderson 
>> Signed-off-by: Laszlo Ersek 
>> ---
>>
>> Notes:
>> I can see that there are test cases under "tests/tcg/i386", but I don't
>> know how to run them.
> 
> Yeah it is not easy to figure...
> 
> Try 'make run-tcg-tests-i386-softmmu'
> but you need docker :^)

That worked, thanks! Even without Docker: I just had to add

  --cross-cc-i386=gcc

to my ./configure flags.

Thanks,
Laszlo

> 
> (There is also 'make check-softfloat', listed in 'make check-help')
> 
>>
>>  include/fpu/softfloat.h  |   1 +
>>  target/i386/fpu_helper.c | 426 +++
>>  2 files changed, 214 insertions(+), 213 deletions(-)
>>
>> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
>> index f1a19df066b7..659218b5c787 100644
>> --- a/include/fpu/softfloat.h
>> +++ b/include/fpu/softfloat.h
>> @@ -822,6 +822,7 @@ static inline bool floatx80_invalid_encoding(floatx80 a)
>>  }
>>  
>>  #define floatx80_zero make_floatx80(0x, 0xLL)
>> +#define floatx80_zero_init make_floatx80_init(0x, 0xLL)
>>  #define floatx80_one make_floatx80(0x3fff, 0x8000LL)
>>  #define floatx80_ln2 make_floatx80(0x3ffe, 0xb17217f7d1cf79acLL)
>>  #define floatx80_pi make_floatx80(0x4000, 0xc90fdaa22168c235LL)
>> diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
>> index f5e6c4b88d4e..4ea73874d836 100644
>> --- a/target/i386/fpu_helper.c
>> +++ b/target/i386/fpu_helper.c
>> @@ -868,201 +868,201 @@ struct f2xm1_data {
>>  };
> ...
> 

Re: [PATCH] target/i386: floatx80: avoid compound literals in static initializers

[Laszlo Ersek]

On 07/16/20 18:31, Alex Bennée wrote:
> 
> Laszlo Ersek  writes:
> 
>> Quoting ISO C99 6.7.8p4, "All the expressions in an initializer for an
>> object that has static storage duration shall be constant expressions or
>> string literals".
>>
>> The compound literal produced by the make_floatx80() macro is not such a
>> constant expression, per 6.6p7-9. (An implementation may accept it,
>> according to 6.6p10, but is not required to.)
>>
>> Therefore using "floatx80_zero" and make_floatx80() for initializing
>> "f2xm1_table" and "fpatan_table" is not portable. And gcc-4.8 in RHEL-7.6
>> actually chokes on them:
>>
>>> target/i386/fpu_helper.c:871:5: error: initializer element is not constant
>>>  { make_floatx80(0xbfff, 0x8000ULL),
>>>  ^
>>
>> We've had the make_floatx80_init() macro for this purpose since commit
>> 3bf7e40ab914 ("softfloat: fix for C99", 2012-03-17), so let's use that
>> macro again.
>>
>> Fixes: eca30647fc07
>> Fixes: ff57bb7b6326
>> Link: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06566.html
>> Link: https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04714.html
>> Cc: Alex Bennée 
>> Cc: Aurelien Jarno 
>> Cc: Eduardo Habkost 
>> Cc: Joseph Myers 
>> Cc: Paolo Bonzini 
>> Cc: Peter Maydell 
>> Cc: Richard Henderson 
>> Signed-off-by: Laszlo Ersek 
>> ---
>>
>> Notes:
>> I can see that there are test cases under "tests/tcg/i386", but I don't
>> know how to run them.
> 
> You can run the TCG tests with:
> 
>   make check-tcg
> 
> or more specifically:
> 
>   make run-tcg-tests-i386-linux-user
> 
> there is also a:
> 
>   make check-softfloat
> 
> although in this case nothing is affected.
> 
> softfloat bits:
> 
> Acked-by: Alex Bennée 

Thank you!
Laszlo

[RFC v2 0/3] Enable virtio-fs on s390x

[Marc Hartmayer]

This RFC is about enabling virtio-fs on s390x. For that we need
 + some shim code (first patch), and we need
 + libvhost-user to deal with virtio endiannes for non-legacy virtio
   devices as mandated by the spec.

libvhost-access.h is based on hw/virtio/virtio-access.h.

How to use?

For general instructions how to use virtio-fs (on x86) please have a
look at https://virtio-fs.gitlab.io/howto-qemu.html. Most of the
instructions can also be applied on s390x.

In short:

1. Install self-compiled QEMU with this patch series applied
2. Prepare host and guest kernel so they support virtio-fs

Start virtiofsd on the host

 $ virtiofsd -f --socket-path=/tmp/vhostqemu -o source=/tmp/shared

Now you can start QEMU in a separate shell on the host:

 $ qemu-system-s390x -machine type=s390-ccw-virtio,accel=kvm,memory-backend=mem 
\
   -object 
memory-backend-file,id=mem,size=2G,mem-path=/dev/shm/virtiofs,share=on,prealloc=on,prealloc-threads=1
 \
   -chardev socket,id=char0,path=/tmp/vhostqemu -device 
vhost-user-fs-ccw,queue-size=1024,chardev=char0,tag=myfs \
   -drive if=virtio,file=disk.qcow2 \
   -m 2G -smp 2 -nographic

Log into the guest and mount it

 $ mount -t virtiofs myfs /mnt

Changelog:
 v1->v2:
  + rebased
  + drop patch "libvhost-user: print invalid address on vu_panic" as it's not 
related to this series
  + drop patch "[RFC 4/4] HACK: Hard-code the libvhost-user.o-cflags for s390x"
  + patch "virtio: add vhost-user-fs-ccw device": replace qdev_set_parent_bus 
and object_property_set_bool by qdev_realize
  + patch "libvhost-user: handle endianness as mandated by the spec":
Drop support for legacy virtio devices
  + add patch to fence legacy virtio devices

Halil Pasic (1):
  virtio: add vhost-user-fs-ccw device

Marc Hartmayer (2):
  libvhost-user: handle endianness as mandated by the spec
  libvhost-user: fence legacy virtio devices

 contrib/libvhost-user/libvhost-access.h |  71 ++
 contrib/libvhost-user/libvhost-user.c   | 125 +---
 hw/s390x/Makefile.objs  |   1 +
 hw/s390x/vhost-user-fs-ccw.c|  73 ++
 4 files changed, 211 insertions(+), 59 deletions(-)
 create mode 100644 contrib/libvhost-user/libvhost-access.h
 create mode 100644 hw/s390x/vhost-user-fs-ccw.c

-- 
2.25.4

[RFC v2 3/3] libvhost-user: fence legacy virtio devices

[Marc Hartmayer]

libvhost-user has no support for legacy virtio devices therefore
let's fence them.

Signed-off-by: Marc Hartmayer 
---
 contrib/libvhost-user/libvhost-access.h | 10 ++
 contrib/libvhost-user/libvhost-user.c   |  6 ++
 2 files changed, 16 insertions(+)

diff --git a/contrib/libvhost-user/libvhost-access.h 
b/contrib/libvhost-user/libvhost-access.h
index 868ba3e7bfb8..aa505ea1ec02 100644
--- a/contrib/libvhost-user/libvhost-access.h
+++ b/contrib/libvhost-user/libvhost-access.h
@@ -1,11 +1,21 @@
 #ifndef LIBVHOST_ACCESS_H
 
+#include 
+
 #include "qemu/bswap.h"
 
 #include "libvhost-user.h"
 
+static inline bool vu_has_feature(VuDev *dev, unsigned int fbit);
+
 static inline bool vu_access_is_big_endian(VuDev *dev)
 {
+/*
+ * TODO: can probably be removed as the fencing is already done in
+ * `vu_set_features_exec`
+ */
+assert(vu_has_feature(dev, VIRTIO_F_VERSION_1));
+
 /* Devices conforming to VIRTIO 1.0 or later are always LE. */
 return false;
 }
diff --git a/contrib/libvhost-user/libvhost-user.c 
b/contrib/libvhost-user/libvhost-user.c
index 0214b04c5291..93c4503b1f53 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -540,6 +540,12 @@ vu_set_features_exec(VuDev *dev, VhostUserMsg *vmsg)
 DPRINT("u64: 0x%016"PRIx64"\n", vmsg->payload.u64);
 
 dev->features = vmsg->payload.u64;
+if (!vu_has_feature(dev, VIRTIO_F_VERSION_1)) {
+/* We only support devices conforming to VIRTIO 1.0 or
+ * later */
+vu_panic(dev, "virtio legacy devices aren't supported by 
libvhost-user");
+return false;
+}
 
 if (!(dev->features & VHOST_USER_F_PROTOCOL_FEATURES)) {
 vu_set_enable_all_rings(dev, true);
-- 
2.25.4

[RFC v2 1/3] virtio: add vhost-user-fs-ccw device

[Marc Hartmayer]

From: Halil Pasic 

Wire up the CCW device for vhost-user-fs.

Signed-off-by: Halil Pasic 
---
 hw/s390x/Makefile.objs   |  1 +
 hw/s390x/vhost-user-fs-ccw.c | 73 
 2 files changed, 74 insertions(+)
 create mode 100644 hw/s390x/vhost-user-fs-ccw.c

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index a46a1c7894e0..c4086ec3171e 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -20,6 +20,7 @@ obj-$(CONFIG_VIRTIO_NET) += virtio-ccw-net.o
 obj-$(CONFIG_VIRTIO_BLK) += virtio-ccw-blk.o
 obj-$(call land,$(CONFIG_VIRTIO_9P),$(CONFIG_VIRTFS)) += virtio-ccw-9p.o
 obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock-ccw.o
+obj-$(CONFIG_VHOST_USER_FS) += vhost-user-fs-ccw.o
 endif
 obj-y += css-bridge.o
 obj-y += ccw-device.o
diff --git a/hw/s390x/vhost-user-fs-ccw.c b/hw/s390x/vhost-user-fs-ccw.c
new file mode 100644
index ..88a7a11a34b4
--- /dev/null
+++ b/hw/s390x/vhost-user-fs-ccw.c
@@ -0,0 +1,73 @@
+/*
+ * Ccw transport wiring for vhost-user-fs
+ *
+ * Copyright 2020 IBM Corp.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
+#include "hw/virtio/vhost-user-fs.h"
+#include "virtio-ccw.h"
+
+typedef struct VHostUserFSCcw {
+VirtioCcwDevice parent_obj;
+VHostUserFS vdev;
+} VHostUserFSCcw;
+
+#define TYPE_VHOST_USER_FS_CCW "vhost-user-fs-ccw"
+#define VHOST_USER_FS_CCW(obj) \
+OBJECT_CHECK(VHostUserFSCcw, (obj), TYPE_VHOST_USER_FS_CCW)
+
+
+static Property vhost_user_fs_ccw_properties[] = {
+DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
+VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
+DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
+   VIRTIO_CCW_MAX_REV),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vhost_user_fs_ccw_realize(VirtioCcwDevice *ccw_dev, Error **errp)
+{
+VHostUserFSCcw *dev = VHOST_USER_FS_CCW(ccw_dev);
+DeviceState *vdev = DEVICE(&dev->vdev);
+
+qdev_realize(vdev, BUS(&ccw_dev->bus), errp);
+}
+
+static void vhost_user_fs_ccw_instance_init(Object *obj)
+{
+VHostUserFSCcw *dev = VHOST_USER_FS_CCW(obj);
+
+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+TYPE_VHOST_USER_FS);
+}
+
+static void vhost_user_fs_ccw_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_CLASS(klass);
+
+k->realize = vhost_user_fs_ccw_realize;
+device_class_set_props(dc,vhost_user_fs_ccw_properties);
+set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+}
+
+static const TypeInfo vhost_user_fs_ccw = {
+.name  = TYPE_VHOST_USER_FS_CCW,
+.parent= TYPE_VIRTIO_CCW_DEVICE,
+.instance_size = sizeof(VHostUserFSCcw),
+.instance_init = vhost_user_fs_ccw_instance_init,
+.class_init= vhost_user_fs_ccw_class_init,
+};
+
+static void vhost_user_fs_ccw_register(void)
+{
+type_register_static(&vhost_user_fs_ccw);
+}
+
+type_init(vhost_user_fs_ccw_register)
-- 
2.25.4

[RFC v2 2/3] libvhost-user: handle endianness as mandated by the spec

[Marc Hartmayer]

Since virtio existed even before it got standardized, the virtio
standard defines the following types of virtio devices:

 + legacy device (pre-virtio 1.0)
 + non-legacy or VIRTIO 1.0 device
 + transitional device (which can act both as legacy and non-legacy)

Virtio 1.0 defines the fields of the virtqueues as little endian,
while legacy uses guest's native endian [1]. Currently libvhost-user
does not handle virtio endianness at all, i.e. it works only if the
native endianness matches with whatever is actually needed. That means
things break spectacularly on big-endian targets. Let us handle virtio
endianness for non-legacy as required by the virtio specification
[1]. We will fence non-legacy virtio devices with the upcoming patch.

[1] 
https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-210003

Signed-off-by: Marc Hartmayer 

---
Note: As we don't support legacy virtio devices there is dead code in
libvhost-access.h that could be removed. But for the sake of
completeness, I left it in the code.
---
 contrib/libvhost-user/libvhost-access.h |  61 
 contrib/libvhost-user/libvhost-user.c   | 119 
 2 files changed, 121 insertions(+), 59 deletions(-)
 create mode 100644 contrib/libvhost-user/libvhost-access.h

diff --git a/contrib/libvhost-user/libvhost-access.h 
b/contrib/libvhost-user/libvhost-access.h
new file mode 100644
index ..868ba3e7bfb8
--- /dev/null
+++ b/contrib/libvhost-user/libvhost-access.h
@@ -0,0 +1,61 @@
+#ifndef LIBVHOST_ACCESS_H
+
+#include "qemu/bswap.h"
+
+#include "libvhost-user.h"
+
+static inline bool vu_access_is_big_endian(VuDev *dev)
+{
+/* Devices conforming to VIRTIO 1.0 or later are always LE. */
+return false;
+}
+
+static inline void vu_stw_p(VuDev *vdev, void *ptr, uint16_t v)
+{
+if (vu_access_is_big_endian(vdev)) {
+stw_be_p(ptr, v);
+} else {
+stw_le_p(ptr, v);
+}
+}
+
+static inline void vu_stl_p(VuDev *vdev, void *ptr, uint32_t v)
+{
+if (vu_access_is_big_endian(vdev)) {
+stl_be_p(ptr, v);
+} else {
+stl_le_p(ptr, v);
+}
+}
+
+static inline void vu_stq_p(VuDev *vdev, void *ptr, uint64_t v)
+{
+if (vu_access_is_big_endian(vdev)) {
+stq_be_p(ptr, v);
+} else {
+stq_le_p(ptr, v);
+}
+}
+
+static inline int vu_lduw_p(VuDev *vdev, const void *ptr)
+{
+if (vu_access_is_big_endian(vdev))
+return lduw_be_p(ptr);
+return lduw_le_p(ptr);
+}
+
+static inline int vu_ldl_p(VuDev *vdev, const void *ptr)
+{
+if (vu_access_is_big_endian(vdev))
+return ldl_be_p(ptr);
+return ldl_le_p(ptr);
+}
+
+static inline uint64_t vu_ldq_p(VuDev *vdev, const void *ptr)
+{
+if (vu_access_is_big_endian(vdev))
+return ldq_be_p(ptr);
+return ldq_le_p(ptr);
+}
+
+#endif /* LIBVHOST_ACCESS_H */
diff --git a/contrib/libvhost-user/libvhost-user.c 
b/contrib/libvhost-user/libvhost-user.c
index d315db139606..0214b04c5291 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -45,6 +45,7 @@
 #include "qemu/memfd.h"
 
 #include "libvhost-user.h"
+#include "libvhost-access.h"
 
 /* usually provided by GLib */
 #ifndef MIN
@@ -1074,7 +1075,7 @@ vu_set_vring_addr_exec(VuDev *dev, VhostUserMsg *vmsg)
 return false;
 }
 
-vq->used_idx = vq->vring.used->idx;
+vq->used_idx = vu_lduw_p(dev, &vq->vring.used->idx);
 
 if (vq->last_avail_idx != vq->used_idx) {
 bool resume = dev->iface->queue_is_processed_in_order &&
@@ -1191,7 +1192,7 @@ vu_check_queue_inflights(VuDev *dev, VuVirtq *vq)
 return 0;
 }
 
-vq->used_idx = vq->vring.used->idx;
+vq->used_idx = vu_lduw_p(dev, &vq->vring.used->idx);
 vq->resubmit_num = 0;
 vq->resubmit_list = NULL;
 vq->counter = 0;
@@ -2019,35 +2020,35 @@ vu_queue_started(const VuDev *dev, const VuVirtq *vq)
 }
 
 static inline uint16_t
-vring_avail_flags(VuVirtq *vq)
+vring_avail_flags(VuDev *dev, VuVirtq *vq)
 {
-return vq->vring.avail->flags;
+return vu_lduw_p(dev, &vq->vring.avail->flags);
 }
 
 static inline uint16_t
-vring_avail_idx(VuVirtq *vq)
+vring_avail_idx(VuDev *dev, VuVirtq *vq)
 {
-vq->shadow_avail_idx = vq->vring.avail->idx;
+vq->shadow_avail_idx = vu_lduw_p(dev, &vq->vring.avail->idx);
 
 return vq->shadow_avail_idx;
 }
 
 static inline uint16_t
-vring_avail_ring(VuVirtq *vq, int i)
+vring_avail_ring(VuDev *dev, VuVirtq *vq, int i)
 {
-return vq->vring.avail->ring[i];
+return vu_lduw_p(dev, &vq->vring.avail->ring[i]);
 }
 
 static inline uint16_t
-vring_get_used_event(VuVirtq *vq)
+vring_get_used_event(VuDev *dev, VuVirtq *vq)
 {
-return vring_avail_ring(vq, vq->vring.num);
+return vring_avail_ring(dev, vq, vq->vring.num);
 }
 
 static int
 virtqueue_num_heads(VuDev *dev, VuVirtq *vq, unsigned int idx)
 {
-uint16_t num_heads = vring_avail_idx(vq) - idx;
+uint16_t num_heads = vring_avail_idx(dev, vq) - idx;
 
 /* Check i

[PATCH v2 1/4] scripts/tracetool: Fix dtrace generation for macOS

[Roman Bolshakov]

dtrace USDT is fully supported since OS X 10.6. There are a few
peculiarities compared to other dtrace flavors.

1. It doesn't accept empty files.
2. It doesn't recognize bool type but accepts C99 _Bool.
3. It converts int8_t * in probe points to char * in
   header files and introduces [-Wpointer-sign] warning.

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 scripts/tracetool/format/d.py | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
index 0afb5f3f47..353722f89c 100644
--- a/scripts/tracetool/format/d.py
+++ b/scripts/tracetool/format/d.py
@@ -13,6 +13,7 @@ __email__  = "stefa...@redhat.com"
 
 
 from tracetool import out
+from sys import platform
 
 
 # Reserved keywords from
@@ -34,7 +35,8 @@ def generate(events, backend, group):
 
 # SystemTap's dtrace(1) warns about empty "provider qemu {}" but is happy
 # with an empty file.  Avoid the warning.
-if not events:
+# But dtrace on macOS can't deal with empty files.
+if not events and platform != "darwin":
 return
 
 out('/* This file is autogenerated by tracetool, do not edit. */'
@@ -44,6 +46,17 @@ def generate(events, backend, group):
 for e in events:
 args = []
 for type_, name in e.args:
+if platform == "darwin":
+# macOS dtrace accepts only C99 _Bool
+if type_ == 'bool':
+type_ = '_Bool'
+if type_ == 'bool *':
+type_ = '_Bool *'
+# It converts int8_t * in probe points to char * in header
+# files and introduces [-Wpointer-sign] warning.
+# Avoid it by changing probe type to signed char * beforehand.
+if type_ == 'int8_t *':
+type_ = 'signed char *'
 if name in RESERVED_WORDS:
 name += '_'
 args.append(type_ + ' ' + name)
-- 
2.26.1

[PATCH v2 0/4] Add dtrace support on macOS

[Roman Bolshakov]

Hi,

This is a small series that enables dtrace tracing backend on macOS.
Whether or not it should go to 5.1 is up to discretion of tracing
maintainers.

Thanks,
Roman

Changes since v1:
 - Fixed a typo ANSI C to C99, wrt to _Bool in the first patch.
 - Prevented a few [-Wpointer-sign] warnings by converting int8_t * to
   signed char * in static probe definitions.
 - Moved COLO packet dump under #ifdef DEBUG_COLO_PACKETS (Daniel).
 - Separated tracepoints in net/filter-rewriter.c to use matching
   is-enabled probe (Daniel).

Roman Bolshakov (4):
  scripts/tracetool: Fix dtrace generation for macOS
  scripts/tracetool: Use void pointer for vcpu
  build: Don't make object files for dtrace on macOS
  net/colo: Match is-enabled probe to tracepoint

 Makefile.objs |  2 ++
 net/colo-compare.c| 42 ++-
 net/filter-rewriter.c | 10 +++--
 net/trace-events  |  2 --
 scripts/tracetool/format/d.py | 15 -
 scripts/tracetool/vcpu.py |  2 +-
 6 files changed, 47 insertions(+), 26 deletions(-)

-- 
2.26.1

[PATCH v2 4/4] net/colo: Match is-enabled probe to tracepoint

[Roman Bolshakov]

Build of QEMU with dtrace fails on macOS:

  LINKx86_64-softmmu/qemu-system-x86_64
error: probe colo_compare_miscompare doesn't exist
error: Could not register probes
ld: error creating dtrace DOF section for architecture x86_64

The reason of the error is explained by Adam Leventhal [1]:

  Note that is-enabled probes don't have the stability magic so I'm not
  sure how things would work if only is-enabled probes were used.

net/colo code uses is-enabled probes to determine if other probes should
be used but colo_compare_miscompare itself is not used explicitly.
Linker doesn't include the symbol and build fails.

The issue can be resolved if is-enabled probe matches the actual trace
point that is used inside the test. Packet dump toggle is replaced with
a compile-time conditional definition.

1. http://markmail.org/message/6grq2ygr5nwdwsnb

Fixes: f4b618360e ("colo-compare: add TCP, UDP, ICMP packet comparison")
Cc: Philippe Mathieu-Daudé 
Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 net/colo-compare.c| 42 ++
 net/filter-rewriter.c | 10 --
 net/trace-events  |  2 --
 3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 398b7546ff..e0be98e494 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -54,6 +54,8 @@ static NotifierList colo_compare_notifiers =
 #define REGULAR_PACKET_CHECK_MS 3000
 #define DEFAULT_TIME_OUT_MS 3000
 
+/* #define DEBUG_COLO_PACKETS */
+
 static QemuMutex colo_compare_mutex;
 static bool colo_compare_active;
 static QemuMutex event_mtx;
@@ -327,7 +329,7 @@ static int colo_compare_packet_payload(Packet *ppkt,
uint16_t len)
 
 {
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(TRACE_COLO_COMPARE_IP_INFO)) {
 char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];
 
 strcpy(pri_ip_src, inet_ntoa(ppkt->ip->ip_src));
@@ -492,12 +494,12 @@ sec:
 g_queue_push_head(&conn->primary_list, ppkt);
 g_queue_push_head(&conn->secondary_list, spkt);
 
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
-qemu_hexdump((char *)ppkt->data, stderr,
-"colo-compare ppkt", ppkt->size);
-qemu_hexdump((char *)spkt->data, stderr,
-"colo-compare spkt", spkt->size);
-}
+#ifdef DEBUG_COLO_PACKETS
+qemu_hexdump((char *)ppkt->data, stderr,
+ "colo-compare ppkt", ppkt->size);
+qemu_hexdump((char *)spkt->data, stderr,
+ "colo-compare spkt", spkt->size);
+#endif
 
 colo_compare_inconsistency_notify(s);
 }
@@ -533,12 +535,12 @@ static int colo_packet_compare_udp(Packet *spkt, Packet 
*ppkt)
 ppkt->size - offset)) {
 trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
 trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
-qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
- ppkt->size);
-qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
- spkt->size);
-}
+#ifdef DEBUG_COLO_PACKETS
+qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
+ ppkt->size);
+qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
+ spkt->size);
+#endif
 return -1;
 } else {
 return 0;
@@ -576,12 +578,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet 
*ppkt)
ppkt->size);
 trace_colo_compare_icmp_miscompare("Secondary pkt size",
spkt->size);
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
-qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
- ppkt->size);
-qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
- spkt->size);
-}
+#ifdef DEBUG_COLO_PACKETS
+qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
+ ppkt->size);
+qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
+ spkt->size);
+#endif
 return -1;
 } else {
 return 0;
@@ -597,7 +599,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet 
*ppkt)
 uint16_t offset = ppkt->vnet_hdr_len;
 
 trace_colo_compare_main("compare other");
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(TRACE_COLO_COMPARE_IP_INFO)) {
 char pri_ip_src[20], pri_ip_dst[20], sec_ip

[PATCH v2 2/4] scripts/tracetool: Use void pointer for vcpu

[Roman Bolshakov]

dtrace on macOS complains that CPUState * is used for a few probes:

  dtrace: failed to compile script trace-dtrace-root.dtrace: line 130: syntax 
error near "CPUState"

A comment in scripts/tracetool/__init__.py mentions that:

  We only want to allow standard C types or fixed sized
  integer types. We don't want QEMU specific types
  as we can't assume trace backends can resolve all the
  typedefs

Fixes: 3d211d9f4dbee ("trace: Add 'vcpu' event property to trace guest vCPU")
Reviewed-by: Daniel P. Berrangé 
Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 scripts/tracetool/vcpu.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/tracetool/vcpu.py b/scripts/tracetool/vcpu.py
index b54e4f4e7a..868b4cb04c 100644
--- a/scripts/tracetool/vcpu.py
+++ b/scripts/tracetool/vcpu.py
@@ -24,7 +24,7 @@ def transform_event(event):
 assert "tcg-trans" not in event.properties
 assert "tcg-exec" not in event.properties
 
-event.args = Arguments([("CPUState *", "__cpu"), event.args])
+event.args = Arguments([("void *", "__cpu"), event.args])
 if "tcg" in event.properties:
 fmt = "\"cpu=%p \""
 event.fmt = [fmt + event.fmt[0],
-- 
2.26.1

[PATCH v2 3/4] build: Don't make object files for dtrace on macOS

[Roman Bolshakov]

dtrace on macOS uses unresolved symbols with a special prefix to define
probes [1], only headers should be generated for USDT (dtrace(1)). But
it doesn't support backwards compatible no-op -G flag [2] and implicit
build rules fail.

1. https://markmail.org/message/6grq2ygr5nwdwsnb
2. https://markmail.org/message/5xrxt2w5m42nojkz

Reviewed-by: Daniel P. Berrangé 
Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 Makefile.objs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile.objs b/Makefile.objs
index d22b3b45d7..982f15ba30 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -211,5 +211,7 @@ trace-events-files = $(SRC_PATH)/trace-events 
$(trace-events-subdirs:%=$(SRC_PAT
 trace-obj-y = trace-root.o
 trace-obj-y += $(trace-events-subdirs:%=%/trace.o)
 trace-obj-$(CONFIG_TRACE_UST) += trace-ust-all.o
+ifneq ($(CONFIG_DARWIN),y)
 trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace-root.o
 trace-obj-$(CONFIG_TRACE_DTRACE) += $(trace-events-subdirs:%=%/trace-dtrace.o)
+endif
-- 
2.26.1

Re: [PULL 0/6] x86 fixes for -rc1

[Peter Maydell]

On Thu, 16 Jul 2020 at 19:19, Eduardo Habkost  wrote:
>
> The following changes since commit ee5128bb00f90dd301991d80d1db5224ce924c84:
>
>   Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into 
> staging (2020-07-16 13:12:05 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/ehabkost/qemu.git tags/x86-next-pull-request
>
> for you to fetch changes up to 818b9f111d64b40661d08f5e23236ac1ca5df505:
>
>   i386: hvf: Explicitly set CR4 guest/host mask (2020-07-16 14:15:13 -0400)
>
> 
> x86 fixes for -rc1
>
> Fixes for x86 that missed hard freeze:
> * Don't trigger warnings for features set by
>   CPU model versions (Xiaoyao Li)
> * Missing features in Icelake-Server, Skylake-Server,
>   Cascadelake-Server CPU models (Chenyi Qiang)
> * Fix hvf x86_64 guest boot crash (Roman Bolshakov)


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM

[PULL 3/6] fuzz: Expect the cmdline in a freeable GString

[Thomas Huth]

From: Alexander Bulekov 

In the initial FuzzTarget, get_init_cmdline returned a char *. With this
API, we had no guarantee about where the string came from. For example,
i440fx-qtest-reboot-fuzz simply returned a pointer to a string literal,
while the QOS-based targets build the arguments out in a GString an
return the gchar *str pointer. Since we did not try to free the cmdline,
we have a leak for any targets that do not simply return string
literals. Clean up this mess by forcing fuzz-targets to return
a GString, that we can free.

Signed-off-by: Alexander Bulekov 
Message-Id: <20200714174616.20709-1-alx...@bu.edu>
Reviewed-by: Darren Kenny 
Signed-off-by: Thomas Huth 
---
 tests/qtest/fuzz/fuzz.c| 13 ++---
 tests/qtest/fuzz/fuzz.h|  6 +++---
 tests/qtest/fuzz/i440fx_fuzz.c |  4 ++--
 tests/qtest/fuzz/qos_fuzz.c|  6 +++---
 4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index 0b66e43409..6bc17ef313 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -199,16 +199,15 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char 
***envp)
 }
 
 /* Run QEMU's softmmu main with the fuzz-target dependent arguments */
-const char *init_cmdline = fuzz_target->get_init_cmdline(fuzz_target);
-init_cmdline = g_strdup_printf("%s -qtest /dev/null -qtest-log %s",
-   init_cmdline,
-   getenv("QTEST_LOG") ? "/dev/fd/2"
-   : "/dev/null");
-
+GString *cmd_line = fuzz_target->get_init_cmdline(fuzz_target);
+g_string_append_printf(cmd_line,
+   " -qtest /dev/null -qtest-log %s",
+   getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null");
 
 /* Split the runcmd into an argv and argc */
 wordexp_t result;
-wordexp(init_cmdline, &result, 0);
+wordexp(cmd_line->str, &result, 0);
+g_string_free(cmd_line, true);
 
 qemu_init(result.we_wordc, result.we_wordv, NULL);
 
diff --git a/tests/qtest/fuzz/fuzz.h b/tests/qtest/fuzz/fuzz.h
index 72d5710f6c..9ca3d107c5 100644
--- a/tests/qtest/fuzz/fuzz.h
+++ b/tests/qtest/fuzz/fuzz.h
@@ -50,10 +50,10 @@ typedef struct FuzzTarget {
 
 
 /*
- * returns the arg-list that is passed to qemu/softmmu init()
- * Cannot be NULL
+ * Returns the arguments that are passed to qemu/softmmu init(). Freed by
+ * the caller.
  */
-const char* (*get_init_cmdline)(struct FuzzTarget *);
+GString *(*get_init_cmdline)(struct FuzzTarget *);
 
 /*
  * will run once, prior to running qemu/softmmu init.
diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
index e2f31e56f9..bf966d478b 100644
--- a/tests/qtest/fuzz/i440fx_fuzz.c
+++ b/tests/qtest/fuzz/i440fx_fuzz.c
@@ -158,9 +158,9 @@ static void i440fx_fuzz_qos_fork(QTestState *s,
 
 static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest"
" -m 0 -display none";
-static const char *i440fx_argv(FuzzTarget *t)
+static GString *i440fx_argv(FuzzTarget *t)
 {
-return i440fx_qtest_argv;
+return g_string_new(i440fx_qtest_argv);
 }
 
 static void fork_init(void)
diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index 0c68f5361f..d52f3ebd83 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -66,7 +66,7 @@ void *qos_allocate_objects(QTestState *qts, QGuestAllocator 
**p_alloc)
 return allocate_objects(qts, current_path + 1, p_alloc);
 }
 
-static const char *qos_build_main_args(void)
+static GString *qos_build_main_args(void)
 {
 char **path = fuzz_path_vec;
 QOSGraphNode *test_node;
@@ -88,7 +88,7 @@ static const char *qos_build_main_args(void)
 /* Prepend the arguments that we need */
 g_string_prepend(cmd_line,
 TARGET_NAME " -display none -machine accel=qtest -m 64 ");
-return cmd_line->str;
+return cmd_line;
 }
 
 /*
@@ -189,7 +189,7 @@ static void walk_path(QOSGraphNode *orig_path, int len)
 g_free(path_str);
 }
 
-static const char *qos_get_cmdline(FuzzTarget *t)
+static GString *qos_get_cmdline(FuzzTarget *t)
 {
 /*
  * Set a global variable that we use to identify the qos_path for our
-- 
2.18.1

[PULL 0/6] Leak fixes for qtests + fuzzer CI

[Thomas Huth]

 Hi Peter,

the following changes since commit 95d1fbabae0cd44156ac4b96d512d143ca7dfd5e:

  Merge remote-tracking branch 
'remotes/kraxel/tags/fixes-20200716-pull-request' into staging (2020-07-16 
18:50:51 +0100)

are available in the Git repository at:

  https://gitlab.com/huth/qemu.git tags/pull-request-2020-07-17

for you to fetch changes up to b610eba335d5c8ac7484dbb1c886b125e2dea058:

  gitlab-ci.yml: Add fuzzer tests (2020-07-17 10:44:23 +0200)


* Leak fixes
* One fix for running with --enable-werror on macOS
* Add fuzzer test to the Gitlab-CI


Alexander Bulekov (1):
  fuzz: Expect the cmdline in a freeable GString

Li Qiang (2):
  qtest: bios-tables-test: fix a memory leak
  tests: qmp-cmd-test: fix memory leak

Markus Armbruster (1):
  qom: Plug memory leak in "info qom-tree"

Thomas Huth (2):
  configure: Fix for running with --enable-werror on macOS
  gitlab-ci.yml: Add fuzzer tests

 .gitlab-ci.yml | 20 +++-
 configure  |  2 +-
 qom/qom-hmp-cmds.c |  6 --
 tests/qtest/bios-tables-test.c |  1 +
 tests/qtest/fuzz/fuzz.c| 13 ++---
 tests/qtest/fuzz/fuzz.h|  6 +++---
 tests/qtest/fuzz/i440fx_fuzz.c |  4 ++--
 tests/qtest/fuzz/qos_fuzz.c|  6 +++---
 tests/qtest/qmp-cmd-test.c | 13 +
 9 files changed, 52 insertions(+), 19 deletions(-)

[PULL 2/6] tests: qmp-cmd-test: fix memory leak

[Thomas Huth]

From: Li Qiang 

Properly free each test response to avoid memory leak and separate
qtest_qmp() calls with spare lines, in a consistent manner.

Fixes: 5b88849e7b9("tests/qmp-cmd-test: Add qmp/object-add-failure-modes")
Reviewed-by: Eric Auger 
Signed-off-by: Li Qiang 
Message-Id: <20200715154117.15456-1-liq...@163.com>
Fixes: 9fc719b869 ("tests/qmp-cmd-test: Add qmp/object-add-duplicate-id")
Reviewed-by: Markus Armbruster 
Signed-off-by: Thomas Huth 
---
 tests/qtest/qmp-cmd-test.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index c68f99f659..f7b1aa7fdc 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -230,6 +230,8 @@ static void test_object_add_failure_modes(void)
  " 'props': {'size': 1048576 } } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
+
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
  " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
  " 'props': {'size': 1048576 } } }");
@@ -241,6 +243,7 @@ static void test_object_add_failure_modes(void)
  " {'id': 'ram1' } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* attempt to create an object with a property of a wrong type */
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
@@ -249,17 +252,20 @@ static void test_object_add_failure_modes(void)
 g_assert_nonnull(resp);
 /* now do it right */
 qmp_assert_error_class(resp, "GenericError");
+
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
  " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
  " 'props': {'size': 1048576 } } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* delete ram1 object */
 resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
  " {'id': 'ram1' } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* attempt to create an object without the id */
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
@@ -267,18 +273,21 @@ static void test_object_add_failure_modes(void)
  " 'props': {'size': 1048576 } } }");
 g_assert_nonnull(resp);
 qmp_assert_error_class(resp, "GenericError");
+
 /* now do it right */
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
  " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
  " 'props': {'size': 1048576 } } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* delete ram1 object */
 resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
  " {'id': 'ram1' } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* attempt to set a non existing property */
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
@@ -286,23 +295,27 @@ static void test_object_add_failure_modes(void)
  " 'props': {'sized': 1048576 } } }");
 g_assert_nonnull(resp);
 qmp_assert_error_class(resp, "GenericError");
+
 /* now do it right */
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
  " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
  " 'props': {'size': 1048576 } } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* delete ram1 object without id */
 resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
  " {'ida': 'ram1' } }");
 g_assert_nonnull(resp);
+qobject_unref(resp);
 
 /* delete ram1 object */
 resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
  " {'id': 'ram1' } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* delete ram1 object that does not exist anymore*/
 resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
-- 
2.18.1

[PULL 4/6] configure: Fix for running with --enable-werror on macOS

[Thomas Huth]

The configure script currently refuses to succeed when run on macOS
with --enable-werror:

 ERROR: configure test passed without -Werror but failed with -Werror.

The information in config.log indicates:

 config-temp/qemu-conf.c:3:55: error: control reaches end of non-void
 function [-Werror,-Wreturn-type]
 static void *f(void *p) { pthread_setname_np("QEMU"); }
  ^
And indeed, the return statement is missing here.

Fixes: 479a57475e ("util: Implement debug-threads for macOS")
Message-Id: <20200716055655.24507-1-th...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index b751c853f5..e93836aaae 100755
--- a/configure
+++ b/configure
@@ -4198,7 +4198,7 @@ pthread_setname_np_wo_tid=no
 cat > $TMPC << EOF
 #include 
 
-static void *f(void *p) { pthread_setname_np("QEMU"); }
+static void *f(void *p) { pthread_setname_np("QEMU"); return NULL; }
 int main(void)
 {
 pthread_t thread;
-- 
2.18.1

[PULL 1/6] qtest: bios-tables-test: fix a memory leak

[Thomas Huth]

From: Li Qiang 

Fixes: 5da7c35e25a("bios-tables-test: Add Q35/TPM-TIS test")
Signed-off-by: Li Qiang 
Message-Id: <20200714153536.66060-1-liq...@163.com>
Reviewed-by: Eric Auger 
Reviewed-by: Igor Mammedov 
Signed-off-by: Thomas Huth 
---
 tests/qtest/bios-tables-test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index c315156858..d49b3988ec 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -924,6 +924,7 @@ static void test_acpi_tcg_tpm(const char *machine, const 
char *tpm_if,
 g_free(variant);
 g_free(tmp_path);
 g_free(tmp_dir_name);
+g_free(args);
 free_test_data(&data);
 #else
 g_test_skip("TPM disabled");
-- 
2.18.1

[PULL 5/6] qom: Plug memory leak in "info qom-tree"

[Thomas Huth]

From: Markus Armbruster 

Commit e8c9e65816 "qom: Make "info qom-tree" show children sorted"
created a memory leak, because I didn't realize
object_get_canonical_path_component()'s value needs to be freed.

Reproducer:

$ qemu-system-x86_64 -nodefaults -display none -S -monitor stdio
QEMU 5.0.50 monitor - type 'help' for more information
(qemu) info qom-tree

This leaks some 4500 path components, 12-13 characters on average,
i.e. roughly 100kBytes depending on the allocator.  A couple of
hundred "info qom-tree" here, a couple of hundred there, and soon
enough we're talking about real memory.

Plug the leak.

Fixes: e8c9e65816f5dbfe18ad3b2be938d0d8192d459a
Signed-off-by: Markus Armbruster 
Reported-by: Reviewed-by: Li Qiang  [sent same patch]
Message-Id: <20200714160202.3121879-3-arm...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 qom/qom-hmp-cmds.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
index 9ed8bb1c9f..aaacadacca 100644
--- a/qom/qom-hmp-cmds.c
+++ b/qom/qom-hmp-cmds.c
@@ -96,8 +96,10 @@ static void print_qom_composition(Monitor *mon, Object *obj, 
int indent);
 
 static int qom_composition_compare(const void *a, const void *b, void *ignore)
 {
-return g_strcmp0(a ? object_get_canonical_path_component(a) : NULL,
- b ? object_get_canonical_path_component(b) : NULL);
+g_autofree char *ac = object_get_canonical_path_component(a);
+g_autofree char *bc = object_get_canonical_path_component(b);
+
+return g_strcmp0(ac, bc);
 }
 
 static int insert_qom_composition_child(Object *obj, void *opaque)
-- 
2.18.1

[PULL 6/6] gitlab-ci.yml: Add fuzzer tests

[Thomas Huth]

So far we neither compile-tested nor run any of the new fuzzers in our CI,
which led to some build failures of the fuzzer code in the past weeks.
To avoid this problem, add a job to compile the fuzzer code and run some
loops (which likely don't find any new bugs via fuzzing, but at least we
know that the code can still be run).

A nice side-effect of this test is that the leak tests are enabled here,
so we should now notice some of the memory leaks in our code base earlier.

Message-Id: <20200716100950.27396-1-th...@redhat.com>
Reviewed-by: Alexander Bulekov 
Signed-off-by: Thomas Huth 
---
 .gitlab-ci.yml | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 5eeba2791b..41597c3603 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -161,9 +161,27 @@ build-clang:
 IMAGE: fedora
 CONFIGURE_ARGS: --cc=clang --cxx=clang++
 TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
-  ppc-softmmu s390x-softmmu x86_64-softmmu arm-linux-user
+  ppc-softmmu s390x-softmmu arm-linux-user
 MAKE_CHECK_ARGS: check
 
+build-fuzzer:
+  <<: *native_build_job_definition
+  variables:
+IMAGE: fedora
+  script:
+- mkdir build
+- cd build
+- ../configure --cc=clang --cxx=clang++ --enable-fuzzing
+   --enable-sanitizers --target-list=x86_64-softmmu
+- make -j"$JOBS" all check-build x86_64-softmmu/fuzz
+- make check
+- for fuzzer in i440fx-qos-fork-fuzz i440fx-qos-noreset-fuzz
+i440fx-qtest-reboot-fuzz virtio-scsi-flags-fuzz virtio-scsi-fuzz ; do
+  echo Testing ${fuzzer} ... ;
+  x86_64-softmmu/qemu-fuzz-x86_64 --fuzz-target=${fuzzer} -runs=1000
+|| exit 1 ;
+  done
+
 build-tci:
   <<: *native_build_job_definition
   variables:
-- 
2.18.1

Re: [PATCH] net: check payload length limit for all frames

[Li Qiang]

P J P  于2020年7月17日周五 下午5:09写道：
>
> +-- On Fri, 17 Jul 2020, Jason Wang wrote --+
> | Thanks but I don't see a direct relation between 64K limit and this
> | calltrace. Maybe you can elaborate more on this?
>
> The use-after-free is not function of the size per say; The reproducer given
> sends large(>64k) packets via loopback interface with gso_type=none(0). The
> proposed patch helps to fix it. The large size & payload_len may result in
> other oob kind of access issues too I think.
>
> @Alex, would it be possible to share the reproduces on the upstream bug
> LP#1886362?

The reproducer of LP#1886362 is here:
--> https://bugs.launchpad.net/qemu/+bug/1886362

Maybe you mean the reproducer of your patch?
If you or Alex could share it, I'm glad to analysis this issue.

Thanks,
Li Qiang

>
> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
>

Re: [RFC PATCH-for-5.1 v2] hw/ide: Cancel pending DMA requests before setting as inactive

[Philippe Mathieu-Daudé]

On 7/17/20 9:53 AM, Philippe Mathieu-Daudé wrote:
> libFuzzer found a case where requests are queued for later in the
> AIO context, but a command set the bus inactive, then when finally
> the requests are processed by the DMA it aborts because it is
> inactive:
> 
>  include/hw/ide/pci.h:59: IDEState *bmdma_active_if(BMDMAState *): Assertion 
> `bmdma->bus->retry_unit != (uint8_t)-1' failed.
> 
> Reproducer available on the BugLink.
> 
> Fix by draining the pending DMA requests before inactivating the bus.
> 

Fixes: 8ccad811e6 ("use AIO for DMA transfers - enabled DMA for CDROMs")

> BugLink: https://bugs.launchpad.net/qemu/+bug/1887303
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> RFC because I don't have much clue about block drive and IDE,
> so block-team please be very careful while reviewing this bug.
> ---
>  hw/ide/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index d997a78e47..f7affafb0c 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -804,7 +804,7 @@ void dma_buf_commit(IDEState *s, uint32_t tx_bytes)
>  
>  void ide_set_inactive(IDEState *s, bool more)
>  {
> -s->bus->dma->aiocb = NULL;
> +ide_cancel_dma_sync(s);
>  ide_clear_retry(s);
>  if (s->bus->dma->ops->set_inactive) {
>  s->bus->dma->ops->set_inactive(s->bus->dma, more);
> 

Re: [RFC PATCH-for-5.1 v2] hw/ide: Cancel pending DMA requests before setting as inactive

[Philippe Mathieu-Daudé]

On 7/17/20 12:08 PM, Philippe Mathieu-Daudé wrote:
> On 7/17/20 9:53 AM, Philippe Mathieu-Daudé wrote:
>> libFuzzer found a case where requests are queued for later in the
>> AIO context, but a command set the bus inactive, then when finally
>> the requests are processed by the DMA it aborts because it is
>> inactive:
>>
>>  include/hw/ide/pci.h:59: IDEState *bmdma_active_if(BMDMAState *): Assertion 
>> `bmdma->bus->retry_unit != (uint8_t)-1' failed.
>>
>> Reproducer available on the BugLink.
>>
>> Fix by draining the pending DMA requests before inactivating the bus.
>>
> 

Sorry I also forgot:

Reported-by: Alexander Bulekov 

> Fixes: 8ccad811e6 ("use AIO for DMA transfers - enabled DMA for CDROMs")
> 
>> BugLink: https://bugs.launchpad.net/qemu/+bug/1887303
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>> RFC because I don't have much clue about block drive and IDE,
>> so block-team please be very careful while reviewing this bug.
>> ---
>>  hw/ide/core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index d997a78e47..f7affafb0c 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -804,7 +804,7 @@ void dma_buf_commit(IDEState *s, uint32_t tx_bytes)
>>  
>>  void ide_set_inactive(IDEState *s, bool more)
>>  {
>> -s->bus->dma->aiocb = NULL;
>> +ide_cancel_dma_sync(s);
>>  ide_clear_retry(s);
>>  if (s->bus->dma->ops->set_inactive) {
>>  s->bus->dma->ops->set_inactive(s->bus->dma, more);
>>
> 

Re: [PATCH] docs/s390x: fix vfio-ccw type

[Eric Farman]

On 7/16/20 10:50 AM, Cornelia Huck wrote:
> Fix the type name in the mdevctl example.
> 
> Signed-off-by: Cornelia Huck 

Yup, that is backwards.

Reviewed-by: Eric Farman 

> ---
> 
> I always seem to get this one wrong, and mdevctl does not complain until
> it wants to start the device...
> 
> ---
>  docs/system/s390x/vfio-ccw.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/docs/system/s390x/vfio-ccw.rst b/docs/system/s390x/vfio-ccw.rst
> index 8f65442c0f56..41e0bad5b489 100644
> --- a/docs/system/s390x/vfio-ccw.rst
> +++ b/docs/system/s390x/vfio-ccw.rst
> @@ -29,7 +29,7 @@ automatically, use
>  
> [root@host ~]# driverctl -b css set-override 0.0.0313 vfio_ccw
> [root@host ~]# mdevctl define -u 7e270a25-e163-4922-af60-757fc8ed48c6 \
> -  -p 0.0.0313 -t vfio-ccw_io -a
> +  -p 0.0.0313 -t vfio_ccw-io -a
>  
>  If using ``mdevctl`` is not possible or wanted, follow the manual procedure
>  below.
> 

[RFC PATCH-for-5.1] hw/ide: Avoid #DIV/0! FPU exception by setting CD-ROM sector count

[Philippe Mathieu-Daudé]

libFuzzer found an undefined behavior (#DIV/0!) in ide_set_sector()
when using a CD-ROM (reproducer available on the BugLink):

  UndefinedBehaviorSanitizer:DEADLYSIGNAL
  ==12163==ERROR: UndefinedBehaviorSanitizer: FPE on unknown address 
0x5616279cffdc (pc 0x5616279cffdc bp 0x7ffcdaabae90 sp 0x7ffcdaabae30 T12163)

Fix by initializing the CD-ROM number of sectors in ide_dev_initfn().

Reported-by: Alexander Bulekov 
Fixes: b2df431407 ("ide scsi: Mess with geometry only for hard disk devices")
BugLink: https://bugs.launchpad.net/qemu/+bug/1887309
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ide/qdev.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 27ff1f7f66..6ce7fc317c 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -201,6 +201,15 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind 
kind, Error **errp)
   errp)) {
 return;
 }
+} else {
+uint64_t nb_sectors;
+
+blk_get_geometry(dev->conf.blk, &nb_sectors);
+if (!nb_sectors) {
+error_setg(errp, "CD-ROM size can not be zero");
+return;
+}
+dev->conf.secs = nb_sectors;
 }
 if (!blkconf_apply_backend_options(&dev->conf, kind == IDE_CD,
kind != IDE_CD, errp)) {
-- 
2.21.3

[PATCH v2] introduce VFIO-over-socket protocol specificaion

[Thanos Makatos]

This patch introduces the VFIO-over-socket protocol specification, which
is designed to allow devices to be emulated outside QEMU, in a separate
process. VFIO-over-socket reuses the existing VFIO defines, structs and
concepts.

It has been earlier discussed as an RFC in:
"RFC: use VFIO over a UNIX domain socket to implement device offloading"

Signed-off-by: John G Johnson 
Signed-off-by: Thanos Makatos 

---

Changed since v1:
  * fix coding style issues
  * update MAINTAINERS for VFIO-over-socket
  * add vfio-over-socket to ToC

Regarding the build failure, I have not been able to reproduce it locally
using the docker image on my Debian 10.4 machine.
---
 MAINTAINERS |6 +
 docs/devel/index.rst|1 +
 docs/devel/vfio-over-socket.rst | 1135 +++
 3 files changed, 1142 insertions(+)
 create mode 100644 docs/devel/vfio-over-socket.rst

diff --git a/MAINTAINERS b/MAINTAINERS
index 030faf0..bb81590 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1732,6 +1732,12 @@ F: hw/vfio/ap.c
 F: docs/system/s390x/vfio-ap.rst
 L: qemu-s3...@nongnu.org
 
+VFIO-over-socket
+M: John G Johnson 
+M: Thanos Makatos 
+S: Supported
+F: docs/devel/vfio-over-socket.rst
+
 vhost
 M: Michael S. Tsirkin 
 S: Supported
diff --git a/docs/devel/index.rst b/docs/devel/index.rst
index ae6eac7..0439460 100644
--- a/docs/devel/index.rst
+++ b/docs/devel/index.rst
@@ -30,3 +30,4 @@ Contents:
reset
s390-dasd-ipl
clocks
+   vfio-over-socket
diff --git a/docs/devel/vfio-over-socket.rst b/docs/devel/vfio-over-socket.rst
new file mode 100644
index 000..9ac0282
--- /dev/null
+++ b/docs/devel/vfio-over-socket.rst
@@ -0,0 +1,1135 @@
+***
+VFIO-over-socket Protocol Specification
+***
+
+Version 0.1
+
+Introduction
+
+VFIO-over-socket, also known as vfio-user, is a protocol that allows a device
+to be virtualized in a separate process outside of QEMU. VFIO-over-socket
+devices consist of a generic VFIO device type, living inside QEMU, which we
+call the client, and the core device implementation, living outside QEMU, which
+we call the server. VFIO-over-socket can be the main transport mechanism for
+multi-process QEMU, however it can be used by other applications offering
+device virtualization. Explaining the advantages of a
+disaggregated/multi-process QEMU, and device virtualization outside QEMU in
+general, is beyond the scope of this document.
+
+This document focuses on specifying the VFIO-over-socket protocol. VFIO has
+been chosen for the following reasons:
+
+1) It is a mature and stable API, backed by an extensively used framework.
+2) The existing VFIO client implementation (qemu/hw/vfio/) can be largely
+   reused.
+
+In a proof of concept implementation it has been demonstrated that using VFIO
+over a UNIX domain socket is a viable option. VFIO-over-socket is designed with
+QEMU in mind, however it could be used by other client applications. The
+VFIO-over-socket protocol does not require that QEMU's VFIO client
+implementation is used in QEMU. None of the VFIO kernel modules are required
+for supporting the protocol, neither in the client nor the server, only the
+source header files are used.
+
+The main idea is to allow a virtual device to function in a separate process in
+the same host over a UNIX domain socket. A UNIX domain socket (AF_UNIX) is
+chosen because we can trivially send file descriptors over it, which in turn
+allows:
+
+* Sharing of guest memory for DMA with the virtual device process.
+* Sharing of virtual device memory with the guest for fast MMIO.
+* Efficient sharing of eventfd's for triggering interrupts.
+
+However, other socket types could be used which allows the virtual device
+process to run in a separate guest in the same host (AF_VSOCK) or remotely
+(AF_INET). Theoretically the underlying transport doesn't necessarily have to
+be a socket, however we don't examine such alternatives. In this document we
+focus on using a UNIX domain socket and introduce basic support for the other
+two types of sockets without considering performance implications.
+
+This document does not yet describe any internal details of the server-side
+implementation, however QEMU's VFIO client implementation will have to be
+adapted according to this protocol in order to support VFIO-over-socket virtual
+devices.
+
+VFIO
+
+VFIO is a framework that allows a physical device to be securely passed through
+to a user space process; the kernel does not drive the device at all.
+Typically, the user space process is a VM and the device is passed through to
+it in order to achieve high performance. VFIO provides an API and the required
+functionality in the kernel. QEMU has adopted VFIO to allow a guest virtual
+machine to directly access physical devices, instead of emulating them in
+software
+
+VFIO-over-socket reuses the core VFIO concepts defined in its API, but
+im

Re: [RFC v2 0/3] Enable virtio-fs on s390x

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200717092929.19453-1-mhart...@linux.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200717092929.19453-1-mhart...@linux.ibm.com
Subject: [RFC v2 0/3] Enable virtio-fs on s390x

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
a6d1b6d libvhost-user: fence legacy virtio devices
968857c libvhost-user: handle endianness as mandated by the spec
2c7bd4b virtio: add vhost-user-fs-ccw device

=== OUTPUT BEGIN ===
1/3 Checking commit 2c7bd4bc5216 (virtio: add vhost-user-fs-ccw device)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#25: 
new file mode 100644

ERROR: space required after that ',' (ctx:VxV)
#85: FILE: hw/s390x/vhost-user-fs-ccw.c:56:
+device_class_set_props(dc,vhost_user_fs_ccw_properties);
  ^

total: 1 errors, 1 warnings, 80 lines checked

Patch 1/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/3 Checking commit 968857cfd9be (libvhost-user: handle endianness as mandated 
by the spec)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

ERROR: braces {} are necessary for all arms of this statement
#75: FILE: contrib/libvhost-user/libvhost-access.h:42:
+if (vu_access_is_big_endian(vdev))
[...]

ERROR: braces {} are necessary for all arms of this statement
#82: FILE: contrib/libvhost-user/libvhost-access.h:49:
+if (vu_access_is_big_endian(vdev))
[...]

ERROR: braces {} are necessary for all arms of this statement
#89: FILE: contrib/libvhost-user/libvhost-access.h:56:
+if (vu_access_is_big_endian(vdev))
[...]

WARNING: line over 80 characters
#347: FILE: contrib/libvhost-user/libvhost-user.c:2512:
+   vu_ldq_p(dev, &desc[i].addr), vu_ldl_p(dev, 
&desc[i].len));

WARNING: line over 80 characters
#356: FILE: contrib/libvhost-user/libvhost-user.c:2520:
+   vu_ldq_p(dev, &desc[i].addr), vu_ldl_p(dev, 
&desc[i].len));

total: 3 errors, 3 warnings, 362 lines checked

Patch 2/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/3 Checking commit a6d1b6dacac8 (libvhost-user: fence legacy virtio devices)
WARNING: Block comments use a leading /* on a separate line
#48: FILE: contrib/libvhost-user/libvhost-user.c:544:
+/* We only support devices conforming to VIRTIO 1.0 or

WARNING: Block comments use a trailing */ on a separate line
#49: FILE: contrib/libvhost-user/libvhost-user.c:545:
+ * later */

WARNING: line over 80 characters
#50: FILE: contrib/libvhost-user/libvhost-user.c:546:
+vu_panic(dev, "virtio legacy devices aren't supported by 
libvhost-user");

total: 0 errors, 3 warnings, 33 lines checked

Patch 3/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200717092929.19453-1-mhart...@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [RFC v2 0/3] Enable virtio-fs on s390x

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200717092929.19453-1-mhart...@linux.ibm.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=bdade74ad18e41e6bde3a38af0ee03ee', '-u', 
'1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-_1lsptde/src/docker-src.2020-07-17-06.22.37.2505:/var/tmp/qemu:z,ro',
 'qemu/fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=bdade74ad18e41e6bde3a38af0ee03ee
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-_1lsptde/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real9m20.974s
user0m9.037s


The full log is available at
http://patchew.org/logs/20200717092929.19453-1-mhart...@linux.ibm.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [RFC PATCH-for-5.1] hw/ide: Avoid #DIV/0! FPU exception by setting CD-ROM sector count

[Philippe Mathieu-Daudé]

On 7/17/20 12:16 PM, Philippe Mathieu-Daudé wrote:
> libFuzzer found an undefined behavior (#DIV/0!) in ide_set_sector()
> when using a CD-ROM (reproducer available on the BugLink):
> 
>   UndefinedBehaviorSanitizer:DEADLYSIGNAL
>   ==12163==ERROR: UndefinedBehaviorSanitizer: FPE on unknown address 
> 0x5616279cffdc (pc 0x5616279cffdc bp 0x7ffcdaabae90 sp 0x7ffcdaabae30 T12163)
> 
> Fix by initializing the CD-ROM number of sectors in ide_dev_initfn().
> 
> Reported-by: Alexander Bulekov 
> Fixes: b2df431407 ("ide scsi: Mess with geometry only for hard disk devices")
> BugLink: https://bugs.launchpad.net/qemu/+bug/1887309
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/ide/qdev.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 27ff1f7f66..6ce7fc317c 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -201,6 +201,15 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind 
> kind, Error **errp)
>errp)) {
>  return;
>  }
> +} else {
> +uint64_t nb_sectors;
> +
> +blk_get_geometry(dev->conf.blk, &nb_sectors);
> +if (!nb_sectors) {
> +error_setg(errp, "CD-ROM size can not be zero");

Hmm this doesn't work because for some machines configure_blockdev()
creates blocks of zero size under your feet:

default_drive(default_cdrom, snapshot,
machine_class->block_default_type, 2,
  CDROM_OPTS);

> +return;
> +}
> +dev->conf.secs = nb_sectors;
>  }
>  if (!blkconf_apply_backend_options(&dev->conf, kind == IDE_CD,
> kind != IDE_CD, errp)) {
> 

[PATCH v1 1/5] shippable: add one more qemu to registry url

[Alex Bennée]

The registry url is //qemu/

Perhaps we should rationalise that some day but for now.

Signed-off-by: Alex Bennée 
---
 .shippable.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.shippable.yml b/.shippable.yml
index f6b742432e5..89d8be4291b 100644
--- a/.shippable.yml
+++ b/.shippable.yml
@@ -27,7 +27,7 @@ env:
   TARGET_LIST=ppc64-softmmu,ppc64-linux-user,ppc64abi32-linux-user
 build:
   pre_ci_boot:
-image_name: registry.gitlab.com/qemu-project/qemu/${IMAGE}
+image_name: registry.gitlab.com/qemu-project/qemu/qemu/${IMAGE}
 image_tag: latest
 pull: true
 options: "-e HOME=/root"
-- 
2.20.1

Re: [GIT PULL] I2C updates

[Peter Maydell]

On Thu, 16 Jul 2020 at 23:26, Corey Minyard  wrote:
>
> On Thu, Jul 16, 2020 at 09:45:41PM +0100, Peter Maydell wrote:
> > Hi; this failed to build on x86-64 Linux (incremental build):
>
> Hmm, I did test this, and I just rebuilt, then rebased on the end of
> master and rebuilt, without issue.
>
> It looks like the smbus code is not being included, but I don't see how
> that can be.

I was wrong about which config failed, sorry. Incremental builds
are fine, but the build that does "make -C builddir clean" first
fails.

It looks like the problem is that in the created config-devices.mak
files, CONFIG_SMBUS_EEPROM is set, but CONFIG_SMBUS is not.
So presumably the problem is in the change
"hw/i2c/Kconfig: Add an entry for the SMBus", or it is a more
general issue with changes to Kconfig files not correctly
resulting in rebuilds of config-devices.mak.

thanks
-- PMM

[PATCH v1 2/5] semihosting: defer connect_chardevs a little more to use serialx

[Alex Bennée]

From: KONRAD Frederic 

With that we can just use -semihosting-config chardev=serial0.

Signed-off-by: KONRAD Frederic 
Message-Id: <1592215252-26742-1-git-send-email-frederic.kon...@adacore.com>
[AJB: tweak commit message]
Signed-off-by: Alex Bennée 
---
 softmmu/vl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index f476ef89edb..4fedbe60c39 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -4131,8 +4131,6 @@ void qemu_init(int argc, char **argv, char **envp)
 
 qemu_opts_foreach(qemu_find_opts("chardev"),
   chardev_init_func, NULL, &error_fatal);
-/* now chardevs have been created we may have semihosting to connect */
-qemu_semihosting_connect_chardevs();
 
 #ifdef CONFIG_VIRTFS
 qemu_opts_foreach(qemu_find_opts("fsdev"),
@@ -4281,6 +4279,9 @@ void qemu_init(int argc, char **argv, char **envp)
 if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
 exit(1);
 
+/* now chardevs have been created we may have semihosting to connect */
+qemu_semihosting_connect_chardevs();
+
 /* If no default VGA is requested, the default is "none".  */
 if (default_vga) {
 vga_model = get_default_vga_model(machine_class);
-- 
2.20.1

[PATCH v1 4/5] util: add qemu_get_host_physmem utility function

[Alex Bennée]

This will be used in a future patch. For POSIX systems _SC_PHYS_PAGES
isn't standardised but at least appears in the man pages for
Open/FreeBSD. The result is advisory so any users of it shouldn't just
fail if we can't work it out.

The win32 stub currently returns 0 until someone with a Windows system
can develop and test a patch.

Signed-off-by: Alex Bennée 
Cc: BALATON Zoltan 
Cc: Christian Ehrhardt 
---
 include/qemu/osdep.h | 10 ++
 util/oslib-posix.c   | 11 +++
 util/oslib-win32.c   |  6 ++
 3 files changed, 27 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 4841b5c6b5f..7ff209983e2 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -665,4 +665,14 @@ static inline void qemu_reset_optind(void)
  */
 char *qemu_get_host_name(Error **errp);
 
+/**
+ * qemu_get_host_physmem:
+ *
+ * Operating system agnostiv way of querying host memory.
+ *
+ * Returns amount of physical memory on the system. This is purely
+ * advisery and may return 0 if we can't work it out.
+ */
+size_t qemu_get_host_physmem(void);
+
 #endif
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 36bf8593f8c..d9da782b896 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -839,3 +839,14 @@ char *qemu_get_host_name(Error **errp)
 
 return g_steal_pointer(&hostname);
 }
+
+size_t qemu_get_host_physmem(void)
+{
+#ifdef _SC_PHYS_PAGES
+long pages = sysconf(_SC_PHYS_PAGES);
+if (pages > 0) {
+return pages * qemu_real_host_page_size;
+}
+#endif
+return 0;
+}
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 7eedbe5859a..31030463cc9 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -828,3 +828,9 @@ char *qemu_get_host_name(Error **errp)
 
 return g_utf16_to_utf8(tmp, size, NULL, NULL, NULL);
 }
+
+size_t qemu_get_host_physmem(void)
+{
+/* currently unimplemented */
+return 0;
+}
-- 
2.20.1

[PATCH v1 0/5] candidate fixes for 5.1-rc1 (shippable, semihosting, OOM tcg)

[Alex Bennée]

Hi,

These are some candidate patches for rc1. 

The first is a quick fix that finally gets shippable up and running
again. We may want to consider a grand renaming of our docker scheme
but I don't think thats something worth dropping in while we are
stabilising - especially if we change the project name on gitlab. 

The semihosting fixes had just slipped off my radar but are pretty
minor bug fixes.

Finally I whipped up the last two because of a report of QEMU OOM'ing
on some CI systems with low memory. It doesn't restore the link to
guest ram size but instead introduces an advisory helper function to
probe guest physical memory.

Please review.

Alex Bennée (3):
  shippable: add one more qemu to registry url
  util: add qemu_get_host_physmem utility function
  accel/tcg: better handle memory constrained systems

KONRAD Frederic (2):
  semihosting: defer connect_chardevs a little more to use serialx
  semihosting: don't send the trailing '\0'

 include/qemu/osdep.h  | 10 ++
 accel/tcg/translate-all.c |  7 ++-
 hw/semihosting/console.c  |  4 +++-
 softmmu/vl.c  |  5 +++--
 util/oslib-posix.c| 11 +++
 util/oslib-win32.c|  6 ++
 .shippable.yml|  2 +-
 7 files changed, 40 insertions(+), 5 deletions(-)

-- 
2.20.1

[PATCH v1 5/5] accel/tcg: better handle memory constrained systems

[Alex Bennée]

It turns out there are some 64 bit systems that have relatively low
amounts of physical memory available to them (typically CI system).
Even with swapping available a 1GB translation buffer that fills up
can put the machine under increased memory pressure. Detect these low
memory situations and reduce tb_size appropriately.

Fixes: 600e17b261
Signed-off-by: Alex Bennée 
Cc: BALATON Zoltan 
Cc: Christian Ehrhardt 
---
 accel/tcg/translate-all.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 2afa46bd2b1..2ff0ba6d19b 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -976,7 +976,12 @@ static inline size_t size_code_gen_buffer(size_t tb_size)
 {
 /* Size the buffer.  */
 if (tb_size == 0) {
-tb_size = DEFAULT_CODE_GEN_BUFFER_SIZE;
+size_t phys_mem = qemu_get_host_physmem();
+if (phys_mem > 0 && phys_mem < (2 * DEFAULT_CODE_GEN_BUFFER_SIZE)) {
+tb_size = phys_mem / 4;
+} else {
+tb_size = DEFAULT_CODE_GEN_BUFFER_SIZE;
+}
 }
 if (tb_size < MIN_CODE_GEN_BUFFER_SIZE) {
 tb_size = MIN_CODE_GEN_BUFFER_SIZE;
-- 
2.20.1

[PATCH for-5.1 0/3] file-posix: Fix check_hdev_writable() with auto-read-only

[Kevin Wolf]

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1828252

Kevin Wolf (3):
  file-posix: Move check_hdev_writable() up
  file-posix: Fix check_hdev_writable() with auto-read-only
  file-posix: Fix leaked fd in raw_open_common() error path

 block/file-posix.c | 96 ++
 1 file changed, 54 insertions(+), 42 deletions(-)

-- 
2.25.4

[PATCH v1 3/5] semihosting: don't send the trailing '\0'

[Alex Bennée]

From: KONRAD Frederic 

Don't send the trailing 0 from the string.

Signed-off-by: KONRAD Frederic 
Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <1592215252-26742-2-git-send-email-frederic.kon...@adacore.com>
---
 hw/semihosting/console.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/semihosting/console.c b/hw/semihosting/console.c
index 22e7827824a..9b4fee92602 100644
--- a/hw/semihosting/console.c
+++ b/hw/semihosting/console.c
@@ -52,7 +52,9 @@ static GString *copy_user_string(CPUArchState *env, 
target_ulong addr)
 
 do {
 if (cpu_memory_rw_debug(cpu, addr++, &c, 1, 0) == 0) {
-s = g_string_append_c(s, c);
+if (c) {
+s = g_string_append_c(s, c);
+}
 } else {
 qemu_log_mask(LOG_GUEST_ERROR,
   "%s: passed inaccessible address " TARGET_FMT_lx,
-- 
2.20.1

[PATCH for-5.1 1/3] file-posix: Move check_hdev_writable() up

[Kevin Wolf]

We'll need to call it in raw_open_common(), so move the function to
avoid a forward declaration.

Signed-off-by: Kevin Wolf 
---
 block/file-posix.c | 66 +++---
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 8067e238cb..e35e15185a 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -401,6 +401,39 @@ static void raw_probe_alignment(BlockDriverState *bs, int 
fd, Error **errp)
 }
 }
 
+static int check_hdev_writable(BDRVRawState *s)
+{
+#if defined(BLKROGET)
+/* Linux block devices can be configured "read-only" using blockdev(8).
+ * This is independent of device node permissions and therefore open(2)
+ * with O_RDWR succeeds.  Actual writes fail with EPERM.
+ *
+ * bdrv_open() is supposed to fail if the disk is read-only.  Explicitly
+ * check for read-only block devices so that Linux block devices behave
+ * properly.
+ */
+struct stat st;
+int readonly = 0;
+
+if (fstat(s->fd, &st)) {
+return -errno;
+}
+
+if (!S_ISBLK(st.st_mode)) {
+return 0;
+}
+
+if (ioctl(s->fd, BLKROGET, &readonly) < 0) {
+return -errno;
+}
+
+if (readonly) {
+return -EACCES;
+}
+#endif /* defined(BLKROGET) */
+return 0;
+}
+
 static void raw_parse_flags(int bdrv_flags, int *open_flags, bool has_writers)
 {
 bool read_write = false;
@@ -3299,39 +3332,6 @@ static int hdev_probe_device(const char *filename)
 return 0;
 }
 
-static int check_hdev_writable(BDRVRawState *s)
-{
-#if defined(BLKROGET)
-/* Linux block devices can be configured "read-only" using blockdev(8).
- * This is independent of device node permissions and therefore open(2)
- * with O_RDWR succeeds.  Actual writes fail with EPERM.
- *
- * bdrv_open() is supposed to fail if the disk is read-only.  Explicitly
- * check for read-only block devices so that Linux block devices behave
- * properly.
- */
-struct stat st;
-int readonly = 0;
-
-if (fstat(s->fd, &st)) {
-return -errno;
-}
-
-if (!S_ISBLK(st.st_mode)) {
-return 0;
-}
-
-if (ioctl(s->fd, BLKROGET, &readonly) < 0) {
-return -errno;
-}
-
-if (readonly) {
-return -EACCES;
-}
-#endif /* defined(BLKROGET) */
-return 0;
-}
-
 static void hdev_parse_filename(const char *filename, QDict *options,
 Error **errp)
 {
-- 
2.25.4

[PATCH for-5.1 3/3] file-posix: Fix leaked fd in raw_open_common() error path

[Kevin Wolf]

Signed-off-by: Kevin Wolf 
---
 block/file-posix.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index 659f780570..b2ed9d7eb2 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -749,6 +749,9 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
 }
 ret = 0;
 fail:
+if (ret < 0 && s->fd != -1) {
+qemu_close(s->fd);
+}
 if (filename && (bdrv_flags & BDRV_O_TEMPORARY)) {
 unlink(filename);
 }
-- 
2.25.4

[PATCH for-5.1 2/3] file-posix: Fix check_hdev_writable() with auto-read-only

[Kevin Wolf]

For Linux block devices, being able to open the device read-write
doesn't necessarily mean that the device is actually writable (one
example is a read-only LV, as you get with lvchange -pr ). We
have check_hdev_writable() to check this condition and fail opening the
image read-write if it's not actually writable.

However, this check doesn't take auto-read-only into account, but
results in a hard failure instead of downgrading to read-only where
possible.

Fix this and do the writable check not based on BDRV_O_RDWR, but only
when this actually results in opening the file read-write. A second
check is inserted in raw_reconfigure_getfd() to have the same check when
dynamic auto-read-only upgrades an image file from read-only to
read-write.

Signed-off-by: Kevin Wolf 
---
 block/file-posix.c | 33 +
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index e35e15185a..659f780570 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -401,7 +401,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int 
fd, Error **errp)
 }
 }
 
-static int check_hdev_writable(BDRVRawState *s)
+static int check_hdev_writable(int fd)
 {
 #if defined(BLKROGET)
 /* Linux block devices can be configured "read-only" using blockdev(8).
@@ -415,7 +415,7 @@ static int check_hdev_writable(BDRVRawState *s)
 struct stat st;
 int readonly = 0;
 
-if (fstat(s->fd, &st)) {
+if (fstat(fd, &st)) {
 return -errno;
 }
 
@@ -423,7 +423,7 @@ static int check_hdev_writable(BDRVRawState *s)
 return 0;
 }
 
-if (ioctl(s->fd, BLKROGET, &readonly) < 0) {
+if (ioctl(fd, BLKROGET, &readonly) < 0) {
 return -errno;
 }
 
@@ -618,6 +618,15 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
 }
 s->fd = fd;
 
+/* Check s->open_flags rather than bdrv_flags due to auto-read-only */
+if (s->open_flags & O_RDWR) {
+ret = check_hdev_writable(s->fd);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "The device is not writable");
+goto fail;
+}
+}
+
 s->perm = 0;
 s->shared_perm = BLK_PERM_ALL;
 
@@ -1010,6 +1019,15 @@ static int raw_reconfigure_getfd(BlockDriverState *bs, 
int flags,
 }
 }
 
+if (fd != -1 && (*open_flags & O_RDWR)) {
+ret = check_hdev_writable(fd);
+if (ret < 0) {
+qemu_close(fd);
+error_setg_errno(errp, -ret, "The device is not writable");
+return -1;
+}
+}
+
 return fd;
 }
 
@@ -3454,15 +3472,6 @@ hdev_open_Mac_error:
 /* Since this does ioctl the device must be already opened */
 bs->sg = hdev_is_sg(bs);
 
-if (flags & BDRV_O_RDWR) {
-ret = check_hdev_writable(s);
-if (ret < 0) {
-raw_close(bs);
-error_setg_errno(errp, -ret, "The device is not writable");
-return ret;
-}
-}
-
 return ret;
 }
 
-- 
2.25.4

Re: [RFC PATCH] tcg/cpu-exec: precise single-stepping after an exception

[Luc Michel]

On 7/16/20 11:08 PM, Richard Henderson wrote:
> On 7/16/20 1:12 PM, Peter Maydell wrote:
>> On Thu, 16 Jul 2020 at 11:08, Luc Michel  wrote:
>>>
>>> When single-stepping with a debugger attached to QEMU, and when an
>>> exception is raised, the debugger misses the first instruction after the
>>> exception:
>>
>> This is a long-standing bug; thanks for looking at it.
>> (https://bugs.launchpad.net/qemu/+bug/757702)
>>
>>
>>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>>> index d95c4848a4..e85fab5d40 100644
>>> --- a/accel/tcg/cpu-exec.c
>>> +++ b/accel/tcg/cpu-exec.c
>>> @@ -502,10 +502,21 @@ static inline bool cpu_handle_exception(CPUState 
>>> *cpu, int *ret)
>>>  CPUClass *cc = CPU_GET_CLASS(cpu);
>>>  qemu_mutex_lock_iothread();
>>>  cc->do_interrupt(cpu);
>>>  qemu_mutex_unlock_iothread();
>>>  cpu->exception_index = -1;
>>> +
>>> +if (unlikely(cpu->singlestep_enabled)) {
>>> +/*
>>> + * After processing the exception, ensure an EXCP_DEBUG is
>>> + * raised when single-stepping so that GDB doesn't miss the
>>> + * next instruction.
>>> + */
>>> +cpu->exception_index = EXCP_DEBUG;
>>> +return cpu_handle_exception(cpu, ret);
>>> +}
>>
>> I like the idea of being able to do this generically in
>> the main loop.
>>
>> How about interrupts? If we are single-stepping and we
>> take an interrupt I guess we want to stop before the first
>> insn of the interrupt handler rather than after it, which
>> would imply a similar change to cpu_handle_interrupt().
> 
> Fair.  I think something like this:
> 
> if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
> replay_interrupt();
> -   cpu->exception_index = -1;
> +   cpu->exception_index =
> +   (cpu->singlestep_enabled ? EXCP_DEBUG : -1);
> *last_tb = NULL;
> }
> 
> I'm not quite sure how to test this though...
I wrote a small test case for the interrupt side that can be run on the
virt board:


8<---
.global _start
.text

#define GIC_DIST_BASE 0x0800
#define GIC_CPU_BASE  0x0801

#define ARCH_TIMER_NS_EL1_IRQ (16 + 14)

.org 0x280
_irq_handler:
nop
nop


.org 0x1000
_start:
ldr x1, =GIC_DIST_BASE  /* GICD_CTLR */
mov w0, #1  /* enable */
str w0, [x1]

ldr x1, =GIC_DIST_BASE + 0x100 /* GICD_ISENABLER0 */
ldr w0, =(1 << ARCH_TIMER_NS_EL1_IRQ)
str w0, [x1]

ldr x1, =GIC_CPU_BASE   /* GICC_CTLR */
mov w0, #1  /* enable */
str w0, [x1]

ldr x1, =GIC_CPU_BASE + 0x4  /* GICC_PMR */
mov w0, #255  /* min priority */
str w0, [x1]

msr daifclr, 2  /* unmask IRQ line */

mov x0, 10  /* timer will trigger in 10 ticks */
msr cntp_tval_el0, x0

mov x0, 1   /* enable timer */
enable_timer:
msr cntp_ctl_el0, x0

1:
b 1b

8<-

$ aarch64-linux-gnu-gcc -g -nostdinc -nostdlib -Wl,-Ttext=0x0 -o foo foo.S

$ qemu-system-aarch64 -display none -M virt -cpu cortex-a53 -kernel foo
-s -S

$ aarch64-linux-gnu-gdb foo
(gdb) tar rem :1234
(gdb) maintenance packet Qqemu.sstep=0x1
(gdb) b enable_timer
(gdb) disp /i $pc
(gdb) c
Continuing.

Breakpoint 1, enable_timer () at foo.S:40
1: x/i $pc
=> 0x1040 :   msr cntp_ctl_el0, x0

(gdb) si
1: x/i $pc
=> 0x1044 : b   0x1044 

(gdb)
1: x/i $pc
=> 0x280 <_irq_handler>:nop

This is with your fix. Without it, the second stepi stops on 0x284.

> 
> Probably best to keep this a separate patch anyway.
Do you want me to send it? If yes, how should I give credit to you?
Should I put your Signed-off-by: in it?

thanks

Luc

> 
> 
> r~
> 

Re: [PATCH for-5.1 v2 1/2] block: Require aligned image size to avoid assertion failure

[Max Reitz]

On 16.07.20 16:26, Kevin Wolf wrote:
> Unaligned requests will automatically be aligned to bl.request_alignment
> and we can't extend write requests to access space beyond the end of the
> image without resizing the image, so if we have the WRITE permission,
> but not the RESIZE one, it's required that the image size is aligned.
> 
> Failing to meet this requirement could cause assertion failures like
> this if RESIZE permissions weren't requested:
> 
> qemu-img: block/io.c:1910: bdrv_co_write_req_prepare: Assertion `end_sector 
> <= bs->total_sectors || child->perm & BLK_PERM_RESIZE' failed.
> 
> This was e.g. triggered by qemu-img converting to a target image with 4k
> request alignment when the image was only aligned to 512 bytes, but not
> to 4k.
> 
> Turn this into a graceful error in bdrv_check_perm() so that WRITE
> without RESIZE can only be taken if the image size is aligned. If a user
> holds both permissions and drops only RESIZE, the function will return
> an error, but bdrv_child_try_set_perm() will ignore the failure silently
> if permissions are only requested to be relaxed and just keep both
> permissions while returning success.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/block.c b/block.c
> index 35a372df57..6371928edb 100644
> --- a/block.c
> +++ b/block.c
> @@ -2025,6 +2025,22 @@ static int bdrv_check_perm(BlockDriverState *bs, 
> BlockReopenQueue *q,
>  return -EPERM;
>  }
>  
> +/*
> + * Unaligned requests will automatically be aligned to 
> bl.request_alignment
> + * and without RESIZE we can't extend requests to write to space beyond 
> the
> + * end of the image, so it's required that the image size is aligned.
> + */
> +if ((cumulative_perms & BLK_PERM_WRITE) &&

What about WRITE_UNCHANGED?  I think this would only matter with nodes
that can have backing files (i.e., qcow2 in practice) because
WRITE_UNCHANGED is only used by COR and block jobs doing something with
a backing chain, so it shouldn’t matter in practice, but, well.

So, either way:

Reviewed-by: Max Reitz 

> +!(cumulative_perms & BLK_PERM_RESIZE))
> +{
> +if ((bs->total_sectors * BDRV_SECTOR_SIZE) % 
> bs->bl.request_alignment) {
> +error_setg(errp, "Cannot get 'write' permission without 
> 'resize': "
> + "Image size is not a multiple of request "
> + "alignment");
> +return -EPERM;
> +}
> +}
> +
>  /* Check this node */
>  if (!drv) {
>  return 0;
> 




signature.asc
Description: OpenPGP digital signature

Re: [PULL 28/53] Makefile: simplify MINIKCONF rules

[Peter Maydell]

On Mon, 6 Jul 2020 at 18:03, Paolo Bonzini  wrote:
>
> There is no reason to write MINIKCONF_DEPS manually, since minikconf.py
> emits a dependency file, and also no reason to list multiple Kconfig
> files on the command line since they can be included from a master file
> in the top-level source directory.
>
> Signed-off-by: Paolo Bonzini 

Hi Paolo. With this change Make seems no longer to realise
that a change to a Kconfig file means it needs to rebuild
the config-devices.mak, which means that we tend to get
weird compile failures for changes which update Kconfig files.

Specifically, this is preventing me getting a clean set of
builds for Corey's latest i2c pullreq. Reverting this
commit allows the pullreq to build on all configs.

> --- a/Makefile
> +++ b/Makefile
> @@ -404,7 +404,7 @@ endif
>  # This has to be kept in sync with Kconfig.host.
>  MINIKCONF_ARGS = \
>  $(CONFIG_MINIKCONF_MODE) \
> -$@ $*/config-devices.mak.d $< $(MINIKCONF_INPUTS) \
> +$@ $*/config-devices.mak.d $< $(SRC_PATH)/Kconfig \
>  CONFIG_TCG=$(CONFIG_TCG) \
>  CONFIG_KVM=$(CONFIG_KVM) \
>  CONFIG_SPICE=$(CONFIG_SPICE) \
> @@ -419,15 +419,9 @@ MINIKCONF_ARGS = \
>  CONFIG_LINUX=$(CONFIG_LINUX) \
>  CONFIG_PVRDMA=$(CONFIG_PVRDMA)
>
> -MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host \
> -   $(SRC_PATH)/backends/Kconfig \
> -   $(SRC_PATH)/accel/Kconfig \
> -   $(SRC_PATH)/hw/Kconfig
> -MINIKCONF_DEPS = $(MINIKCONF_INPUTS) \
> - $(wildcard $(SRC_PATH)/hw/*/Kconfig)
>  MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py
>
> -$(SUBDIR_DEVICES_MAK): %/config-devices.mak: default-configs/%.mak 
> $(MINIKCONF_DEPS) $(BUILD_DIR)/config-host.mak
> +$(SUBDIR_DEVICES_MAK): %/config-devices.mak: default-configs/%.mak 
> $(SRC_PATH)/Kconfig $(BUILD_DIR)/config-host.mak

Specifically here, the $(MINIKCONF_DEPS) long list of Kconfig
files has been removed from the dependency list of
config-devices.mak.

There doesn't seem to be any machinery for creating .d
files for make to include to tell it that Kconfig has a
dependency on hw/Kconfig which has a dependency on hw/i2c/Kconfig etc.
How is this intended to work ?

For 5.1, should we just revert this commit ?

thanks
-- PMM

Re: [PATCH v2] introduce VFIO-over-socket protocol specificaion

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1594981096-58580-1-git-send-email-thanos.maka...@nutanix.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1594981096-58580-1-git-send-email-thanos.maka...@nutanix.com
Subject: [PATCH v2] introduce VFIO-over-socket protocol specificaion

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
c657ae6 introduce VFIO-over-socket protocol specificaion

=== OUTPUT BEGIN ===
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#47: 
new file mode 100644

ERROR: trailing whitespace
#905: FILE: docs/devel/vfio-over-socket.rst:854:
+guest unmasks the interrupt. $

total: 1 errors, 1 warnings, 1151 lines checked

Commit c657ae6fb755 (introduce VFIO-over-socket protocol specificaion) has 
style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1594981096-58580-1-git-send-email-thanos.maka...@nutanix.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH for-5.1 v2 2/2] file-posix: Allow byte-aligned O_DIRECT with NFS

[Max Reitz]

On 16.07.20 16:26, Kevin Wolf wrote:
> Since commit a6b257a08e3 ('file-posix: Handle undetectable alignment'),
> we assume that if we open a file with O_DIRECT and alignment probing
> returns 1, we just couldn't find out the real alignment requirement
> because some filesystems make the requirement only for allocated blocks.
> In this case, a safe default of 4k is used.
> 
> This is too strict for NFS, which does actually allow byte-aligned
> requests even with O_DIRECT. Because we can't distinguish both cases
> with generic code, let's just look at the file system magic and disable
> s->needs_alignment for NFS. This way, O_DIRECT can still be used on NFS
> for images that are not aligned to 4k.
> 
> Signed-off-by: Kevin Wolf 
> Reviewed-by: Eric Blake 
> ---
>  block/file-posix.c | 26 +-
>  1 file changed, 25 insertions(+), 1 deletion(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [PULL 28/53] Makefile: simplify MINIKCONF rules

[Paolo Bonzini]

Il ven 17 lug 2020, 13:03 Peter Maydell  ha
scritto:

> On Mon, 6 Jul 2020 at 18:03, Paolo Bonzini  wrote:
> >
> > There is no reason to write MINIKCONF_DEPS manually, since minikconf.py
> > emits a dependency file, and also no reason to list multiple Kconfig
> > files on the command line since they can be included from a master file
> > in the top-level source directory.
> >
> > Signed-off-by: Paolo Bonzini 
>
> Hi Paolo. With this change Make seems no longer to realise
> that a change to a Kconfig file means it needs to rebuild
> the config-devices.mak, which means that we tend to get
> weird compile failures for changes which update Kconfig files.
>
> Specifically here, the $(MINIKCONF_DEPS) long list of Kconfig
> files has been removed from the dependency list of
> config-devices.mak.
>
> There doesn't seem to be any machinery for creating .d
> files for make to include to tell it that Kconfig has a
> dependency on hw/Kconfig which has a dependency on hw/i2c/Kconfig etc.
> How is this intended to work ?
>

I cannot look at a build tree right now, but shouldn't that be in the .d
file produced by minikconf.py Those are passed to minikconf.py as the
second argument and included with "include $(SUBDIR_DEVICES_MAK_DEP)".

No objection against reverting though.

Paolo


> For 5.1, should we just revert this commit ?
>
> thanks
> -- PMM
>
>

[PATCH v3] introduce VFIO-over-socket protocol specificaion

[Thanos Makatos]

This patch introduces the VFIO-over-socket protocol specification, which
is designed to allow devices to be emulated outside QEMU, in a separate
process. VFIO-over-socket reuses the existing VFIO defines, structs and
concepts.

It has been earlier discussed as an RFC in:
"RFC: use VFIO over a UNIX domain socket to implement device offloading"

Signed-off-by: John G Johnson 
Signed-off-by: Thanos Makatos 

---

Changed since v1:
  * fix coding style issues
  * update MAINTAINERS for VFIO-over-socket
  * add vfio-over-socket to ToC

Changed since v2:
  * fix whitespace

Regarding the build failure, I have not been able to reproduce it locally
using the docker image on my Debian 10.4 machine.
---
 MAINTAINERS |6 +
 docs/devel/index.rst|1 +
 docs/devel/vfio-over-socket.rst | 1135 +++
 3 files changed, 1142 insertions(+)
 create mode 100644 docs/devel/vfio-over-socket.rst

diff --git a/MAINTAINERS b/MAINTAINERS
index 030faf0..bb81590 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1732,6 +1732,12 @@ F: hw/vfio/ap.c
 F: docs/system/s390x/vfio-ap.rst
 L: qemu-s3...@nongnu.org
 
+VFIO-over-socket
+M: John G Johnson 
+M: Thanos Makatos 
+S: Supported
+F: docs/devel/vfio-over-socket.rst
+
 vhost
 M: Michael S. Tsirkin 
 S: Supported
diff --git a/docs/devel/index.rst b/docs/devel/index.rst
index ae6eac7..0439460 100644
--- a/docs/devel/index.rst
+++ b/docs/devel/index.rst
@@ -30,3 +30,4 @@ Contents:
reset
s390-dasd-ipl
clocks
+   vfio-over-socket
diff --git a/docs/devel/vfio-over-socket.rst b/docs/devel/vfio-over-socket.rst
new file mode 100644
index 000..b474f23
--- /dev/null
+++ b/docs/devel/vfio-over-socket.rst
@@ -0,0 +1,1135 @@
+***
+VFIO-over-socket Protocol Specification
+***
+
+Version 0.1
+
+Introduction
+
+VFIO-over-socket, also known as vfio-user, is a protocol that allows a device
+to be virtualized in a separate process outside of QEMU. VFIO-over-socket
+devices consist of a generic VFIO device type, living inside QEMU, which we
+call the client, and the core device implementation, living outside QEMU, which
+we call the server. VFIO-over-socket can be the main transport mechanism for
+multi-process QEMU, however it can be used by other applications offering
+device virtualization. Explaining the advantages of a
+disaggregated/multi-process QEMU, and device virtualization outside QEMU in
+general, is beyond the scope of this document.
+
+This document focuses on specifying the VFIO-over-socket protocol. VFIO has
+been chosen for the following reasons:
+
+1) It is a mature and stable API, backed by an extensively used framework.
+2) The existing VFIO client implementation (qemu/hw/vfio/) can be largely
+   reused.
+
+In a proof of concept implementation it has been demonstrated that using VFIO
+over a UNIX domain socket is a viable option. VFIO-over-socket is designed with
+QEMU in mind, however it could be used by other client applications. The
+VFIO-over-socket protocol does not require that QEMU's VFIO client
+implementation is used in QEMU. None of the VFIO kernel modules are required
+for supporting the protocol, neither in the client nor the server, only the
+source header files are used.
+
+The main idea is to allow a virtual device to function in a separate process in
+the same host over a UNIX domain socket. A UNIX domain socket (AF_UNIX) is
+chosen because we can trivially send file descriptors over it, which in turn
+allows:
+
+* Sharing of guest memory for DMA with the virtual device process.
+* Sharing of virtual device memory with the guest for fast MMIO.
+* Efficient sharing of eventfd's for triggering interrupts.
+
+However, other socket types could be used which allows the virtual device
+process to run in a separate guest in the same host (AF_VSOCK) or remotely
+(AF_INET). Theoretically the underlying transport doesn't necessarily have to
+be a socket, however we don't examine such alternatives. In this document we
+focus on using a UNIX domain socket and introduce basic support for the other
+two types of sockets without considering performance implications.
+
+This document does not yet describe any internal details of the server-side
+implementation, however QEMU's VFIO client implementation will have to be
+adapted according to this protocol in order to support VFIO-over-socket virtual
+devices.
+
+VFIO
+
+VFIO is a framework that allows a physical device to be securely passed through
+to a user space process; the kernel does not drive the device at all.
+Typically, the user space process is a VM and the device is passed through to
+it in order to achieve high performance. VFIO provides an API and the required
+functionality in the kernel. QEMU has adopted VFIO to allow a guest virtual
+machine to directly access physical devices, instead of emulating them in
+software
+
+VFIO-over-socket reuses the core VFI

Re: [GIT PULL] I2C updates

[Philippe Mathieu-Daudé]

On 7/17/20 12:50 PM, Peter Maydell wrote:
> On Thu, 16 Jul 2020 at 23:26, Corey Minyard  wrote:
>>
>> On Thu, Jul 16, 2020 at 09:45:41PM +0100, Peter Maydell wrote:
>>> Hi; this failed to build on x86-64 Linux (incremental build):
>>
>> Hmm, I did test this, and I just rebuilt, then rebased on the end of
>> master and rebuilt, without issue.
>>
>> It looks like the smbus code is not being included, but I don't see how
>> that can be.
> 
> I was wrong about which config failed, sorry. Incremental builds
> are fine, but the build that does "make -C builddir clean" first
> fails.
> 
> It looks like the problem is that in the created config-devices.mak
> files, CONFIG_SMBUS_EEPROM is set, but CONFIG_SMBUS is not.
> So presumably the problem is in the change
> "hw/i2c/Kconfig: Add an entry for the SMBus", or it is a more
> general issue with changes to Kconfig files not correctly
> resulting in rebuilds of config-devices.mak.

To Corey, this is likely the later (buildsys), see:
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg05475.html

Re: [PATCH for-5.1 v2 1/2] block: Require aligned image size to avoid assertion failure

[Kevin Wolf]

Am 17.07.2020 um 13:02 hat Max Reitz geschrieben:
> On 16.07.20 16:26, Kevin Wolf wrote:
> > Unaligned requests will automatically be aligned to bl.request_alignment
> > and we can't extend write requests to access space beyond the end of the
> > image without resizing the image, so if we have the WRITE permission,
> > but not the RESIZE one, it's required that the image size is aligned.
> > 
> > Failing to meet this requirement could cause assertion failures like
> > this if RESIZE permissions weren't requested:
> > 
> > qemu-img: block/io.c:1910: bdrv_co_write_req_prepare: Assertion `end_sector 
> > <= bs->total_sectors || child->perm & BLK_PERM_RESIZE' failed.
> > 
> > This was e.g. triggered by qemu-img converting to a target image with 4k
> > request alignment when the image was only aligned to 512 bytes, but not
> > to 4k.
> > 
> > Turn this into a graceful error in bdrv_check_perm() so that WRITE
> > without RESIZE can only be taken if the image size is aligned. If a user
> > holds both permissions and drops only RESIZE, the function will return
> > an error, but bdrv_child_try_set_perm() will ignore the failure silently
> > if permissions are only requested to be relaxed and just keep both
> > permissions while returning success.
> > 
> > Signed-off-by: Kevin Wolf 
> > ---
> >  block.c | 16 
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/block.c b/block.c
> > index 35a372df57..6371928edb 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -2025,6 +2025,22 @@ static int bdrv_check_perm(BlockDriverState *bs, 
> > BlockReopenQueue *q,
> >  return -EPERM;
> >  }
> >  
> > +/*
> > + * Unaligned requests will automatically be aligned to 
> > bl.request_alignment
> > + * and without RESIZE we can't extend requests to write to space 
> > beyond the
> > + * end of the image, so it's required that the image size is aligned.
> > + */
> > +if ((cumulative_perms & BLK_PERM_WRITE) &&
> 
> What about WRITE_UNCHANGED?  I think this would only matter with nodes
> that can have backing files (i.e., qcow2 in practice) because
> WRITE_UNCHANGED is only used by COR and block jobs doing something with
> a backing chain, so it shouldn’t matter in practice, but, well.

So basically just replacing the line with this?

if ((cumulative_perms & (BLK_PERM_WRITE | BDRV_PERM_WRITE_UNCHANGED)) &&

I can do that while applying if it is what you mean.

> So, either way:
> 
> Reviewed-by: Max Reitz 

Thanks!

Kevin

> > +!(cumulative_perms & BLK_PERM_RESIZE))
> > +{
> > +if ((bs->total_sectors * BDRV_SECTOR_SIZE) % 
> > bs->bl.request_alignment) {
> > +error_setg(errp, "Cannot get 'write' permission without 
> > 'resize': "
> > + "Image size is not a multiple of request "
> > + "alignment");
> > +return -EPERM;
> > +}
> > +}
> > +
> >  /* Check this node */
> >  if (!drv) {
> >  return 0;
> > 
> 
> 





signature.asc
Description: PGP signature

Re: [PATCH for-5.1 v2 1/2] block: Require aligned image size to avoid assertion failure

[Max Reitz]

On 17.07.20 13:32, Kevin Wolf wrote:
> Am 17.07.2020 um 13:02 hat Max Reitz geschrieben:
>> On 16.07.20 16:26, Kevin Wolf wrote:
>>> Unaligned requests will automatically be aligned to bl.request_alignment
>>> and we can't extend write requests to access space beyond the end of the
>>> image without resizing the image, so if we have the WRITE permission,
>>> but not the RESIZE one, it's required that the image size is aligned.
>>>
>>> Failing to meet this requirement could cause assertion failures like
>>> this if RESIZE permissions weren't requested:
>>>
>>> qemu-img: block/io.c:1910: bdrv_co_write_req_prepare: Assertion `end_sector 
>>> <= bs->total_sectors || child->perm & BLK_PERM_RESIZE' failed.
>>>
>>> This was e.g. triggered by qemu-img converting to a target image with 4k
>>> request alignment when the image was only aligned to 512 bytes, but not
>>> to 4k.
>>>
>>> Turn this into a graceful error in bdrv_check_perm() so that WRITE
>>> without RESIZE can only be taken if the image size is aligned. If a user
>>> holds both permissions and drops only RESIZE, the function will return
>>> an error, but bdrv_child_try_set_perm() will ignore the failure silently
>>> if permissions are only requested to be relaxed and just keep both
>>> permissions while returning success.
>>>
>>> Signed-off-by: Kevin Wolf 
>>> ---
>>>  block.c | 16 
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/block.c b/block.c
>>> index 35a372df57..6371928edb 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -2025,6 +2025,22 @@ static int bdrv_check_perm(BlockDriverState *bs, 
>>> BlockReopenQueue *q,
>>>  return -EPERM;
>>>  }
>>>  
>>> +/*
>>> + * Unaligned requests will automatically be aligned to 
>>> bl.request_alignment
>>> + * and without RESIZE we can't extend requests to write to space 
>>> beyond the
>>> + * end of the image, so it's required that the image size is aligned.
>>> + */
>>> +if ((cumulative_perms & BLK_PERM_WRITE) &&
>>
>> What about WRITE_UNCHANGED?  I think this would only matter with nodes
>> that can have backing files (i.e., qcow2 in practice) because
>> WRITE_UNCHANGED is only used by COR and block jobs doing something with
>> a backing chain, so it shouldn’t matter in practice, but, well.
> 
> So basically just replacing the line with this?
> 
> if ((cumulative_perms & (BLK_PERM_WRITE | BDRV_PERM_WRITE_UNCHANGED)) &&
> 
> I can do that while applying if it is what you mean.

Yes. :)

>> So, either way:
>>
>> Reviewed-by: Max Reitz 
> 
> Thanks!
> 
> Kevin
> 
>>> +!(cumulative_perms & BLK_PERM_RESIZE))
>>> +{
>>> +if ((bs->total_sectors * BDRV_SECTOR_SIZE) % 
>>> bs->bl.request_alignment) {
>>> +error_setg(errp, "Cannot get 'write' permission without 
>>> 'resize': "
>>> + "Image size is not a multiple of request "
>>> + "alignment");
>>> +return -EPERM;
>>> +}
>>> +}
>>> +
>>>  /* Check this node */
>>>  if (!drv) {
>>>  return 0;
>>>
>>
>>
> 
> 
> 




signature.asc
Description: OpenPGP digital signature

Re: [PATCH] virtiofsd: Remove "norace" from cmdline help

[Sergio Lopez]

On Fri, Jul 17, 2020 at 11:14:14AM +0200, Stefano Garzarella wrote:
> On Thu, Jul 16, 2020 at 12:14:42PM +0200, Sergio Lopez wrote:
> > Commit 93bb3d8d4cda ("virtiofsd: remove symlink fallbacks") removed
> > the implementation of the "norace" option, so remove it from the
> > cmdline help too.
> > 
> > Signed-off-by: Sergio Lopez 
> > ---
> >  tools/virtiofsd/helper.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
> > index 3105b6c23a..7bc5d7dc5a 100644
> > --- a/tools/virtiofsd/helper.c
> > +++ b/tools/virtiofsd/helper.c
> > @@ -159,8 +159,6 @@ void fuse_cmdline_help(void)
> > "-o max_idle_threadsthe maximum number of idle 
> > worker "
> > "threads\n"
> > "   allowed (default: 10)\n"
> > -   "-o norace  disable racy fallback\n"
> > -   "   default: false\n"
> > "-o posix_lock|no_posix_lock\n"
> > "   enable/disable remote posix 
> > lock\n"
> > "   default: posix_lock\n"
> > -- 
> > 2.26.2
> > 
> > 
> 
> I noticed that 'norace' is also described in docs/tools/virtiofsd.rst,
> so I think we need to remove it there too:
> 
> diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
> index 824e713491..58666a4495 100644
> --- a/docs/tools/virtiofsd.rst
> +++ b/docs/tools/virtiofsd.rst
> @@ -63,9 +63,6 @@ Options
>  Print only log messages matching LEVEL or more severe.  LEVEL is one 
> of
>  ``err``, ``warn``, ``info``, or ``debug``.  The default is ``info``.
> 
> -  * norace -
> -Disable racy fallback.  The default is false.
> -
>* posix_lock|no_posix_lock -
>  Enable/disable remote POSIX locks.  The default is ``posix_lock``.

Good catch, thanks. I'll send a v2.

Sergio.

> With that fixed:
> Reviewed-by: Stefano Garzarella 
> 
> Thanks,
> Stefano
> 


signature.asc
Description: PGP signature

Re: [PATCH for-5.1 1/3] file-posix: Move check_hdev_writable() up

[Max Reitz]

On 17.07.20 12:54, Kevin Wolf wrote:
> We'll need to call it in raw_open_common(), so move the function to
> avoid a forward declaration.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/file-posix.c | 66 +++---
>  1 file changed, 33 insertions(+), 33 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [PATCH for-5.1 0/3] file-posix: Fix check_hdev_writable() with auto-read-only

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200717105426.51134-1-kw...@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TESTcheck-qtest-x86_64: tests/qtest/qmp-test
  TESTcheck-qtest-x86_64: tests/qtest/qmp-cmd-test
**
ERROR:/tmp/qemu-test/src/tests/qtest/libqtest.c:97:init_socket: assertion 
failed (ret != -1): (-1 != -1)
ERROR qmp-cmd-test - Bail out! 
ERROR:/tmp/qemu-test/src/tests/qtest/libqtest.c:97:init_socket: assertion 
failed (ret != -1): (-1 != -1)
make: *** [check-qtest-x86_64] Error 1
make: *** Waiting for unfinished jobs
  TESTiotest-qcow2: 080
  TESTiotest-qcow2: 086
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=fc3463a17cea454c9a0fcd472358db4f', '-u', 
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-c3bzzbt8/src/docker-src.2020-07-17-07.34.41.28182:/var/tmp/qemu:z,ro',
 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=fc3463a17cea454c9a0fcd472358db4f
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-c3bzzbt8/src'
make: *** [docker-run-test-quick@centos7] Error 2

real14m51.065s
user0m8.978s


The full log is available at
http://patchew.org/logs/20200717105426.51134-1-kw...@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH for-5.1 2/3] file-posix: Fix check_hdev_writable() with auto-read-only

[Max Reitz]

On 17.07.20 12:54, Kevin Wolf wrote:
> For Linux block devices, being able to open the device read-write
> doesn't necessarily mean that the device is actually writable (one
> example is a read-only LV, as you get with lvchange -pr ). We
> have check_hdev_writable() to check this condition and fail opening the
> image read-write if it's not actually writable.
> 
> However, this check doesn't take auto-read-only into account, but
> results in a hard failure instead of downgrading to read-only where
> possible.
> 
> Fix this and do the writable check not based on BDRV_O_RDWR, but only
> when this actually results in opening the file read-write. A second
> check is inserted in raw_reconfigure_getfd() to have the same check when
> dynamic auto-read-only upgrades an image file from read-only to
> read-write.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/file-posix.c | 33 +
>  1 file changed, 21 insertions(+), 12 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [PATCH for-5.1 3/3] file-posix: Fix leaked fd in raw_open_common() error path

[Max Reitz]

On 17.07.20 12:54, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  block/file-posix.c | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

[Bug 1887854] Re: Spurious Data Abort on qemu-system-aarch64

[K]

I would have thought that TLB considerations would not apply when the
MMU is disabled (RTEMS runs in a completely flat memory space). I'll try
to reproduce on more modern QEMU today. Thanks for taking a look at
this.

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

Title:
  Spurious Data Abort on qemu-system-aarch64

Status in QEMU:
  New

Bug description:
  When running RTEMS test psxndbm01.exe built for AArch64-ilp32 (this code is 
not yet publically available), the test generates a spurious data abort (the 
MMU and alignment checks should be disabled according to bits 1, 0 of 
SCTLR_EL1). The abort information is as follows:
  Taking exception 4 [Data Abort]
  ...from EL1 to EL1
  ...with ESR 0x25/0x9610
  ...with FAR 0x104010ca28
  ...with ELR 0x400195d8
  ...to EL1 PC 0x40018200 PSTATE 0x3c5

  The ESR indicates that a synchronous external abort has occurred.
  ESR EC field: 0b100101

  From the ARMv8 technical manual: Data Abort taken without a change in
  Exception level. Used for MMU faults generated by data accesses,
  alignment faults other than those caused by Stack Pointer
  misalignment, and synchronous External aborts, including synchronous
  parity or ECC errors. Not used for debug related exceptions.

  ESR ISS field: 0b1

  From the ARMv8 technical manual: Synchronous External abort, not on
  translation table walk or hardware update of translation table.

  The following command line is used to invoke qemu:
  qemu-system-aarch64 -machine virt -cpu cortex-a53 -m 256M -no-reboot 
-nographic -serial mon:stdio -kernel 
build/aarch64/a53_ilp32_qemu/testsuites/psxtests/psxndbm01.exe -D qemu.log -d 
in_asm,int,cpu_reset,unimp,guest_errors

  This occurs on Qemu 3.1.0 as distributed via Debian and on Qemu 4.1 as
  built by the RTEMS source builder (4.1+minor patches).

  Edit: This bug can be worked around by getting and setting SCTLR
  without changing its value before each data abort would occur. This
  test needs 6 of these workarounds to operate successfully.

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

Re: [PATCH v3 for-5.1 0/2] Fix crash due to NBD export leak

[Kevin Wolf]

Am 14.07.2020 um 18:22 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Hi all!
> 
> We've faced crash bug, which is reproducing on master branch as well.
> The case is described in 01, where fix is suggested.
> New iotest in 02 crashes without that fix.
> 
> v3: resend for convenience, as all preparatory patches are merged.
> 01-02: add Eric's r-b and t-b marks
> 
> 
> 
> This is a crash-fix, so it would be good to fix in 5.1. Still neither
> Eric nor I are sure in patch 01: is AIO_WAIT_WHILE used correctly?

Anything specific you had doubts about?

At first sight it looks good to me. It's always called in the main loop
and we don't hold any AioContext locks, so using NULL as the context is
fine. You also made sure to use aio_wait_kick() so that we won't hang
even though the condition has become false.

I'm applying this to my block branch now. If your doubts were about
something more subtle that I missed, we can unstage/revert the patch.

Kevin

[PATCH v2] virtiofsd: Remove "norace" from cmdline help and docs

[Sergio Lopez]

Commit 93bb3d8d4cda ("virtiofsd: remove symlink fallbacks") removed
the implementation of the "norace" option, so remove it from the
cmdline help and the documentation too.

Signed-off-by: Sergio Lopez 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Stefano Garzarella 
---
v2:
 * Drop "norace" from the documentation too (Stefano Garzarella)
---
 docs/tools/virtiofsd.rst | 3 ---
 tools/virtiofsd/helper.c | 2 --
 2 files changed, 5 deletions(-)

diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
index 824e713491..58666a4495 100644
--- a/docs/tools/virtiofsd.rst
+++ b/docs/tools/virtiofsd.rst
@@ -63,9 +63,6 @@ Options
 Print only log messages matching LEVEL or more severe.  LEVEL is one of
 ``err``, ``warn``, ``info``, or ``debug``.  The default is ``info``.
 
-  * norace -
-Disable racy fallback.  The default is false.
-
   * posix_lock|no_posix_lock -
 Enable/disable remote POSIX locks.  The default is ``posix_lock``.
 
diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
index 3105b6c23a..7bc5d7dc5a 100644
--- a/tools/virtiofsd/helper.c
+++ b/tools/virtiofsd/helper.c
@@ -159,8 +159,6 @@ void fuse_cmdline_help(void)
"-o max_idle_threadsthe maximum number of idle worker "
"threads\n"
"   allowed (default: 10)\n"
-   "-o norace  disable racy fallback\n"
-   "   default: false\n"
"-o posix_lock|no_posix_lock\n"
"   enable/disable remote posix lock\n"
"   default: posix_lock\n"
-- 
2.26.2

Re: [PATCH] slirp: update to v4.3.1

[Marc-André Lureau]

Hi

On Mon, Jul 13, 2020 at 12:33 PM Marc-André Lureau <
marcandre.lur...@redhat.com> wrote:

> Switch from stable-4.2 branch back to master (which is actually
> maintained, I think we tend to forget about stable...).
>
> git shortlog 2faae0f7..a62d3673:
>
> 5eraph (2):
>   disable_dns option
>   limit vnameserver_addr to port 53
>
> Akihiro Suda (1):
>   libslirp.h: fix SlirpConfig v3 documentation
>
> Jindrich Novy (4):
>   Fix possible infinite loops and use-after-free
>   Use secure string copy to avoid overflow
>   Be sure to initialize sockaddr structure
>   Check lseek() for failure
>
> Marc-André Lureau (12):
>   Merge branch 'master' into 'master'
>   Merge branch 'fix-slirpconfig-3-doc' into 'master'
>   Fix use-afte-free in ip_reass() (CVE-2020-1983)
>   Update CHANGELOG
>   Merge branch 'cve-2020-1983' into 'master'
>   Release v4.3.0
>   Merge branch 'release-v4.3.0' into 'master'
>   changelog: post-release
>   util: do not silently truncate
>   Merge branch 'slirp-fmt-truncate' into 'master'
>   Release v4.3.1
>   Merge branch 'release-v4.3.1' into 'master'
>
> Philippe Mathieu-Daudé (3):
>   Fix win32 builds by using the SLIRP_PACKED definition
>   Fix constness warnings
>   Remove unnecessary break
>
> Ralf Haferkamp (2):
>   Drop bogus IPv6 messages
>   Fix MTU check
>
> Samuel Thibault (1):
>   Merge branch 'ip6_payload_len' into 'master'
>
> Signed-off-by: Marc-André Lureau 
> ---
>  slirp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/slirp b/slirp
> index 2faae0f778f..a62d36734ff 16
> --- a/slirp
> +++ b/slirp
> @@ -1 +1 @@
> -Subproject commit 2faae0f778f818fadc873308f983289df697eb93
> +Subproject commit a62d36734ffe9828d0f70df1b3898a3b4fbda755
> --
> 2.27.0.221.ga08a83db2b
>
>
>
Anyone willing to ack this, or nack it and then someone (me?) will have to
backport the fixes in the slirp stable-4.2 branch.


-- 
Marc-André Lureau

Re: [PATCH v6 07/13] hw/arm: Load -bios image as a boot ROM for npcm7xx

[Cédric Le Goater]

On 7/17/20 8:02 AM, Havard Skinnemoen wrote:
> If a -bios option is specified on the command line, load the image into
> the internal ROM memory region, which contains the first instructions
> run by the CPU after reset.
> 
> If -bios is not specified, the vbootrom included with qemu is loaded by
> default.
> 
> Reviewed-by: Tyrone Ting 
> Signed-off-by: Havard Skinnemoen 
> ---
>  hw/arm/npcm7xx_boards.c | 31 +++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
> index 0b9dce2b35..f32557e0e1 100644
> --- a/hw/arm/npcm7xx_boards.c
> +++ b/hw/arm/npcm7xx_boards.c
> @@ -18,12 +18,41 @@
>  
>  #include "hw/arm/npcm7xx.h"
>  #include "hw/core/cpu.h"
> +#include "hw/loader.h"
>  #include "qapi/error.h"
> +#include "qemu-common.h"
>  #include "qemu/units.h"
> +#include "sysemu/sysemu.h"
>  
>  #define NPCM750_EVB_POWER_ON_STRAPS 0x1ff7
>  #define QUANTA_GSJ_POWER_ON_STRAPS 0x1fff
>  
> +static const char npcm7xx_default_bootrom[] = "npcm7xx_bootrom.bin";
> +
> +static void npcm7xx_load_bootrom(NPCM7xxState *soc)
> +{
> +g_autofree char *filename = NULL;
> +const char *bootrom;
> +int ret;
> +
> +if (bios_name) {
> +bootrom = bios_name;
> +} else {
> +bootrom = npcm7xx_default_bootrom;

you could simply assign bios_name. No need to resend for that.

Reviewed-by: Cédric Le Goater 

C.

> +}
> +
> +filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bootrom);
> +if (!filename) {
> +error_report("Could not find ROM image '%s'", bootrom);
> +exit(1);
> +}
> +ret = load_image_mr(filename, &soc->irom);
> +if (ret < 0) {
> +error_report("Failed to load ROM image '%s'", filename);
> +exit(1);
> +}
> +}
> +
>  static void npcm7xx_connect_dram(NPCM7xxState *soc, MemoryRegion *dram)
>  {
>  memory_region_add_subregion(get_system_memory(), NPCM7XX_DRAM_BA, dram);
> @@ -60,6 +89,7 @@ static void npcm750_evb_init(MachineState *machine)
>  npcm7xx_connect_dram(soc, machine->ram);
>  qdev_realize(DEVICE(soc), NULL, &error_fatal);
>  
> +npcm7xx_load_bootrom(soc);
>  npcm7xx_load_kernel(machine, soc);
>  }
>  
> @@ -71,6 +101,7 @@ static void quanta_gsj_init(MachineState *machine)
>  npcm7xx_connect_dram(soc, machine->ram);
>  qdev_realize(DEVICE(soc), NULL, &error_fatal);
>  
> +npcm7xx_load_bootrom(soc);
>  npcm7xx_load_kernel(machine, soc);
>  }
>  
> 

Re: [PATCH] introduce VFIO-over-socket protocol specificaion

[Stefan Hajnoczi]

On Thu, Jul 16, 2020 at 08:31:43AM -0700, Thanos Makatos wrote:
> This patch introduces the VFIO-over-socket protocol specification, which
> is designed to allow devices to be emulated outside QEMU, in a separate
> process. VFIO-over-socket reuses the existing VFIO defines, structs and
> concepts.
> 
> It has been earlier discussed as an RFC in:
> "RFC: use VFIO over a UNIX domain socket to implement device offloading"
> 
> Signed-off-by: John G Johnson 
> Signed-off-by: Thanos Makatos 
> ---
>  docs/devel/vfio-over-socket.rst | 1135 
> +++
>  1 files changed, 1135 insertions(+), 0 deletions(-)
>  create mode 100644 docs/devel/vfio-over-socket.rst

This is exciting! The spec is clear enough that I feel I could start
writing a client/server. There is enough functionality here to implement
real-world devices. Can you share links to client/server
implementations?

It would be useful to introduce a standard way of enumerating,
launching, and controlling device emulation processes. That doesn't need
to be part of this specification document though. In vhost-user there
are conventions for command-line parameters, process lifecycle, etc that
make it easier for management tools to run device processes (the
"Backend program conventions" section in vhost-user.rst).

> diff --git a/docs/devel/vfio-over-socket.rst b/docs/devel/vfio-over-socket.rst
> new file mode 100644
> index 000..723b944
> --- /dev/null
> +++ b/docs/devel/vfio-over-socket.rst
> @@ -0,0 +1,1135 @@
> +***
> +VFIO-over-socket Protocol Specification
> +***
> +
> +Version 0.1

Please include a reference to the section below explaining how
versioning works.

Also, are there rules about noting versions when updating the spec? For
example:

  When making a change to this specification, the protocol version
  number must be included:

The `foo` field contains ... Added in version 1.3.

> +
> +Introduction
> +
> +VFIO-over-socket, also known as vfio-user, is a protocol that allows a device

vfio-user is shorted. Now is the best time to start consistently using
"vfio-user" as the name for this protocol. Want to drop the name
VFIO-over-socket?

> +to be virtualized in a separate process outside of QEMU. VFIO-over-socket

Is there anything QEMU-specific about this protocol?

I think the scope of this protocol is more general and it could be
described as:

  allows device emulation in a separate process outside of a Virtual
  Machine Monitor (VMM).

(Or "emulator" instead of VMM, if you prefer.)

> +devices consist of a generic VFIO device type, living inside QEMU, which we

s/QEMU/the VMM/

> +call the client, and the core device implementation, living outside QEMU, 
> which
> +we call the server. VFIO-over-socket can be the main transport mechanism for
> +multi-process QEMU, however it can be used by other applications offering
> +device virtualization. Explaining the advantages of a
> +disaggregated/multi-process QEMU, and device virtualization outside QEMU in
> +general, is beyond the scope of this document.
> +
> +This document focuses on specifying the VFIO-over-socket protocol. VFIO has
> +been chosen for the following reasons:

It's a little subtle here that "VFIO" means the Linux VFIO ioctl
interface. Expanding it a bit makes the rest of the text clearer:

  The `Linux VFIO ioctl
  interface
  `_ has
  been chosen as the base for this protocol for the following reasons:

> +
> +1) It is a mature and stable API, backed by an extensively used framework.
> +2) The existing VFIO client implementation (qemu/hw/vfio/) can be largely
> +   reused.
> +
> +In a proof of concept implementation it has been demonstrated that using VFIO
> +over a UNIX domain socket is a viable option. VFIO-over-socket is designed 
> with
> +QEMU in mind, however it could be used by other client applications. The
> +VFIO-over-socket protocol does not require that QEMU's VFIO client
> +implementation is used in QEMU. None of the VFIO kernel modules are required
> +for supporting the protocol, neither in the client nor the server, only the
> +source header files are used.

In the long run only the last sentence in this paragraph needs to be in
the specification.

The proof of concept and QEMU-specific info is good to know for the
discussion right now, but I don't think this needs to be in the
specification.

> +
> +The main idea is to allow a virtual device to function in a separate process 
> in
> +the same host over a UNIX domain socket. A UNIX domain socket (AF_UNIX) is
> +chosen because we can trivially send file descriptors over it, which in turn
> +allows:
> +
> +* Sharing of guest memory for DMA with the virtual device process.

Should the spec start consistently using "server" instead of various
terms like "virtual device process"?

> +* Sharing of virtual device memory with the guest for fas

Re: [PATCH v6 04/13] hw/arm: Add NPCM730 and NPCM750 SoC models

[Cédric Le Goater]

On 7/17/20 8:02 AM, Havard Skinnemoen wrote:
> The Nuvoton NPCM7xx SoC family are used to implement Baseboard
> Management Controllers in servers. While the family includes four SoCs,
> this patch implements limited support for two of them: NPCM730 (targeted
> for Data Center applications) and NPCM750 (targeted for Enterprise
> applications).
> 
> This patch includes little more than the bare minimum needed to boot a
> Linux kernel built with NPCM7xx support in direct-kernel mode:
> 
>   - Two Cortex-A9 CPU cores with built-in periperhals.
>   - Global Configuration Registers.
>   - Clock Management.
>   - 3 Timer Modules with 5 timers each.
>   - 4 serial ports.
> 
> The chips themselves have a lot more features, some of which will be
> added to the model at a later stage.
> 
> Reviewed-by: Tyrone Ting 
> Reviewed-by: Joel Stanley 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Havard Skinnemoen 
> ---
>  include/hw/arm/npcm7xx.h |  85 
>  hw/arm/npcm7xx.c | 407 +++
>  hw/arm/Kconfig   |   5 +
>  hw/arm/Makefile.objs |   1 +
>  4 files changed, 498 insertions(+)
>  create mode 100644 include/hw/arm/npcm7xx.h
>  create mode 100644 hw/arm/npcm7xx.c
> 
> diff --git a/include/hw/arm/npcm7xx.h b/include/hw/arm/npcm7xx.h
> new file mode 100644
> index 00..e68d9c79e6
> --- /dev/null
> +++ b/include/hw/arm/npcm7xx.h
> @@ -0,0 +1,85 @@
> +/*
> + * Nuvoton NPCM7xx SoC family.
> + *
> + * Copyright 2020 Google LLC
> + *
> + * 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.
> + */
> +#ifndef NPCM7XX_H
> +#define NPCM7XX_H
> +
> +#include "hw/boards.h"
> +#include "hw/cpu/a9mpcore.h"
> +#include "hw/misc/npcm7xx_clk.h"
> +#include "hw/misc/npcm7xx_gcr.h"
> +#include "hw/timer/npcm7xx_timer.h"
> +#include "target/arm/cpu.h"
> +
> +#define NPCM7XX_MAX_NUM_CPUS(2)
> +
> +/* The first half of the address space is reserved for DDR4 DRAM. */
> +#define NPCM7XX_DRAM_BA (0x)
> +#define NPCM7XX_DRAM_SZ (2 * GiB)
> +
> +/* Magic addresses for setting up direct kernel booting and SMP boot stubs. 
> */
> +#define NPCM7XX_LOADER_START(0x)  /* Start of SDRAM */
> +#define NPCM7XX_SMP_LOADER_START(0x)  /* Boot ROM */
> +#define NPCM7XX_SMP_BOOTREG_ADDR(0xf080013c)  /* GCR.SCRPAD */
> +#define NPCM7XX_GIC_CPU_IF_ADDR (0xf03fe100)  /* GIC within A9 */
> +
> +typedef struct NPCM7xxState {
> +DeviceState parent;
> +
> +ARMCPU  cpu[NPCM7XX_MAX_NUM_CPUS];
> +A9MPPrivState   a9mpcore;
> +
> +MemoryRegionsram;
> +MemoryRegionirom;
> +MemoryRegionram3;
> +MemoryRegion*dram;
> +
> +NPCM7xxGCRState gcr;
> +NPCM7xxCLKState clk;
> +NPCM7xxTimerCtrlState tim[3];
> +} NPCM7xxState;
> +
> +#define TYPE_NPCM7XX"npcm7xx"
> +#define NPCM7XX(obj)OBJECT_CHECK(NPCM7xxState, (obj), TYPE_NPCM7XX)
> +
> +#define TYPE_NPCM730"npcm730"
> +#define TYPE_NPCM750"npcm750"
> +
> +typedef struct NPCM7xxClass {
> +DeviceClass parent;
> +
> +/* Bitmask of modules that are permanently disabled on this chip. */
> +uint32_tdisabled_modules;
> +/* Number of CPU cores enabled in this SoC class (may be 1 or 2). */
> +uint32_tnum_cpus;
> +} NPCM7xxClass;
> +
> +#define NPCM7XX_CLASS(klass)\
> +OBJECT_CLASS_CHECK(NPCM7xxClass, (klass), TYPE_NPCM7XX)
> +#define NPCM7XX_GET_CLASS(obj)  \
> +OBJECT_GET_CLASS(NPCM7xxClass, (obj), TYPE_NPCM7XX)
> +
> +/**
> + * npcm7xx_load_kernel - Loads memory with everything needed to boot
> + * @machine - The machine containing the SoC to be booted.
> + * @soc - The SoC containing the CPU to be booted.
> + *
> + * This will set up the ARM boot info structure for the specific NPCM7xx
> + * derivative and call arm_load_kernel() to set up loading of the kernel, 
> etc.
> + * into memory, if requested by the user.
> + */
> +void npcm7xx_load_kernel(MachineState *machine, NPCM7xxState *soc);
> +
> +#endif /* NPCM7XX_H */
> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> new file mode 100644
> index 00..9669ac5fa0
> --- /dev/null
> +++ b/hw/arm/npcm7xx.c
> @@ -0,0 +1,407 @@
> +/*
> + * Nuvoton NPCM7xx SoC family.
> + *
> + * Copyright 2020 Google LLC
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms

Re: [PATCH v6 06/13] roms: Add virtual Boot ROM for NPCM7xx SoCs

[Cédric Le Goater]

On 7/17/20 8:02 AM, Havard Skinnemoen wrote:
> This is a minimalistic boot ROM written specifically for use with QEMU.
> It supports loading the second-stage loader from SPI flash into RAM, SMP
> boot, and not much else.
> 
> Signed-off-by: Havard Skinnemoen 
> ---
>  Makefile|   1 +
>  .gitmodules |   3 +++
>  pc-bios/npcm7xx_bootrom.bin | Bin 0 -> 768 bytes
>  roms/Makefile   |   7 +++
>  roms/vbootrom   |   1 +

pc-bios/README
MAINTAINERS

need an update.

C. 

>  5 files changed, 12 insertions(+)
>  create mode 100644 pc-bios/npcm7xx_bootrom.bin
>  create mode 16 roms/vbootrom
> 
> diff --git a/Makefile b/Makefile
> index 32345c610e..56473c788d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -838,6 +838,7 @@ s390-ccw.img s390-netboot.img \
>  slof.bin skiboot.lid \
>  palcode-clipper \
>  u-boot.e500 u-boot-sam460-20100605.bin \
> +npcm7xx_bootrom.bin \
>  qemu_vga.ndrv \
>  edk2-licenses.txt \
>  hppa-firmware.img \
> diff --git a/.gitmodules b/.gitmodules
> index 9c0501a4d4..c95eaf8284 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -58,3 +58,6 @@
>  [submodule "roms/qboot"]
>   path = roms/qboot
>   url = https://github.com/bonzini/qboot
> +[submodule "roms/vbootrom"]
> + path = roms/vbootrom
> + url = https://github.com/google/vbootrom.git
> diff --git a/pc-bios/npcm7xx_bootrom.bin b/pc-bios/npcm7xx_bootrom.bin
> new file mode 100644
> index 
> ..38f89d1b97b0c2e133af2a9fbed0521be132065b
> GIT binary patch
> literal 768
> zcmd5)JxClu6n- zwKtZg3J4a0aCM3_X(ZL&4g;46VVk5e$K;z;L99|b@aE%v^S$rQ8)h(Vm@cB9IYc+2
> z2SHd4^NwTIGE%w>9S05p1#kf90Sj5Z(jG8}+)IZIp~iXK=T&)dL`%d-q*8aR#mq{7
> z9`=6;Dr(H0ACe72R5x?!)^86Qj-X%{+!K9iZNA@*wkBAV&iZ(l^I9?!Gz=S2I_*1d
> zr+tTQDHjvyzKnw(hu00yX`u!Fv z@P}yDV-bci(K9XL$FU!som2C`c)?Uc&294s^}Wzumap{hg1X^jN|V25M5tQZ=<9lN
> z%(zKz#t-qCwHKb;HygOCpvCNL_4@1tXV1YGf^XUE_$zr{g8zWh-6gz-teI(eibtxo
> z?0OZI4%rU0741PgUD`2xq@H|*4=+Rs?%N)Ox5G+q>C;DilBe_YlkeSUVHA-crNk+k
> jtiF_MudA 
> literal 0
> HcmV?d1
> 
> diff --git a/roms/Makefile b/roms/Makefile
> index f9acf39954..97e54fedeb 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -34,6 +34,7 @@ find-cross-gcc = $(firstword $(wildcard $(patsubst 
> %ld,%gcc,$(call find-cross-ld
>  # finally strip off path + toolname so we get the prefix
>  find-cross-prefix = $(subst gcc,,$(notdir $(call find-cross-gcc,$(1
>  
> +arm_cross_prefix := $(call find-cross-prefix,arm)
>  powerpc64_cross_prefix := $(call find-cross-prefix,powerpc64)
>  powerpc_cross_prefix := $(call find-cross-prefix,powerpc)
>  x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
> @@ -63,6 +64,7 @@ default help:
>   @echo "  skiboot-- update skiboot.lid"
>   @echo "  u-boot.e500-- update u-boot.e500"
>   @echo "  u-boot.sam460  -- update u-boot.sam460"
> + @echo "  npcm7xx_bootrom-- update vbootrom for npcm7xx"
>   @echo "  efi-- update UEFI (edk2) platform firmware"
>   @echo "  opensbi32-virt -- update OpenSBI for 32-bit virt machine"
>   @echo "  opensbi64-virt -- update OpenSBI for 64-bit virt machine"
> @@ -198,6 +200,10 @@ bios-microvm:
>   $(MAKE) -C qboot
>   cp qboot/bios.bin ../pc-bios/bios-microvm.bin
>  
> +npcm7xx_bootrom:
> + $(MAKE) -C vbootrom CROSS_COMPILE=$(arm_cross_prefix)
> + cp vbootrom/npcm7xx_bootrom.bin ../pc-bios/npcm7xx_bootrom.bin
> +
>  clean:
>   rm -rf seabios/.config seabios/out seabios/builds
>   $(MAKE) -C sgabios clean
> @@ -211,3 +217,4 @@ clean:
>   $(MAKE) -f Makefile.edk2 clean
>   $(MAKE) -C opensbi clean
>   $(MAKE) -C qboot clean
> + $(MAKE) -C vbootrom clean
> diff --git a/roms/vbootrom b/roms/vbootrom
> new file mode 16
> index 00..0c37a43527
> --- /dev/null
> +++ b/roms/vbootrom
> @@ -0,0 +1 @@
> +Subproject commit 0c37a43527f0ee2b9584e7fb2fdc805e902635ac
> 

Re: [GIT PULL] I2C updates

[Corey Minyard]

On Fri, Jul 17, 2020 at 01:30:35PM +0200, Philippe Mathieu-Daudé wrote:
> On 7/17/20 12:50 PM, Peter Maydell wrote:
> > On Thu, 16 Jul 2020 at 23:26, Corey Minyard  wrote:
> >>
> >> On Thu, Jul 16, 2020 at 09:45:41PM +0100, Peter Maydell wrote:
> >>> Hi; this failed to build on x86-64 Linux (incremental build):
> >>
> >> Hmm, I did test this, and I just rebuilt, then rebased on the end of
> >> master and rebuilt, without issue.
> >>
> >> It looks like the smbus code is not being included, but I don't see how
> >> that can be.
> > 
> > I was wrong about which config failed, sorry. Incremental builds
> > are fine, but the build that does "make -C builddir clean" first
> > fails.
> > 
> > It looks like the problem is that in the created config-devices.mak
> > files, CONFIG_SMBUS_EEPROM is set, but CONFIG_SMBUS is not.
> > So presumably the problem is in the change
> > "hw/i2c/Kconfig: Add an entry for the SMBus", or it is a more
> > general issue with changes to Kconfig files not correctly
> > resulting in rebuilds of config-devices.mak.
> 
> To Corey, this is likely the later (buildsys), see:
> https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg05475.html

Thanks, I was planning to try to figure this out this morning, but 
Peter beat me to it.

Thanks Peter.

-corey

Re: [PATCH v6 13/13] tests/acceptance: console boot tests for quanta-gsj

[Cédric Le Goater]

On 7/17/20 8:02 AM, Havard Skinnemoen wrote:
> This adds two acceptance tests for the quanta-gsj machine.
> 
> One test downloads a lightly patched openbmc flash image from github and
> verifies that it boots all the way to the login prompt.
> 
> The other test downloads a kernel, initrd and dtb built from the same
> openbmc source and verifies that the kernel detects all CPUs and boots
> to the point where it can't find the root filesystem (because we have no
> flash image in this case).
> 
> Signed-off-by: Havard Skinnemoen 

It looks good but I am not sure it's a good idea to have tests 
point to URLs like : 

https://github.com/hskinnemoen/openbmc/releases/download/20200711-gsj-qemu-0/obmc-phosphor-initramfs-gsj.cpio.xz

Philippe, Peter, is that OK ? 


If so, Joel, Andrew, could we host FW images on the OpenBMC github ? 
and do the same for Aspeed.

Thanks,

C. 

> ---
>  tests/acceptance/boot_linux_console.py | 65 ++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/tests/acceptance/boot_linux_console.py 
> b/tests/acceptance/boot_linux_console.py
> index 73cc69c499..1d82fc7ff8 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -569,6 +569,71 @@ class BootLinuxConsole(LinuxKernelTest):
>  'sda')
>  # cubieboard's reboot is not functioning; omit reboot test.
>  
> +def test_arm_quanta_gsj(self):
> +"""
> +:avocado: tags=arch:arm
> +:avocado: tags=machine:quanta-gsj
> +"""
> +# 25 MiB compressed, 32 MiB uncompressed.
> +image_url = (
> +'https://github.com/hskinnemoen/openbmc/releases/download/'
> +'20200711-gsj-qemu-0/obmc-phosphor-image-gsj.static.mtd.gz')
> +image_hash = '14895e634923345cb5c8776037ff7876df96f6b1'
> +image_path_gz = self.fetch_asset(image_url, asset_hash=image_hash)
> +image_name = os.path.splitext(os.path.basename(image_path_gz))[0]
> +image_path = os.path.join(self.workdir, image_name)
> +archive.gzip_uncompress(image_path_gz, image_path)
> +
> +self.vm.set_console()
> +drive_args = 'file=' + image_path + ',if=mtd,bus=0,unit=0'
> +self.vm.add_args('-drive', drive_args)
> +self.vm.launch()
> +
> +self.wait_for_console_pattern('> BootBlock by Nuvoton')
> +self.wait_for_console_pattern('>Device: Poleg BMC NPCM730')
> +self.wait_for_console_pattern('>Skip DDR init.')
> +self.wait_for_console_pattern('U-Boot ')
> +self.wait_for_console_pattern('Booting Linux on physical CPU 0x0')
> +self.wait_for_console_pattern('CPU1: thread -1, cpu 1, socket 0')
> +self.wait_for_console_pattern('OpenBMC Project Reference Distro')
> +self.wait_for_console_pattern('gsj login:')
> +
> +def test_arm_quanta_gsj_initrd(self):
> +"""
> +:avocado: tags=arch:arm
> +:avocado: tags=machine:quanta-gsj
> +"""
> +initrd_url = (
> +'https://github.com/hskinnemoen/openbmc/releases/download/'
> +'20200711-gsj-qemu-0/obmc-phosphor-initramfs-gsj.cpio.xz')
> +initrd_hash = '98fefe5d7e56727b1eb17d5c00311b1b5c945300'
> +initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
> +kernel_url = (
> +'https://github.com/hskinnemoen/openbmc/releases/download/'
> +'20200711-gsj-qemu-0/uImage-gsj.bin')
> +kernel_hash = 'fa67b2f141d56d39b3c54305c0e8a899c99eb2c7'
> +kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> +dtb_url = (
> +'https://github.com/hskinnemoen/openbmc/releases/download/'
> +'20200711-gsj-qemu-0/nuvoton-npcm730-gsj.dtb')
> +dtb_hash = '18315f7006d7b688d8312d5c727eecd819aa36a4'
> +dtb_path = self.fetch_asset(dtb_url, asset_hash=dtb_hash)
> +
> +self.vm.set_console()
> +kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> +   'console=ttyS0,115200n8 '
> +   'earlycon=uart8250,mmio32,0xf0001000')
> +self.vm.add_args('-kernel', kernel_path,
> + '-initrd', initrd_path,
> + '-dtb', dtb_path,
> + '-append', kernel_command_line)
> +self.vm.launch()
> +
> +self.wait_for_console_pattern('Booting Linux on physical CPU 0x0')
> +self.wait_for_console_pattern('CPU1: thread -1, cpu 1, socket 0')
> +self.wait_for_console_pattern(
> +'Give root password for system maintenance')
> +
>  def test_arm_orangepi(self):
>  """
>  :avocado: tags=arch:arm
> 

[Bug 1887854] Re: Spurious Data Abort on qemu-system-aarch64

[Peter Maydell]

It does still crash on current QEMU. The proximate cause of the crash is
that you are trying to read from an address which is way outside RAM:

Trace 0: 0x7f8d50054340 [/400195d8/0x82104000] strcmp
 PC=400195d8 X00=00104010ca28 X01=4001ec28
X02=0fe8 X03=401098c8 X04=4010ba40
X05=5641526f44654b00 X06=1f276f6c62717372 X07=
X08=ffda X09=401097d0 X10=0101010101010101
X11= X12= X13=
X14= X15= X16=40014610
X17=0008 X18= X19=4010b9f0
X20=4001ec28 X21=00084001ec20 X22=4001ec60
X23=4001ec40 X24=4001f548 X25=00104001ec28
X26=00104001ec40 X27=00034001ec38 X28=
X29=401098d0 X30=40008a38  SP=401098d0
PSTATE=4005 -Z-- EL1h
Taking exception 4 [Data Abort]
...from EL1 to EL1
...with ESR 0x25/0x9610
...with FAR 0x104010ca28
...with ELR 0x400195d8
...to EL1 PC 0x40018200 PSTATE 0x3c5

where the insn at 0x400195d8 is (inside strcmp)
0x400195d8:  f8408402  ldr  x2, [x0], #8

You can see that x0 is is 00104010ca28, so QEMU is correct to give
the data abort here. Further diagnosis would require working back
through the log to find out where that address came from, which will be
easier for you to do since you have the source.

NB: I recommend these options for producing the logfile:
 /tmp/q.log -d in_asm,int,cpu_reset,exec,cpu,guest_errors,nochain -singlestep
Execution will be slower, but the crash here is pretty quick so that's not a 
problem, and these options mean that every insn executed will produce a "Trace" 
line and a CPU register dump. That's easier to understand and read (especially 
reading backwards) than logs produced when QEMU is doing its normal 
optimisations of chaining TBs together and putting multiple guest insns in each 
TB.

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

Title:
  Spurious Data Abort on qemu-system-aarch64

Status in QEMU:
  New

Bug description:
  When running RTEMS test psxndbm01.exe built for AArch64-ilp32 (this code is 
not yet publically available), the test generates a spurious data abort (the 
MMU and alignment checks should be disabled according to bits 1, 0 of 
SCTLR_EL1). The abort information is as follows:
  Taking exception 4 [Data Abort]
  ...from EL1 to EL1
  ...with ESR 0x25/0x9610
  ...with FAR 0x104010ca28
  ...with ELR 0x400195d8
  ...to EL1 PC 0x40018200 PSTATE 0x3c5

  The ESR indicates that a synchronous external abort has occurred.
  ESR EC field: 0b100101

  From the ARMv8 technical manual: Data Abort taken without a change in
  Exception level. Used for MMU faults generated by data accesses,
  alignment faults other than those caused by Stack Pointer
  misalignment, and synchronous External aborts, including synchronous
  parity or ECC errors. Not used for debug related exceptions.

  ESR ISS field: 0b1

  From the ARMv8 technical manual: Synchronous External abort, not on
  translation table walk or hardware update of translation table.

  The following command line is used to invoke qemu:
  qemu-system-aarch64 -machine virt -cpu cortex-a53 -m 256M -no-reboot 
-nographic -serial mon:stdio -kernel 
build/aarch64/a53_ilp32_qemu/testsuites/psxtests/psxndbm01.exe -D qemu.log -d 
in_asm,int,cpu_reset,unimp,guest_errors

  This occurs on Qemu 3.1.0 as distributed via Debian and on Qemu 4.1 as
  built by the RTEMS source builder (4.1+minor patches).

  Edit: This bug can be worked around by getting and setting SCTLR
  without changing its value before each data abort would occur. This
  test needs 6 of these workarounds to operate successfully.

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

[PULL 02/12] Remove VXHS block device

[Kevin Wolf]

From: Marc-André Lureau 

The vxhs code doesn't compile since v2.12.0. There's no point in fixing
and then adding CI for a config that our users have demonstrated that
they do not use; better to just remove it.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
Message-Id: <20200711065926.2204721-1-marcandre.lur...@redhat.com>
Signed-off-by: Kevin Wolf 
---
 qapi/block-core.json |  22 +-
 docs/system/deprecated.rst   |   8 +
 configure|  39 --
 block/vxhs.c | 587 ---
 block/Makefile.objs  |   2 -
 block/trace-events   |  17 -
 tests/qemu-iotests/017   |   1 -
 tests/qemu-iotests/029   |   1 -
 tests/qemu-iotests/073   |   1 -
 tests/qemu-iotests/114   |   1 -
 tests/qemu-iotests/130   |   1 -
 tests/qemu-iotests/134   |   1 -
 tests/qemu-iotests/156   |   1 -
 tests/qemu-iotests/158   |   1 -
 tests/qemu-iotests/282   |   1 -
 tests/qemu-iotests/check |  10 -
 tests/qemu-iotests/common.filter |   1 -
 tests/qemu-iotests/common.rc |  33 --
 18 files changed, 10 insertions(+), 718 deletions(-)
 delete mode 100644 block/vxhs.c

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 463ffd83da..ab7bf3c612 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2790,7 +2790,6 @@
 #
 # Drivers that are supported in block device operations.
 #
-# @vxhs: Since 2.10
 # @throttle: Since 2.11
 # @nvme: Since 2.12
 # @copy-on-read: Since 3.0
@@ -2808,7 +2807,7 @@
 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd',
 { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' },
 'sheepdog',
-'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] 
}
+'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
 
 ##
 # @BlockdevOptionsFile:
@@ -3895,22 +3894,6 @@
   'base': 'BlockdevOptionsGenericFormat',
   'data': { '*offset': 'int', '*size': 'int' } }
 
-##
-# @BlockdevOptionsVxHS:
-#
-# Driver specific block device options for VxHS
-#
-# @vdisk-id: UUID of VxHS volume
-# @server: vxhs server IP, port
-# @tls-creds: TLS credentials ID
-#
-# Since: 2.10
-##
-{ 'struct': 'BlockdevOptionsVxHS',
-  'data': { 'vdisk-id': 'str',
-'server': 'InetSocketAddressBase',
-'*tls-creds': 'str' } }
-
 ##
 # @BlockdevOptionsThrottle:
 #
@@ -4010,8 +3993,7 @@
   'vhdx':   'BlockdevOptionsGenericFormat',
   'vmdk':   'BlockdevOptionsGenericCOWFormat',
   'vpc':'BlockdevOptionsGenericFormat',
-  'vvfat':  'BlockdevOptionsVVFAT',
-  'vxhs':   'BlockdevOptionsVxHS'
+  'vvfat':  'BlockdevOptionsVVFAT'
   } }
 
 ##
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 971b65be75..851dbdeb8a 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -618,3 +618,11 @@ to achieve the same fake NUMA effect or a properly 
configured
 New machine versions (since 5.1) will not accept the option but it will still
 work with old machine types. User can check the QAPI schema to see if the 
legacy
 option is supported by looking at MachineInfo::numa-mem-supported property.
+
+Block devices
+-
+
+VXHS backend (removed in 5.1)
+'
+
+The VXHS code does not compile since v2.12.0. It was removed in 5.1.
diff --git a/configure b/configure
index b751c853f5..8227962b45 100755
--- a/configure
+++ b/configure
@@ -501,7 +501,6 @@ numa=""
 tcmalloc="no"
 jemalloc="no"
 replication="yes"
-vxhs=""
 bochs="yes"
 cloop="yes"
 dmg="yes"
@@ -1541,10 +1540,6 @@ for opt do
   ;;
   --enable-replication) replication="yes"
   ;;
-  --disable-vxhs) vxhs="no"
-  ;;
-  --enable-vxhs) vxhs="yes"
-  ;;
   --disable-bochs) bochs="no"
   ;;
   --enable-bochs) bochs="yes"
@@ -1932,7 +1927,6 @@ disabled with --disable-FEATURE, default is enabled if 
available:
   xfsctl  xfsctl support
   qom-cast-debug  cast debugging support
   tools   build qemu-io, qemu-nbd and qemu-img tools
-  vxhsVeritas HyperScale vDisk backend support
   bochs   bochs image format support
   cloop   cloop image format support
   dmg dmg image format support
@@ -6249,33 +6243,6 @@ if compile_prog "" "" ; then
 have_sysmacros=yes
 fi
 
-##
-# Veritas HyperScale block driver VxHS
-# Check if libvxhs is installed
-
-if test "$vxhs" != "no" ; then
-  cat > $TMPC <
-#include 
-
-void *vxhs_callback;
-
-int main(void) {
-iio_init(QNIO_VERSION, vxhs_callback);
-return 0;
-}
-EOF
-  vxhs_libs="-lvxhs -lssl"
-  if compile_prog "" "$vxhs_libs" ; then
-vxhs=yes
-  else
-if test "$vxhs" = "yes" ; then
-  feature_not_found "vxhs block device" "Install libvxhs See github"
-fi
-vxhs=no
-  fi
-fi
-
 ##
 # check fo

[PULL 01/12] vvfat: set status to odd fixes

[Kevin Wolf]

From: Prasad J Pandit 

Virtual VFAT driver is quite old and rarely used. Set its status
to Odd Fixes.

Signed-off-by: Prasad J Pandit 
Message-Id: <20200710190451.761286-1-ppan...@redhat.com>
Signed-off-by: Kevin Wolf 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 030faf0249..5d9c56e441 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2987,7 +2987,7 @@ F: block/vpc.c
 vvfat
 M: Kevin Wolf 
 L: qemu-bl...@nongnu.org
-S: Supported
+S: Odd Fixes
 F: block/vvfat.c
 
 Image format fuzzer
-- 
2.25.4