Re: [Qemu-devel] [libvirt] clean/simple Q35 support in libvirt+QEMU for guest OSes that don't support virtio-1.0

[Gerd Hoffmann]

  Hi,

> b) Rather than a "legacy-only" model for virtio-0.9, it would be more
> useful to have "transitional". This way the config would work for older
> OSes that don't support virtio-1.0, and when/if the OS was upgraded such
> that it supported virtio-1.0, that would be automatically used without
> needing to change the config.

Having a legacy-only (instead of transitional) model could be useful for
regression-testing (i.e. whenever virtio-0.9 mode still works properly
in guests with virtio-1.0 support).

But that is pretty much the only reason I can think of to prefer the
virtio-0.9 devices being legacy-only instead of transitional.

> A) libosinfo starts telling consumers that the preferred virtio device
> model for the relevant OSes is "virtio-0.9", and leaves the
> recommendation for other OSes as "virtio".
> 
> B) libvirt adds a "virtio-0.9" model for all virtio devices that
> actually have virtio-0.9 support (a couple of devices never existed
> prior to virtio-1.0 (rng and ???) so virtio-0.9 would be nonsensical for
> them).

input, gpu are 1.0 only too.

> C) inside libvirt, the implementation of the "virtio-0.9" model is
> identical to "virtio", except that the VIR_PCI_CONNECT_TYPE flags for
> these devices contain VIR_PCI_CONNECT_TYPE_PCI rather than
> VIR_PCI_CONNECT_TYPE_PCIE, resulting in those devices being assigned to
> a legacy PCI slot, and thus they would be transitional mode by default.

Looks good to me.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 24/56] json: Accept overlong \xC0\x80 as U+0000 ("modified UTF-8")

[Markus Armbruster]

Markus Armbruster  writes:

> Eric Blake  writes:
>
>> On 08/10/2018 10:48 AM, Eric Blake wrote:
>>> On 08/08/2018 07:03 AM, Markus Armbruster wrote:
 This is consistent with qobject_to_json().  See commit e2ec3f97680.
>>>
>>> Side note: that commit mentions that on output, ASCII DEL (0x7f) is
>>> always escaped. RFC 7159 does not require it to be escaped on input,
>
> Weird, isn't it?
>
>>> but I wonder if any of your earlier testsuite improvements should
>>> specifically cover \x7f vs. \u007f on input being canonicalized to
>>> \u007f on round trip output.
>
> From utf8_string():
>
> /* 2.2.1  1 byte U+007F */
> {
> "\x7F",
> "\x7F",
> "\\u007F",
> },
>
> We test parsing of JSON "\x7F" (expecting C string "\x7F"), unparsing of
> that C string (expecting JSON "\\u007F"), and after PATCH 29 parsing of
> that JSON (expecting the C string again).  Sufficient?
>

 Signed-off-by: Markus Armbruster 
 ---
   qobject/json-lexer.c  | 2 +-
   qobject/json-parser.c | 2 +-
   tests/check-qjson.c   | 8 +---
   3 files changed, 3 insertions(+), 9 deletions(-)

 diff --git a/qobject/json-lexer.c b/qobject/json-lexer.c
 index ca1e0e2c03..36fb665b12 100644
 --- a/qobject/json-lexer.c
 +++ b/qobject/json-lexer.c
 @@ -93,7 +93,7 @@
*   interpolation = %((l|ll|I64)[du]|[ipsf])
*
    * Note:
 - * - Input must be encoded in UTF-8.
 + * - Input must be encoded in modified UTF-8.
>>>
>>> Worth documenting this in the QMP doc as an explicit extension?
>
> qmp-spec.txt:
>
> The sever expects its input to be encoded in UTF-8, and sends its
> output encoded in ASCII.
>
> The obvious update would be to stick in "modified".

Not really necessary, because:

* Before this patch, the JSON parser rejects \0 as ASCII control
  character, and \xC0\x80 as overlong UTF-8.

  Note that PATCH 17 fixed rejection of \0 in JSON strings.  PATCH 21
  fixed rejection of invalid UTF-8, but \xC0\x80 wasn't broken.

* This patch makes \xC0\x80 pass the "invalid UTF-8" check, only to get
  rejected as ASCII control character.  The error message changes,
  that's all.

The patch's benefit is consistency with the other direction:
qobject_to_json() maps \xC0\x80 to \\u.  I guess my commit message
should explain this a bit better.

[...]

Re: [Qemu-devel] Bugs when cross-compiling qemu for Windows with mingw 8.1, executable doesn't run

[David Hildenbrand]

On 18.07.2018 08:33, Howard Spoelstra wrote:
> Hi all,
> 
> I have two issues when cross compiling current master for Windows with
> mingw 8.1. Host is Fedora29. See further below for gcc and mingw
> versions.
> 
> Reproduce with:
> ./configure --cross-prefix=x86_64-w64-mingw32-
> --target-list="ppc-softmmu" --enable-gtk --with-gtkabi=3.0
> --enable-sdl --with-sdlabi=2.0
> 
> Issue 1: Two bugs show up during compilation related to strncpy.
> Replacing the offenders with memcpy seems to fix this.
> 
> First time:
> block/sheepdog.c: In function 'find_vdi_name':
> block/sheepdog.c:1239:5: error: 'strncpy' specified bound 256 equals
> destination size [-Werror=stringop-truncation]
>  strncpy(buf + SD_MAX_VDI_LEN, tag, SD_MAX_VDI_TAG_LEN);
>  ^~
> 
> Second time:
> migration/global_state.c: In function 'global_state_store_running':
> migration/global_state.c:45:5: error: 'strncpy' specified bound 100
> equals destination size [-Werror=stringop-truncation]
>  strncpy((char *)global_state.runstate,
>  ^~
> state, sizeof(global_state.runstate));
> ~
> 

These two reports should be independent of general functionality
(sheepdog, migration). So what you see is most likely unrelated to this.

> Issue 2: once strncpy has been replaced with memcpy in these two
> instances, I can successfully compile, but the executable doesn't run
> in Windows.
> I tried to debug, and this is what gdb told me:
> 
> (gdb) run
> Starting program: c:\qemu-fedora29beta\qemu-system-ppc-debug.exe -L
> c:\qemu-fedora29beta\pc-bios -boot c -m 256 -M "mac99,via=pmu"
> -prom-env "boot-args=-v" -prom-env "auto-boot?=true" -prom-env
> "vga-ndrv?=true" -hda c:\Mac-disks\9.2.qcow2 -netdev
> "user,id=network01" -device "sungem,netdev=network01" -sdl -d int
> [New Thread 948.0x6d8]
> [New Thread 948.0x2778]
> [New Thread 948.0x286c]
> [New Thread 948.0x3d0]
> 
> Program received signal SIGSEGV, Segmentation fault.
> getpagesize () at util/oslib-win32.c:535
> 535 util/oslib-win32.c: No such file or directory.

This warning is just from GDB, not able to locate you sources I guess.

> (gdb) bt full
> #0  getpagesize () at util/oslib-win32.c:535

Wonder why we should get a SEGFAULT in that simple function. As
discussed offline, the functionality in general seems to work (if this
function is compiled and run independently in your environment).

But maybe this backtrace is just misleading.

> system_info = {{dwOemId = 56491488, {wProcessorArchitecture =
> 64992, wReserved = 861}}, dwPageSize = 0,
>   lpMinimumApplicationAddress = 0x99cca4
> , lpMaximumApplicationAddress = 0x3,
>   dwActiveProcessorMask = 11102192, dwNumberOfProcessors =
> 56584576, dwProcessorType = 0,
>   dwAllocationGranularity = 200, wProcessorLevel = 0,
> wProcessorRevision = 0}

dwPageSize = 0, I assume this is some random data on the stack and
GetSystemInfo never got executed. I wonder where this segfault comes from.

> #1  0x009b7fcd in init_real_host_page_size () at util/pagesize.c:16
> No locals.
> #2  0x009bc5f2 in __do_global_ctors ()
> No symbol table info available.
> #3  0x004013ca in __tmainCRTStartup () at ../crt/crtexe.c:324
> lock_free = 
> fiberid = 
> nested = 
> lpszCommandLine = 
> StartupInfo = {cb = 104, lpReserved = 0x3778e00 "", lpDesktop
> = 0x377f440 "Winsta0\\Default",
>   lpTitle = 0x3786280
> "c:\\qemu-fedora29beta\\qemu-system-ppc-debug.exe", dwX = 0, dwY = 0,
> dwXSize = 0,
>   dwYSize = 0, dwXCountChars = 0, dwYCountChars = 0,
> dwFillAttribute = 0, dwFlags = 0, wShowWindow = 0,
>   cbReserved2 = 0, lpReserved2 = 0x0, hStdInput =
> 0x, hStdOutput = 0x,
>   hStdError = 0x}
> inDoubleQuote = 
> #4  0x004014fb in WinMainCRTStartup () at ../crt/crtexe.c:184
> ret = 255
> (gdb)
> 
> 
> Gcc and mingw versions used:
> 
> [hsp@localhost qemu-master]$ gcc -v
> Using built-in specs.
> COLLECT_GCC=gcc
> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/8/lto-wrapper
> OFFLOAD_TARGET_NAMES=nvptx-none
> OFFLOAD_TARGET_DEFAULT=1
> Target: x86_64-redhat-linux
> Configured with: ../configure --enable-bootstrap
> --enable-languages=c,c++,fortran,objc,obj-c++,ada,go,lto --prefix=/usr
> --mandir=/usr/share/man --infodir=/usr/share/info
> --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared
> --enable-threads=posix --enable-checking=release --enable-multilib
> --with-system-zlib --enable-__cxa_atexit
> --disable-libunwind-exceptions --enable-gnu-unique-object
> --enable-linker-build-id --with-gcc-major-version-only
> --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array
> --with-isl --enable-libmpx --enable-offload-targets=nvptx-none
> --without-cuda-driver --enable-gnu-indirect-f

[Qemu-devel] [RFC PATCH] qom: Rename object_new_with_props to object_new_child

[Thomas Huth]

While adding the object_initialize_child() function, Paolo suggested
to rename the similar object_new_with_props() function accordingly:

http://marc.info/?i=e034610d-9a1d-a8a5-ee92-b2e3f0ba2...@redhat.com

This way it is more obvious that this function creates a new object
as a child of another object.

Signed-off-by: Thomas Huth 
---
 hw/misc/auxbus.c |  4 +--
 include/crypto/tlscredsanon.h|  4 +--
 include/crypto/tlscredspsk.h |  4 +--
 include/crypto/tlscredsx509.h|  4 +--
 include/qom/object.h | 44 +--
 iothread.c   |  6 ++--
 qom/object.c | 22 +++---
 tests/check-qom-proplist.c   | 66 +++-
 tests/test-crypto-block.c|  2 +-
 tests/test-crypto-secret.c   | 44 +--
 tests/test-crypto-tlscredsx509.c |  2 +-
 tests/test-crypto-tlssession.c   |  4 +--
 tests/test-io-channel-tls.c  |  2 +-
 ui/vnc.c | 28 -
 14 files changed, 116 insertions(+), 120 deletions(-)

diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c
index 0e56d9a..c3729e5 100644
--- a/hw/misc/auxbus.c
+++ b/hw/misc/auxbus.c
@@ -67,8 +67,8 @@ AUXBus *aux_init_bus(DeviceState *parent, const char *name)
 Object *auxtoi2c;
 
 bus = AUX_BUS(qbus_create(TYPE_AUX_BUS, parent, name));
-auxtoi2c = object_new_with_props(TYPE_AUXTOI2C, OBJECT(bus), "i2c",
- &error_abort, NULL);
+auxtoi2c = object_new_child(TYPE_AUXTOI2C, OBJECT(bus), "i2c",
+&error_abort, NULL);
 qdev_set_parent_bus(DEVICE(auxtoi2c), BUS(bus));
 
 bus->bridge = AUXTOI2C(auxtoi2c);
diff --git a/include/crypto/tlscredsanon.h b/include/crypto/tlscredsanon.h
index 4d6b7e4..7cfa03f 100644
--- a/include/crypto/tlscredsanon.h
+++ b/include/crypto/tlscredsanon.h
@@ -41,14 +41,14 @@ typedef struct QCryptoTLSCredsAnonClass 
QCryptoTLSCredsAnonClass;
  * due to lacking MITM attack protection amongst other problems.
  *
  * This is a user creatable object, which can be instantiated
- * via object_new_propv():
+ * via object_new_child():
  *
  * 
  *   Creating anonymous TLS credential objects in code
  *   
  *   Object *obj;
  *   Error *err = NULL;
- *   obj = object_new_propv(TYPE_QCRYPTO_TLS_CREDS_ANON,
+ *   obj = object_new_child(TYPE_QCRYPTO_TLS_CREDS_ANON,
  *  "tlscreds0",
  *  &err,
  *  "endpoint", "server",
diff --git a/include/crypto/tlscredspsk.h b/include/crypto/tlscredspsk.h
index 306d36c..781ac56 100644
--- a/include/crypto/tlscredspsk.h
+++ b/include/crypto/tlscredspsk.h
@@ -39,14 +39,14 @@ typedef struct QCryptoTLSCredsPSKClass 
QCryptoTLSCredsPSKClass;
  * of the Pre-Shared Key credential used to perform a TLS handshake.
  *
  * This is a user creatable object, which can be instantiated
- * via object_new_propv():
+ * via object_new_child():
  *
  * 
  *   Creating TLS-PSK credential objects in code
  *   
  *   Object *obj;
  *   Error *err = NULL;
- *   obj = object_new_propv(TYPE_QCRYPTO_TLS_CREDS_PSK,
+ *   obj = object_new_child(TYPE_QCRYPTO_TLS_CREDS_PSK,
  *  "tlscreds0",
  *  &err,
  *  "dir", "/path/to/dir",
diff --git a/include/crypto/tlscredsx509.h b/include/crypto/tlscredsx509.h
index 66ad6a7..d0ef017 100644
--- a/include/crypto/tlscredsx509.h
+++ b/include/crypto/tlscredsx509.h
@@ -45,14 +45,14 @@ typedef struct QCryptoTLSCredsX509Class 
QCryptoTLSCredsX509Class;
  * of x509 credentials used to perform a TLS handshake.
  *
  * This is a user creatable object, which can be instantiated
- * via object_new_propv():
+ * via object_new_child():
  *
  * 
  *   Creating x509 TLS credential objects in code
  *   
  *   Object *obj;
  *   Error *err = NULL;
- *   obj = object_new_propv(TYPE_QCRYPTO_TLS_CREDS_X509,
+ *   obj = object_new_child(TYPE_QCRYPTO_TLS_CREDS_X509,
  *  "tlscreds0",
  *  &err,
  *  "endpoint", "server",
diff --git a/include/qom/object.h b/include/qom/object.h
index f0b0bf3..88f9b59 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -609,7 +609,7 @@ struct InterfaceClass
 Object *object_new(const char *typename);
 
 /**
- * object_new_with_props:
+ * object_new_child:
  * @typename:  The name of the type of the object to instantiate.
  * @parent: the parent object
  * @id: The unique ID of the object
@@ -635,15 +635,15 @@ Object *object_new(const char *typename);
  *   Error *err = NULL;
  *   Object *obj;
  *
- *   obj = object_new_with_props(TYPE_MEMORY_BACKEND_FILE,
- *   object_get_objects_root(),
- *   "hostmem0",
- *   &err,
- *   "share", "yes",
- *   "mem-path", "/dev/shm

Re: [Qemu-devel] Bugs when cross-compiling qemu for Windows with mingw 8.1, executable doesn't run

[Howard Spoelstra]

On Fri, Aug 17, 2018 at 9:32 AM, David Hildenbrand  wrote:
> On 18.07.2018 08:33, Howard Spoelstra wrote:
>> Hi all,
>>
>> I have two issues when cross compiling current master for Windows with
>> mingw 8.1. Host is Fedora29. See further below for gcc and mingw
>> versions.
>>
>> Reproduce with:
>> ./configure --cross-prefix=x86_64-w64-mingw32-
>> --target-list="ppc-softmmu" --enable-gtk --with-gtkabi=3.0
>> --enable-sdl --with-sdlabi=2.0
>>
>> Issue 1: Two bugs show up during compilation related to strncpy.
>> Replacing the offenders with memcpy seems to fix this.
>>
>> First time:
>> block/sheepdog.c: In function 'find_vdi_name':
>> block/sheepdog.c:1239:5: error: 'strncpy' specified bound 256 equals
>> destination size [-Werror=stringop-truncation]
>>  strncpy(buf + SD_MAX_VDI_LEN, tag, SD_MAX_VDI_TAG_LEN);
>>  ^~
>>
>> Second time:
>> migration/global_state.c: In function 'global_state_store_running':
>> migration/global_state.c:45:5: error: 'strncpy' specified bound 100
>> equals destination size [-Werror=stringop-truncation]
>>  strncpy((char *)global_state.runstate,
>>  ^~
>> state, sizeof(global_state.runstate));
>> ~
>>
>
> These two reports should be independent of general functionality
> (sheepdog, migration). So what you see is most likely unrelated to this.
>
>> Issue 2: once strncpy has been replaced with memcpy in these two
>> instances, I can successfully compile, but the executable doesn't run
>> in Windows.
>> I tried to debug, and this is what gdb told me:
>>
>> (gdb) run
>> Starting program: c:\qemu-fedora29beta\qemu-system-ppc-debug.exe -L
>> c:\qemu-fedora29beta\pc-bios -boot c -m 256 -M "mac99,via=pmu"
>> -prom-env "boot-args=-v" -prom-env "auto-boot?=true" -prom-env
>> "vga-ndrv?=true" -hda c:\Mac-disks\9.2.qcow2 -netdev
>> "user,id=network01" -device "sungem,netdev=network01" -sdl -d int
>> [New Thread 948.0x6d8]
>> [New Thread 948.0x2778]
>> [New Thread 948.0x286c]
>> [New Thread 948.0x3d0]
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> getpagesize () at util/oslib-win32.c:535
>> 535 util/oslib-win32.c: No such file or directory.
>
> This warning is just from GDB, not able to locate you sources I guess.
>
>> (gdb) bt full
>> #0  getpagesize () at util/oslib-win32.c:535
>
> Wonder why we should get a SEGFAULT in that simple function. As
> discussed offline, the functionality in general seems to work (if this
> function is compiled and run independently in your environment).
>
> But maybe this backtrace is just misleading.
>
>> system_info = {{dwOemId = 56491488, {wProcessorArchitecture =
>> 64992, wReserved = 861}}, dwPageSize = 0,
>>   lpMinimumApplicationAddress = 0x99cca4
>> , lpMaximumApplicationAddress = 0x3,
>>   dwActiveProcessorMask = 11102192, dwNumberOfProcessors =
>> 56584576, dwProcessorType = 0,
>>   dwAllocationGranularity = 200, wProcessorLevel = 0,
>> wProcessorRevision = 0}
>
> dwPageSize = 0, I assume this is some random data on the stack and
> GetSystemInfo never got executed. I wonder where this segfault comes from.
>
>> #1  0x009b7fcd in init_real_host_page_size () at util/pagesize.c:16
>> No locals.
>> #2  0x009bc5f2 in __do_global_ctors ()
>> No symbol table info available.
>> #3  0x004013ca in __tmainCRTStartup () at ../crt/crtexe.c:324
>> lock_free = 
>> fiberid = 
>> nested = 
>> lpszCommandLine = 
>> StartupInfo = {cb = 104, lpReserved = 0x3778e00 "", lpDesktop
>> = 0x377f440 "Winsta0\\Default",
>>   lpTitle = 0x3786280
>> "c:\\qemu-fedora29beta\\qemu-system-ppc-debug.exe", dwX = 0, dwY = 0,
>> dwXSize = 0,
>>   dwYSize = 0, dwXCountChars = 0, dwYCountChars = 0,
>> dwFillAttribute = 0, dwFlags = 0, wShowWindow = 0,
>>   cbReserved2 = 0, lpReserved2 = 0x0, hStdInput =
>> 0x, hStdOutput = 0x,
>>   hStdError = 0x}
>> inDoubleQuote = 
>> #4  0x004014fb in WinMainCRTStartup () at ../crt/crtexe.c:184
>> ret = 255
>> (gdb)
>>
>>
>> Gcc and mingw versions used:
>>
>> [hsp@localhost qemu-master]$ gcc -v
>> Using built-in specs.
>> COLLECT_GCC=gcc
>> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/8/lto-wrapper
>> OFFLOAD_TARGET_NAMES=nvptx-none
>> OFFLOAD_TARGET_DEFAULT=1
>> Target: x86_64-redhat-linux
>> Configured with: ../configure --enable-bootstrap
>> --enable-languages=c,c++,fortran,objc,obj-c++,ada,go,lto --prefix=/usr
>> --mandir=/usr/share/man --infodir=/usr/share/info
>> --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared
>> --enable-threads=posix --enable-checking=release --enable-multilib
>> --with-system-zlib --enable-__cxa_atexit
>> --disable-libunwind-exceptions --enable-gnu-unique-object
>> --enable-linker-build-id --with-gcc-major-version-only
>> --

Re: [Qemu-devel] [PATCH v3 2/4] kvm: Use inhibit to prevent ballooning without synchronous mmu

[Paolo Bonzini]

On 16/08/2018 20:15, Alex Williamson wrote:
> On Tue,  7 Aug 2018 13:31:23 -0600
> Alex Williamson  wrote:
> 
>> Remove KVM specific tests in balloon_page(), instead marking
>> ballooning as inhibited without KVM_CAP_SYNC_MMU support.
>>
>> Reviewed-by: David Hildenbrand 
>> Reviewed-by: Peter Xu 
>> Reviewed-by: Cornelia Huck 
>> Signed-off-by: Alex Williamson 
>> ---
>>  accel/kvm/kvm-all.c| 4 
>>  hw/virtio/virtio-balloon.c | 4 +---
>>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> Paolo and Michael, can I get an ack for this one?  Otherwise I can drop
> it from the series and let this continue to be a special case.  Thanks,
> 
> Alex
>  
>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>> index eb7db92a5e3b..38f468d8e2b1 100644
>> --- a/accel/kvm/kvm-all.c
>> +++ b/accel/kvm/kvm-all.c
>> @@ -39,6 +39,7 @@
>>  #include "trace.h"
>>  #include "hw/irq.h"
>>  #include "sysemu/sev.h"
>> +#include "sysemu/balloon.h"
>>  
>>  #include "hw/boards.h"
>>  
>> @@ -1698,6 +1699,9 @@ static int kvm_init(MachineState *ms)
>>  s->many_ioeventfds = kvm_check_many_ioeventfds();
>>  
>>  s->sync_mmu = !!kvm_vm_check_extension(kvm_state, KVM_CAP_SYNC_MMU);
>> +if (!s->sync_mmu) {
>> +qemu_balloon_inhibit(true);
>> +}
>>  
>>  return 0;
>>  
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 1f7a87f09429..b5425080c5fb 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -21,7 +21,6 @@
>>  #include "hw/mem/pc-dimm.h"
>>  #include "sysemu/balloon.h"
>>  #include "hw/virtio/virtio-balloon.h"
>> -#include "sysemu/kvm.h"
>>  #include "exec/address-spaces.h"
>>  #include "qapi/error.h"
>>  #include "qapi/qapi-events-misc.h"
>> @@ -36,8 +35,7 @@
>>  
>>  static void balloon_page(void *addr, int deflate)
>>  {
>> -if (!qemu_balloon_is_inhibited() && (!kvm_enabled() ||
>> - kvm_has_sync_mmu())) {
>> +if (!qemu_balloon_is_inhibited()) {
>>  qemu_madvise(addr, BALLOON_PAGE_SIZE,
>>  deflate ? QEMU_MADV_WILLNEED : QEMU_MADV_DONTNEED);
>>  }
> 

Acked-by: Paolo Bonzini 

Thanks,

Paolo

Re: [Qemu-devel] [PATCH v4 6/7] qmp: add pmemload command

[Simon Ruderich]

On Thu, Aug 16, 2018 at 03:01:31PM -0500, Eric Blake wrote:
>> +}
>> +if (!has_size) {
>> +struct stat s;
>> +if (fstat(fd, &s)) {
>> +error_setg_errno(errp, errno, "could not fstat fd to get size");
>> +goto exit;
>> +}
>> +size = s.st_size;
>> +}
>
> This works for regular files, but not for block devices.
>
> Do we have a helper function for easily determining the size of an arbitrary
> fd (whether file or block device)?  If not, should we?  As there are
> multiple spots in the code that grab this sort of information.

I found raw_getlength() in block/file-posix.c, but its static and
it takes a BlockDriverState* as argument. For most systems it
could be extracted without too much trouble, but some (e.g
__sun__ and BSD) are quite entangled with BlockDriverState code.

> Otherwise looks okay to me.

Should I check for S_ISCHR() and S_ISBLK() and abort with an
error? I don't think it's a common use case for pmemload anyway.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9


signature.asc
Description: PGP signature

Re: [Qemu-devel] How do you do when write more than 16TB data to qcow2 on ext4?

[lampahome]

Really? How to mount a blk device to /dev/nbdN?
I always find tips to mount from file-like image to /dev/nbdN

2018-08-16 19:46 GMT+08:00 Eric Blake :

> On 08/16/2018 03:22 AM, Daniel P. Berrangé wrote:
>
>> On Thu, Aug 16, 2018 at 09:35:52AM +0800, lampahome wrote:
>>
>>> We all know there's a file size limit 16TB in ext4 and other fs has their
>>> limit,too.
>>>
>>> If I create an qcow2 20TB on ext4 and write to it more than 16TB. Data
>>> more
>>> than 16TB can't be written to qcow2.
>>>
>>> So, is there any better ways to solve this situation?
>>>
>>
>> I'd really just recommend using a different filesystem, in particular XFS
>> has massively higher file size limit - tested to 500 TB in RHEL-7, with a
>> theoretical max size of 8 EB. It is a very mature filesystem & the default
>> in RHEL-7.
>>
>
> Or target raw block devices instead of using a filesystem. LVM works great.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>

Re: [Qemu-devel] [PATCH 55/56] json: Clean up headers

[Markus Armbruster]

Eric Blake  writes:

> On 08/08/2018 07:03 AM, Markus Armbruster wrote:
>> The JSON parser has three public headers, json-lexer.h, json-parser.h,
>> json-streamer.h.  They all contain stuff that is of no interest
>> outside qobject/json-*.c.
>>
>> Collect the public interface in include/qapi/qmp/json-parser.h, and
>> everything else in qobject/json-parser-int.h.
>>
>> Signed-off-by: Markus Armbruster 
>> ---
>
> Nice separation.
>
>>   10 files changed, 51 insertions(+), 76 deletions(-)
>>   delete mode 100644 include/qapi/qmp/json-streamer.h
>>   rename include/qapi/qmp/json-lexer.h => qobject/json-parser-int.h (62%)
>
>
>>
>> diff --git a/include/qapi/qmp/json-parser.h b/include/qapi/qmp/json-parser.h
>> index 55f75954c3..7345a9bd5c 100644
>> --- a/include/qapi/qmp/json-parser.h
>> +++ b/include/qapi/qmp/json-parser.h
>> @@ -1,5 +1,5 @@
>>   /*
>> - * JSON Parser
>> + * JSON Parser
>
> I'm not sure what git tried to flag here.

Trailing whitespace cleaned up.

> Otherwise, looks like a good reorganization.
>
> Reviewed-by: Eric Blake 

Thanks!

[Qemu-devel] [PATCH 1/2] linux-headers: update to mainline 5c60a7389d79

[Jason Wang]

Sync linux headers to 5c60a7389d79 ("Merge tag 'for-linus-4.19-ofs1' of 
git://git.kernel.org/pub/scm/linux/kernel/git/hubcap/linux").

Signed-off-by: Jason Wang 
---
 include/standard-headers/drm/drm_fourcc.h  | 176 +
 include/standard-headers/linux/ethtool.h   |  11 +-
 include/standard-headers/linux/pci_regs.h  |   4 +-
 include/standard-headers/linux/virtio_config.h |  16 ++-
 linux-headers/asm-generic/unistd.h |   4 +-
 linux-headers/asm-mips/unistd.h|  18 ++-
 linux-headers/asm-powerpc/kvm.h|   1 +
 linux-headers/asm-powerpc/unistd.h |   1 +
 linux-headers/asm-s390/unistd_32.h |   2 +
 linux-headers/asm-s390/unistd_64.h |   2 +
 linux-headers/linux/kvm.h  |   1 +
 linux-headers/linux/vhost.h|  18 +++
 12 files changed, 238 insertions(+), 16 deletions(-)

diff --git a/include/standard-headers/drm/drm_fourcc.h 
b/include/standard-headers/drm/drm_fourcc.h
index 11912fd..b53f8d7 100644
--- a/include/standard-headers/drm/drm_fourcc.h
+++ b/include/standard-headers/drm/drm_fourcc.h
@@ -182,6 +182,7 @@ extern "C" {
 #define DRM_FORMAT_MOD_VENDOR_QCOM0x05
 #define DRM_FORMAT_MOD_VENDOR_VIVANTE 0x06
 #define DRM_FORMAT_MOD_VENDOR_BROADCOM 0x07
+#define DRM_FORMAT_MOD_VENDOR_ARM 0x08
 /* add more to the end as needed */
 
 #define DRM_FORMAT_RESERVED  ((1ULL << 56) - 1)
@@ -297,6 +298,19 @@ extern "C" {
  */
 #define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE  fourcc_mod_code(SAMSUNG, 1)
 
+/*
+ * Qualcomm Compressed Format
+ *
+ * Refers to a compressed variant of the base format that is compressed.
+ * Implementation may be platform and base-format specific.
+ *
+ * Each macrotile consists of m x n (mostly 4 x 4) tiles.
+ * Pixel data pitch/stride is aligned with macrotile width.
+ * Pixel data height is aligned with macrotile height.
+ * Entire pixel data buffer is aligned with 4k(bytes).
+ */
+#define DRM_FORMAT_MOD_QCOM_COMPRESSED fourcc_mod_code(QCOM, 1)
+
 /* Vivante framebuffer modifiers */
 
 /*
@@ -384,6 +398,23 @@ extern "C" {
fourcc_mod_code(NVIDIA, 0x15)
 
 /*
+ * Some Broadcom modifiers take parameters, for example the number of
+ * vertical lines in the image. Reserve the lower 32 bits for modifier
+ * type, and the next 24 bits for parameters. Top 8 bits are the
+ * vendor code.
+ */
+#define __fourcc_mod_broadcom_param_shift 8
+#define __fourcc_mod_broadcom_param_bits 48
+#define fourcc_mod_broadcom_code(val, params) \
+   fourcc_mod_code(BROADCOM, uint64_t)params) << 
__fourcc_mod_broadcom_param_shift) | val))
+#define fourcc_mod_broadcom_param(m) \
+   ((int)(((m) >> __fourcc_mod_broadcom_param_shift) & \
+  ((1ULL << __fourcc_mod_broadcom_param_bits) - 1)))
+#define fourcc_mod_broadcom_mod(m) \
+   ((m) & ~(((1ULL << __fourcc_mod_broadcom_param_bits) - 1) <<\
+__fourcc_mod_broadcom_param_shift))
+
+/*
  * Broadcom VC4 "T" format
  *
  * This is the primary layout that the V3D GPU can texture from (it
@@ -404,6 +435,151 @@ extern "C" {
  */
 #define DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED fourcc_mod_code(BROADCOM, 1)
 
+/*
+ * Broadcom SAND format
+ *
+ * This is the native format that the H.264 codec block uses.  For VC4
+ * HVS, it is only valid for H.264 (NV12/21) and RGBA modes.
+ *
+ * The image can be considered to be split into columns, and the
+ * columns are placed consecutively into memory.  The width of those
+ * columns can be either 32, 64, 128, or 256 pixels, but in practice
+ * only 128 pixel columns are used.
+ *
+ * The pitch between the start of each column is set to optimally
+ * switch between SDRAM banks. This is passed as the number of lines
+ * of column width in the modifier (we can't use the stride value due
+ * to various core checks that look at it , so you should set the
+ * stride to width*cpp).
+ *
+ * Note that the column height for this format modifier is the same
+ * for all of the planes, assuming that each column contains both Y
+ * and UV.  Some SAND-using hardware stores UV in a separate tiled
+ * image from Y to reduce the column height, which is not supported
+ * with these modifiers.
+ */
+
+#define DRM_FORMAT_MOD_BROADCOM_SAND32_COL_HEIGHT(v) \
+   fourcc_mod_broadcom_code(2, v)
+#define DRM_FORMAT_MOD_BROADCOM_SAND64_COL_HEIGHT(v) \
+   fourcc_mod_broadcom_code(3, v)
+#define DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(v) \
+   fourcc_mod_broadcom_code(4, v)
+#define DRM_FORMAT_MOD_BROADCOM_SAND256_COL_HEIGHT(v) \
+   fourcc_mod_broadcom_code(5, v)
+
+#define DRM_FORMAT_MOD_BROADCOM_SAND32 \
+   DRM_FORMAT_MOD_BROADCOM_SAND32_COL_HEIGHT(0)
+#define DRM_FORMAT_MOD_BROADCOM_SAND64 \
+   DRM_FORMAT_MOD_BROADCOM_SAND64_COL_HEIGHT(0)
+#define DRM_FORMAT_MOD_BROADCOM_SAND128 \
+   DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(0)
+#define DRM_FORMAT_MOD_BROADCOM_SAND256 \
+   DRM_FORMAT_MOD_BROADCOM_SAND256_COL

[Qemu-devel] [PATCH 2/2] vhost: switch to use IOTLB v2 format

[Jason Wang]

This patch tries to switch to use new kernel IOTLB format V2. Previous
version may have inconsistent ABI between 32bit and 64bit machines
because of the hole after type field. Refer kernel commit
("429711aec282 vhost: switch to use new message format") for more
information.

To enable this feature, qemu need to use a new ioctl
VHOST_SET_BACKEND_FEATURE with VHOST_BACKEND_F_IOTLB_MSG_V2 bit. A new
pair of setting and getting backend features ops was introduced. And
when we try to set features for vhost dev, we will examine the support
of new IOTLB format and enable it. This process is total transparent
to guest, which means we can have different IOTLB message type in src
and dst during migration.

The conversion of IOTLB message is straightforward, just check the
type and behave accordingly.

Signed-off-by: Jason Wang 
---
 hw/virtio/vhost-backend.c | 77 +++
 hw/virtio/vhost.c | 18 +
 include/hw/virtio/vhost-backend.h |  6 +++
 include/hw/virtio/vhost.h |  2 +
 4 files changed, 87 insertions(+), 16 deletions(-)

diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 7f09efa..1a68719 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -151,12 +151,24 @@ static int vhost_kernel_set_features(struct vhost_dev 
*dev,
 return vhost_kernel_call(dev, VHOST_SET_FEATURES, &features);
 }
 
+static int vhost_kernel_set_backend_features(struct vhost_dev *dev,
+ uint64_t features)
+{
+return vhost_kernel_call(dev, VHOST_SET_BACKEND_FEATURES, &features);
+}
+
 static int vhost_kernel_get_features(struct vhost_dev *dev,
  uint64_t *features)
 {
 return vhost_kernel_call(dev, VHOST_GET_FEATURES, features);
 }
 
+static int vhost_kernel_get_backend_features(struct vhost_dev *dev,
+ uint64_t *features)
+{
+return vhost_kernel_call(dev, VHOST_GET_BACKEND_FEATURES, features);
+}
+
 static int vhost_kernel_set_owner(struct vhost_dev *dev)
 {
 return vhost_kernel_call(dev, VHOST_SET_OWNER, NULL);
@@ -190,34 +202,65 @@ static int vhost_kernel_vsock_set_running(struct 
vhost_dev *dev, int start)
 static void vhost_kernel_iotlb_read(void *opaque)
 {
 struct vhost_dev *dev = opaque;
-struct vhost_msg msg;
 ssize_t len;
 
-while ((len = read((uintptr_t)dev->opaque, &msg, sizeof msg)) > 0) {
-if (len < sizeof msg) {
-error_report("Wrong vhost message len: %d", (int)len);
-break;
+if (dev->acked_backend_cap & 0x1ULL <<
+VHOST_BACKEND_F_IOTLB_MSG_V2) {
+struct vhost_msg_v2 msg;
+
+while ((len = read((uintptr_t)dev->opaque, &msg, sizeof msg)) > 0) {
+if (len < sizeof msg) {
+error_report("Wrong vhost message len: %d", (int)len);
+break;
+}
+if (msg.type != VHOST_IOTLB_MSG_V2) {
+error_report("Unknown vhost iotlb message type");
+break;
+}
+
+vhost_backend_handle_iotlb_msg(dev, &msg.iotlb);
 }
-if (msg.type != VHOST_IOTLB_MSG) {
-error_report("Unknown vhost iotlb message type");
-break;
+} else {
+struct vhost_msg msg;
+
+while ((len = read((uintptr_t)dev->opaque, &msg, sizeof msg)) > 0) {
+if (len < sizeof msg) {
+error_report("Wrong vhost message len: %d", (int)len);
+break;
+}
+if (msg.type != VHOST_IOTLB_MSG) {
+error_report("Unknown vhost iotlb message type");
+break;
+}
+
+vhost_backend_handle_iotlb_msg(dev, &msg.iotlb);
 }
-
-vhost_backend_handle_iotlb_msg(dev, &msg.iotlb);
 }
 }
 
 static int vhost_kernel_send_device_iotlb_msg(struct vhost_dev *dev,
   struct vhost_iotlb_msg *imsg)
 {
-struct vhost_msg msg;
+if (dev->acked_backend_cap & 1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) {
+struct vhost_msg_v2 msg;
+
+msg.type = VHOST_IOTLB_MSG_V2;
+msg.iotlb = *imsg;
 
-msg.type = VHOST_IOTLB_MSG;
-msg.iotlb = *imsg;
+if (write((uintptr_t)dev->opaque, &msg, sizeof msg) != sizeof msg) {
+error_report("Fail to update device iotlb");
+return -EFAULT;
+}
+} else {
+struct vhost_msg msg;
+
+msg.type = VHOST_IOTLB_MSG;
+msg.iotlb = *imsg;
 
-if (write((uintptr_t)dev->opaque, &msg, sizeof msg) != sizeof msg) {
-error_report("Fail to update device iotlb");
-return -EFAULT;
+if (write((uintptr_t)dev->opaque, &msg, sizeof msg) != sizeof msg) {
+error_report("Fail to update device iotlb");
+return -EFAULT;
+}
 }
 
 return 0;
@@ -254,7 +297,9 @@ static const VhostOps kernel_ops = {
 .vhost

Re: [Qemu-devel] [PATCH 56/56] docs/interop/qmp-spec: How to force known good parser state

[Markus Armbruster]

Eric Blake  writes:

> On 08/08/2018 07:03 AM, Markus Armbruster wrote:
>> Section "QGA Synchronization" specifies that sending "a raw 0xFF
>> sentinel byte" makes the server "reset its state and discard all
>> pending data prior to the sentinel."  What actually happens there is a
>> lexical error, which will produce one ore more error responses.
>> Moreover, it's not specific to QGA.
>
> Hoisting my review of this, as you may want to move it sooner in the series.
>
>>
>> Create new section "Forcing the JSON parser into known-good state" to
>> document the technique properly.  Rewrite section "QGA
>> Synchronization" to document just the other direction, i.e. command
>> guest-sync-delimited.
>>
>> Section "Protocol Specification" mentions "synchronization bytes
>> (documented below)".  Delete that.
>>
>> While there, fix it not to claim '"Server" is QEMU itself', but
>> '"Server" is either QEMU or the QEMU Guest Agent'.
>>
>> Signed-off-by: Markus Armbruster 
>> ---
>>   docs/interop/qmp-spec.txt | 37 -
>>   1 file changed, 24 insertions(+), 13 deletions(-)
>>
>> diff --git a/docs/interop/qmp-spec.txt b/docs/interop/qmp-spec.txt
>> index 1566b8ae5e..d4a42fe2cc 100644
>> --- a/docs/interop/qmp-spec.txt
>> +++ b/docs/interop/qmp-spec.txt
>> @@ -20,9 +20,9 @@ operating system.
>>   2. Protocol Specification
>>   =
>>   -This section details the protocol format. For the purpose of this
>> document
>> -"Client" is any application which is using QMP to communicate with QEMU and
>> -"Server" is QEMU itself.
>> +This section details the protocol format. For the purpose of this
>> +document, "Server" is either QEMU or the QEMU Guest Agent, and
>> +"Client" is any application communicating with it via QMP.
>>   
>
> Broadens the term "QMP" to mean any client speaking to a qemu
> machine-readable server (previously, we tended to treat "QMP" as the
> direct-to-qemu service, and "QGA" as the guest agent service). I can
> live with that, especially since this document was already mentioning
> QGA.

And by that it already had QMP denote two disctinct things: the protocol
and one of its two applications.  I'm not really making this worse.  I'm
not really improving it, either.

>>   JSON data structures, when mentioned in this document, are always in the
>>   following format:
>> @@ -34,9 +34,8 @@ by the JSON standard:
>> http://www.ietf.org/rfc/rfc7159.txt
>>   -The protocol is always encoded in UTF-8 except for
>> synchronization
>> -bytes (documented below); although thanks to json-string escape
>> -sequences, the server will reply using only the strict ASCII subset.
>> +The sever expects its input to be encoded in UTF-8, and sends its
>> +output encoded in ASCII.
>>   
>
> Perhaps worth documenting is the range of JSON numbers produced by
> qemu (maybe as a separate patch). Libvirt just hit a bug with the
> jansson library making it extremely difficult to parse JSON containing
> numbers larger than INT64_MAX, when compared to yajl which had a way
> to support up to UINT64_MAX.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1614569
>
> Knowing that qemu sends numbers larger than INT64_MAX with the intent
> that they not be truncated/rounded by conversion to double can be a
> vital piece of information for implementing a client, when it comes to
> picking a particular library for JSON parsing.

Good point.  Doesn't really fit into this commit, though.  Care to
propose a patch?

>>   For convenience, json-object members mentioned in this document will
>>   be in a certain order. However, in real protocol usage they can be in
>> @@ -215,16 +214,28 @@ Some events are rate-limited to at most one per 
>> second.  If additional
>>   dropped, and the last one is delayed.  "Similar" normally means same
>>   event type.  See qmp-events.txt for details.
>>   -2.6 QGA Synchronization
>> +2.6 Forcing the JSON parser into known-good state
>> +-
>> +
>> +Incomplete or invalid input can leave the server's JSON parser in a
>> +state where it can't parse additional commands.  To get it back into
>> +known-good state, the client should provoke a lexical error.
>> +
>> +The cleanest way to do that is sending an ASCII control character
>> +other than '\t' (horizontal tab), '\r' (carriage return), and '\n'
>
> s/and/or/

Done.

>> +(new line).
>> +
>> +Sadly, older versions of QEMU can fail to flag this as an error.  If a
>> +client needs to deal with them, it should send a 0xFF byte.
>
> Here's where we have the choice of whether to intentionally document
> 0xff as an intentional parser reset, instead of a lexical error. If
> so, the advice to provoke a lexical error via an ASCII control (of
> which I would be most likely to use 0x00 NUL or 0x1b ESC) vs. an
> intentional use of 0xff may need different wording here.
>
> But if you don't want to give 0xff any more special treatment than
> what it already has as a lexical error (and

Re: [Qemu-devel] [PULL v2 00/15] MIPS queue for QEMU upstream, August 16, 2018

[Peter Maydell]

On 16 August 2018 at 18:48, Aleksandar Markovic
 wrote:
> From: Aleksandar Markovic 
>
> The following changes since commit c542a9f9794ec8e0bc3fcf5956d3cc8bce667789:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-tests-2018-08-16' 
> into staging (2018-08-16 09:50:54 +0100)
>
> are available in the git repository at:
>
>   https://github.com/AMarkovic/qemu tags/mips-queue-aug-2018
>
> for you to fetch changes up to 8639c5c95472ee79391a7e7826d1af23948b71f6:
>
>   qemu-doc: Amend MIPS-related items (2018-08-16 19:18:45 +0200)
>
> 
> MIPS queue for QEMU upstream, August 16, 2018
>
> This is v2 of the pull request sent on August 14, 2018 - in
> this pull request clang build errors are fixed. The pull request
> is based on the first part (patches 1-15) of v9 of the nanoMIPS
> support series.
>
>   - gcc/clang bisect were tested to work (build ok after each
> patch).
>   - Two occurences of "O64" are replaced with "N64" in the last
> patch, as minor corrections.
>   - "Reviewed-by" lines were also updated compared to v9.
>
> -
> Aleksandar Markovic (9):
>   MAINTAINERS: Update target/mips maintainer's email addresses
>   target/mips: Avoid case statements formulated by ranges - part 1
>   target/mips: Mark switch fallthroughs with interpretable comments
>   target/mips: Fix two instances of shadow variables
>   target/mips: Update some CP0 registers bit definitions
>   elf: Remove duplicate preprocessor constant definition
>   elf: Add ELF flags for MIPS machine variants
>   linux-user: Update MIPS syscall numbers up to kernel 4.18 headers
>   qemu-doc: Amend MIPS-related items
>
> Aleksandar Rikalo (2):
>   target/mips: Avoid case statements formulated by ranges - part 2
>   linux-user: Add preprocessor availability control to some syscalls
>
> Stefan Markovic (2):
>   target/mips: Add CP0 BadInstrX register
>   target/mips: Implement CP0 Config1.WR bit functionality
>
> Yongbok Kim (2):
>   target/mips: Don't update BadVAddr register in Debug Mode
>   target/mips: Check ELPA flag only in some cases of MFHC0 and MTHC0

Applied, thanks.

-- PMM

Re: [Qemu-devel] Pipe key broken on US keyboards

[Daniel P . Berrangé]

On Thu, Aug 16, 2018 at 03:44:54PM -0400, Phillip Susi wrote:
> On 8/16/2018 1:27 PM, Daniel P. Berrangé wrote:
> > Did you actually 'git bisect' to that commit, or is that just a guess ?
> 
> No, I haven't actually tried to build it from sources myself yet so I
> just found the source file that handles the keyboard, saw a bunch of
> scancode translation stuff in it, did a git log on that file and that
> first commit sticks out like a sore thumb; it changes the way scancodes
> are translated in a big way and the timing of it seems about right.  The
> other commits talk about fixing specific key codes that are unrelated to
> pipe, and it looks like this commit threw out the code those commits
> touched anyhow and refactored the whole thing.

Oh one other thing, is whether your QEMU process has an explicit keymap
configured (this is the -k arg to QEMU), as when that it set, it
completely changes the way keyboard input is handled in VNC. That code
has also been massively refactored recently.

> > If not, could you 'git bisect' to confirm the actual commit, as there
> > have been lots of changes in this area.
> 
> I'll try.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] nbd oldstyle negotiation

[Daniel P . Berrangé]

On Thu, Aug 16, 2018 at 02:56:10PM -0500, Eric Blake wrote:
> On 08/16/2018 02:02 PM, Vladimir Sementsov-Ogievskiy wrote:
> > Hi Eric!
> > 
> > There is a small problem with our qemu-nbd cmdline interface: people
> > forget to use option -x or don't know about it and face into problems
> > with old protocol version. One of may colleagues after such situation
> > (because of this option, he could not use utility nbd-client, which
> > doesn't support old version) asked me to add comments about version
> > selection by -x option into --help of qemu-nbd. And really, it's not an
> > obvious interface solution...
> > 
> > Now I think, should not we use new version by default? Or, even drop old
> > version support at all? Anybody uses it?
> 
> nbdkit 1.3 switched its default to newstyle (Jan 2018):
> https://github.com/libguestfs/nbdkit/commit/b2a8aecc
> https://github.com/libguestfs/nbdkit/commit/8158e773
> 
> and you are correct that nbd 3.10 dropped oldstyle long ago (Mar 2015):
> https://github.com/NetworkBlockDevice/nbd/commit/36940193
> 
> oldstyle is severely lacking in features (it can't do TLS or efficient
> sparse file handling, for example).  Do we need a formal qemu deprecation
> period (where you still get oldstyle if -x was not used, but a nice big
> warning) before flipping the default in 3.2/3.3 [1], or are we okay flipping
> it in 3.1?
> 
> [1] Okay, I know that our new equally-arbitrary policy on when to bump qemu
> version number components means that we'll probably see 3.1 in 2018 then 4.0
> in 2019, rather than 3.2.
> 
> And, if we switch the default now (which I am in favor of), do we want to
> add a -o switch to still make it possible to select oldstyle for a couple
> more releases in case someone was relying on it?  Then again, if you have to
> amend your script to use -o to keep things from breaking, why not just amend
> your setup to use newstyle?
> 
> My personal preference: completely drop oldstyle support from qemu, for 3.1,
> with no deprecation period. Do so by making '-x ""' the default for any
> qemu-nbd command line that did not otherwise specify an export name.  If
> someone still needs interoperability, they can use nbdkit in the middle,
> which will still support oldstyle, and provides a plugin for connecting:

So the key question is if the server switches to newstyle with "" export
name, what versions of qemu client will cease to work. Originally the QEMU
client would refuse to use new style protocol if not export name was given.

I fixed that in

commit 69b49502d8b7b582af79fac5bef7b7ccc2dc9c1e
Author: Daniel P. Berrange 
Date:   Wed Feb 10 18:41:10 2016 +

nbd: use "" as a default export name if none provided


So if we mandate new style in the server then we break compat with any
QEMU client that is < 2.6.0

I think that is acceptable as this point in time, as that'll be about
3 years old by time 3.1 comes out.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] clean/simple Q35 support in libvirt+QEMU for guest OSes that don't support virtio-1.0

[Daniel P . Berrangé]

On Thu, Aug 16, 2018 at 06:20:29PM -0400, Laine Stump wrote:
> Summary of the problem:
> 
> 1) We want to persuade libvirt+QEMU users to move away from the i440fx
> machinetype in favor of Q35. (NB: Someday this *might* lead to the
> ability to deprecate and even remove the 440fx machinetype, but even if
> that were to happen, it would be a *very long* time from now, so this
> discussion is *not* about that!)

There are plenty of OS that will never support Q35 and are still interesting
to use under Q35. The set which could use Q35, but lack virtio1.0 is fairly
small. So removal of i440fx is really only something for downstream KVM vendors
to consider. Those vendors only care about modern OS, but upstream is much
more open minded about what QEMU is used for, so I see it probably living
forever in upstream, or at least long enough that current maintaniers will
be retired ;-P.


> 2) When Q35 machinetype is used, libvirt assigns virtio devices to a
> slot on a PCI Express controller (because why have modern PCIe
> controllers/slots available but force everything onto clunky old legacy
> controllers?).
> 
> 3) When a virtio device is plugged into an Express controller, QEMU
> disables the device's IO port space, and it is put into "modern-only"
> mode (this is done to avoid a rapid exhaustion of limited IO port space).
> 
> 4) modern-only virtio devices won't work with a legacy (virtio-0.9-only)
> guest driver, because virtio-0.9 requires IO port space.
> 
> 5) Some guest OSes that we still want to support (and which would
> otherwise work okay on a Q35 virtual machine) have virtio drivers too
> old to support virtio-1.0 (CentOS6 and RHEL6 are examples of such OSes),
> but due to the chain of reasons listed above, the "standard" config for
> a Q35 guest generated by libvirt doesn't support virtio-0.9, hence
> doesn't support these guest OSes.

Note when talking about "support" you're really saying it from the
downstream vendor, specifically RHEL, POV. From upstream or Fedora POV
essentially all x86 OS ever made are in scope for running under QEMU
if suitable virtual hardware models have been provided. QEMU doesn't
maintain any whitelist of "supported" OS that differs from what is
technically capable of being run, in the way downstream vendors do.

> And here's a list of possible solutions to this problem (note that
> "consumers" means management applications such as OpenStack, oVirt,
> virt-manager, virt-install, gnome-boxes, etc. In all cases, it's assumed
> that the consumer's decision on the action to take will be based on
> information from libosinfo). For completeness, I've included even the
> possibilities that have been rejected, along with a brief synopsis of
> (at least part of) the reason for rejection:
> 
>   (1) Add some way libvirt consumers can ask libvirt to place
>   virtio devices on a legacy pci slot instead of pcie when
>   the machinetype is q35 (qemu sets virtio devices in legacy
>   PCI slots to transitional mode, so io port space is enabled
>   and virtio-0.0 drivers will work).
> 
>   This has been proposed on libvir-list, but rejected. Here is
>   the most elquently stated reasoning for the rejection I could
>   find (with thanks to Dan Berrange):
> 
>  The domain XML is a way to express the configuration
>  of the guest virtual machine.  What we're talking about
>  here is a policy tunable for an internal libvirt QEMU
>  driver algorithm, as so does not belong anywhere in the
>  domain XML.

Indeed, that's a guiding principal in general, not just for this PCI
question.

>   (2) Add full-blown pci enumeration support to all libvirt consumers
>   (i.e. they will need to build a model of the PCI bus topology
>   of each guest, and keep track of which addresses are in use).
>   They can then manually place virtio devices on legacy pci slots
>   (again, triggering transitional mode) when the intended guest
>   OS doesn't support virtio-0.9.
> 
>   (This is seen as requiring too much duplicated effort for
>   development and support/maintenance, since up until now libvirt
>   has been the single point of action for PCI address assignment
>   (well, QEMU can do it too, but for > 10 years libvirt has
>   *always* provided full PCI addresses for all devices)

It really depends on the scope of the mgmt app - at some point the mgmt
apps needs to take charge to some degree if it has particular ideas
about how a machine should look. Libvirt's placement strategy is a good
default for 95% of use cases, but it'll never be 100%. An example is
setting up a particular PCI topology that is guest NUMA node aware,
using expander buses.

So some apps might take this option, but in the common case it is
undesirable.

>   (3) Add virtio-1.0 support to all guest OSes. If this is done,
>   existing libvirt configs will work.
> 
>   (Aside from the difficulty of backporting, and the fact that
>

Re: [Qemu-devel] Qemu and Spectre_V4 + l1tf + IBRS_FW

[Daniel P . Berrangé]

On Fri, Aug 17, 2018 at 08:44:38AM +0200, Stefan Priebe - Profihost AG wrote:
> Hello,
> 
> i haven't found anything on the web regarding qemu and mentioned variants.
> 
> While my host says:
> l1tf:Mitigation: PTE Inversion; VMX: SMT vulnerable, L1D conditional
> cache flushes
> meltdown:Mitigation: PTI
> spec_store_bypass:Mitigation: Speculative Store Bypass disabled via
> prctl and seccomp
> spectre_v1:Mitigation: __user pointer sanitization
> spectre_v2:Mitigation: Full generic retpoline, IBPB, IBRS_FW
> 
> My guests bootet with pcid and spec-ctrl only say:
> l1tf:Mitigation: PTE Inversion
> meltdown:Mitigation: PTI
> spec_store_bypass:Vulnerable
> spectre_v1:Mitigation: __user pointer sanitization
> spectre_v2:Mitigation: Full generic retpoline, IBPB
> 
> * What is about spec_store_bypass in Qemu?

The guest needs an 'ssbd' feature for Intel CPU models and either a
'virt-ssbd' or 'amd-ssbd' feature for AMD CPU models.

> * What is about IBRS_FW feature?

I'm not sure what IBRS_FW is referring to, but don't worry about it.
The fact the the guest kernel says "Mitigation" instead of "Vulnerable"
means you are protected with your current config.

For Intel CPU models Spectre v2 needs the guest to have the 'spec-ctrl'
feature. On AMD models Spectre v2 the guest needs 'ibpb' feature.

> * What is about L1TF?

No extra CPU flags are required for QEMU guests for L1TF. The new CPU
feature is merely an perf optimization for the host hypervisor fixes.

Note that with L1TF there are extra steps you need to consider wrt
hyperthreading, that won't be reflected in the 'vulnerabilities'
data published by the kernel.

You can read more about the procedure for dealing with L1TF in
virt hosts in the "Resolve" tab of this article:

  https://access.redhat.com/security/vulnerabilities/L1TF

> Or are those just irrelevant to Qemu guests? Would be great to have some
> informations.

We have some QEMU docs providing guidance on guest CPU model/feature config
but they are not yet published. In the meantime this blog post of mine gives
the same info, covering what's needed for Spectre v2, Meltdown and SSBD and
guidance in general for CPU config:

  
https://www.berrange.com/posts/2018/06/29/cpu-model-configuration-for-qemu-kvm-on-x86-hosts/


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [RFC PATCH] qom: Rename object_new_with_props to object_new_child

[Daniel P . Berrangé]

On Fri, Aug 17, 2018 at 09:33:33AM +0200, Thomas Huth wrote:
> While adding the object_initialize_child() function, Paolo suggested
> to rename the similar object_new_with_props() function accordingly:
> 
> http://marc.info/?i=e034610d-9a1d-a8a5-ee92-b2e3f0ba2...@redhat.com
> 
> This way it is more obvious that this function creates a new object
> as a child of another object.

I'd expect 'object_new_with_child' to be the same as 'object_new',
but with only 'parent' & 'id' args added, which isn't the case here.

If we want the full & consistent design then we should have

  object_new(typename)
  object_new_with_child(typename, parent, id)
  object_new_props(typename, ...)
  object_new_propv(typename, va_arg props)
  object_new_with_child_props(typename, parent, id, ...)
  object_new_with_child_propv(typename, parent, id, va_arg props)


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] sdhci: add i.MX SD Stable Clock bit

[no-reply]

Hi,

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

Type: series
Message-id: 1534422540-18815-1-git-send-email-hans-erik.flo...@rt-labs.com
Subject: [Qemu-devel] [PATCH] sdhci: add i.MX SD Stable Clock bit

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
eeea783b72 sdhci: add i.MX SD Stable Clock bit

=== OUTPUT BEGIN ===
Checking PATCH 1/1: sdhci: add i.MX SD Stable Clock bit...
ERROR: suspect code indent for conditional statements (7, 10)
#34: FILE: hw/sd/sdhci.c:1657:
+   if (s->clkcon & SDHC_CLOCK_INT_STABLE)
+  ret |= ESDHC_PRNSTS_SDSTB;

ERROR: braces {} are necessary for all arms of this statement
#34: FILE: hw/sd/sdhci.c:1657:
+   if (s->clkcon & SDHC_CLOCK_INT_STABLE)
[...]

total: 2 errors, 0 warnings, 19 lines checked

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

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [RFC PATCH] qom: Rename object_new_with_props to object_new_child

[Thomas Huth]

On 08/17/2018 11:48 AM, Daniel P. Berrangé wrote:
> On Fri, Aug 17, 2018 at 09:33:33AM +0200, Thomas Huth wrote:
>> While adding the object_initialize_child() function, Paolo suggested
>> to rename the similar object_new_with_props() function accordingly:
>>
>> http://marc.info/?i=e034610d-9a1d-a8a5-ee92-b2e3f0ba2...@redhat.com
>>
>> This way it is more obvious that this function creates a new object
>> as a child of another object.
> 
> I'd expect 'object_new_with_child' to be the same as 'object_new',
> but with only 'parent' & 'id' args added, which isn't the case here.
> 
> If we want the full & consistent design then we should have
> 
>   object_new(typename)
>   object_new_with_child(typename, parent, id)
>   object_new_props(typename, ...)
>   object_new_propv(typename, va_arg props)
>   object_new_with_child_props(typename, parent, id, ...)
>   object_new_with_child_propv(typename, parent, id, va_arg props)

"new_with_child" sounds wrong, too, since the parent is not created
here, but the child. Anyway, I guess the naming of these functions is
too much subject to bikeshedding, so never mind, let's keep it as it
currently is.

 Thomas

Re: [Qemu-devel] [RFC PATCH] qom: Rename object_new_with_props to object_new_child

[Daniel P . Berrangé]

On Fri, Aug 17, 2018 at 11:58:07AM +0200, Thomas Huth wrote:
> On 08/17/2018 11:48 AM, Daniel P. Berrangé wrote:
> > On Fri, Aug 17, 2018 at 09:33:33AM +0200, Thomas Huth wrote:
> >> While adding the object_initialize_child() function, Paolo suggested
> >> to rename the similar object_new_with_props() function accordingly:
> >>
> >> http://marc.info/?i=e034610d-9a1d-a8a5-ee92-b2e3f0ba2...@redhat.com
> >>
> >> This way it is more obvious that this function creates a new object
> >> as a child of another object.
> > 
> > I'd expect 'object_new_with_child' to be the same as 'object_new',
> > but with only 'parent' & 'id' args added, which isn't the case here.
> > 
> > If we want the full & consistent design then we should have
> > 
> >   object_new(typename)
> >   object_new_with_child(typename, parent, id)
> >   object_new_props(typename, ...)
> >   object_new_propv(typename, va_arg props)
> >   object_new_with_child_props(typename, parent, id, ...)
> >   object_new_with_child_propv(typename, parent, id, va_arg props)
> 
> "new_with_child" sounds wrong, too, since the parent is not created
> here, but the child. Anyway, I guess the naming of these functions is
> too much subject to bikeshedding, so never mind, let's keep it as it
> currently is.

True, 'new_with_parent' is a better choice in retrospect :-)

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [RFC PATCH] qom: Rename object_new_with_props to object_new_child

[Daniel P . Berrangé]

On Fri, Aug 17, 2018 at 10:59:44AM +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 17, 2018 at 11:58:07AM +0200, Thomas Huth wrote:
> > On 08/17/2018 11:48 AM, Daniel P. Berrangé wrote:
> > > On Fri, Aug 17, 2018 at 09:33:33AM +0200, Thomas Huth wrote:
> > >> While adding the object_initialize_child() function, Paolo suggested
> > >> to rename the similar object_new_with_props() function accordingly:
> > >>
> > >> http://marc.info/?i=e034610d-9a1d-a8a5-ee92-b2e3f0ba2...@redhat.com
> > >>
> > >> This way it is more obvious that this function creates a new object
> > >> as a child of another object.
> > > 
> > > I'd expect 'object_new_with_child' to be the same as 'object_new',
> > > but with only 'parent' & 'id' args added, which isn't the case here.
> > > 
> > > If we want the full & consistent design then we should have
> > > 
> > >   object_new(typename)
> > >   object_new_with_child(typename, parent, id)
> > >   object_new_props(typename, ...)
> > >   object_new_propv(typename, va_arg props)
> > >   object_new_with_child_props(typename, parent, id, ...)
> > >   object_new_with_child_propv(typename, parent, id, va_arg props)
> > 
> > "new_with_child" sounds wrong, too, since the parent is not created
> > here, but the child. Anyway, I guess the naming of these functions is
> > too much subject to bikeshedding, so never mind, let's keep it as it
> > currently is.
> 
> True, 'new_with_parent' is a better choice in retrospect :-)

Or indeed 'object_new_child' and 'object_new_child_prop{s,v}' approx as
you had suggested


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 3/4] scripts/qemu: add render_block_graph method for QEMUMachine

[Vladimir Sementsov-Ogievskiy]

17.08.2018 04:54, Eduardo Habkost wrote:

On Thu, Aug 16, 2018 at 08:20:26PM +0300, Vladimir Sementsov-Ogievskiy wrote:

Render block nodes graph with help of graphviz

Signed-off-by: Vladimir Sementsov-Ogievskiy 

Thanks for your patch.  Comments below:


---
  scripts/qemu.py | 53 +
  1 file changed, 53 insertions(+)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index f099ce7278..cff562c713 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -460,3 +460,56 @@ class QEMUMachine(object):
   socket.SOCK_STREAM)
  self._console_socket.connect(self._console_address)
  return self._console_socket
+
+def render_block_graph(self, filename):
+'''
+Render graph in text (dot) representation into "filename" and graphical
+representation into pdf file "filename".pdf
+'''
+
+try:
+from graphviz import Digraph

I'm not convinced that this belongs to qemu.py, if most code
using the qemu.py module will never use it.  Do you see any
problem in moving this code to a scripts/render_block_graph.py
script?


Not a problem, I can do it.. Just one thought:
Isn't it better to keep all function doing something with one vm as 
methods, not separate functions?





+except ImportError:
+print "Can't import graphviz. Please run 'pip install graphviz'"

If you really want to make this part of qemu.py, I suggest
raising an appropriate exception instead of printing to stdout
directly.


Ok


(But just moving this code to a separate script would probably be
simpler)

Also, keep in mind that this module needs to work with both
Python 3 and Python 2, so you need to use print("message ...")
with parenthesis.



+return
+
+nodes = self.qmp('query-named-block-nodes')['return']
+edges = self.qmp('x-query-block-nodes-relations')['return']

I suggest you use .command() instead of .qmp().  It handles
['return'] and ['error'] for you.


Cool, thanks





+node_names = []
+
+graph = Digraph(comment='Block Nodes Graph')
+graph.node('permission symbols:\l'
+   ' w - Write\l'
+   ' r - consistent-Read\l'
+   ' u - write - Unchanged\l'
+   ' g - Graph-mod\l'
+   ' s - reSize\l'
+   'edge label scheme:\l'
+   ' \l'
+   ' \l'
+   ' \l', shape='none')
+
+def perm(arr):
+s = 'w' if 'write' in arr else '_'
+s += 'r' if 'consistent-read' in arr else '_'
+s += 'u' if 'write-unchanged' in arr else '_'
+s += 'g' if 'graph-mod' in arr else '_'
+s += 's' if 'resize' in arr else '_'
+return s
+
+for n in nodes:
+node_names.append(n['node-name'])
+label = n['node-name'] + ' [' + n['drv'] + ']'
+if n['drv'] == 'file':
+label = '<%s%s>' % (label, os.path.basename(n['file']))
+graph.node(n['node-name'], label)
+
+for r in edges:
+if r['parent'] not in node_names:
+graph.node(r['parent'], shape='box')
+
+label = '%s\l%s\l%s\l' % (r['name'], perm(r['perm']),
+  perm(r['shared-perm']))
+graph.edge(r['parent'], r['child'], label=label)
+
+graph.render(filename)

The rest of the code looks OK to me.




--
Best regards,
Vladimir

Re: [Qemu-devel] [Qemu-arm] [PATCH 00/10] target/arm: Some pieces of support for 32-bit Hyp mode

[Peter Maydell]

On 14 August 2018 at 13:42, Peter Maydell  wrote:
> Now we have virtualization support in the GICv2 emulation,
> I thought I'd have a look at how much we were still missing
> for being able to enable EL2 support for AArch32.
> This set of patches fixes some minor missing pieces:
>  * various small bugs in cp15 registers or places where
>we were missing the 32-bit version of a 64-bit register
>  * a bugfix for MSR/MRS (banked), which were not allowing
>Hyp mode to access ELR_Hyp
>  * implementation of the ERET instruction for A32/T32
>  * support for taking exceptions to Hyp mode (the largest
>of these missing bits)
>
> This isn't complete, but I thought I'd push these patches
> out for review. My test setup is that I have another
> couple of patches, one which fixes up hw/arm/boot.c to
> boot AArch32 kernels in Hyp mode if it exists, and one
> which sets ARM_FEATURE_EL2 on our A15 model. With those I
> can get an outer kernel to boot with KVM support and try
> to run an inner guest kernel. The inner kernel boots OK
> but gets random segfaults in its userspace -- I haven't
> tracked down why this is yet...

I've put patches 1, 2, 4, 6, 7, 8, 9:

>   target/arm: Correct typo in HAMAIR1 regdef name
>   target/arm: Add missing .cp = 15 to HMAIR1 and HAMAIR1 regdefs
>   target/arm: Implement RAZ/WI HACTLR2
>   target/arm: Implement AArch32 HVBAR
>   target/arm: Implement AArch32 HCR and HCR2
>   target/arm: Implement AArch32 Hyp FARs
>   target/arm: Implement ESR_EL2/HSR for AArch32 and no-EL2
>   target/arm: Permit accesses to ELR_Hyp from Hyp mode via MSR/MRS
> (banked)
>   target/arm: Implement AArch32 ERET instruction

into target-arm.next.

Patches 3, 5, 10 had issues in code review and I'll
rework those and send a v2 at some point:

>   target/arm: Implement RAZ/WI HACTLR2
>   target/arm: Implement AArch32 HCR and HCR2
>   target/arm: Implement support for taking exceptions to Hyp mode

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/8] tests/migration-test: Silence the kvm_hv message by default

[Juan Quintela]

Thomas Huth  wrote:
> When running "make check" on a non-POWER host, the output is quite
> distorted like this:
>
>   [...]
>   GTESTER check-qtest-nios2
>   GTESTER check-qtest-or1k
>   GTESTER check-qtest-ppc64
> Skipping test: kvm_hv not available Skipping test: kvm_hv not available 
> Skipping test: kvm_hv not available Skipping test: kvm_hv not available   
> GTESTER check-qtest-ppcemb
>   GTESTER check-qtest-ppc
>   GTESTER check-qtest-riscv32
>   GTESTER check-qtest-riscv64
>   [...]
>
> Move the check to the beginning of the main function instead, so that
> we do not have to test the condition again and again for each test,
> and better use g_test_message() instead of g_print() here, like it is
> also done in ufd_version_check() already.
>
> Reviewed-by: Markus Armbruster 
> Signed-off-by: Thomas Huth 

Reviewed-by: Juan Quintela 

[Qemu-devel] vm-tests images disks filling up?

[Peter Maydell]

I just ran into a build failure using the tests/vm/ BSD build tests,
because the NetBSD build image's disk filled up.

Looking more closely there seemed to be 9 stale build trees in
the VM's /var/tmp/qemu-test.* , which is why the disk was full
(they'd used up about 18GB between them).

The other VMs (freebsd, openbsd) also had the same problem of
/var/tmp gradually filling with stale trees, they just hadn't
quite run out of space yet...

What's the process for managing the disk space on these images?
How are stale or completed build trees deleted ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 3/8] hw/timer/mc146818rtc: White space clean-up

[Juan Quintela]

Thomas Huth  wrote:
> mc146818rtc.c still contains some TABs. Replace them with spaces.
> And while we're at it, also delete trailing whitespace in this file.
>
> Reviewed-by: Markus Armbruster 
> Signed-off-by: Thomas Huth 

Reviewed-by: Juan Quintela 

Re: [Qemu-devel] [PATCH v2 4/8] hw/timer/mc146818rtc: Fix introspection problem

[Juan Quintela]

Thomas Huth  wrote:
> There is currently a funny problem with the "mc146818rtc" device:
> 1) Start QEMU like this:
>qemu-system-ppc64 -M pseries -S
> 2) At the HMP monitor, enter "info qom-tree". Note that there is an
>entry for "/rtc (spapr-rtc)".
> 3) Introspect the mc146818rtc device like this:
>device_add mc146818rtc,help
> 4) Run "info qom-tree" again. The "/rtc" entry is gone now!
>
> The rtc_finalize() function of the mc146818rtc device has two bugs: First,
> it tries to remove a "rtc" property, while the rtc_realizefn() added a
> "rtc-time" property instead. And second, it should have been done in an
> unrealize function, not in a finalize function, to avoid that this causes
> problems during introspection.
>
> But since adding aliases to the global machine state should not be done
> from a device's realize function anyway, let's rather fix this issue
> by moving the creation of the alias to the code that creates the device
> (and thus is run from the machine init functions instead), i.e. the
> mc146818_rtc_init() function for most machines. The prep machines are
> special, since the mc146818rtc device is created here in the realize
> function of the i82378 device. Since we certainly don't want to add the
> alias there, we add it to some code that is called from the ibm_40p_init()
> machine init function instead.
> Since the alias is now only created during the machine init, we can remove
> the object_property_del() completely.
>
> Fixes: 654a36d857ff949e0d1989904b76f53fded9dc83
> Reviewed-by: Markus Armbruster 
> Signed-off-by: Thomas Huth 

Reviewed-by: Juan Quintela 

Re: [Qemu-devel] [PATCH v2 5/8] tests: Skip old versioned machine types in quick testing mode

[Juan Quintela]

Thomas Huth  wrote:
> The tests that check something for all machine types currently spend
> a lot of time checking old machine types (like "pc-i440fx-2.0" for
> example). The chances that we find something new there in addition
> to checking the latest version of a machine type are pretty low, so
> we should not waste the time of the developers by testing this again
> and again in the "quick" testing mode.
> Thus let's add some code to determine whether we are testing a current
> machine type or an old one, and only test the old types if we are
> running in "SPEED=slow" mode.
> This decreases the testing time quite a bit now, e.g. the qom-test
> now finishes within 4 seconds for qemu-system-x86_64 instead of 30
> seconds when testing all machines.
>
> Signed-off-by: Thomas Huth 

Reviewed-by: Juan Quintela 

Thanks!

Re: [Qemu-devel] [PATCH v2 0/8] Various qtest-related patches and a mc146818rtc fix

[Paolo Bonzini]

On 16/08/2018 13:35, Thomas Huth wrote:
> The basic idea for this patch series are the two patches that improve
> the device-inrospection test: We now check that the qom-tree and the
> qtree do not change during introspection, and the we also turn on the
> device introspection test with all machines when running in SPEED=slow
> mode. The other patches are a bug fix for the mc146818rtc device which
> has been discovered with the improved device-introspection test, two
> patches to shut up annoying messages during "make check", a patch to
> speed up the tests that iterate over all machine types and an update
> to the MAINTAINERS file.
> 
> v2:
>  - Added Reviewed-bys from v1
>  - Use g_test_message() instead of g_debug()
>  - Improved the patch descriptions a little bit
>  - Added patch to skip old versioned machine types in quick mode
>  - Added Paolo's update to MAINTAINERS
> 
> Paolo Bonzini (1):
>   MAINTAINERS: add maintainers for qtest
> 
> Thomas Huth (7):
>   tests/migration-test: Silence the kvm_hv message by default
>   net: Silence 'has no peer' messages in testing mode
>   hw/timer/mc146818rtc: White space clean-up
>   hw/timer/mc146818rtc: Fix introspection problem
>   tests: Skip old versioned machine types in quick testing mode
>   tests/device-introspection: Check that the qom-tree and qtree do not
> change
>   tests/device-introspect: Test with all machines, not only with "none"
> 
>  MAINTAINERS| 10 
>  hw/ppc/prep.c  |  3 +++
>  hw/timer/mc146818rtc.c | 20 ++-
>  tests/cpu-plug-test.c  |  6 ++---
>  tests/device-introspect-test.c | 55 
> --
>  tests/libqtest.c   | 52 +--
>  tests/libqtest.h   |  4 ++-
>  tests/migration-test.c | 20 ---
>  tests/qom-test.c   |  2 +-
>  tests/test-hmp.c   |  2 +-
>  vl.c   |  3 +--
>  11 files changed, 138 insertions(+), 39 deletions(-)
> 

Queued, thanks.

Paolo

Re: [Qemu-devel] [PATCH v2 6/8] tests/device-introspection: Check that the qom-tree and qtree do not change

[Juan Quintela]

Thomas Huth  wrote:
> Introspection should not change the qom-tree / qtree, so we should check
> this in the device-introspect-test, too. This patch helped to find lots
> of instrospection bugs during the QEMU v3.0 soft/hard-freeze period in the
> last two months.
>
> Signed-off-by: Thomas Huth 

Reviewed-by: Juan Quintela 

Re: [Qemu-devel] [libvirt] clean/simple Q35 support in libvirt+QEMU for guest OSes that don't support virtio-1.0

[Andrea Bolognani]

On Fri, 2018-08-17 at 10:29 +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 16, 2018 at 06:20:29PM -0400, Laine Stump wrote:
> > 5) Some guest OSes that we still want to support (and which would
> > otherwise work okay on a Q35 virtual machine) have virtio drivers too
> > old to support virtio-1.0 (CentOS6 and RHEL6 are examples of such OSes),
> > but due to the chain of reasons listed above, the "standard" config for
> > a Q35 guest generated by libvirt doesn't support virtio-0.9, hence
> > doesn't support these guest OSes.
> 
> Note when talking about "support" you're really saying it from the
> downstream vendor, specifically RHEL, POV. From upstream or Fedora POV
> essentially all x86 OS ever made are in scope for running under QEMU
> if suitable virtual hardware models have been provided. QEMU doesn't
> maintain any whitelist of "supported" OS that differs from what is
> technically capable of being run, in the way downstream vendors do.

Well, at least in the case of RHEL 6, "not supported" means that it
will not boot at all on q35 with the default guest topology created
by libvirt, so that's not really a downstream-only problem :)

> > a) we don't really need the virtio-1.0 model, since that's what you
> > currently get anyway when you ask for "virtio" on Q35 (and on 440fx,
> > "virtio" gives you transitional, which works for everybody).
> 
> At some point we might have a virtio-2.0 and find ourselves in a
> similar problem again. IMHO it is preferrable to have both explicit
> versioned models, and discourage use of the magical 'virtio' model from
> mgmt apps. Use libosinfo to identify which virtio model is supported
> for the OS in question and use that explicitly.  Only use the magical
> 'virtio' model if there's no information about what version the OS
> supports.

Agreed.

> > c) Even if it's possible to force a device on an Express slot into
> > transitional mode, this is extremely wasteful of io port space, so
> > libvirt should consider virtio-0.9 devices to be legacy PCI, and thus
> > plug them into legacy PCI slots. And once we're doing this, it's
> > unnecessary to add any extra option to the qemu commandline to force
> > legacy support (i.e. transitional mode), as that is what QEMU already
> > does when the device is connected to a legacy PCI slot.
> 
> Yes, it should plug them into legacy PCI slots by default, but if a
> mgmt app has done explicit placement itself, it should honour that
> even if it means wasting IO space.

This is consistent with what already happens with PCI addresses: a
PCIe device will never be assigned to a PCI slot automatically, but
the user can still force that to happen by providing the PCI address
themselves.

> > C) inside libvirt, the implementation of the "virtio-0.9" model is
> > identical to "virtio", except that the VIR_PCI_CONNECT_TYPE flags for
> > these devices contain VIR_PCI_CONNECT_TYPE_PCI rather than
> > VIR_PCI_CONNECT_TYPE_PCIE, resulting in those devices being assigned to
> > a legacy PCI slot, and thus they would be transitional mode by default.
> 
> For 'virtio-0.9' libvirt should set "disable-modern=yes" in QEMU args
> 
> For 'virtio-1.0' libvirt should set "disable-legacy=yes" in QEMU args

If we decide we want to explicitly spell out the options instead
of relying on QEMU changing behavior based on the slot type, which
is probably a good idea anyway, I think we should have

  virtio-0.9 => disable-legacy=no,disable-modern=no
  virtio-1.0 => disable-legacy=yes,disable-modern=no

There's basically no reason to have a device legacy-only rather
than transitional, and spelling out both options instead of only
one of them just seems more robust.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [Qemu-devel] [PATCH v2 0/9] synchronization profiler

[Paolo Bonzini]

On 17/08/2018 07:18, Emilio G. Cota wrote:
> v1: https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg02186.html
> 
> Changes since v1:
> 
> - Rebase on current master.
> - Update copyright to 2018.
> - Add -m option to the HMP info command to sort by average wait time,
>   as suggested by Paolo.
> - Add -n option to the HMP info command to NOT merge call sites.
>   The default does merge call sites, as suggested by Paolo (thanks
>   to Peter for the clarification).
> - Switch to camel case in qsp.c
> - Update the commit logs of the two HMP patches to clearly state
>   that this is only for developers, so it is HMP-only.
> - Rename the HMP command from "sync" to "sync-profile", as suggested
>   by Markus.
> - Use int for line info consistently (was using unsigned sometimes)
> - Drop qsp_init from qsp_cond_wait, as suggested by Fam.
> - #undef QSP_GEN_{VOID,RET1} once they're no longer used.
> - Add qsp_reset()
>   This uses a snapshot to avoid deleting items, which would require
>   adding rcu_read_lock/unlock to the fast path.
> - Convert to run-time option, as suggested by Fam
>   - Add -enable-sync-profile to qemu-options
>   - Add sync-profile HMP command: "sync-profile on|off|reset"
> - allocate QSPEntry with g_new0
> 
> I added most new bits as separate patches to ease review.
> 
> The first patch has some perf numbers; the last patch shows
> sample output from the monitor.
> 
> Checkpatch gives some errors, but they're false positives.
> 
> You can fetch this series from:
>   https://github.com/cota/qemu/tree/sync-profiler-v2
> 
> Diffstat below.
> 
> Thanks,
> 
>   Emilio
> ---
>  cpus.c  |  10 +-
>  hmp-commands-info.hx|  22 ++
>  hmp-commands.hx |  15 +
>  hmp.c   |  24 ++
>  hmp.h   |   1 +
>  include/qemu/main-loop.h|   4 +-
>  include/qemu/qht.h  |   1 +
>  include/qemu/qsp.h  |  29 ++
>  include/qemu/thread-posix.h |   4 +-
>  include/qemu/thread-win32.h |   5 +-
>  include/qemu/thread.h   |  66 +++-
>  monitor.c   |  11 +
>  qemu-options.hx |  10 +
>  stubs/iothread-lock.c   |   2 +-
>  tests/atomic_add-bench.c|   6 +-
>  util/Makefile.objs  |   1 +
>  util/qemu-thread-win32.c|   4 +-
>  util/qht.c  |  47 ++-
>  util/qsp.c  | 779 +++
>  vl.c|   3 +
>  20 files changed, 1016 insertions(+), 28 deletions(-)
>  create mode 100644 include/qemu/qsp.h
>  create mode 100644 util/qsp.c
> 
> 

Queued, I'll wait for more comments before sending a pull request.

Paolo

Re: [Qemu-devel] [libvirt] clean/simple Q35 support in libvirt+QEMU for guest OSes that don't support virtio-1.0

[Daniel P . Berrangé]

On Fri, Aug 17, 2018 at 12:35:11PM +0200, Andrea Bolognani wrote:
> On Fri, 2018-08-17 at 10:29 +0100, Daniel P. Berrangé wrote:
> > On Thu, Aug 16, 2018 at 06:20:29PM -0400, Laine Stump wrote:
> > > 5) Some guest OSes that we still want to support (and which would
> > > otherwise work okay on a Q35 virtual machine) have virtio drivers too
> > > old to support virtio-1.0 (CentOS6 and RHEL6 are examples of such OSes),
> > > but due to the chain of reasons listed above, the "standard" config for
> > > a Q35 guest generated by libvirt doesn't support virtio-0.9, hence
> > > doesn't support these guest OSes.
> > 
> > Note when talking about "support" you're really saying it from the
> > downstream vendor, specifically RHEL, POV. From upstream or Fedora POV
> > essentially all x86 OS ever made are in scope for running under QEMU
> > if suitable virtual hardware models have been provided. QEMU doesn't
> > maintain any whitelist of "supported" OS that differs from what is
> > technically capable of being run, in the way downstream vendors do.
> 
> Well, at least in the case of RHEL 6, "not supported" means that it
> will not boot at all on q35 with the default guest topology created
> by libvirt, so that's not really a downstream-only problem :)

I mean from an upstream POV we still support RHEL-6 fine in i440fx,
so there's no reason to particularly care about RHEL-6 with q35
upstream. It is only downstream decision to try to force it to
use q35, despite it not working right today.

> > > C) inside libvirt, the implementation of the "virtio-0.9" model is
> > > identical to "virtio", except that the VIR_PCI_CONNECT_TYPE flags for
> > > these devices contain VIR_PCI_CONNECT_TYPE_PCI rather than
> > > VIR_PCI_CONNECT_TYPE_PCIE, resulting in those devices being assigned to
> > > a legacy PCI slot, and thus they would be transitional mode by default.
> > 
> > For 'virtio-0.9' libvirt should set "disable-modern=yes" in QEMU args
> > 
> > For 'virtio-1.0' libvirt should set "disable-legacy=yes" in QEMU args
> 
> If we decide we want to explicitly spell out the options instead
> of relying on QEMU changing behavior based on the slot type, which
> is probably a good idea anyway, I think we should have
> 
>   virtio-0.9 => disable-legacy=no,disable-modern=no
>   virtio-1.0 => disable-legacy=yes,disable-modern=no
> 
> There's basically no reason to have a device legacy-only rather
> than transitional, and spelling out both options instead of only
> one of them just seems more robust.

>From a testing POV it is desirable to be able to have legacy-only.
There is also possibility that guest OS impl 1.0 in a buggy manner,
so forcing legacy only is desirable.

The existing device still already provides a transitional option
on i440fx, and on Q35 if you do explicit placement in a PCI slot.
So I don't think there's a good reason to have a second transitional
device type, especially if we're naming it virtio-0.9, it is rather
misleading if it would be in fact able to run virtio-1.0 mode.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] vm-tests images disks filling up?

[Daniel P . Berrangé]

On Fri, Aug 17, 2018 at 11:26:39AM +0100, Peter Maydell wrote:
> I just ran into a build failure using the tests/vm/ BSD build tests,
> because the NetBSD build image's disk filled up.
> 
> Looking more closely there seemed to be 9 stale build trees in
> the VM's /var/tmp/qemu-test.* , which is why the disk was full
> (they'd used up about 18GB between them).
> 
> The other VMs (freebsd, openbsd) also had the same problem of
> /var/tmp gradually filling with stale trees, they just hadn't
> quite run out of space yet...
> 
> What's the process for managing the disk space on these images?
> How are stale or completed build trees deleted ?

I'd prefer to see the test process honouring the build directory instead
of putting stuff in /var/tmp, so that a developer's normal approach to
cleaning up build artifacts works.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 8/9] hmp-commands: add sync-profile

[Dr. David Alan Gilbert]

* Emilio G. Cota (c...@braap.org) wrote:
> The command introduced here is just for developers. This means that:
> 
> - the interface implemented here could change in the future
> - the command is only meant to be used from HMP, not from QMP
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  hmp.h   |  1 +
>  hmp.c   | 24 
>  hmp-commands.hx | 15 +++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/hmp.h b/hmp.h
> index 33354f1bdd..5f1addcca2 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -42,6 +42,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict);
>  void hmp_info_iothreads(Monitor *mon, const QDict *qdict);
>  void hmp_quit(Monitor *mon, const QDict *qdict);
>  void hmp_stop(Monitor *mon, const QDict *qdict);
> +void hmp_sync_profile(Monitor *mon, const QDict *qdict);
>  void hmp_system_reset(Monitor *mon, const QDict *qdict);
>  void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
>  void hmp_exit_preconfig(Monitor *mon, const QDict *qdict);
> diff --git a/hmp.c b/hmp.c
> index 2aafb50e8e..d94a47f7c7 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1062,6 +1062,30 @@ void hmp_stop(Monitor *mon, const QDict *qdict)
>  qmp_stop(NULL);
>  }
>  
> +void hmp_sync_profile(Monitor *mon, const QDict *qdict)
> +{
> +const char *op = qdict_get_try_str(qdict, "op");
> +
> +if (op == NULL) {
> +bool on = qsp_is_enabled();
> +
> +monitor_printf(mon, "sync-profile is %s\n", on ? "on" : "off");
> +return;
> +}
> +if (!strcmp(op, "on")) {
> +qsp_enable();
> +} else if (!strcmp(op, "off")) {
> +qsp_disable();
> +} else if (!strcmp(op, "reset")) {
> +qsp_reset();
> +} else {
> +Error *err = NULL;
> +
> +error_setg(&err, QERR_INVALID_PARAMETER, op);
> +hmp_handle_error(mon, &err);
> +}
> +}
> +
>  void hmp_system_reset(Monitor *mon, const QDict *qdict)
>  {
>  qmp_system_reset(NULL);
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index c1fc747403..db0c681f74 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -643,6 +643,21 @@ sendkey ctrl-alt-f1
>  
>  This command is useful to send keys that your graphical user interface
>  intercepts at low level, such as @code{ctrl-alt-f1} in X Window.
> +ETEXI
> +{
> +.name   = "sync-profile",
> +.args_type  = "op:s?",
> +.params = "[on|off|reset]",
> +.help   = "enable, disable or reset synchronization profiling. "
> +  "With no arguments, prints whether profiling is on or 
> off.",
> +.cmd= hmp_sync_profile,
> +},

That's OK; it'a  little odd way to do it (I'd have expected a couple of
booleans instead of the string); but it's OK, so


Reviewed-by: Dr. David Alan Gilbert 

> +STEXI
> +@item sync-profile [on|off|reset]
> +@findex sync-profile
> +Enable, disable or reset synchronization profiling. With no arguments, prints
> +whether profiling is on or off.
>  ETEXI
>  
>  {
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PATCH correct] checkpatch: allow space in more places before a bracket

[Paolo Bonzini]

From: Heinrich Schuchardt 

Allow a space between a colon and subsequent opening bracket.  This
sequence may occur in inline assembler statements like

asm(
"ldr %[out], [%[in]]\n\t"
: [out] "=r" (ret)
: [in] "r" (addr)
);

Allow a space between a comma and subsequent opening bracket.  This
sequence may occur in designated initializers.

To ease backporting the patch, I am also changing the comma-bracket
detection (added in QEMU by commit 409db6eb7199af7a2f09f746bd1b793e9daefe5f)
to use the same regex as brackets and colons (as done independently
by Linux commit daebc534ac15f991961a5bb433e515988220e9bf).

Link: http://lkml.kernel.org/r/20180403191655.23700-1-xypron.g...@gmx.de
Signed-off-by: Heinrich Schuchardt 
Acked-by: Joe Perches 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Signed-off-by: Paolo Bonzini 
---
 scripts/checkpatch.pl | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 52ab18bfce..33b5771120 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1977,9 +1977,8 @@ sub process {
my ($where, $prefix) = ($-[1], $1);
if ($prefix !~ /$Type\s+$/ &&
($where != 0 || $prefix !~ /^.\s+$/) &&
-   $prefix !~ /{\s+$/ &&
$prefix !~ /\#\s*define[^(]*\([^)]*\)\s+$/ &&
-   $prefix !~ /,\s+$/) {
+   $prefix !~ /[,{:]\s+$/) {
ERROR("space prohibited before open square 
bracket '['\n" . $herecurr);
}
}
-- 
2.17.1

Re: [Qemu-devel] [RFC PATCH] qom: Rename object_new_with_props to object_new_child

[Paolo Bonzini]

On 17/08/2018 12:04, Daniel P. Berrangé wrote:
> On Fri, Aug 17, 2018 at 10:59:44AM +0100, Daniel P. Berrangé wrote:
>> On Fri, Aug 17, 2018 at 11:58:07AM +0200, Thomas Huth wrote:
>>> On 08/17/2018 11:48 AM, Daniel P. Berrangé wrote:
 On Fri, Aug 17, 2018 at 09:33:33AM +0200, Thomas Huth wrote:
> While adding the object_initialize_child() function, Paolo suggested
> to rename the similar object_new_with_props() function accordingly:
>
> http://marc.info/?i=e034610d-9a1d-a8a5-ee92-b2e3f0ba2...@redhat.com
>
> This way it is more obvious that this function creates a new object
> as a child of another object.

 I'd expect 'object_new_with_child' to be the same as 'object_new',
 but with only 'parent' & 'id' args added, which isn't the case here.

 If we want the full & consistent design then we should have

   object_new(typename)
   object_new_with_child(typename, parent, id)
   object_new_props(typename, ...)
   object_new_propv(typename, va_arg props)
   object_new_with_child_props(typename, parent, id, ...)
   object_new_with_child_propv(typename, parent, id, va_arg props)
>>>
>>> "new_with_child" sounds wrong, too, since the parent is not created
>>> here, but the child. Anyway, I guess the naming of these functions is
>>> too much subject to bikeshedding, so never mind, let's keep it as it
>>> currently is.
>>
>> True, 'new_with_parent' is a better choice in retrospect :-)
> 
> Or indeed 'object_new_child' and 'object_new_child_prop{s,v}' approx as
> you had suggested

That would work for me too.  Really anything works as long as the
initialize and new names are consistent.

In practice object_new_child would be unused, hence the shortcut of
adding the props argument directly to object_new_child{,v}.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 9/9] hmp-commands-info: add sync-profile

[Dr. David Alan Gilbert]

* Emilio G. Cota (c...@braap.org) wrote:
> The command introduced here is just for developers. This means that:
> 
> - the info displayed and the output format could change in the future
> - the command is only meant to be used from HMP, not from QMP
> 
> Sample output:
> 
> (qemu) sync-profile
> sync-profile is off
> (qemu) info sync-profile
> Type   Object  Call site  Wait Time (s) Count  Average 
> (us)
> ---
> ---
> (qemu) sync-profile on
> (qemu) sync-profile
> sync-profile is on
> (qemu) info sync-profile 15
> Type   Object  Call site Wait Time (s) 
> Count  Average (us)
> --
> condvar0x55a01813ced0  cpus.c:116591.38235  
> 2842  32154.24
> BQL mutex  0x55a0171b7140  cpus.c:143412.56490  
> 5787   2171.23
> BQL mutex  0x55a0171b7140  accel/tcg/cpu-exec.c:4327.75846  
> 2844   2728.01
> BQL mutex  0x55a0171b7140  accel/tcg/cputlb.c:870  5.09889  
> 2884   1767.99
> BQL mutex  0x55a0171b7140  accel/tcg/cpu-exec.c:5293.46140  
> 3254   1063.74
> BQL mutex  0x55a0171b7140  accel/tcg/cputlb.c:804  0.76333  
> 8655 88.20
> BQL mutex  0x55a0171b7140  cpus.c:1466 0.60893  
> 2941207.05
> BQL mutex  0x55a0171b7140  util/main-loop.c:2360.00894  
> 6425  1.39
> mutex  [   3]  util/qemu-timer.c:520   0.00342 
> 50611  0.07
> mutex  [   2]  util/qemu-timer.c:426   0.00254 
> 31336  0.08
> mutex  [   3] util/qemu-timer.c:234   0.00107 
> 19275  0.06
> mutex  0x55a0171d9960  vl.c:7630.00043  
> 6425  0.07
> mutex  0x55a0180d1bb0  monitor.c:458   0.00015  
> 1603  0.09
> mutex  0x55a0180e4c78  chardev/char.c:109  0.2   
> 217  0.08
> mutex  0x55a0180d1bb0  monitor.c:448   0.1   
> 162  0.08
> --
> (qemu) info sync-profile -m 15
> Type   Object  Call site Wait Time (s) 
> Count  Average (us)
> --
> condvar0x55a01813ced0  cpus.c:116595.11196  
> 3051  31174.03
> BQL mutex  0x55a0171b7140  accel/tcg/cpu-exec.c:4327.92108  
> 3052   2595.37
> BQL mutex  0x55a0171b7140  cpus.c:143413.38253  
> 6210   2155.00
> BQL mutex  0x55a0171b7140  accel/tcg/cputlb.c:870  5.09901  
> 3093   1648.57
> BQL mutex  0x55a0171b7140  accel/tcg/cpu-exec.c:5294.21123  
> 3468   1214.31
> BQL mutex  0x55a0171b7140  cpus.c:1466 0.60895  
> 3156192.95
> BQL mutex  0x55a0171b7140  accel/tcg/cputlb.c:804  0.76337  
> 9282 82.24
> BQL mutex  0x55a0171b7140  util/main-loop.c:2360.00944  
> 6889  1.37
> mutex  0x55a01813ce80  tcg/tcg.c:397   0.0
> 24  0.15
> mutex  0x55a0180d1bb0  monitor.c:458   0.00018  
> 1922  0.09
> mutex  [   2]  util/qemu-timer.c:426   0.00266 
> 32710  0.08
> mutex  0x55a0180e4c78  chardev/char.c:109  0.2   
> 260  0.08
> mutex  0x55a0180d1bb0  monitor.c:448   0.1   
> 187  0.08
> mutex  0x55a0171d9960  vl.c:7630.00047  
> 6889  0.07
> mutex  [   3]  util/qemu-timer.c:520   0.00362 
> 53377  0.07
> --
> (qemu) info sync-profile -m -n 15
> Type   Object  Call site Wait Time (s) 
> Count  Average (us)
> --
> condvar0x55a01813ced0  cpus.c:1165   101.39331  
> 3398  29839.12
> BQL mutex  0x55a0171b7140  accel/tcg/cpu-exec.c:4327.92112  
> 3399   2330.43
> BQL mutex  0x55a0171b7140  cpus.c:143414.28280  
> 6922   2063.39
> BQL mutex  0x55a0171b7140  accel/tcg/cputlb.c:870  5.77505  
> 3445   1676.36
> BQL mutex  0x55a0171b7140  accel/tcg/cpu-exec.c:529 

Re: [Qemu-devel] vm-tests images disks filling up?

[Peter Maydell]

On 17 August 2018 at 11:47, Daniel P. Berrangé  wrote:
> I'd prefer to see the test process honouring the build directory instead
> of putting stuff in /var/tmp, so that a developer's normal approach to
> cleaning up build artifacts works.

That's tricky because the build directory is outside the VM and
the build is done inside it (using a copy of the sources): the
VM doesn't have access to the build directory.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 7/8] tests/device-introspect: Test with all machines, not only with "none"

[Juan Quintela]

Thomas Huth  wrote:
> Certain device introspection crashes used to only happen if you were
> using a certain machine, e.g. if the machine was using serial_hd() or
> nd_table[], and a device was trying to use these in its instance_init
> function, too.
>
> To be able to catch these problems, let's extend the device-introspect
> test to check the devices on all machine types, with and without the
> "-nodefaults" parameter (since this makes a difference sometimes, too).
> Since this is a rather slow operation, and most of the problems are
> already handled by testing with the "none" machine only, the test with
> all machines is only run in the "make check SPEED=slow" mode.
>
> Reviewed-by: Markus Armbruster 
> Signed-off-by: Thomas Huth 

Reviewed-by: Juan Quintela 


> ---
>  tests/device-introspect-test.c | 32 +---
>  1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
> index a38193b..a25092d 100644
> --- a/tests/device-introspect-test.c
> +++ b/tests/device-introspect-test.c
> @@ -221,13 +221,13 @@ static void test_device_intro_abstract(void)
>  qtest_end();
>  }
>  
> -static void test_device_intro_concrete(void)
> +static void test_device_intro_concrete(const void *args)
  

I hate this game of casts, but I have no good suggestions, sorry.

Re: [Qemu-devel] [PATCH v2 8/8] MAINTAINERS: add maintainers for qtest

[Juan Quintela]

Thomas Huth  wrote:
> From: Paolo Bonzini 
>
> Thomas has been doing a lot of work on qom-test and device-introspection-test,
> and Laurent has ported libqos to sPAPR and co-mentored Emanuele on the
> upcoming qtest device framework.  They deserve recognition. :)
>
> Signed-off-by: Paolo Bonzini 
> Reviewed-by: Laurent Vivier 
> Reviewed-by: John Snow 
> [thuth: removed the hunk that deleted the tests/qom-test.c entry]
> Signed-off-by: Thomas Huth 

Reviewed-by: Juan Quintela 

Re: [Qemu-devel] vm-tests images disks filling up?

[Daniel P . Berrangé]

On Fri, Aug 17, 2018 at 11:47:06AM +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 17, 2018 at 11:26:39AM +0100, Peter Maydell wrote:
> > I just ran into a build failure using the tests/vm/ BSD build tests,
> > because the NetBSD build image's disk filled up.
> > 
> > Looking more closely there seemed to be 9 stale build trees in
> > the VM's /var/tmp/qemu-test.* , which is why the disk was full
> > (they'd used up about 18GB between them).
> > 
> > The other VMs (freebsd, openbsd) also had the same problem of
> > /var/tmp gradually filling with stale trees, they just hadn't
> > quite run out of space yet...
> > 
> > What's the process for managing the disk space on these images?
> > How are stale or completed build trees deleted ?
> 
> I'd prefer to see the test process honouring the build directory instead
> of putting stuff in /var/tmp, so that a developer's normal approach to
> cleaning up build artifacts works.

Oh, wait I'm stupid,  you're talking about the /var/tmp inside the guest
disk images, not the host !

IIUC, the problem is that we create the tests/vm/freebsd.img file
by deep copying the template cached under $HOME and never purge it,
so it gradually fills up.

It would be desirable to guarantee a clean image for every build test
to avoid risk of previous problems affecting subsequent builds.

So how about we create tests/vm/freebsd.img as a qcow2 overlay that
points back to the cached image in $HOME. That makes it cheap to create,
so on every test we can just throw away the previous overlay to get
pristine state.

NB, don't delete the overlay at end of build - only start of the next
build. That gives devs a way to launch the VM to debug a failed build

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH V11 05/20] COLO: Add block replication into colo process

[Dr. David Alan Gilbert]

* Zhang Chen (zhangc...@gmail.com) wrote:
> Make sure master start block replication after slave's block
> replication started.
> 
> Besides, we need to activate VM's blocks before goes into
> COLO state.
> 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Li Zhijian 
> Signed-off-by: Zhang Chen 
> Signed-off-by: Zhang Chen 

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  migration/colo.c  | 43 +++
>  migration/migration.c |  9 +
>  2 files changed, 52 insertions(+)
> 
> diff --git a/migration/colo.c b/migration/colo.c
> index 081df1835f..e06640c3d6 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -27,6 +27,7 @@
>  #include "replication.h"
>  #include "net/colo-compare.h"
>  #include "net/colo.h"
> +#include "block/block.h"
>  
>  static bool vmstate_loading;
>  static Notifier packets_compare_notifier;
> @@ -56,6 +57,7 @@ static void secondary_vm_do_failover(void)
>  {
>  int old_state;
>  MigrationIncomingState *mis = migration_incoming_get_current();
> +Error *local_err = NULL;
>  
>  /* Can not do failover during the process of VM's loading VMstate, Or
>   * it will break the secondary VM.
> @@ -73,6 +75,11 @@ static void secondary_vm_do_failover(void)
>  migrate_set_state(&mis->state, MIGRATION_STATUS_COLO,
>MIGRATION_STATUS_COMPLETED);
>  
> +replication_stop_all(true, &local_err);
> +if (local_err) {
> +error_report_err(local_err);
> +}
> +
>  if (!autostart) {
>  error_report("\"-S\" qemu option will be ignored in secondary side");
>  /* recover runstate to normal migration finish state */
> @@ -110,6 +117,7 @@ static void primary_vm_do_failover(void)
>  {
>  MigrationState *s = migrate_get_current();
>  int old_state;
> +Error *local_err = NULL;
>  
>  migrate_set_state(&s->state, MIGRATION_STATUS_COLO,
>MIGRATION_STATUS_COMPLETED);
> @@ -133,6 +141,13 @@ static void primary_vm_do_failover(void)
>   FailoverStatus_str(old_state));
>  return;
>  }
> +
> +replication_stop_all(true, &local_err);
> +if (local_err) {
> +error_report_err(local_err);
> +local_err = NULL;
> +}
> +
>  /* Notify COLO thread that failover work is finished */
>  qemu_sem_post(&s->colo_exit_sem);
>  }
> @@ -356,6 +371,11 @@ static int colo_do_checkpoint_transaction(MigrationState 
> *s,
>  qemu_savevm_state_header(fb);
>  qemu_savevm_state_setup(fb);
>  qemu_mutex_lock_iothread();
> +replication_do_checkpoint_all(&local_err);
> +if (local_err) {
> +qemu_mutex_unlock_iothread();
> +goto out;
> +}
>  qemu_savevm_state_complete_precopy(fb, false, false);
>  qemu_mutex_unlock_iothread();
>  
> @@ -446,6 +466,12 @@ static void colo_process_checkpoint(MigrationState *s)
>  object_unref(OBJECT(bioc));
>  
>  qemu_mutex_lock_iothread();
> +replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
> +if (local_err) {
> +qemu_mutex_unlock_iothread();
> +goto out;
> +}
> +
>  vm_start();
>  qemu_mutex_unlock_iothread();
>  trace_colo_vm_state_change("stop", "run");
> @@ -585,6 +611,11 @@ void *colo_process_incoming_thread(void *opaque)
>  object_unref(OBJECT(bioc));
>  
>  qemu_mutex_lock_iothread();
> +replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
> +if (local_err) {
> +qemu_mutex_unlock_iothread();
> +goto out;
> +}
>  vm_start();
>  trace_colo_vm_state_change("stop", "run");
>  qemu_mutex_unlock_iothread();
> @@ -665,6 +696,18 @@ void *colo_process_incoming_thread(void *opaque)
>  goto out;
>  }
>  
> +replication_get_error_all(&local_err);
> +if (local_err) {
> +qemu_mutex_unlock_iothread();
> +goto out;
> +}
> +/* discard colo disk buffer */
> +replication_do_checkpoint_all(&local_err);
> +if (local_err) {
> +qemu_mutex_unlock_iothread();
> +goto out;
> +}
> +
>  vmstate_loading = false;
>  vm_start();
>  trace_colo_vm_state_change("stop", "run");
> diff --git a/migration/migration.c b/migration/migration.c
> index 33a2fd74c6..71492c642d 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -385,6 +385,7 @@ static void process_incoming_migration_co(void *opaque)
>  MigrationIncomingState *mis = migration_incoming_get_current();
>  PostcopyState ps;
>  int ret;
> +Error *local_err = NULL;
>  
>  assert(mis->from_src_file);
>  mis->largest_page_size = qemu_ram_pagesize_largest();
> @@ -416,6 +417,14 @@ static void process_incoming_migration_co(void *opaque)
>  
>  /* we get COLO info, and know if we are in COLO mode */
>  if (!ret && migration_incoming_enable_colo()) {
> +/* Make sure all file formats flush their mu

Re: [Qemu-devel] [PATCH V10 05/20] COLO: Add block replication into colo process

[Dr. David Alan Gilbert]

* Zhang Chen (zhangc...@gmail.com) wrote:
> On Tue, Aug 7, 2018 at 10:30 PM Dr. David Alan Gilbert 
> wrote:
> 
> > * Zhang Chen (zhangc...@gmail.com) wrote:
> > > Make sure master start block replication after slave's block
> > > replication started.
> > >
> > > Besides, we need to activate VM's blocks before goes into
> > > COLO state.
> > >
> > > Signed-off-by: zhanghailiang 
> > > Signed-off-by: Li Zhijian 
> > > Signed-off-by: Zhang Chen 
> > > ---
> > >  migration/colo.c  | 43 +++
> > >  migration/migration.c |  9 +
> > >  2 files changed, 52 insertions(+)
> > >
> > > diff --git a/migration/colo.c b/migration/colo.c
> > > index 081df1835f..e06640c3d6 100644
> > > --- a/migration/colo.c
> > > +++ b/migration/colo.c
> > > @@ -27,6 +27,7 @@
> > >  #include "replication.h"
> > >  #include "net/colo-compare.h"
> > >  #include "net/colo.h"
> > > +#include "block/block.h"
> > >
> > >  static bool vmstate_loading;
> > >  static Notifier packets_compare_notifier;
> > > @@ -56,6 +57,7 @@ static void secondary_vm_do_failover(void)
> > >  {
> > >  int old_state;
> > >  MigrationIncomingState *mis = migration_incoming_get_current();
> > > +Error *local_err = NULL;
> > >
> > >  /* Can not do failover during the process of VM's loading VMstate,
> > Or
> > >   * it will break the secondary VM.
> > > @@ -73,6 +75,11 @@ static void secondary_vm_do_failover(void)
> > >  migrate_set_state(&mis->state, MIGRATION_STATUS_COLO,
> > >MIGRATION_STATUS_COMPLETED);
> > >
> > > +replication_stop_all(true, &local_err);
> > > +if (local_err) {
> > > +error_report_err(local_err);
> > > +}
> > > +
> > >  if (!autostart) {
> > >  error_report("\"-S\" qemu option will be ignored in secondary
> > side");
> > >  /* recover runstate to normal migration finish state */
> > > @@ -110,6 +117,7 @@ static void primary_vm_do_failover(void)
> > >  {
> > >  MigrationState *s = migrate_get_current();
> > >  int old_state;
> > > +Error *local_err = NULL;
> > >
> > >  migrate_set_state(&s->state, MIGRATION_STATUS_COLO,
> > >MIGRATION_STATUS_COMPLETED);
> > > @@ -133,6 +141,13 @@ static void primary_vm_do_failover(void)
> > >   FailoverStatus_str(old_state));
> > >  return;
> > >  }
> > > +
> > > +replication_stop_all(true, &local_err);
> > > +if (local_err) {
> > > +error_report_err(local_err);
> > > +local_err = NULL;
> > > +}
> > > +
> > >  /* Notify COLO thread that failover work is finished */
> > >  qemu_sem_post(&s->colo_exit_sem);
> > >  }
> > > @@ -356,6 +371,11 @@ static int
> > colo_do_checkpoint_transaction(MigrationState *s,
> > >  qemu_savevm_state_header(fb);
> > >  qemu_savevm_state_setup(fb);
> > >  qemu_mutex_lock_iothread();
> > > +replication_do_checkpoint_all(&local_err);
> > > +if (local_err) {
> > > +qemu_mutex_unlock_iothread();
> > > +goto out;
> > > +}
> >
> > In docs/block-replication.txt it says:
> >   b. replication_do_checkpoint_all()
> >This interface is called after all VM state is transferred to
> >Secondary QEMU. The Disk buffer will be dropped in this interface.
> >The caller must hold the I/O mutex lock if it is in migration/checkpoint
> >thread.
> >
> > but we're making this call before the call below that actually transfers
> > all the state.  Which is right?
> >
> 
> Hi Dave,
> 
> The "docs/block-replication.txt" means we should call the
> replication_do_checkpoint_all() after VM state is transferred in secondary
> node,
> and in primary node this function no need to call before transfers all the
> state.

OK, it might be worth clarifying the docs.

Dave

> Thanks
> Zhang Chen
> 
> 
> 
> >
> > Other than that I think it's OK.
> >
> > Dave
> >
> > >  qemu_savevm_state_complete_precopy(fb, false, false);
> > >  qemu_mutex_unlock_iothread();
> > >
> > > @@ -446,6 +466,12 @@ static void colo_process_checkpoint(MigrationState
> > *s)
> > >  object_unref(OBJECT(bioc));
> > >
> > >  qemu_mutex_lock_iothread();
> > > +replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
> > > +if (local_err) {
> > > +qemu_mutex_unlock_iothread();
> > > +goto out;
> > > +}
> > > +
> > >  vm_start();
> > >  qemu_mutex_unlock_iothread();
> > >  trace_colo_vm_state_change("stop", "run");
> > > @@ -585,6 +611,11 @@ void *colo_process_incoming_thread(void *opaque)
> > >  object_unref(OBJECT(bioc));
> > >
> > >  qemu_mutex_lock_iothread();
> > > +replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
> > > +if (local_err) {
> > > +qemu_mutex_unlock_iothread();
> > > +goto out;
> > > +}
> > >  vm_start();
> > >  trace_colo_vm_state_change("stop", "run");
> > >  qemu_mutex_unlock_iothread();
> > > @@ -665,6 +696,18 @@ void *colo

Re: [Qemu-devel] [PATCH 56/56] docs/interop/qmp-spec: How to force known good parser state

[Markus Armbruster]

Eric Blake  writes:

> On 08/08/2018 07:03 AM, Markus Armbruster wrote:
>> Section "QGA Synchronization" specifies that sending "a raw 0xFF
>> sentinel byte" makes the server "reset its state and discard all
>> pending data prior to the sentinel."  What actually happens there is a
>> lexical error, which will produce one ore more error responses.
>> Moreover, it's not specific to QGA.
>
> Hoisting my review of this, as you may want to move it sooner in the series.
>
>>
>> Create new section "Forcing the JSON parser into known-good state" to
>> document the technique properly.  Rewrite section "QGA
>> Synchronization" to document just the other direction, i.e. command
>> guest-sync-delimited.
>>
>> Section "Protocol Specification" mentions "synchronization bytes
>> (documented below)".  Delete that.
>>
>> While there, fix it not to claim '"Server" is QEMU itself', but
>> '"Server" is either QEMU or the QEMU Guest Agent'.
>>
>> Signed-off-by: Markus Armbruster 
>> ---
>>   docs/interop/qmp-spec.txt | 37 -
>>   1 file changed, 24 insertions(+), 13 deletions(-)
>>
>> diff --git a/docs/interop/qmp-spec.txt b/docs/interop/qmp-spec.txt
>> index 1566b8ae5e..d4a42fe2cc 100644
>> --- a/docs/interop/qmp-spec.txt
>> +++ b/docs/interop/qmp-spec.txt
>> @@ -20,9 +20,9 @@ operating system.
>>   2. Protocol Specification
>>   =
>>   -This section details the protocol format. For the purpose of this
>> document
>> -"Client" is any application which is using QMP to communicate with QEMU and
>> -"Server" is QEMU itself.
>> +This section details the protocol format. For the purpose of this
>> +document, "Server" is either QEMU or the QEMU Guest Agent, and
>> +"Client" is any application communicating with it via QMP.
>>   
[...]
>>   JSON data structures, when mentioned in this document, are always in the
>>   following format:
>> @@ -34,9 +34,8 @@ by the JSON standard:
>> http://www.ietf.org/rfc/rfc7159.txt
>>   -The protocol is always encoded in UTF-8 except for
>> synchronization
>> -bytes (documented below); although thanks to json-string escape
>> -sequences, the server will reply using only the strict ASCII subset.
>> +The sever expects its input to be encoded in UTF-8, and sends its
>> +output encoded in ASCII.
>>   
[...]
>>   For convenience, json-object members mentioned in this document will
>>   be in a certain order. However, in real protocol usage they can be in
>> @@ -215,16 +214,28 @@ Some events are rate-limited to at most one per 
>> second.  If additional
>>   dropped, and the last one is delayed.  "Similar" normally means same
>>   event type.  See qmp-events.txt for details.
>>   -2.6 QGA Synchronization
>> +2.6 Forcing the JSON parser into known-good state
>> +-
>> +
>> +Incomplete or invalid input can leave the server's JSON parser in a
>> +state where it can't parse additional commands.  To get it back into
>> +known-good state, the client should provoke a lexical error.
>> +
>> +The cleanest way to do that is sending an ASCII control character
>> +other than '\t' (horizontal tab), '\r' (carriage return), and '\n'
>
> s/and/or/
>
>> +(new line).
>> +
>> +Sadly, older versions of QEMU can fail to flag this as an error.  If a
>> +client needs to deal with them, it should send a 0xFF byte.
[...]
>> +
>> +2.7 QGA Synchronization
>>   ---
>> When using QGA, an additional synchronization feature is built
>> into
>> -the protocol.  If the Client sends a raw 0xFF sentinel byte (not valid
>> -JSON), then the Server will reset its state and discard all pending
>> -data prior to the sentinel.  Conversely, if the Client makes use of
>> -the 'guest-sync-delimited' command, the Server will send a raw 0xFF
>> -sentinel byte prior to its response, to aid the Client in discarding
>> -any data prior to the sentinel.
>> +the protocol. If the Client makes use of the 'guest-sync-delimited'
>> +command, the Server will send a raw 0xFF sentinel byte prior to its
>> +response, to aid the Client in discarding any data prior to the
>> +sentinel.
>
> Maybe worth mentioning "including error messages reported about any
> lexical errors received prior to the guest-sync-delimited command"
>
>>   3. QMP Examples
>>

What about:

2.7 QGA Synchronization
---

When a client connects to QGA over a transport lacking proper
connection semantics such as virtio-serial, QGA may have read partial
input from a previous client.  The client needs to force QGA's parser
into known-good state using the previous section's technique.
Moreover, the client may receive output a previous client didn't read.
To help with skipping that output, QGA provides the
'guest-sync-delimited' command.  Refer to its documentation for
details.

[Qemu-devel] [PATCH 2/3] memory: Remove MMIO request_ptr APIs

[Peter Maydell]

Remove the obsolete MMIO request_ptr APIs; they have no
users now.

Signed-off-by: Peter Maydell 
---
 include/exec/memory.h |  35 --
 memory.c  | 110 --
 2 files changed, 145 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 448d41a7529..68636561821 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -141,15 +141,6 @@ struct MemoryRegionOps {
 uint64_t data,
 unsigned size,
 MemTxAttrs attrs);
-/* Instruction execution pre-callback:
- * @addr is the address of the access relative to the @mr.
- * @size is the size of the area returned by the callback.
- * @offset is the location of the pointer inside @mr.
- *
- * Returns a pointer to a location which contains guest code.
- */
-void *(*request_ptr)(void *opaque, hwaddr addr, unsigned *size,
- unsigned *offset);
 
 enum device_endian endianness;
 /* Guest-visible constraints: */
@@ -1667,32 +1658,6 @@ void memory_global_dirty_log_stop(void);
 void mtree_info(fprintf_function mon_printf, void *f, bool flatview,
 bool dispatch_tree, bool owner);
 
-/**
- * memory_region_request_mmio_ptr: request a pointer to an mmio
- * MemoryRegion. If it is possible map a RAM MemoryRegion with this pointer.
- * When the device wants to invalidate the pointer it will call
- * memory_region_invalidate_mmio_ptr.
- *
- * @mr: #MemoryRegion to check
- * @addr: address within that region
- *
- * Returns true on success, false otherwise.
- */
-bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr);
-
-/**
- * memory_region_invalidate_mmio_ptr: invalidate the pointer to an mmio
- * previously requested.
- * In the end that means that if something wants to execute from this area it
- * will need to request the pointer again.
- *
- * @mr: #MemoryRegion associated to the pointer.
- * @offset: offset within the memory region
- * @size: size of that area.
- */
-void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
-   unsigned size);
-
 /**
  * memory_region_dispatch_read: perform a read directly to the specified
  * MemoryRegion.
diff --git a/memory.c b/memory.c
index 2ea16e7bfb0..8b44672c132 100644
--- a/memory.c
+++ b/memory.c
@@ -29,7 +29,6 @@
 #include "exec/ram_addr.h"
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
-#include "hw/misc/mmio_interface.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 
@@ -2680,115 +2679,6 @@ void memory_listener_unregister(MemoryListener 
*listener)
 listener->address_space = NULL;
 }
 
-bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr)
-{
-void *host;
-unsigned size = 0;
-unsigned offset = 0;
-Object *new_interface;
-
-if (!mr || !mr->ops->request_ptr) {
-return false;
-}
-
-/*
- * Avoid an update if the request_ptr call
- * memory_region_invalidate_mmio_ptr which seems to be likely when we use
- * a cache.
- */
-memory_region_transaction_begin();
-
-host = mr->ops->request_ptr(mr->opaque, addr - mr->addr, &size, &offset);
-
-if (!host || !size) {
-memory_region_transaction_commit();
-return false;
-}
-
-new_interface = object_new("mmio_interface");
-qdev_prop_set_uint64(DEVICE(new_interface), "start", offset);
-qdev_prop_set_uint64(DEVICE(new_interface), "end", offset + size - 1);
-qdev_prop_set_bit(DEVICE(new_interface), "ro", true);
-qdev_prop_set_ptr(DEVICE(new_interface), "host_ptr", host);
-qdev_prop_set_ptr(DEVICE(new_interface), "subregion", mr);
-object_property_set_bool(OBJECT(new_interface), true, "realized", NULL);
-
-memory_region_transaction_commit();
-return true;
-}
-
-typedef struct MMIOPtrInvalidate {
-MemoryRegion *mr;
-hwaddr offset;
-unsigned size;
-int busy;
-int allocated;
-} MMIOPtrInvalidate;
-
-#define MAX_MMIO_INVALIDATE 10
-static MMIOPtrInvalidate mmio_ptr_invalidate_list[MAX_MMIO_INVALIDATE];
-
-static void memory_region_do_invalidate_mmio_ptr(CPUState *cpu,
- run_on_cpu_data data)
-{
-MMIOPtrInvalidate *invalidate_data = (MMIOPtrInvalidate *)data.host_ptr;
-MemoryRegion *mr = invalidate_data->mr;
-hwaddr offset = invalidate_data->offset;
-unsigned size = invalidate_data->size;
-MemoryRegionSection section = memory_region_find(mr, offset, size);
-
-qemu_mutex_lock_iothread();
-
-/* Reset dirty so this doesn't happen later. */
-cpu_physical_memory_test_and_clear_dirty(offset, size, 1);
-
-if (section.mr != mr) {
-/* memory_region_find add a ref on section.mr */
-memory_region_unref(section.mr);
-if (MMIO_INTERFACE(section.mr->owner)) {
-/* We found the interface just 

[Qemu-devel] [PATCH 3/3] hw/misc: Remove mmio_interface device

[Peter Maydell]

The mmio_interface device was a purely internal artifact
of the implementation of the memory subsystem's request_ptr
APIs. Now that we have removed those APIs, we can remove
the mmio_interface device too.

Signed-off-by: Peter Maydell 
---
 hw/misc/Makefile.objs|   1 -
 include/hw/misc/mmio_interface.h |  49 ---
 hw/misc/mmio_interface.c | 135 ---
 3 files changed, 185 deletions(-)
 delete mode 100644 include/hw/misc/mmio_interface.h
 delete mode 100644 hw/misc/mmio_interface.c

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 51d27b3af1e..22714b08510 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -71,5 +71,4 @@ obj-$(CONFIG_PVPANIC) += pvpanic.o
 obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
 obj-$(CONFIG_AUX) += auxbus.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
-obj-y += mmio_interface.o
 obj-$(CONFIG_MSF2) += msf2-sysreg.o
diff --git a/include/hw/misc/mmio_interface.h b/include/hw/misc/mmio_interface.h
deleted file mode 100644
index 90d34fb2286..000
--- a/include/hw/misc/mmio_interface.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * mmio_interface.h
- *
- *  Copyright (C) 2017 : GreenSocs
- *  http://www.greensocs.com/ , email: i...@greensocs.com
- *
- *  Developed by :
- *  Frederic Konrad   
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 2 of the License, or
- * (at your option)any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, see .
- *
- */
-
-#ifndef MMIO_INTERFACE_H
-#define MMIO_INTERFACE_H
-
-#include "exec/memory.h"
-
-#define TYPE_MMIO_INTERFACE "mmio_interface"
-#define MMIO_INTERFACE(obj) OBJECT_CHECK(MMIOInterface, (obj), 
\
- TYPE_MMIO_INTERFACE)
-
-typedef struct MMIOInterface {
-DeviceState parent_obj;
-
-MemoryRegion *subregion;
-MemoryRegion ram_mem;
-uint64_t start;
-uint64_t end;
-bool ro;
-uint64_t id;
-void *host_ptr;
-} MMIOInterface;
-
-void mmio_interface_map(MMIOInterface *s);
-void mmio_interface_unmap(MMIOInterface *s);
-
-#endif /* MMIO_INTERFACE_H */
diff --git a/hw/misc/mmio_interface.c b/hw/misc/mmio_interface.c
deleted file mode 100644
index 3b0e2039a36..000
--- a/hw/misc/mmio_interface.c
+++ /dev/null
@@ -1,135 +0,0 @@
-/*
- * mmio_interface.c
- *
- *  Copyright (C) 2017 : GreenSocs
- *  http://www.greensocs.com/ , email: i...@greensocs.com
- *
- *  Developed by :
- *  Frederic Konrad   
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 2 of the License, or
- * (at your option)any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, see .
- *
- */
-
-#include "qemu/osdep.h"
-#include "qemu/log.h"
-#include "trace.h"
-#include "hw/qdev-properties.h"
-#include "hw/misc/mmio_interface.h"
-#include "qapi/error.h"
-
-#ifndef DEBUG_MMIO_INTERFACE
-#define DEBUG_MMIO_INTERFACE 0
-#endif
-
-static uint64_t mmio_interface_counter;
-
-#define DPRINTF(fmt, ...) do { 
\
-if (DEBUG_MMIO_INTERFACE) {
\
-qemu_log("mmio_interface: 0x%" PRIX64 ": " fmt, s->id, ## 
__VA_ARGS__);\
-}  
\
-} while (0)
-
-static void mmio_interface_init(Object *obj)
-{
-MMIOInterface *s = MMIO_INTERFACE(obj);
-
-if (DEBUG_MMIO_INTERFACE) {
-s->id = mmio_interface_counter++;
-}
-
-DPRINTF("interface created\n");
-s->host_ptr = 0;
-s->subregion = 0;
-}
-
-static void mmio_interface_realize(DeviceState *dev, Error **errp)
-{
-MMIOInterface *s = MMIO_INTERFACE(dev);
-
-DPRINTF("realize from 0x%" PRIX64 " to 0x%" PRIX64 " map host pointer"
-" %p\n", s->start, s->end, s->host_ptr);
-
-if (!s->host_ptr) {
-error_setg(errp, "host_ptr property must be set");
-return;
-}
-
-if (!s->subregion) {
-error_setg(errp, "subregion property must be

[Qemu-devel] [PATCH 1/3] hw/ssi/xilinx_spips: Remove unneeded MMIO request_ptr code

[Peter Maydell]

We now support direct execution from MMIO regions in the
core memory subsystem. This means that we don't need to
have device-specific support for it, and we can remove
the request_ptr handling from the Xilinx SPIPS device.
(It was broken anyway due to race conditions, and disabled
by default.)

This device is the only in-tree user of this API.

Signed-off-by: Peter Maydell 
---
 hw/ssi/xilinx_spips.c | 46 ---
 1 file changed, 46 deletions(-)

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index c052bfc4b3c..16f88f74029 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -1031,14 +1031,6 @@ static const MemoryRegionOps spips_ops = {
 
 static void xilinx_qspips_invalidate_mmio_ptr(XilinxQSPIPS *q)
 {
-XilinxSPIPS *s = &q->parent_obj;
-
-if ((q->mmio_execution_enabled) && (q->lqspi_cached_addr != ~0ULL)) {
-/* Invalidate the current mapped mmio */
-memory_region_invalidate_mmio_ptr(&s->mmlqspi, q->lqspi_cached_addr,
-  LQSPI_CACHE_SIZE);
-}
-
 q->lqspi_cached_addr = ~0ULL;
 }
 
@@ -1207,23 +1199,6 @@ static void lqspi_load_cache(void *opaque, hwaddr addr)
 }
 }
 
-static void *lqspi_request_mmio_ptr(void *opaque, hwaddr addr, unsigned *size,
-unsigned *offset)
-{
-XilinxQSPIPS *q = opaque;
-hwaddr offset_within_the_region;
-
-if (!q->mmio_execution_enabled) {
-return NULL;
-}
-
-offset_within_the_region = addr & ~(LQSPI_CACHE_SIZE - 1);
-lqspi_load_cache(opaque, offset_within_the_region);
-*size = LQSPI_CACHE_SIZE;
-*offset = offset_within_the_region;
-return q->lqspi_buf;
-}
-
 static uint64_t
 lqspi_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -1245,7 +1220,6 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size)
 
 static const MemoryRegionOps lqspi_ops = {
 .read = lqspi_read,
-.request_ptr = lqspi_request_mmio_ptr,
 .endianness = DEVICE_NATIVE_ENDIAN,
 .valid = {
 .min_access_size = 1,
@@ -1322,15 +1296,6 @@ static void xilinx_qspips_realize(DeviceState *dev, 
Error **errp)
 sysbus_init_mmio(sbd, &s->mmlqspi);
 
 q->lqspi_cached_addr = ~0ULL;
-
-/* mmio_execution breaks migration better aborting than having strange
- * bugs.
- */
-if (q->mmio_execution_enabled) {
-error_setg(&q->migration_blocker,
-   "enabling mmio_execution breaks migration");
-migrate_add_blocker(q->migration_blocker, &error_fatal);
-}
 }
 
 static void xlnx_zynqmp_qspips_realize(DeviceState *dev, Error **errp)
@@ -1427,16 +1392,6 @@ static Property xilinx_zynqmp_qspips_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };
 
-static Property xilinx_qspips_properties[] = {
-/* We had to turn this off for 2.10 as it is not compatible with migration.
- * It can be enabled but will prevent the device to be migrated.
- * This will go aways when a fix will be released.
- */
-DEFINE_PROP_BOOL("x-mmio-exec", XilinxQSPIPS, mmio_execution_enabled,
- false),
-DEFINE_PROP_END_OF_LIST(),
-};
-
 static Property xilinx_spips_properties[] = {
 DEFINE_PROP_UINT8("num-busses", XilinxSPIPS, num_busses, 1),
 DEFINE_PROP_UINT8("num-ss-bits", XilinxSPIPS, num_cs, 4),
@@ -1450,7 +1405,6 @@ static void xilinx_qspips_class_init(ObjectClass *klass, 
void * data)
 XilinxSPIPSClass *xsc = XILINX_SPIPS_CLASS(klass);
 
 dc->realize = xilinx_qspips_realize;
-dc->props = xilinx_qspips_properties;
 xsc->reg_ops = &qspips_ops;
 xsc->rx_fifo_size = RXFF_A_Q;
 xsc->tx_fifo_size = TXFF_A_Q;
-- 
2.18.0

[Qemu-devel] [PATCH 0/3] Drop obsolete memory system request_ptr API

[Peter Maydell]

Now that support has hit master for direct execution from
arbitrary MMIO regions, we can remove the MMIO request_ptr
API, which required special case support in each device that
wanted to handle it, and also had bad race conditions that
resulted in crashes if you tried to use it heavily.

This API was only ever used in one device in the source
tree, the Xilinx SPIPS. These three patches remove the
now-unneeded code from the Xilinx device and then the
core memory subsystem code.

thanks
-- PMM

Peter Maydell (3):
  hw/ssi/xilinx_spips: Remove unneeded MMIO request_ptr code
  memory: Remove MMIO request_ptr APIs
  hw/misc: Remove mmio_interface device

 hw/misc/Makefile.objs|   1 -
 include/exec/memory.h|  35 
 include/hw/misc/mmio_interface.h |  49 ---
 hw/misc/mmio_interface.c | 135 ---
 hw/ssi/xilinx_spips.c|  46 ---
 memory.c | 110 -
 6 files changed, 376 deletions(-)
 delete mode 100644 include/hw/misc/mmio_interface.h
 delete mode 100644 hw/misc/mmio_interface.c

-- 
2.18.0

Re: [Qemu-devel] [PULL 0/8] x86 queue, 2018-08-16

[Peter Maydell]

On 17 August 2018 at 02:33, Eduardo Habkost  wrote:
> The following changes since commit bb16c0412a572c2c9cd44496deb3ad430bc49c1a:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20180816' into staging (2018-08-16 
> 14:35:50 +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 7210a02c58572b2686a3a8d610c6628f87864aed:
>
>   i386: Disable TOPOEXT by default on "-cpu host" (2018-08-16 13:43:01 -0300)
>
> 
> x86 queue, 2018-08-16
>
> Bug fix:
> * Some guests may crash when using "-cpu host" due to TOPOEXT,
>   disable it by default
>
> Features:
> * PV_SEND_IPI feature bit
> * Icelake-{Server,Client} CPU models
> * New CPUID feature bits: PV_SEND_IPI, WBNOINVD, PCONFIG, ARCH_CAPABILITIES
>
> Documentation:
> * docs/qemu-cpu-models.texi

Applied, thanks.

-- PMM

Re: [Qemu-devel] vm-tests images disks filling up?

[Fam Zheng]

On Fri, 08/17 11:26, Peter Maydell wrote:
> I just ran into a build failure using the tests/vm/ BSD build tests,
> because the NetBSD build image's disk filled up.
> 
> Looking more closely there seemed to be 9 stale build trees in
> the VM's /var/tmp/qemu-test.* , which is why the disk was full
> (they'd used up about 18GB between them).
> 
> The other VMs (freebsd, openbsd) also had the same problem of
> /var/tmp gradually filling with stale trees, they just hadn't
> quite run out of space yet...
> 
> What's the process for managing the disk space on these images?
> How are stale or completed build trees deleted ?

Ouch. We used to always use snapshot when running tests until 983c2a777be.

One possibility is to use a fixed temp dir (image locking makes sure no two
tests boot the same image concurrently). I.e. something like:

  $ sed -i -e 's:cd.*mktemp.*:mkdir -p /var/tmp/qemu-vm-test; cd 
/var/tmp/qemu-vm-test:' tests/vm/*

Fam

[Qemu-devel] [PATCH v2] sdhci: add i.MX SD Stable Clock bit

[Hans-Erik Floryd]

Add the ESDHC PRSSTAT_SDSTB bit, using the value of SDHC_CLOCK_INT_STABLE.
Freescale recommends checking this bit when changing clock frequency.

Signed-off-by: Hans-Erik Floryd 
---
 hw/sd/sdhci-internal.h | 2 ++
 hw/sd/sdhci.c  | 8 
 2 files changed, 10 insertions(+)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index 756ef3f..19665fd 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -302,4 +302,6 @@ extern const VMStateDescription sdhci_vmstate;
 #define ESDHC_CTRL_4BITBUS  (0x1 << 1)
 #define ESDHC_CTRL_8BITBUS  (0x2 << 1)
 
+#define ESDHC_PRNSTS_SDSTB  (1 << 3)
+
 #endif
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 8f58c31..23069e8 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1651,6 +1651,14 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, 
unsigned size)
 
 break;
 
+case SDHC_PRNSTS:
+   /* Add SDSTB (SD Clock Stable) bit to PRNSTS */
+   ret = sdhci_read(opaque, offset, size) & ~ESDHC_PRNSTS_SDSTB;
+   if (s->clkcon & SDHC_CLOCK_INT_STABLE) {
+  ret |= ESDHC_PRNSTS_SDSTB;
+   }
+   break;
+
 case ESDHC_DLL_CTRL:
 case ESDHC_TUNE_CTRL_STATUS:
 case ESDHC_UNDOCUMENTED_REG27:
-- 
2.7.4

[Qemu-devel] [PATCH v3] sdhci: add i.MX SD Stable Clock bit

[Hans-Erik Floryd]

Add the ESDHC PRSSTAT_SDSTB bit, using the value of SDHC_CLOCK_INT_STABLE.
Freescale recommends checking this bit when changing clock frequency.

Signed-off-by: Hans-Erik Floryd 
---
 hw/sd/sdhci-internal.h | 2 ++
 hw/sd/sdhci.c  | 8 
 2 files changed, 10 insertions(+)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index 756ef3f..19665fd 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -302,4 +302,6 @@ extern const VMStateDescription sdhci_vmstate;
 #define ESDHC_CTRL_4BITBUS  (0x1 << 1)
 #define ESDHC_CTRL_8BITBUS  (0x2 << 1)
 
+#define ESDHC_PRNSTS_SDSTB  (1 << 3)
+
 #endif
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 8f58c31..5201f3c 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1651,6 +1651,14 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, 
unsigned size)
 
 break;
 
+case SDHC_PRNSTS:
+   /* Add SDSTB (SD Clock Stable) bit to PRNSTS */
+   ret = sdhci_read(opaque, offset, size) & ~ESDHC_PRNSTS_SDSTB;
+   if (s->clkcon & SDHC_CLOCK_INT_STABLE) {
+ret |= ESDHC_PRNSTS_SDSTB;
+   }
+   break;
+
 case ESDHC_DLL_CTRL:
 case ESDHC_TUNE_CTRL_STATUS:
 case ESDHC_UNDOCUMENTED_REG27:
-- 
2.7.4

[Qemu-devel] what should we do to make it easier for new contributors to get started on QEMU?

[Peter Maydell]

Hi; this is something of a survey question. I'm particularly
interested in answers from people who've recently had to go through
the process of getting up to speed with working on QEMU, so
I've cc'd our current Outreachy and GSoC students, but input
from anybody is welcome.

* If we did one thing to make it easier for new contributors to get
  started, what should it be?

Very wide focus -- could be anything, better documentation, better
processes, test infrastructure, code changes, whatever.
Some possible things I've thought of:
  * better documentation of top-level qemu internals, like a
source layout roadmap?
  * specific "missing pieces" that trip people up (eg not having
a good intro to how to write QOM/qdev devices)?
  * better curation of 'bite sized tasks' list?
But I've been working on QEMU for years, so my impressions could
well be wrong. I'm hoping we can come up with a top 2 or 3 things
to work on improving/fixing, to try to make a difference for future
new contributors...

thanks in advance
-- PMM

[Qemu-devel] [PATCH v2 3/7] block/qcow2-refcount: check_refcounts_l2: refactor compressed case

[Vladimir Sementsov-Ogievskiy]

Separate offset and size of compressed cluster.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2-refcount.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 566c19fbfa..0ea01e3ee2 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1570,7 +1570,7 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 BDRVQcow2State *s = bs->opaque;
 uint64_t *l2_table, l2_entry;
 uint64_t next_contiguous_offset = 0;
-int i, l2_size, nb_csectors, ret;
+int i, l2_size, ret;
 
 /* Read L2 table from disk */
 l2_size = s->l2_size * sizeof(uint64_t);
@@ -1589,6 +1589,9 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 
 switch (qcow2_get_cluster_type(l2_entry)) {
 case QCOW2_CLUSTER_COMPRESSED:
+{
+int64_t csize, coffset;
+
 /* Compressed clusters don't have QCOW_OFLAG_COPIED */
 if (l2_entry & QCOW_OFLAG_COPIED) {
 fprintf(stderr, "ERROR: coffset=0x%" PRIx64 ": "
@@ -1599,12 +1602,13 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 }
 
 /* Mark cluster as used */
-nb_csectors = ((l2_entry >> s->csize_shift) &
-   s->csize_mask) + 1;
-l2_entry &= s->cluster_offset_mask;
+csize = (((l2_entry >> s->csize_shift) & s->csize_mask) + 1) *
+BDRV_SECTOR_SIZE;
+coffset = l2_entry & s->cluster_offset_mask &
+  ~(BDRV_SECTOR_SIZE - 1);
 ret = qcow2_inc_refcounts_imrt(bs, res,
refcount_table, refcount_table_size,
-   l2_entry & ~511, nb_csectors * 512);
+   coffset, csize);
 if (ret < 0) {
 goto fail;
 }
@@ -1621,6 +1625,7 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 res->bfi.fragmented_clusters++;
 }
 break;
+}
 
 case QCOW2_CLUSTER_ZERO_ALLOC:
 case QCOW2_CLUSTER_NORMAL:
-- 
2.11.1

[Qemu-devel] [PATCH 0/7] qcow2 check improvements

[Vladimir Sementsov-Ogievskiy]

Hi all!

v2:
02, 06: check bdrv_getlength error return code

v1:

We've faced the following problem: after host fs corruption, vm images
becomes invalid. And which is interesting, starting qemu-img check on
them led to allocating of the whole RAM and then killing qemu-img by
OOM Killer.

This was due to corrupted l2 entries, which referenced clusters far-far
beyond the end of the qcow2 file.
02 is a generic fix for the bug, 01 is unrelated improvement, 03-07 are
additional info and fixing for such corrupted table entries.

Questions on 02, 06 and 07:
1. Should restrictions be more or less strict?
2. Are there valid cases, when such entries should not be considered as
   corrupted?

Vladimir Sementsov-Ogievskiy (7):
  block/qcow2-refcount: fix check_oflag_copied
  block/qcow2-refcount: avoid eating RAM
  block/qcow2-refcount: check_refcounts_l2: refactor compressed case
  block/qcow2-refcount: check_refcounts_l2: reduce ignored overlaps
  block/qcow2-refcount: check_refcounts_l2: split fix_l2_entry_to_zero
  block/qcow2-refcount: fix out-of-file L1 entries to be zero
  block/qcow2-refcount: fix out-of-file L2 entries to be read-as-zero

 block/qcow2-refcount.c | 270 +++--
 1 file changed, 219 insertions(+), 51 deletions(-)

-- 
2.11.1

[Qemu-devel] [PATCH v2 5/7] block/qcow2-refcount: check_refcounts_l2: split fix_l2_entry_to_zero

[Vladimir Sementsov-Ogievskiy]

Split entry repairing to separate function, to be reused later.

Note: entry in in-memory l2 table (local variable in
check_refcounts_l2) is not updated after this patch.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2-refcount.c | 147 -
 1 file changed, 109 insertions(+), 38 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 3b057b635d..d9c8cd666b 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1554,6 +1554,99 @@ enum {
 CHECK_FRAG_INFO = 0x2,  /* update BlockFragInfo counters */
 };
 
+/* Update entry in L1 or L2 table
+ *
+ * Returns: -errno if overlap check failed
+ *  0 if write failed
+ *  1 on success
+ */
+static int write_table_entry(BlockDriverState *bs, const char *table_name,
+ uint64_t table_offset, int entry_index,
+ uint64_t new_val, int ign)
+{
+int ret;
+uint64_t entry_offset =
+table_offset + (uint64_t)entry_index * sizeof(new_val);
+
+cpu_to_be64s(&new_val);
+ret = qcow2_pre_write_overlap_check(bs, ign, entry_offset, 
sizeof(new_val));
+if (ret < 0) {
+fprintf(stderr,
+"ERROR: Can't write %s table entry: overlap check failed: 
%s\n",
+table_name, strerror(-ret));
+return ret;
+}
+
+ret = bdrv_pwrite_sync(bs->file, entry_offset, &new_val, sizeof(new_val));
+if (ret < 0) {
+fprintf(stderr, "ERROR: Failed to overwrite %s table entry: %s\n",
+table_name, strerror(-ret));
+return 0;
+}
+
+return 1;
+}
+
+/* Try to fix (if allowed) entry in L1 or L2 table. Update @res 
correspondingly.
+ *
+ * Returns: -errno if overlap check failed
+ *  0 if entry was not updated for other reason
+ *(fixing disabled or write failed)
+ *  1 on success
+ */
+static int fix_table_entry(BlockDriverState *bs, BdrvCheckResult *res,
+   BdrvCheckMode fix, const char *table_name,
+   uint64_t table_offset, int entry_index,
+   uint64_t new_val, int ign,
+   const char *fmt, va_list args)
+{
+int ret;
+
+fprintf(stderr, fix & BDRV_FIX_ERRORS ? "Repairing: " : "ERROR: ");
+vfprintf(stderr, fmt, args);
+fprintf(stderr, "\n");
+
+if (!(fix & BDRV_FIX_ERRORS)) {
+res->corruptions++;
+return 0;
+}
+
+ret = write_table_entry(bs, table_name, table_offset, entry_index, new_val,
+ign);
+
+if (ret == 1) {
+res->corruptions_fixed++;
+} else {
+res->check_errors++;
+}
+
+return ret;
+}
+
+/* Make L2 entry to be QCOW2_CLUSTER_ZERO_PLAIN
+ *
+ * Returns: -errno if overlap check failed
+ *  0 if write failed
+ *  1 on success
+ */
+static int fix_l2_entry_to_zero(BlockDriverState *bs, BdrvCheckResult *res,
+BdrvCheckMode fix, int64_t l2_offset,
+int l2_index, bool active,
+const char *fmt, ...)
+{
+int ret;
+int ign = active ? QCOW2_OL_ACTIVE_L2 : QCOW2_OL_INACTIVE_L2;
+uint64_t l2_entry = QCOW_OFLAG_ZERO;
+va_list args;
+
+va_start(args, fmt);
+ret = fix_table_entry(bs, res, fix, "L2", l2_offset, l2_index, l2_entry,
+  ign, fmt, args);
+va_end(args);
+
+return ret;
+}
+
 /*
  * Increases the refcount in the given refcount table for the all clusters
  * referenced in the L2 table. While doing so, performs some checks on L2
@@ -1646,46 +1739,24 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 if (qcow2_get_cluster_type(l2_entry) ==
 QCOW2_CLUSTER_ZERO_ALLOC)
 {
-fprintf(stderr, "%s offset=%" PRIx64 ": Preallocated zero "
-"cluster is not properly aligned; L2 entry "
-"corrupted.\n",
-fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR",
+ret = fix_l2_entry_to_zero(
+bs, res, fix, l2_offset, i, active,
+"offset=%" PRIx64 ": Preallocated zero cluster is "
+"not properly aligned; L2 entry corrupted.",
 offset);
-if (fix & BDRV_FIX_ERRORS) {
-uint64_t l2e_offset =
-l2_offset + (uint64_t)i * sizeof(uint64_t);
-int ign = active ? QCOW2_OL_ACTIVE_L2 :
-   QCOW2_OL_INACTIVE_L2;
-
-l2_entry = QCOW_OFLAG_ZERO;
-l2_table[i] = cpu_to_be64(l2_entry);
-ret = qcow2_pre_write_overlap_check(bs, ign,
-l

[Qemu-devel] [PATCH v2 2/7] block/qcow2-refcount: avoid eating RAM

[Vladimir Sementsov-Ogievskiy]

qcow2_inc_refcounts_imrt() (through realloc_refcount_array()) can eat
an unpredictable amount of memory on corrupted table entries, which are
referencing regions far beyond the end of file.

Prevent this, by skipping such regions from further processing.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2-refcount.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 615847eb09..566c19fbfa 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1499,12 +1499,26 @@ int qcow2_inc_refcounts_imrt(BlockDriverState *bs, 
BdrvCheckResult *res,
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t start, last, cluster_offset, k, refcount;
+int64_t file_len;
 int ret;
 
 if (size <= 0) {
 return 0;
 }
 
+file_len = bdrv_getlength(bs->file->bs);
+if (file_len < 0) {
+return file_len;
+}
+
+if (offset + size - file_len > s->cluster_size) {
+fprintf(stderr, "ERROR: counting reference for region exceeding the "
+"end of the file by more than one cluster: offset 0x%" PRIx64
+" size 0x%" PRIx64 "\n", offset, size);
+res->corruptions++;
+return 0;
+}
+
 start = start_of_cluster(s, offset);
 last = start_of_cluster(s, offset + size - 1);
 for(cluster_offset = start; cluster_offset <= last;
-- 
2.11.1

[Qemu-devel] [PATCH v2 1/7] block/qcow2-refcount: fix check_oflag_copied

[Vladimir Sementsov-Ogievskiy]

Increase corruptions_fixed only after successful fix.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2-refcount.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 3c539f02e5..615847eb09 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1816,7 +1816,7 @@ static int check_oflag_copied(BlockDriverState *bs, 
BdrvCheckResult *res,
 for (i = 0; i < s->l1_size; i++) {
 uint64_t l1_entry = s->l1_table[i];
 uint64_t l2_offset = l1_entry & L1E_OFFSET_MASK;
-bool l2_dirty = false;
+int l2_fixed_entries = 0;
 
 if (!l2_offset) {
 continue;
@@ -1878,8 +1878,7 @@ static int check_oflag_copied(BlockDriverState *bs, 
BdrvCheckResult *res,
 l2_table[j] = cpu_to_be64(refcount == 1
 ? l2_entry |  QCOW_OFLAG_COPIED
 : l2_entry & ~QCOW_OFLAG_COPIED);
-l2_dirty = true;
-res->corruptions_fixed++;
+l2_fixed_entries++;
 } else {
 res->corruptions++;
 }
@@ -1887,7 +1886,7 @@ static int check_oflag_copied(BlockDriverState *bs, 
BdrvCheckResult *res,
 }
 }
 
-if (l2_dirty) {
+if (l2_fixed_entries > 0) {
 ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L2,
 l2_offset, s->cluster_size);
 if (ret < 0) {
@@ -1905,6 +1904,7 @@ static int check_oflag_copied(BlockDriverState *bs, 
BdrvCheckResult *res,
 res->check_errors++;
 goto fail;
 }
+res->corruptions_fixed += l2_fixed_entries;
 }
 }
 
-- 
2.11.1

[Qemu-devel] [PATCH v2 4/7] block/qcow2-refcount: check_refcounts_l2: reduce ignored overlaps

[Vladimir Sementsov-Ogievskiy]

Reduce number of structures ignored in overlap check: when checking
active table ignore active tables, when checking inactive table ignore
inactive ones.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2-refcount.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 0ea01e3ee2..3b057b635d 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1565,7 +1565,7 @@ enum {
 static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,
   void **refcount_table,
   int64_t *refcount_table_size, int64_t l2_offset,
-  int flags, BdrvCheckMode fix)
+  int flags, BdrvCheckMode fix, bool active)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t *l2_table, l2_entry;
@@ -1654,11 +1654,12 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 if (fix & BDRV_FIX_ERRORS) {
 uint64_t l2e_offset =
 l2_offset + (uint64_t)i * sizeof(uint64_t);
+int ign = active ? QCOW2_OL_ACTIVE_L2 :
+   QCOW2_OL_INACTIVE_L2;
 
 l2_entry = QCOW_OFLAG_ZERO;
 l2_table[i] = cpu_to_be64(l2_entry);
-ret = qcow2_pre_write_overlap_check(bs,
-QCOW2_OL_ACTIVE_L2 | QCOW2_OL_INACTIVE_L2,
+ret = qcow2_pre_write_overlap_check(bs, ign,
 l2e_offset, sizeof(uint64_t));
 if (ret < 0) {
 fprintf(stderr, "ERROR: Overlap check failed\n");
@@ -1732,7 +1733,7 @@ static int check_refcounts_l1(BlockDriverState *bs,
   void **refcount_table,
   int64_t *refcount_table_size,
   int64_t l1_table_offset, int l1_size,
-  int flags, BdrvCheckMode fix)
+  int flags, BdrvCheckMode fix, bool active)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t *l1_table = NULL, l2_offset, l1_size2;
@@ -1788,7 +1789,7 @@ static int check_refcounts_l1(BlockDriverState *bs,
 /* Process and check L2 entries */
 ret = check_refcounts_l2(bs, res, refcount_table,
  refcount_table_size, l2_offset, flags,
- fix);
+ fix, active);
 if (ret < 0) {
 goto fail;
 }
@@ -2074,7 +2075,7 @@ static int calculate_refcounts(BlockDriverState *bs, 
BdrvCheckResult *res,
 /* current L1 table */
 ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters,
  s->l1_table_offset, s->l1_size, CHECK_FRAG_INFO,
- fix);
+ fix, true);
 if (ret < 0) {
 return ret;
 }
@@ -2097,7 +2098,8 @@ static int calculate_refcounts(BlockDriverState *bs, 
BdrvCheckResult *res,
 continue;
 }
 ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters,
- sn->l1_table_offset, sn->l1_size, 0, fix);
+ sn->l1_table_offset, sn->l1_size, 0, fix,
+ false);
 if (ret < 0) {
 return ret;
 }
-- 
2.11.1

[Qemu-devel] [PATCH v2 7/7] block/qcow2-refcount: fix out-of-file L2 entries to be read-as-zero

[Vladimir Sementsov-Ogievskiy]

Rewrite corrupted L2 table entry, which reference space out of
underlying file.

Make this L2 table entry read-as-all-zeros without any allocation.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2-refcount.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 3c004e5bfe..3de3768a3c 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1720,8 +1720,30 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 /* Mark cluster as used */
 csize = (((l2_entry >> s->csize_shift) & s->csize_mask) + 1) *
 BDRV_SECTOR_SIZE;
+if (csize > s->cluster_size) {
+ret = fix_l2_entry_to_zero(
+bs, res, fix, l2_offset, i, active,
+"compressed cluster larger than cluster: size 0x%"
+PRIx64, csize);
+if (ret < 0) {
+goto fail;
+}
+continue;
+}
+
 coffset = l2_entry & s->cluster_offset_mask &
   ~(BDRV_SECTOR_SIZE - 1);
+if (coffset >= bdrv_getlength(bs->file->bs)) {
+ret = fix_l2_entry_to_zero(
+bs, res, fix, l2_offset, i, active,
+"compressed cluster out of file: offset 0x%" PRIx64,
+coffset);
+if (ret < 0) {
+goto fail;
+}
+continue;
+}
+
 ret = qcow2_inc_refcounts_imrt(bs, res,
refcount_table, refcount_table_size,
coffset, csize);
@@ -1748,6 +1770,16 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 {
 uint64_t offset = l2_entry & L2E_OFFSET_MASK;
 
+if (offset >= bdrv_getlength(bs->file->bs)) {
+ret = fix_l2_entry_to_zero(
+bs, res, fix, l2_offset, i, active,
+"cluster out of file: offset 0x%" PRIx64, offset);
+if (ret < 0) {
+goto fail;
+}
+continue;
+}
+
 if (flags & CHECK_FRAG_INFO) {
 res->bfi.allocated_clusters++;
 if (next_contiguous_offset &&
-- 
2.11.1

Re: [Qemu-devel] [PATCH 0/3] Drop obsolete memory system request_ptr API

[Paolo Bonzini]

On 17/08/2018 13:46, Peter Maydell wrote:
> Now that support has hit master for direct execution from
> arbitrary MMIO regions, we can remove the MMIO request_ptr
> API, which required special case support in each device that
> wanted to handle it, and also had bad race conditions that
> resulted in crashes if you tried to use it heavily.
> 
> This API was only ever used in one device in the source
> tree, the Xilinx SPIPS. These three patches remove the
> now-unneeded code from the Xilinx device and then the
> core memory subsystem code.
> 
> thanks
> -- PMM
> 
> Peter Maydell (3):
>   hw/ssi/xilinx_spips: Remove unneeded MMIO request_ptr code
>   memory: Remove MMIO request_ptr APIs
>   hw/misc: Remove mmio_interface device
> 
>  hw/misc/Makefile.objs|   1 -
>  include/exec/memory.h|  35 
>  include/hw/misc/mmio_interface.h |  49 ---
>  hw/misc/mmio_interface.c | 135 ---
>  hw/ssi/xilinx_spips.c|  46 ---
>  memory.c | 110 -
>  6 files changed, 376 deletions(-)
>  delete mode 100644 include/hw/misc/mmio_interface.h
>  delete mode 100644 hw/misc/mmio_interface.c
> 

The diffstat speaks for itself, go ahead and include it via the ARM tree.

Thanks,

Paolo

[Qemu-devel] [PATCH v2 6/7] block/qcow2-refcount: fix out-of-file L1 entries to be zero

[Vladimir Sementsov-Ogievskiy]

Zero out corrupted L1 table entry, which reference L2 table out of
underlying file.
Zero L1 table entry means that "the L2 table and all clusters described
by this L2 table are unallocated."

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2-refcount.c | 44 
 1 file changed, 44 insertions(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index d9c8cd666b..3c004e5bfe 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1647,6 +1647,29 @@ static int fix_l2_entry_to_zero(BlockDriverState *bs, 
BdrvCheckResult *res,
 return ret;
 }
 
+/* Zero out L1 entry
+ *
+ * Returns: -errno if overlap check failed
+ *  0 if write failed
+ *  1 on success
+ */
+static int fix_l1_entry_to_zero(BlockDriverState *bs, BdrvCheckResult *res,
+BdrvCheckMode fix, int64_t l1_offset,
+int l1_index, bool active,
+const char *fmt, ...)
+{
+int ret;
+int ign = active ? QCOW2_OL_ACTIVE_L2 : QCOW2_OL_INACTIVE_L2;
+va_list args;
+
+va_start(args, fmt);
+ret = fix_table_entry(bs, res, fix, "L1", l1_offset, l1_index, 0, ign,
+  fmt, args);
+va_end(args);
+
+return ret;
+}
+
 /*
  * Increases the refcount in the given refcount table for the all clusters
  * referenced in the L2 table. While doing so, performs some checks on L2
@@ -1808,6 +1831,7 @@ static int check_refcounts_l1(BlockDriverState *bs,
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t *l1_table = NULL, l2_offset, l1_size2;
+int64_t file_len;
 int i, ret;
 
 l1_size2 = l1_size * sizeof(uint64_t);
@@ -1837,12 +1861,32 @@ static int check_refcounts_l1(BlockDriverState *bs,
 be64_to_cpus(&l1_table[i]);
 }
 
+file_len = bdrv_getlength(bs->file->bs);
+if (file_len < 0) {
+ret = file_len;
+goto fail;
+}
+
 /* Do the actual checks */
 for(i = 0; i < l1_size; i++) {
 l2_offset = l1_table[i];
 if (l2_offset) {
 /* Mark L2 table as used */
 l2_offset &= L1E_OFFSET_MASK;
+if (l2_offset >= file_len) {
+ret = fix_l1_entry_to_zero(
+bs, res, fix, l1_table_offset, i, active,
+"l2 table offset out of file: offset 0x%" PRIx64,
+l2_offset);
+if (ret < 0) {
+/* Something is seriously wrong, so abort checking
+ * this L1 table */
+goto fail;
+}
+
+continue;
+}
+
 ret = qcow2_inc_refcounts_imrt(bs, res,
refcount_table, refcount_table_size,
l2_offset, s->cluster_size);
-- 
2.11.1

Re: [Qemu-devel] [PATCH v6 07/11] migration: poll the cm event while wait RDMA work request completion

[Dr. David Alan Gilbert]

* Lidong Chen (jemmy858...@gmail.com) wrote:
> From: Lidong Chen 
> 
> If the peer qemu is crashed, the qemu_rdma_wait_comp_channel function
> maybe loop forever. so we should also poll the cm event fd, and when
> receive RDMA_CM_EVENT_DISCONNECTED and RDMA_CM_EVENT_DEVICE_REMOVAL,
> we consider some error happened.
> 
> Signed-off-by: Lidong Chen 
> Signed-off-by: Gal Shachaf 
> Signed-off-by: Aviad Yehezkel 

I found the doc in the man page for rdma_create_event_channel
that said 'Users may make the fd non-blocking, poll or select the fd,
etc', so:


Reviewed-by: Dr. David Alan Gilbert 

> ---
>  migration/rdma.c | 33 ++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index d6bbf28..673f126 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1489,6 +1489,9 @@ static uint64_t qemu_rdma_poll(RDMAContext *rdma, 
> uint64_t *wr_id_out,
>   */
>  static int qemu_rdma_wait_comp_channel(RDMAContext *rdma)
>  {
> +struct rdma_cm_event *cm_event;
> +int ret = -1;
> +
>  /*
>   * Coroutine doesn't start until migration_fd_process_incoming()
>   * so don't yield unless we know we're running inside of a coroutine.
> @@ -1505,13 +1508,37 @@ static int qemu_rdma_wait_comp_channel(RDMAContext 
> *rdma)
>   * without hanging forever.
>   */
>  while (!rdma->error_state  && !rdma->received_error) {
> -GPollFD pfds[1];
> +GPollFD pfds[2];
>  pfds[0].fd = rdma->comp_channel->fd;
>  pfds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
> +pfds[0].revents = 0;
> +
> +pfds[1].fd = rdma->channel->fd;
> +pfds[1].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
> +pfds[1].revents = 0;
> +
>  /* 0.1s timeout, should be fine for a 'cancel' */
> -switch (qemu_poll_ns(pfds, 1, 100 * 1000 * 1000)) {
> +switch (qemu_poll_ns(pfds, 2, 100 * 1000 * 1000)) {
> +case 2:
>  case 1: /* fd active */
> -return 0;
> +if (pfds[0].revents) {
> +return 0;
> +}
> +
> +if (pfds[1].revents) {
> +ret = rdma_get_cm_event(rdma->channel, &cm_event);
> +if (!ret) {
> +rdma_ack_cm_event(cm_event);
> +}
> +
> +error_report("receive cm event while wait comp channel,"
> + "cm event is %d", cm_event->event);
> +if (cm_event->event == RDMA_CM_EVENT_DISCONNECTED ||
> +cm_event->event == RDMA_CM_EVENT_DEVICE_REMOVAL) {
> +return -EPIPE;
> +}
> +}
> +break;
>  
>  case 0: /* Timeout, go around again */
>  break;
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] Pipe key broken on US keyboards

[Phillip Susi]

On 8/17/2018 4:56 AM, Daniel P. Berrangé wrote:
> Oh one other thing, is whether your QEMU process has an explicit keymap
> configured (this is the -k arg to QEMU), as when that it set, it
> completely changes the way keyboard input is handled in VNC. That code
> has also been massively refactored recently.

I'm not using that argument.  I just checked the man page and it says it
defaults to en-us, which should be correct.




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] Pipe key broken on US keyboards

[Daniel P . Berrangé]

On Thu, Aug 16, 2018 at 01:11:05PM -0400, Phillip Susi wrote:
> Hello, I recently upgraded my Xen server from Ubuntu 16.04 to 18.04 and
> am no longer able to type a | over vnc to the xen vms.  When I press \
> it works, but when I hold down shift and press \ which should generate a
> |, the vm sees the scan code for some key that the keymap thinks should
> sit between left shift and Z, but does not exist on US keyboards, and
> that scan code produces a > when combined with shift.  I'm betting it
> was this commit that broke it:
> 
> commit ab8f9d49d62c82a12409475547e4420a46da56ed
> Author: Daniel P. Berrange 
> Date:   Wed Jan 17 16:41:15 2018 +
> 
> hw: convert ps2 device to keycodemapdb
> 
> Replace the qcode_to_keycode_set1, qcode_to_keycode_set2,
> and qcode_to_keycode_set3 tables with automatically
> generated tables.
> 
> Any ideas?

Ubuntu 18.04 apparently has QEMU 2.11.0:

   https://packages.ubuntu.com/source/bionic/qemu

The commit you mention above is from the 2.12.0 release, so that can't
be the cause.  Ubuntu 16.04 was on QEMU 2.5.0, so that's a huge  range
of possible changes.

My suggestion would be to try with the very latest QEMU 3.0.0 to see
if that fixes the problem first.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] Pipe key broken on US keyboards

[Daniel P . Berrangé]

On Fri, Aug 17, 2018 at 08:44:36AM -0400, Phillip Susi wrote:
> On 8/17/2018 4:56 AM, Daniel P. Berrangé wrote:
> > Oh one other thing, is whether your QEMU process has an explicit keymap
> > configured (this is the -k arg to QEMU), as when that it set, it
> > completely changes the way keyboard input is handled in VNC. That code
> > has also been massively refactored recently.
> 
> I'm not using that argument.  I just checked the man page and it says it
> defaults to en-us, which should be correct.

Actually for QEMU there is no default localized keymap. This is desirable
because it allows VNC to activate its raw scancode extension which is more
reliable than passing translated X11 key symbols.

I guess there's a chance that Xen might secretly pass a '-k' arg anyway,
but hopefully it doesn't...


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[Qemu-devel] [Bug 1787505] Re: Solaris host: no network connection, mouse pointer mismatch

[Thomas Huth]

You should maybe try a different NIC model. According to
https://wiki.qemu.org/Documentation/Networking the rtl8139 seems to be a
good choice?

Concerning USB, you've also got to enable an emulated "USB host
controller" to use USB devices. The easiest way to do that is to simply
start QEMU with the "-usb" parameter.

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

Title:
  Solaris host: no network connection, mouse pointer mismatch

Status in QEMU:
  New

Bug description:
  This is probably a bit far afield but on a Solaris 10 SPARC host (Sun
  M3000) running a Windows XP guest like this:

  ./qemu-system-x86_64 -m 1024 -boot d  -smp 3 -net nic -net user -hda
  /bkpool/qemuimages/XP.img -cdrom /bkpool/qemuimages/xp.iso &

  the vnc server starts up and Windows boots normally.  However, there
  is no network connectivity.  There are no network devices visible in
  XP's Networking tab of Control Panel and a ping of the local router
  reports "unreachable".

  Also, the keyboard works fine but the guest mouse pointer is offset
  from the host mouse position by an amount that varies by screen
  position.  This makes it impossible to point to locations near the
  edge of the qemu window.  This seems to be a previously reported
  problem, but the suggested fix, " -device usb-tablet", prevents qemu
  from even starting:

  qemu-system-x86_64: -device usb-tablet: No 'usb-bus' bus found for
  device 'usb-tablet'

  The physical mouse is connected to the USB port of a Sun Ray 2fs
  controlling the M3000 via Sun Ray server.  I apologize if this is a
  configuration issue and not a bug but I don't know where else to
  report it and have been unable to find a solution in the
  documentation.

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

Re: [Qemu-devel] [PATCH correct] checkpatch: allow space in more places before a bracket

[Markus Armbruster]

Paolo Bonzini  writes:

> From: Heinrich Schuchardt 
>
> Allow a space between a colon and subsequent opening bracket.  This
> sequence may occur in inline assembler statements like
>
>   asm(
>   "ldr %[out], [%[in]]\n\t"
>   : [out] "=r" (ret)
>   : [in] "r" (addr)
>   );
>
> Allow a space between a comma and subsequent opening bracket.  This
> sequence may occur in designated initializers.
>
> To ease backporting the patch, I am also changing the comma-bracket
> detection (added in QEMU by commit 409db6eb7199af7a2f09f746bd1b793e9daefe5f)
> to use the same regex as brackets and colons (as done independently
> by Linux commit daebc534ac15f991961a5bb433e515988220e9bf).
>
> Link: http://lkml.kernel.org/r/20180403191655.23700-1-xypron.g...@gmx.de
> Signed-off-by: Heinrich Schuchardt 
> Acked-by: Joe Perches 
> Signed-off-by: Andrew Morton 
> Signed-off-by: Linus Torvalds 
> Signed-off-by: Paolo Bonzini 
> ---
>  scripts/checkpatch.pl | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 52ab18bfce..33b5771120 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1977,9 +1977,8 @@ sub process {
>   my ($where, $prefix) = ($-[1], $1);
>   if ($prefix !~ /$Type\s+$/ &&
>   ($where != 0 || $prefix !~ /^.\s+$/) &&
> - $prefix !~ /{\s+$/ &&
>   $prefix !~ /\#\s*define[^(]*\([^)]*\)\s+$/ &&
> - $prefix !~ /,\s+$/) {
> + $prefix !~ /[,{:]\s+$/) {
>   ERROR("space prohibited before open square 
> bracket '['\n" . $herecurr);
>   }
>   }

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] Pipe key broken on US keyboards

[Phillip Susi]

On 8/17/2018 8:58 AM, Daniel P. Berrangé wrote:
> Actually for QEMU there is no default localized keymap. This is desirable
> because it allows VNC to activate its raw scancode extension which is more
> reliable than passing translated X11 key symbols.
> 
> I guess there's a chance that Xen might secretly pass a '-k' arg anyway,
> but hopefully it doesn't...

It also happens when I just run qemu.  How can I check to see if it is
using raw scancodes or keycodes?





signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] Pipe key broken on US keyboards

[Daniel P . Berrangé]

On Fri, Aug 17, 2018 at 09:10:54AM -0400, Phillip Susi wrote:
> On 8/17/2018 8:58 AM, Daniel P. Berrangé wrote:
> > Actually for QEMU there is no default localized keymap. This is desirable
> > because it allows VNC to activate its raw scancode extension which is more
> > reliable than passing translated X11 key symbols.
> > 
> > I guess there's a chance that Xen might secretly pass a '-k' arg anyway,
> > but hopefully it doesn't...
> 
> It also happens when I just run qemu.  How can I check to see if it is
> using raw scancodes or keycodes?

It depends on what the VNC client asks for too. With no '-k' arg, if the
VNC client asks for raw scancodes it'll get that, otherwise QEMU will
fallback to en-US.GTK-VNC based clients (remote-viewer, virt-viewer,
vinagre, virt-manager, GNOME boxes) use the raw scancodes. Most other
clients do not.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [libvirt] clean/simple Q35 support in libvirt+QEMU for guest OSes that don't support virtio-1.0

[Markus Armbruster]

Daniel P. Berrangé  writes:

> On Fri, Aug 17, 2018 at 12:35:11PM +0200, Andrea Bolognani wrote:
>> On Fri, 2018-08-17 at 10:29 +0100, Daniel P. Berrangé wrote:
>> > On Thu, Aug 16, 2018 at 06:20:29PM -0400, Laine Stump wrote:
>> > > 5) Some guest OSes that we still want to support (and which would
>> > > otherwise work okay on a Q35 virtual machine) have virtio drivers too
>> > > old to support virtio-1.0 (CentOS6 and RHEL6 are examples of such OSes),
>> > > but due to the chain of reasons listed above, the "standard" config for
>> > > a Q35 guest generated by libvirt doesn't support virtio-0.9, hence
>> > > doesn't support these guest OSes.
>> > 
>> > Note when talking about "support" you're really saying it from the
>> > downstream vendor, specifically RHEL, POV. From upstream or Fedora POV
>> > essentially all x86 OS ever made are in scope for running under QEMU
>> > if suitable virtual hardware models have been provided. QEMU doesn't
>> > maintain any whitelist of "supported" OS that differs from what is
>> > technically capable of being run, in the way downstream vendors do.
>> 
>> Well, at least in the case of RHEL 6, "not supported" means that it
>> will not boot at all on q35 with the default guest topology created
>> by libvirt, so that's not really a downstream-only problem :)
>
> I mean from an upstream POV we still support RHEL-6 fine in i440fx,
> so there's no reason to particularly care about RHEL-6 with q35
> upstream.

Only true if Q35 provides nothing of value over i440FX for RHEL-6
guests.  Does it?

>   It is only downstream decision to try to force it to
> use q35, despite it not working right today.

[Qemu-devel] [Bug 1787505] Re: Solaris host: no network connection, mouse pointer mismatch

[Peter Maydell]

If you have access to a Linux box, I'd definitely recommend testing the
same setup there. That way you can distinguish "this doesn't work on
Solaris" from "this doesn't work generally" -- the latter are (a) more
likely to be config/command line issues and (b) are easier for us to
work on where there are bugs.

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

Title:
  Solaris host: no network connection, mouse pointer mismatch

Status in QEMU:
  New

Bug description:
  This is probably a bit far afield but on a Solaris 10 SPARC host (Sun
  M3000) running a Windows XP guest like this:

  ./qemu-system-x86_64 -m 1024 -boot d  -smp 3 -net nic -net user -hda
  /bkpool/qemuimages/XP.img -cdrom /bkpool/qemuimages/xp.iso &

  the vnc server starts up and Windows boots normally.  However, there
  is no network connectivity.  There are no network devices visible in
  XP's Networking tab of Control Panel and a ping of the local router
  reports "unreachable".

  Also, the keyboard works fine but the guest mouse pointer is offset
  from the host mouse position by an amount that varies by screen
  position.  This makes it impossible to point to locations near the
  edge of the qemu window.  This seems to be a previously reported
  problem, but the suggested fix, " -device usb-tablet", prevents qemu
  from even starting:

  qemu-system-x86_64: -device usb-tablet: No 'usb-bus' bus found for
  device 'usb-tablet'

  The physical mouse is connected to the USB port of a Sun Ray 2fs
  controlling the M3000 via Sun Ray server.  I apologize if this is a
  configuration issue and not a bug but I don't know where else to
  report it and have been unable to find a solution in the
  documentation.

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

Re: [Qemu-devel] [PATCH 0/3] Drop obsolete memory system request_ptr API

[Edgar E. Iglesias]

On Fri, Aug 17, 2018 at 12:46:16PM +0100, Peter Maydell wrote:
> Now that support has hit master for direct execution from
> arbitrary MMIO regions, we can remove the MMIO request_ptr
> API, which required special case support in each device that
> wanted to handle it, and also had bad race conditions that
> resulted in crashes if you tried to use it heavily.
> 
> This API was only ever used in one device in the source
> tree, the Xilinx SPIPS. These three patches remove the
> now-unneeded code from the Xilinx device and then the
> core memory subsystem code.


Reviewed-by: Edgar E. Iglesias 



> 
> thanks
> -- PMM
> 
> Peter Maydell (3):
>   hw/ssi/xilinx_spips: Remove unneeded MMIO request_ptr code
>   memory: Remove MMIO request_ptr APIs
>   hw/misc: Remove mmio_interface device
> 
>  hw/misc/Makefile.objs|   1 -
>  include/exec/memory.h|  35 
>  include/hw/misc/mmio_interface.h |  49 ---
>  hw/misc/mmio_interface.c | 135 ---
>  hw/ssi/xilinx_spips.c|  46 ---
>  memory.c | 110 -
>  6 files changed, 376 deletions(-)
>  delete mode 100644 include/hw/misc/mmio_interface.h
>  delete mode 100644 hw/misc/mmio_interface.c
> 
> -- 
> 2.18.0
> 

Re: [Qemu-devel] [PATCH v3 2/3] kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl

[Eduardo Habkost]

Thanks for the patch, comments below:

On Fri, Aug 10, 2018 at 10:06:28PM +0800, Robert Hoo wrote:
> Add kvm_get_supported_feature_msrs() to get supported MSR feature index list.
> Add kvm_arch_get_supported_msr_feature() to get each MSR features value.
> 
> kvm_get_supported_feature_msrs() is called in kvm_arch_init().
> kvm_arch_get_supported_msr_feature() is called by
> x86_cpu_get_supported_feature_word().
> 
> Signed-off-by: Robert Hoo 
> ---
>  include/sysemu/kvm.h |  2 ++
>  target/i386/kvm.c| 79 
> 
>  2 files changed, 81 insertions(+)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 0b64b8e..97d8d9d 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -463,6 +463,8 @@ int kvm_vm_check_extension(KVMState *s, unsigned int 
> extension);
>  
>  uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
>uint32_t index, int reg);
> +uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index);
> +
>  
>  void kvm_set_sigmask_len(KVMState *s, unsigned int sigmask_len);
>  
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 9313602..70d8606 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -107,6 +107,7 @@ static int has_pit_state2;
>  static bool has_msr_mcg_ext_ctl;
>  
>  static struct kvm_cpuid2 *cpuid_cache;
> +static struct kvm_msr_list *kvm_feature_msrs;

I was going to suggest moving this to KVMState, but KVMState is a
arch-independent struct.  I guess we'll have to live with this by
now.

>  
>  int kvm_has_pit_state2(void)
>  {
> @@ -420,6 +421,41 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, 
> uint32_t function,
>  return ret;
>  }
>  
> +uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
> +{
> +struct {
> +struct kvm_msrs info;
> +struct kvm_msr_entry entries[1];
> +} msr_data;
> +uint32_t ret;
> +
> +/*Check if the requested feature MSR is supported*/
> +int i;
> +for (i = 0; i < kvm_feature_msrs->nmsrs; i++) {
> +if (index == kvm_feature_msrs->indices[i]) {
> +break;
> +}
> +}

We are duplicating the logic that's already inside KVM (checking
the list of supported MSRs and returning an error).  Can't we
make this simpler and just remove this check?  If the MSR is not
supported, KVM_GET_MSRS would just return 0.

> +if (i >= kvm_feature_msrs->nmsrs) {
> +fprintf(stderr, "Requested MSR (index= %d) is not supported.\n", 
> index);

This error message is meaningless for QEMU users.  Do we really
need to print it?

> +return 0;

Returning 0 for MSRs that are not supported is probably OK, but
we need to see this function being used, to be sure (I didn't
look at all the patches in this series yet).

> +}
> +
> +msr_data.info.nmsrs = 1;
> +msr_data.entries[0].index = index;
> +
> +ret = kvm_ioctl(s, KVM_GET_MSRS, &msr_data);
> +
> +if (ret != 1) {
> +fprintf(stderr, "KVM get MSR (index=0x%x) feature failed, %s\n",
> +index, strerror(-ret));
> +exit(1);

I'm not a fan of APIs that write to stdout/stderr or exit(),
unless they are clearly just initialization functions that should
never fail in normal circumstances.

But if you call KVM_GET_MSRS for all MSRs at
kvm_get_supported_feature_msrs() below, this function would never
need to report an error.

We are already going through the trouble of allocating
kvm_feature_msrs in kvm_get_supported_feature_msrs() anyway.

> +}
> +
> +return msr_data.entries[0].data;
> +}
> +
> +
>  typedef struct HWPoisonPage {
>  ram_addr_t ram_addr;
>  QLIST_ENTRY(HWPoisonPage) list;
> @@ -1238,7 +1274,45 @@ void kvm_arch_do_init_vcpu(X86CPU *cpu)
>  env->mp_state = KVM_MP_STATE_INIT_RECEIVED;
>  }
>  }

Nit: please add an extra newline between functions.

> +static int kvm_get_supported_feature_msrs(KVMState *s)
> +{
> +static int kvm_supported_feature_msrs;
> +int ret = 0;
> +
> +if (kvm_supported_feature_msrs == 0) {

Do you really need the kvm_supported_feature_msrs variable?  You
could just check if kvm_feature_msrs is NULL.

I also suggest doing this:

if (already initialized) {
   return 0;
}

/* regular initialization code here */

to reduce one indentation level.


> +struct kvm_msr_list msr_list;
> +
> +kvm_supported_feature_msrs++;
> +
> +msr_list.nmsrs = 0;
> +ret = kvm_ioctl(s, KVM_GET_MSR_FEATURE_INDEX_LIST, &msr_list);
> +if (ret < 0 && ret != -E2BIG) {
> +return ret;

What if the host KVM version doesn't support
KVM_GET_MSR_FEATURE_INDEX_LIST?

> +}
> +
> +assert(msr_list.nmsrs > 0);
> +kvm_feature_msrs = (struct kvm_msr_list *) \
> +g_malloc0(sizeof(msr_list) +
> + msr_list.nmsrs * sizeof(msr_list.indices[0]));
> +if (kvm_feature_msrs

Re: [Qemu-devel] Pipe key broken on US keyboards

[Phillip Susi]

On 8/17/2018 9:12 AM, Daniel P. Berrangé wrote:
> It depends on what the VNC client asks for too. With no '-k' arg, if the
> VNC client asks for raw scancodes it'll get that, otherwise QEMU will
> fallback to en-US.GTK-VNC based clients (remote-viewer, virt-viewer,
> vinagre, virt-manager, GNOME boxes) use the raw scancodes. Most other
> clients do not.

I'm using "tightvnc" from my Windows computer at work and it says it is
using keyboard layout 0409 ( whatever that means ), so it sounds
like it is sending a keycode and qemu is translating it into the wrong
scancode.




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [libvirt] clean/simple Q35 support in libvirt+QEMU for guest OSes that don't support virtio-1.0

[Daniel P . Berrangé]

On Fri, Aug 17, 2018 at 03:13:22PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé  writes:
> 
> > On Fri, Aug 17, 2018 at 12:35:11PM +0200, Andrea Bolognani wrote:
> >> On Fri, 2018-08-17 at 10:29 +0100, Daniel P. Berrangé wrote:
> >> > On Thu, Aug 16, 2018 at 06:20:29PM -0400, Laine Stump wrote:
> >> > > 5) Some guest OSes that we still want to support (and which would
> >> > > otherwise work okay on a Q35 virtual machine) have virtio drivers too
> >> > > old to support virtio-1.0 (CentOS6 and RHEL6 are examples of such 
> >> > > OSes),
> >> > > but due to the chain of reasons listed above, the "standard" config for
> >> > > a Q35 guest generated by libvirt doesn't support virtio-0.9, hence
> >> > > doesn't support these guest OSes.
> >> > 
> >> > Note when talking about "support" you're really saying it from the
> >> > downstream vendor, specifically RHEL, POV. From upstream or Fedora POV
> >> > essentially all x86 OS ever made are in scope for running under QEMU
> >> > if suitable virtual hardware models have been provided. QEMU doesn't
> >> > maintain any whitelist of "supported" OS that differs from what is
> >> > technically capable of being run, in the way downstream vendors do.
> >> 
> >> Well, at least in the case of RHEL 6, "not supported" means that it
> >> will not boot at all on q35 with the default guest topology created
> >> by libvirt, so that's not really a downstream-only problem :)
> >
> > I mean from an upstream POV we still support RHEL-6 fine in i440fx,
> > so there's no reason to particularly care about RHEL-6 with q35
> > upstream.
> 
> Only true if Q35 provides nothing of value over i440FX for RHEL-6
> guests.  Does it?

Q35 has little technical benefit over i440fx for the majority of guest
deployments, regardless of guest OS.

It provides a more moderning looking platform (nice, but few users are
going to especially care about that), and lets you do secure boot with
OVMF firmware (blocker if you want that feature).

The desire to have everything use Q35 instead of i440fx is more about
downstream vendor testing / support, rather than a strong technical
feature gap requiring it.

> >   It is only downstream decision to try to force it to
> > use q35, despite it not working right today.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v3 3/3] Change other funcitons referring to feature_word_info[]

[Eduardo Habkost]

On Fri, Aug 10, 2018 at 10:06:29PM +0800, Robert Hoo wrote:
> Add an util function feature_word_description(), which help construct the 
> string
> describing the feature word (both CPUID and MSR types).
> 
> report_unavailable_features(): add MSR_FEATURE_WORD type support.
> x86_cpu_get_feature_words(): limit to CPUID_FEATURE_WORD only.
> x86_cpu_get_supported_feature_word(): add MSR_FEATURE_WORD type support.
> x86_cpu_adjust_feat_level(): assert the requested feature must be
> CPUID_FEATURE_WORD type.
> 
> Signed-off-by: Robert Hoo 
> ---
>  target/i386/cpu.c | 77 
> +--
>  1 file changed, 58 insertions(+), 19 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index f7c70d9..21ed599 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3024,21 +3024,51 @@ static const TypeInfo host_x86_cpu_type_info = {
>  
>  #endif
>  
> +/*
> +*caller should have input str no less than 64 byte length.
> +*/
> +#define FEATURE_WORD_DESCPTION_LEN 64
> +static int feature_word_description(char str[], FeatureWordInfo *f,
> +uint32_t bit)

I agree with Eric: g_strup_printf() would be much simpler here.

> +{
> +int ret;
> +
> +assert(f->type == CPUID_FEATURE_WORD ||
> +f->type == MSR_FEATURE_WORD);
> +switch (f->type) {
> +case CPUID_FEATURE_WORD:
> +{
> +const char *reg = get_register_name_32(f->cpuid.reg);
> +assert(reg);
> +ret = snprintf(str, FEATURE_WORD_DESCPTION_LEN,
> +"CPUID.%02XH:%s%s%s [bit %d]",
> +f->cpuid.eax, reg,
> +f->feat_names[bit] ? "." : "",
> +f->feat_names[bit] ? f->feat_names[bit] : "", bit);
> +break;
> +}
> +case MSR_FEATURE_WORD:
> +ret = snprintf(str, FEATURE_WORD_DESCPTION_LEN,
> +"MSR(%xH).%s [bit %d]",
> +f->msr.index,
> +f->feat_names[bit] ? f->feat_names[bit] : "", bit);

What about leaving the (f->feat_names[bit] ? ... : ...) part
in report_unavailable_features() and just make this function
return "CPUID[...]" or "MSR[...]"?


> +break;
> +}
> +return ret > 0;
> +}
> +
>  static void report_unavailable_features(FeatureWord w, uint32_t mask)
>  {
>  FeatureWordInfo *f = &feature_word_info[w];
>  int i;
> +char feat_word_dscrp_str[FEATURE_WORD_DESCPTION_LEN];
>  
>  for (i = 0; i < 32; ++i) {
>  if ((1UL << i) & mask) {
> -const char *reg = get_register_name_32(f->cpuid_reg);
> -assert(reg);
> -warn_report("%s doesn't support requested feature: "
> -"CPUID.%02XH:%s%s%s [bit %d]",
> +feature_word_description(feat_word_dscrp_str, f, i);
> +warn_report("%s doesn't support requested feature: %s",
>  accel_uses_host_cpuid() ? "host" : "TCG",
> -f->cpuid_eax, reg,
> -f->feat_names[i] ? "." : "",
> -f->feat_names[i] ? f->feat_names[i] : "", i);
> +feat_word_dscrp_str);
>  }
>  }
>  }
> @@ -3276,17 +3306,17 @@ static void x86_cpu_get_feature_words(Object *obj, 
> Visitor *v,
>  {
>  uint32_t *array = (uint32_t *)opaque;
>  FeatureWord w;
> -X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { };
> -X86CPUFeatureWordInfoList list_entries[FEATURE_WORDS] = { };
> +X86CPUFeatureWordInfo word_infos[FEATURE_WORDS_NUM_CPUID] = { };
> +X86CPUFeatureWordInfoList list_entries[FEATURE_WORDS_NUM_CPUID] = { };
>  X86CPUFeatureWordInfoList *list = NULL;
>  
> -for (w = 0; w < FEATURE_WORDS; w++) {
> +for (w = 0; w < FEATURE_WORDS_NUM_CPUID; w++) {
>  FeatureWordInfo *wi = &feature_word_info[w];
>  X86CPUFeatureWordInfo *qwi = &word_infos[w];
> -qwi->cpuid_input_eax = wi->cpuid_eax;
> -qwi->has_cpuid_input_ecx = wi->cpuid_needs_ecx;
> -qwi->cpuid_input_ecx = wi->cpuid_ecx;
> -qwi->cpuid_register = x86_reg_info_32[wi->cpuid_reg].qapi_enum;
> +qwi->cpuid_input_eax = wi->cpuid.eax;
> +qwi->has_cpuid_input_ecx = wi->cpuid.needs_ecx;
> +qwi->cpuid_input_ecx = wi->cpuid.ecx;
> +qwi->cpuid_register = x86_reg_info_32[wi->cpuid.reg].qapi_enum;

Looks OK, but I would add an
assert(wi->type == CPUID_FEATURE_WORD) on every block of code
that uses the wi->cpuid field.

I would also get rid of FEATURE_WORDS_NUM_CPUID and
FEATURE_WORDS_FIRST_MSR to reduce the chance of future mistakes.
We can use FEATURE_WORDS in this loop and just check wi->type on
each iteration.

>  qwi->features = array[w];
>  
>  /* List will be in reverse order, but order shouldn't matter */
> @@ -3659,12 +3689,20 @@ static uint32_t 
> x86_cpu_get_supported_feature_word(FeatureWord w,
> bool migratable_o

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 25 July 2018 at 06:30, Hongbo Zhang  wrote:
> For the Aarch64, there is one machine 'virt', it is primarily meant to
> run on KVM and execute virtualization workloads, but we need an
> environment as faithful as possible to physical hardware, for supporting
> firmware and OS development for pysical Aarch64 machines.
>
> This patch introduces new machine type 'Enterprise' with main features:
>  - Based on 'virt' machine type.
>  - Re-designed memory map.
>  - EL2 and EL3 are enabled by default.
>  - GIC version 3 by default.
>  - AHCI controller attached to system bus, and then CDROM and hard disc
>can be added to it.
>  - EHCI controller attached to system bus, with USB mouse and key board
>installed by default.
>  - E1000E ethernet card on PCIE bus.
>  - VGA display adaptor on PCIE bus.
>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>  - No virtio functions enabled, since this is to emulate real hardware.
>  - No paravirtualized fw_cfg device either.
>
> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>
> Signed-off-by: Hongbo Zhang 
> ---
> Changes since v1:
>  - rebase on v3.0.0-rc0
>  - introduce another auxillary patch as 1/2, so this is 2/2
>  - rename 'sbsa' to 'enterprise'
>  - remove paravirualized fw_cfg
>  - set gic_vertion to 3 instead of 2
>  - edit commit message to describe purpose of this platform

Hi; there's been a lot of discussion on this thread. I'm going
to assume that you don't need any further review commentary for
the moment, and will wait for a v3. If there are any specific
questions you want guidance on to produce that v3, let me know.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

[no-reply]

Hi,

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

Type: series
Message-id: 20180810030139.25916-1-pavel.zbits...@gmail.com
Subject: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions 
support

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
bd3e0382ce target/s390x: implement CVB, CVBY and CVBG
f4fba58f1c target/s390x: fix PACK reading 1 byte less and writing 1 byte more
164afdcf5f target/s390x: add EX support for TRT and TRTR
56d0f419b7 target/s390x: fix IPM polluting irrelevant bits
f48adc25a1 target/s390x: fix CSST decoding and runtime alignment check
63fd9ce84b target/s390x: add BAL and BALR instructions
d7d028e76f tests/tcg: add a simple s390x test

=== OUTPUT BEGIN ===
Checking PATCH 1/7: tests/tcg: add a simple s390x test...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#14: 
new file mode 100644

total: 0 errors, 1 warnings, 10 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/7: target/s390x: add BAL and BALR instructions...
Checking PATCH 3/7: target/s390x: fix CSST decoding and runtime alignment 
check...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#72: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#98: FILE: tests/tcg/s390x/csst.c:22:
+: [op1] "+m" (op1),

ERROR: space prohibited before open square bracket '['
#102: FILE: tests/tcg/s390x/csst.c:26:
+: [flags] "K" (0x0301),

total: 2 errors, 1 warnings, 66 lines checked

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

Checking PATCH 4/7: target/s390x: fix IPM polluting irrelevant bits...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#57: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#74: FILE: tests/tcg/s390x/ipm.c:13:
+: [cc] "+r" (cc)

ERROR: space prohibited before open square bracket '['
#75: FILE: tests/tcg/s390x/ipm.c:14:
+: [op1] "r" (&op1),

total: 2 errors, 1 warnings, 53 lines checked

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

Checking PATCH 5/7: target/s390x: add EX support for TRT and TRTR...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#70: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#101: FILE: tests/tcg/s390x/exrl-trt.c:27:
+: [r1] "+r" (r1),

ERROR: space prohibited before open square bracket '['
#104: FILE: tests/tcg/s390x/exrl-trt.c:30:
+: [op1] "r" (&op1),

ERROR: space prohibited before open square bracket '['
#155: FILE: tests/tcg/s390x/exrl-trtr.c:27:
+: [r1] "+r" (r1),

ERROR: space prohibited before open square bracket '['
#158: FILE: tests/tcg/s390x/exrl-trtr.c:30:
+: [op1] "r" (&op1),

total: 4 errors, 1 warnings, 141 lines checked

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

Checking PATCH 6/7: target/s390x: fix PACK reading 1 byte less and writing 1 
byte more...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#56: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#72: FILE: tests/tcg/s390x/pack.c:12:
+: [data] "r" (&data[0])

total: 1 errors, 1 warnings, 43 lines checked

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

Checking PATCH 7/7: target/s390x: implement CVB, CVBY and CVBG...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#139: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#154: FILE: tests/tcg/s390x/cvb.c:11:
+: [result] "+r" (result)

ERROR: space prohibited before open square bracket '['
#155: FILE: tests/tcg/s390x/cvb.c:12:
+: [data] "m" (data));

total: 2 errors, 1 warnings, 117 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, s

Re: [Qemu-devel] [PATCH 1/4] block: Add bdrv_get_request_alignment()

[Vladimir Sementsov-Ogievskiy]

02.08.2018 17:48, Eric Blake wrote:

The next patch needs access to a device's minimum permitted
alignment, since NBD wants to advertise this to clients. Add
an accessor function, borrowing from blk_get_max_transfer()
for accessing a backend's block limits.

Signed-off-by: Eric Blake 


Reviewed-by: Vladimir Sementsov-Ogievskiy 



--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH] qemu-img.c: increase spacing between commands in documentation

[Eric Blake]

On 08/16/2018 08:27 PM, Programmingkid wrote:


I am by no means an expert at qemu-img. But I did try my best to create what I think 
should be the new output for qemu-img  --help. This is just the text I 
plan on using in a future patch. It is easier to read right now than it will be in 
patch form, so please let me know if there are any errors in this documentation. 
Thank you.






Just reviewing the first for now, to give you a feel for what to consider.


usage: qemu-img amend [--object objectdef] [--image-opts] [-p] [-q] [-f fmt]
[-t cache] -o options filename

Command parameters:
  -f 'fmt' is the disk image format. It is guessed automatically
 in most cases.


Bad advice. We WANT users to use -f (if you don't, and the automatic 
guessing sees something that is not raw, but your image SHOULD have been 
-f raw, then you have a security hole: the guest can write anything into 
a raw image to make the host access files it shouldn't based on 
interpreting the raw file as something else).  I'd drop the last 
sentence, and use just the first.




--image-optsTreat filename as a set of image options, instead of a plain
 filename.

  -o Used with a comma separated list of format specific options in 
a
 name=value format. Use "-o ?" for an overview of the options


Please spell that "-o help", not "-o ?".   Otherwise, the user has to 
quote the ? to avoid it globbing into any single-byte file lying around 
in the current directory.



 supported by the used format

--object'objectdef' is a QEMU user creatable object definition. See the
 qemu(1) manual page for a description of the object properties.
 The most common object type is a 'secret', which is used to
 supply passwords and/or encryption keys.

  -p Display progress bar. If the -p option is not used for a 
command
 that supports it, the progress is reported when the process
 receives a "SIGUSR1" signal.

  -q Quiet mode - do not print any output (except errors). There's 
no
 progress bar in case both -q and -p options are used.


Not your fault, but I don't like it when interfaces take mutually 
exclusive operations but the last one does not win.  Either '-p -q' 
should behave like '-q', and '-q -p' behave like '-p' (because we accept 
the mutual exclusion but last one wins), or both forms should error 
(because they are incompatible).  But having both forms silently behave 
like '-q' is evil.  So, as long as you're willing to patch up 
interfaces, that's a project to consider.




  -t Specifies the cache mode that should be used with the
 destination file.


And what are those modes?  If you're going to be wordy, then give the 
user enough information to be useful.  Otherwise, being terse in --help 
is fine (and let the man page be wordy instead).


Example:
 qemu-img amend -o compat=0.10 -f qcow2 image.qcow2


Where's an example with --image-opts and --object secret?

We're trying to move away from compat=0.10 (also spelled compat=v2), and 
instead start encouraging compat=v3.


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

Re: [Qemu-devel] [PATCH 3/4] scripts/qemu: add render_block_graph method for QEMUMachine

[Eduardo Habkost]

On Fri, Aug 17, 2018 at 01:08:42PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 17.08.2018 04:54, Eduardo Habkost wrote:
> > On Thu, Aug 16, 2018 at 08:20:26PM +0300, Vladimir Sementsov-Ogievskiy 
> > wrote:
> > > Render block nodes graph with help of graphviz
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > Thanks for your patch.  Comments below:
> > 
> > > ---
> > >   scripts/qemu.py | 53 
> > > +
> > >   1 file changed, 53 insertions(+)
> > > 
> > > diff --git a/scripts/qemu.py b/scripts/qemu.py
> > > index f099ce7278..cff562c713 100644
> > > --- a/scripts/qemu.py
> > > +++ b/scripts/qemu.py
> > > @@ -460,3 +460,56 @@ class QEMUMachine(object):
> > >socket.SOCK_STREAM)
> > >   self._console_socket.connect(self._console_address)
> > >   return self._console_socket
> > > +
> > > +def render_block_graph(self, filename):
> > > +'''
> > > +Render graph in text (dot) representation into "filename" and 
> > > graphical
> > > +representation into pdf file "filename".pdf
> > > +'''
> > > +
> > > +try:
> > > +from graphviz import Digraph
> > I'm not convinced that this belongs to qemu.py, if most code
> > using the qemu.py module will never use it.  Do you see any
> > problem in moving this code to a scripts/render_block_graph.py
> > script?
> 
> Not a problem, I can do it.. Just one thought:
> Isn't it better to keep all function doing something with one vm as methods,
> not separate functions?

I don't think so.  We don't need to create a new QEMUMachine
method every time we write a function that takes a QEMUMachine as
argument in our scripts.

There are cases when we're forced to create a method: when the
new code is tightly coupled to the QEMUMachine code and need
access to its private attributes.

There are cases where we probably want to place the code in
qemu.py (as a method): when we expect many users of qemu.py to
call it.

But in other cases, I don't see any problem with another
script/module having a regular function like:

  def my_special_purpose_function(vm, ...):
 ...


-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/4] nbd/server: Advertise actual minimum block size

[Vladimir Sementsov-Ogievskiy]

02.08.2018 17:48, Eric Blake wrote:

Both NBD_CMD_BLOCK_STATUS and structured NBD_CMD_READ will split
their reply according to bdrv_block_status() boundaries. If the
block device has a request_alignment smaller than 512, but we
advertise a block alignment of 512 to the client, then this can
result in the server reply violating client expectations by
reporting a smaller region of the export than what the client
is permitted to address.  Thus, it is imperative that we
advertise the actual minimum block limit, rather than blindly
rounding it up to 512 (bdrv_block_status() cannot return status
aligned any smaller than request_alignment).

Signed-off-by: Eric Blake 
---
  nbd/server.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index ea5fe0eb336..cd3c41f895b 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -608,10 +608,12 @@ static int nbd_negotiate_handle_info(NBDClient *client, 
uint16_t myflags,
  /* Send NBD_INFO_BLOCK_SIZE always, but tweak the minimum size
   * according to whether the client requested it, and according to
   * whether this is OPT_INFO or OPT_GO. */
-/* minimum - 1 for back-compat, or 512 if client is new enough.
- * TODO: consult blk_bs(blk)->bl.request_alignment? */
-sizes[0] =
-(client->opt == NBD_OPT_INFO || blocksize) ? BDRV_SECTOR_SIZE : 1;
+/* minimum - 1 for back-compat, or actual if client will obey it. */
+if (client->opt == NBD_OPT_INFO || blocksize) {
+sizes[0] = blk_get_request_alignment(exp->blk);
+} else {
+sizes[0] = 1;
+}
  /* preferred - Hard-code to 4096 for now.
   * TODO: is blk_bs(blk)->bl.opt_transfer appropriate? */
  sizes[1] = 4096;


Here, what about dirty bitmap export through BLOCK_STATUS? Could it's 
granularity be less than request_alignment? Shouldn't we handle it somehow?


--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v9 40/84] target/mips: Fix pre-nanoMIPS MT ASE instructions availability control

[Aleksandar Markovic]

> > I think some of the previously-implemented similar cases involving 
> > read-only bits were handled the same way, and we just built on that. What 
> > would you suggest as a more appropriate solution in such cases (of 
> > accessing "preset by hardware" bits)?
>
> Well, ctx->insn_flags and ctx->CP0_Config1 are good examples.
> These are 100% read-only and fixed at cpu instantiation.
> 
> I see that CP0_Config3 has one writable bit for micromips, but
> is fully readonly for nanomips.  Therefore XNP and MT need not
> be copied to hflags because they will never vary.
> 
> I'd suggest copying CP0_Config3 to ctx as with Config1.
>
>
> r~

Hi, Richard,

The opinion within the team is that we should leave such changes for follow-up 
clean-up - clean-up of CP0-related functionalities is scheduled anyway soon.

The reason is that the current implementation (in v9) works fine, and this is 
very late in our dev cycle to change features with no observed bugs.

All other your concerns will be addressed in v10, which is planned to be sent 
soon.

Yours,
Aleksandar

[Qemu-devel] [PATCH 0/4] Fix socket chardev regression

[Marc-André Lureau]

Hi,

In commit 25679e5d58e "chardev: tcp: postpone async connection setup"
(and its follow up 99f2f54174a59), Peter moved chardev socket
connection to machine_done event. However, chardev created later will
no longer attempt to connect, and chardev created in tests do not have
machine_done event (breaking some of vhost-user-test).

The goal was to move the "connect" source to the chardev frontend
context (the monitor thread context in his case). chr->gcontext is set
with qemu_chr_fe_set_handlers(). But there is no guarantee that the
function will be called in general, so we can't delay connection until
then: the chardev should still attempt to connect during open(), using
the main context.

An alternative would be to specify the iothread during chardev
creation. Setting up monitor OOB would be quite different too, it
would take the same iothread as argument.

99f2f54174a595e is also a bit problematic, since it will behave
differently before and after machine_done (the first case gives a
chance to use a different context reliably, the second looks racy)

In the end, I am not sure this is all necessary, as chardev callbacks
are called after qemu_chr_fe_set_handlers(), at which point the
context of sources are updated. In "char-socket: update all ioc
handlers when changing context", I moved also the hup handler to the
updated context. So unless the main thread is already stuck, we can
setup a different context for the chardev at that time. Or not?

Marc-André Lureau (4):
  Revert "chardev: tcp: postpone TLS work until machine done"
  Revert "chardev: tcp: postpone async connection setup"
  char-socket: update all ioc handlers when changing context
  test-char: add socket reconnect test

 chardev/char-socket.c | 86 ++-
 tests/test-char.c | 18 +++--
 2 files changed, 50 insertions(+), 54 deletions(-)

-- 
2.18.0.547.g1d89318c48

[Qemu-devel] [PATCH 1/4] Revert "chardev: tcp: postpone TLS work until machine done"

[Marc-André Lureau]

This reverts commit 99f2f54174a595e3ada6e4332fcd2b37ebb0d55d.

See next commit reverting 25679e5d58e258e9950685ffbd0cae4cd40d9cc2 as
well for rationale.

Signed-off-by: Marc-André Lureau 
---
 chardev/char-socket.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index efbad6ee7c..e28d2cca4c 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -32,7 +32,6 @@
 #include "qapi/error.h"
 #include "qapi/clone-visitor.h"
 #include "qapi/qapi-visit-sockets.h"
-#include "sysemu/sysemu.h"
 
 #include "chardev/char-io.h"
 
@@ -724,11 +723,6 @@ static void tcp_chr_tls_init(Chardev *chr)
 Error *err = NULL;
 gchar *name;
 
-if (!machine_init_done) {
-/* This will be postponed to machine_done notifier */
-return;
-}
-
 if (s->is_listen) {
 tioc = qio_channel_tls_new_server(
 s->ioc, s->tls_creds,
@@ -1169,10 +1163,6 @@ static int tcp_chr_machine_done_hook(Chardev *chr)
 tcp_chr_connect_async(chr);
 }
 
-if (s->ioc && s->tls_creds) {
-tcp_chr_tls_init(chr);
-}
-
 return 0;
 }
 
-- 
2.18.0.547.g1d89318c48

[Qemu-devel] [PATCH 3/4] char-socket: update all ioc handlers when changing context

[Marc-André Lureau]

So far, tcp_chr_update_read_handler() only updated the read
handler. Let's also update the hup handler.

Factorize the code while at it. (note that s->ioc != NULL when
s->connected)

Signed-off-by: Marc-André Lureau 
---
 chardev/char-socket.c | 59 ---
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 14f6567f6a..7cd0ae2824 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -353,6 +353,15 @@ static GSource *tcp_chr_add_watch(Chardev *chr, 
GIOCondition cond)
 return qio_channel_create_watch(s->ioc, cond);
 }
 
+static void remove_hup_source(SocketChardev *s)
+{
+if (s->hup_source != NULL) {
+g_source_destroy(s->hup_source);
+g_source_unref(s->hup_source);
+s->hup_source = NULL;
+}
+}
+
 static void tcp_chr_free_connection(Chardev *chr)
 {
 SocketChardev *s = SOCKET_CHARDEV(chr);
@@ -367,11 +376,7 @@ static void tcp_chr_free_connection(Chardev *chr)
 s->read_msgfds_num = 0;
 }
 
-if (s->hup_source != NULL) {
-g_source_destroy(s->hup_source);
-g_source_unref(s->hup_source);
-s->hup_source = NULL;
-}
+remove_hup_source(s);
 
 tcp_set_msgfds(chr, NULL, 0);
 remove_fd_in_watch(chr);
@@ -540,6 +545,27 @@ static char *sockaddr_to_str(struct sockaddr_storage *ss, 
socklen_t ss_len,
 }
 }
 
+static void update_ioc_handlers(SocketChardev *s)
+{
+Chardev *chr = CHARDEV(s);
+
+if (!s->connected) {
+return;
+}
+
+remove_fd_in_watch(chr);
+chr->gsource = io_add_watch_poll(chr, s->ioc,
+ tcp_chr_read_poll,
+ tcp_chr_read, chr,
+ chr->gcontext);
+
+remove_hup_source(s);
+s->hup_source = qio_channel_create_watch(s->ioc, G_IO_HUP);
+g_source_set_callback(s->hup_source, (GSourceFunc)tcp_chr_hup,
+  chr, NULL);
+g_source_attach(s->hup_source, chr->gcontext);
+}
+
 static void tcp_chr_connect(void *opaque)
 {
 Chardev *chr = CHARDEV(opaque);
@@ -552,16 +578,7 @@ static void tcp_chr_connect(void *opaque)
 s->is_listen, s->is_telnet);
 
 s->connected = 1;
-chr->gsource = io_add_watch_poll(chr, s->ioc,
-   tcp_chr_read_poll,
-   tcp_chr_read,
-   chr, chr->gcontext);
-
-s->hup_source = qio_channel_create_watch(s->ioc, G_IO_HUP);
-g_source_set_callback(s->hup_source, (GSourceFunc)tcp_chr_hup,
-  chr, NULL);
-g_source_attach(s->hup_source, chr->gcontext);
-
+update_ioc_handlers(s);
 qemu_chr_be_event(chr, CHR_EVENT_OPENED);
 }
 
@@ -592,17 +609,7 @@ static void tcp_chr_update_read_handler(Chardev *chr)
 tcp_chr_telnet_init(CHARDEV(s));
 }
 
-if (!s->connected) {
-return;
-}
-
-remove_fd_in_watch(chr);
-if (s->ioc) {
-chr->gsource = io_add_watch_poll(chr, s->ioc,
-   tcp_chr_read_poll,
-   tcp_chr_read, chr,
-   chr->gcontext);
-}
+update_ioc_handlers(s);
 }
 
 static gboolean tcp_chr_telnet_init_io(QIOChannel *ioc,
-- 
2.18.0.547.g1d89318c48

[Qemu-devel] [PATCH 4/4] test-char: add socket reconnect test

[Marc-André Lureau]

This test exhibits a regression fixed by the previous reverts.

Signed-off-by: Marc-André Lureau 
---
 tests/test-char.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tests/test-char.c b/tests/test-char.c
index 5905d31441..d99742d86d 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -309,7 +309,7 @@ static int socket_can_read_hello(void *opaque)
 return 10;
 }
 
-static void char_socket_test_common(Chardev *chr)
+static void char_socket_test_common(Chardev *chr, bool reconnect)
 {
 Chardev *chr_client;
 QObject *addr;
@@ -329,7 +329,8 @@ static void char_socket_test_common(Chardev *chr)
 addr = object_property_get_qobject(OBJECT(chr), "addr", &error_abort);
 qdict = qobject_to(QDict, addr);
 port = qdict_get_str(qdict, "port");
-tmp = g_strdup_printf("tcp:127.0.0.1:%s", port);
+tmp = g_strdup_printf("tcp:127.0.0.1:%s%s", port,
+  reconnect ? ",reconnect=1" : "");
 qobject_unref(qdict);
 
 qemu_chr_fe_init(&be, chr, &error_abort);
@@ -370,7 +371,15 @@ static void char_socket_basic_test(void)
 {
 Chardev *chr = qemu_chr_new("server", "tcp:127.0.0.1:0,server,nowait");
 
-char_socket_test_common(chr);
+char_socket_test_common(chr, false);
+}
+
+
+static void char_socket_reconnect_test(void)
+{
+Chardev *chr = qemu_chr_new("server", "tcp:127.0.0.1:0,server,nowait");
+
+char_socket_test_common(chr, true);
 }
 
 
@@ -402,7 +411,7 @@ static void char_socket_fdpass_test(void)
 
 qemu_opts_del(opts);
 
-char_socket_test_common(chr);
+char_socket_test_common(chr, false);
 }
 
 
@@ -823,6 +832,7 @@ int main(int argc, char **argv)
 g_test_add_func("/char/file-fifo", char_file_fifo_test);
 #endif
 g_test_add_func("/char/socket/basic", char_socket_basic_test);
+g_test_add_func("/char/socket/reconnect", char_socket_reconnect_test);
 g_test_add_func("/char/socket/fdpass", char_socket_fdpass_test);
 g_test_add_func("/char/udp", char_udp_test);
 #ifdef HAVE_CHARDEV_SERIAL
-- 
2.18.0.547.g1d89318c48

[Qemu-devel] [PATCH 2/4] Revert "chardev: tcp: postpone async connection setup"

[Marc-André Lureau]

This reverts commit 25679e5d58e258e9950685ffbd0cae4cd40d9cc2.

This commit broke "reconnect socket" chardev that are created after
"machine_done": they no longer try to connect. It broke also
vhost-user-test that uses chardev while there is no "machine_done"
event.

The goal of this patch was to move the "connect" source to the
frontend context. chr->gcontext is set with
qemu_chr_fe_set_handlers(). But there is no guarantee that it will be
called, so we can't delay connection until then: the chardev should
still attempt to connect during open(). qemu_chr_fe_set_handlers() is
eventually called later and will update the context.

Unless there is a good reason to not use initially the default
context, I think we should revert to the previous state to fix the
regressions.

Signed-off-by: Marc-André Lureau 
---
 chardev/char-socket.c | 17 +++--
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index e28d2cca4c..14f6567f6a 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1005,8 +1005,9 @@ static void qmp_chardev_open_socket(Chardev *chr,
 s->reconnect_time = reconnect;
 }
 
-/* If reconnect_time is set, will do that in chr_machine_done. */
-if (!s->reconnect_time) {
+if (s->reconnect_time) {
+tcp_chr_connect_async(chr);
+} else {
 if (s->is_listen) {
 char *name;
 s->listener = qio_net_listener_new();
@@ -1155,17 +1156,6 @@ char_socket_get_connected(Object *obj, Error **errp)
 return s->connected;
 }
 
-static int tcp_chr_machine_done_hook(Chardev *chr)
-{
-SocketChardev *s = SOCKET_CHARDEV(chr);
-
-if (s->reconnect_time) {
-tcp_chr_connect_async(chr);
-}
-
-return 0;
-}
-
 static void char_socket_class_init(ObjectClass *oc, void *data)
 {
 ChardevClass *cc = CHARDEV_CLASS(oc);
@@ -1181,7 +1171,6 @@ static void char_socket_class_init(ObjectClass *oc, void 
*data)
 cc->chr_add_client = tcp_chr_add_client;
 cc->chr_add_watch = tcp_chr_add_watch;
 cc->chr_update_read_handler = tcp_chr_update_read_handler;
-cc->chr_machine_done = tcp_chr_machine_done_hook;
 
 object_class_property_add(oc, "addr", "SocketAddress",
   char_socket_get_addr, NULL,
-- 
2.18.0.547.g1d89318c48

Re: [Qemu-devel] [PATCH 4/4] nbd/client: Deal with unaligned size from server

[Vladimir Sementsov-Ogievskiy]

02.08.2018 17:48, Eric Blake wrote:

When a server advertises an unaligned size but no block sizes,
the code was rounding up to a sector-aligned size (a known
limitation of bdrv_getlength()), then assuming a request_alignment
of 512 (the recommendation of the NBD spec for maximum portability).
However, this means that qemu will actually attempt to access the
padding bytes of the trailing partial sector.

An easy demonstration, using nbdkit as the server:
$ nbdkit -fv random size=1023
$ qemu-io -r -f raw -c 'r -v 0 1023' nbd://localhost:10809
read failed: Invalid argument

because the client rounded the request up to 1024 bytes, which
nbdkit then rejected as beyond the advertised size of 1023.

Note that qemu as the server refuses to send an unaligned size, as
it has already rounded the unaligned image up to sector size, and
then happily resizes the image on access (at least when serving a
POSIX file over NBD).

Reported-by: Richard W.M. Jones 
Signed-off-by: Eric Blake 


Reviewed-by: Vladimir Sementsov-Ogievskiy 



--
Best regards,
Vladimir

Re: [Qemu-devel] [libvirt] clean/simple Q35 support in libvirt+QEMU for guest OSes that don't support virtio-1.0

[Andrea Bolognani]

On Fri, 2018-08-17 at 11:43 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 17, 2018 at 12:35:11PM +0200, Andrea Bolognani wrote:
> > If we decide we want to explicitly spell out the options instead
> > of relying on QEMU changing behavior based on the slot type, which
> > is probably a good idea anyway, I think we should have
> > 
> >   virtio-0.9 => disable-legacy=no,disable-modern=no
> >   virtio-1.0 => disable-legacy=yes,disable-modern=no
> > 
> > There's basically no reason to have a device legacy-only rather
> > than transitional, and spelling out both options instead of only
> > one of them just seems more robust.
> 
> From a testing POV it is desirable to be able to have legacy-only.
> There is also possibility that guest OS impl 1.0 in a buggy manner,
> so forcing legacy only is desirable.
> 
> The existing device still already provides a transitional option
> on i440fx, and on Q35 if you do explicit placement in a PCI slot.
> So I don't think there's a good reason to have a second transitional
> device type, especially if we're naming it virtio-0.9, it is rather
> misleading if it would be in fact able to run virtio-1.0 mode.

Sounds reasonable.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [Qemu-devel] [PATCH v6 09/11] migration: poll the cm event for destination qemu

[Dr. David Alan Gilbert]

* Lidong Chen (jemmy858...@gmail.com) wrote:
> The destination qemu only poll the comp_channel->fd in
> qemu_rdma_wait_comp_channel. But when source qemu disconnnect
> the rdma connection, the destination qemu should be notified.
> 
> Signed-off-by: Lidong Chen 

OK, this could do with an update to the migration_incoming_co comment in
migration.h, since previously it was only used by colo; if we merge this
first please post a patch to update the comment.

Other than that, I think I'm OK:

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  migration/migration.c |  3 ++-
>  migration/rdma.c  | 32 +++-
>  2 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index df0c2cf..f7d6e26 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -389,6 +389,7 @@ static void process_incoming_migration_co(void *opaque)
>  int ret;
>  
>  assert(mis->from_src_file);
> +mis->migration_incoming_co = qemu_coroutine_self();
>  mis->largest_page_size = qemu_ram_pagesize_largest();
>  postcopy_state_set(POSTCOPY_INCOMING_NONE);
>  migrate_set_state(&mis->state, MIGRATION_STATUS_NONE,
> @@ -418,7 +419,6 @@ static void process_incoming_migration_co(void *opaque)
>  
>  /* we get COLO info, and know if we are in COLO mode */
>  if (!ret && migration_incoming_enable_colo()) {
> -mis->migration_incoming_co = qemu_coroutine_self();
>  qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
>   colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE);
>  mis->have_colo_incoming_thread = true;
> @@ -442,6 +442,7 @@ static void process_incoming_migration_co(void *opaque)
>  }
>  mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
>  qemu_bh_schedule(mis->bh);
> +mis->migration_incoming_co = NULL;
>  }
>  
>  static void migration_incoming_setup(QEMUFile *f)
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 1affc46..ae07515 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3226,6 +3226,35 @@ err:
>  
>  static void rdma_accept_incoming_migration(void *opaque);
>  
> +static void rdma_cm_poll_handler(void *opaque)
> +{
> +RDMAContext *rdma = opaque;
> +int ret;
> +struct rdma_cm_event *cm_event;
> +MigrationIncomingState *mis = migration_incoming_get_current();
> +
> +ret = rdma_get_cm_event(rdma->channel, &cm_event);
> +if (ret) {
> +error_report("get_cm_event failed %d", errno);
> +return;
> +}
> +rdma_ack_cm_event(cm_event);
> +
> +if (cm_event->event == RDMA_CM_EVENT_DISCONNECTED ||
> +cm_event->event == RDMA_CM_EVENT_DEVICE_REMOVAL) {
> +error_report("receive cm event, cm event is %d", cm_event->event);
> +rdma->error_state = -EPIPE;
> +if (rdma->return_path) {
> +rdma->return_path->error_state = -EPIPE;
> +}
> +
> +if (mis->migration_incoming_co) {
> +qemu_coroutine_enter(mis->migration_incoming_co);
> +}
> +return;
> +}
> +}
> +
>  static int qemu_rdma_accept(RDMAContext *rdma)
>  {
>  RDMACapabilities cap;
> @@ -3326,7 +3355,8 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>  NULL,
>  (void *)(intptr_t)rdma->return_path);
>  } else {
> -qemu_set_fd_handler(rdma->channel->fd, NULL, NULL, NULL);
> +qemu_set_fd_handler(rdma->channel->fd, rdma_cm_poll_handler,
> +NULL, rdma);
>  }
>  
>  ret = rdma_accept(rdma->cm_id, &conn_param);
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PATCH v10 01/65] target/mips: Add preprocessor constants for nanoMIPS

[Aleksandar Markovic]

From: Aleksandar Markovic 

Add ISA_NANOMIPS32 and CPU_NANOMIPS32 preprocessor constants.

Reviewed-by: Richard Henderson 
Signed-off-by: Yongbok Kim 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 target/mips/mips-defs.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
index d239069..c8e9979 100644
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -39,6 +39,7 @@
 #define   ISA_MIPS64R5  0x1000
 #define   ISA_MIPS32R6  0x2000
 #define   ISA_MIPS64R6  0x4000
+#define   ISA_NANOMIPS32  0x8000
 
 /* MIPS ASEs. */
 #define   ASE_MIPS160x0001
@@ -87,6 +88,9 @@
 #define CPU_MIPS32R6 (CPU_MIPS32R5 | ISA_MIPS32R6)
 #define CPU_MIPS64R6 (CPU_MIPS64R5 | CPU_MIPS32R6 | ISA_MIPS64R6)
 
+/* Wave Computing: "nanoMIPS" */
+#define CPU_NANOMIPS32 (CPU_MIPS32R6 | ISA_NANOMIPS32)
+
 /* Strictly follow the architecture standard:
- Disallow "special" instruction handling for PMON/SPIM.
Note that we still maintain Count/Compare to match the host clock. */
-- 
2.7.4

[Qemu-devel] [PATCH v10 00/65] Add nanoMIPS support to QEMU

[Aleksandar Markovic]

From: Aleksandar Markovic 

v9->v10:

  - first 15 patches removed since they are applied now to the
main tree
  - completely removed connections to and dependance on
  MIPS_HFLAG_M16 in nanoMIPS code
  - completed reworking of nanoMIPS branch handling
  - some fixes and improvements in DSP ASE patches
  - other fixes and improvements that originate from code reviews
  - rebased to the latest code

v8->v9:

  - reoganized (moved/squashed) patches so that clang build bisect
 works (as the result, number of patches dropped from 87 to 84)
  - re-examined and reworked 32-bit nanoMIPS branch instructions
  - minor improvements in commit messages  
  - rebased to the latest code

v7->v8:

  - the series is slightly reorganized so that:
  - patches 1-19 are fixes and improvements that are not
  dependent on the existence of nanoMIPS (even though most
  of them are logicaly connected to (and necessary for)
  nanoMIPS support) - they fix and improve pre-nanoMIPS code
  - patches 20-65 introduce core nanoMIPS functionality, but
  do not contain any dependence on or reference to nanoMIPS
  Linux ABI
  - patches 66-87 mostly deal with Linux user mode-related
  nanoMIPS functionality, therefore dependent on nanoMIPS
  Linux ABI
  - the series will probably be split into three (corresponding
  to the organization mentioned above) in near future

  - added support for availability control via bit config XNP
  - fixed availabitily control for LLWP/SCWP
  - added support for availability control via bit config MT
  - fixed availabitily control for pre-nanoMIPS MT ASE
  - fixed availabitily control for nanoMIPS MT ASE
  - completely removed case-statements defined by integer range
  from translate.c
  - patch on nanoMIPS specifics in ELF headers split into two
  - patch on GT64120-related functionality in nanoMIPS bootloader
  updated with comments and reorganizes with respect to
  endianness
  - replaced one instance of shift/mask with extract32()
  - fixed one instance of missing default case in decoding engine
  - removed several instances of unnecessary default case in
decoding engine
  - minor tweaks related to variable scope and naming
  - fixed several spelling mistakes in commit messages
  - rebased to the latest code

v6->v7:

  - found a better place for MIPS_ARCH in elf.h
  - improved patch for LLWP and SCWP
  - fixed missing availability control, alignment, usage
of extract32() in DSP patches
  - added disassembler support for microMIPS and nanoMIPS
  - removed unnecessary addition of one empty line in the
patch on WR bit
  - improved statx() syscall translation
  - improved nanoMIPS items in binfmt script
  - amended pre-nanoMIPS items in qemu-doc.texi
  - added nanoMIPS items in qemu-doc.texi
  - changed slightly patch order to be logicaly more
comprehensive 
  - rebased to the latest code
  - NOTE: there will be some sheckpatch.pl errors and warning
for this series; however, we think those are flase positives
in these particular circumstances - therefore we will not
change any patch related to these checkpatch.pl messages

v5->v6:

  - used names offset and imm instead of rd and rs when
appropriate
  - used gen_op_addr_addi when appropriate in one more place
  - avoided usage of tcg_temp_local_new
  - avoided unnecessary sign extension related to addr_add
  - fixed unprotected storing to cpu_gpr[0]
  - removed some unnecessary testing for ISA_NANOMIPS
  - updated patch for LLWP and SCWP
  - extract32 inserted instead of shift/mask in DSP patches
  - removed useless casts from DSP patches
  - reorganized functions to eliminated duplicated loading of
gpr values into tcg variables in DSP patches
  - check Config1.WR bit for Watch registers only when using
in runtime
  - removed duplicated check for bad address in PC register
  - added support for statx system call
  - updated script qemu-binfmt-conf.sh for nanoMIPS
  - rebased to the latest code

v4->v5:

  - merged series "Mips maintenance and misc fixes and improvements"
and this one for easier handling (there are build dependencies)
  - eliminated shadow variables from translate.c
  - replaced shift/mask combination with extract32()
  - added new function gen_op_addr_addi()
  - added patch for LLWP and SCWP
  - added "fall through" comments at appropriate places
  - eliminated micromips flag from I7200 definition
  - numerous other enhancements originating from reviewer's
comments
  - some of the patches split into two or more for easier
handling and review
  - rebased to the latest code

v3->v4:

  - added support for nanoMIPS user mode functionality and
configuration
  - DSP patch split into three for easier review and handling
  - corrected indentation in all decoding engine patches
  - shift/mask replaced with equivalent extract32() in some
patches
  - added missing default cases in some patches
  - refactored invocation logic ar

[Qemu-devel] [PATCH v10 04/65] target/mips: Add placeholder and invocation of decode_nanomips_opc()

[Aleksandar Markovic]

From: Aleksandar Markovic 

Add empty body and invocation of decode_nanomips_opc() if the bit
ISA_NANOMIPS32 is set in ctx->insn_flags.

Reviewed-by: Richard Henderson 
Signed-off-by: Yongbok Kim 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 target/mips/translate.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 4f95b9a..b71d3fe 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -16586,6 +16586,19 @@ enum {
 NM_EVP  = 0x01,
 };
 
+
+/*
+ *
+ * nanoMIPS decoding engine
+ *
+ */
+
+static int decode_nanomips_opc(CPUMIPSState *env, DisasContext *ctx)
+{
+return 2;
+}
+
+
 /* SmartMIPS extension to MIPS32 */
 
 #if defined(TARGET_MIPS64)
@@ -21402,7 +21415,10 @@ static void mips_tr_translate_insn(DisasContextBase 
*dcbase, CPUState *cs)
 int is_slot;
 
 is_slot = ctx->hflags & MIPS_HFLAG_BMASK;
-if (!(ctx->hflags & MIPS_HFLAG_M16)) {
+if (ctx->insn_flags & ISA_NANOMIPS32) {
+ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
+insn_bytes = decode_nanomips_opc(env, ctx);
+} else if (!(ctx->hflags & MIPS_HFLAG_M16)) {
 ctx->opcode = cpu_ldl_code(env, ctx->base.pc_next);
 insn_bytes = 4;
 decode_opc(env, ctx);
@@ -21841,8 +21857,8 @@ void cpu_state_reset(CPUMIPSState *env)
 env->CP0_Status |= (1 << CP0St_FR);
 }
 
-if (env->CP0_Config3 & (1 << CP0C3_ISA)) {
-/*  microMIPS on reset when Config3.ISA == {1, 3} */
+if (env->CP0_Config3 & (1 << (CP0C3_ISA + 1))) {
+/*  microMIPS on reset when Config3.ISA == 3  */
 env->hflags |= MIPS_HFLAG_M16;
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH v10 09/65] target/mips: Add emulation of nanoMIPS 16-bit misc instructions

[Aleksandar Markovic]

From: Yongbok Kim 

Add emulation of misc nanoMIPS 16-bit instructions.

Reviewed-by: Richard Henderson 
Signed-off-by: Yongbok Kim 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 target/mips/translate.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 0c24120..fe8a760 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -16763,6 +16763,40 @@ static int decode_nanomips_opc(CPUMIPSState *env, 
DisasContext *ctx)
 op = extract32(ctx->opcode, 10, 6);
 switch (op) {
 case NM_P16_MV:
+rt = NANOMIPS_EXTRACT_RD5(ctx->opcode);
+if (rt != 0) {
+/* MOVE */
+rs = NANOMIPS_EXTRACT_RS5(ctx->opcode);
+gen_arith(ctx, OPC_ADDU, rt, rs, 0);
+} else {
+/* P16.RI */
+switch (extract32(ctx->opcode, 3, 2)) {
+case NM_P16_SYSCALL:
+if (extract32(ctx->opcode, 2, 1) == 0) {
+generate_exception_end(ctx, EXCP_SYSCALL);
+} else {
+generate_exception_end(ctx, EXCP_RI);
+}
+break;
+case NM_BREAK16:
+generate_exception_end(ctx, EXCP_BREAK);
+break;
+case NM_SDBBP16:
+if (is_uhi(extract32(ctx->opcode, 0, 3))) {
+gen_helper_do_semihosting(cpu_env);
+} else {
+if (ctx->hflags & MIPS_HFLAG_SBRI) {
+generate_exception_end(ctx, EXCP_RI);
+} else {
+generate_exception_end(ctx, EXCP_DBp);
+}
+}
+break;
+default:
+generate_exception_end(ctx, EXCP_RI);
+break;
+}
+}
 break;
 case NM_P16_SHIFT:
 {
@@ -16842,6 +16876,13 @@ static int decode_nanomips_opc(CPUMIPSState *env, 
DisasContext *ctx)
 }
 break;
 case NM_LI16:
+{
+int imm = extract32(ctx->opcode, 0, 7);
+imm = (imm == 0x7f ? -1 : imm);
+if (rt != 0) {
+tcg_gen_movi_tl(cpu_gpr[rt], imm);
+}
+}
 break;
 case NM_ANDI16:
 break;
-- 
2.7.4

[Qemu-devel] [PATCH v10 02/65] target/mips: Add nanoMIPS base instruction set opcodes

[Aleksandar Markovic]

From: Yongbok Kim 

Add nanoMIPS opcodes. nanoMIPS instruction are organized by so-called
instruction pools. Each pool contains a set of opcodes, that in turn
can be instruction opcodes or instruction pool opcodes.

Reviewed-by: Richard Henderson 
Signed-off-by: Yongbok Kim 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 target/mips/translate.c | 670 
 1 file changed, 670 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index bdd880b..0075dbc 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -15701,6 +15701,676 @@ static int decode_micromips_opc (CPUMIPSState *env, 
DisasContext *ctx)
 return 2;
 }
 
+/*
+ *
+ * nanoMIPS opcodes
+ *
+ */
+
+/* MAJOR, P16, and P32 pools opcodes */
+enum {
+NM_P_ADDIU  = 0x00,
+NM_ADDIUPC  = 0x01,
+NM_MOVE_BALC= 0x02,
+NM_P16_MV   = 0x04,
+NM_LW16 = 0x05,
+NM_BC16 = 0x06,
+NM_P16_SR   = 0x07,
+
+NM_POOL32A  = 0x08,
+NM_P_BAL= 0x0a,
+NM_P16_SHIFT= 0x0c,
+NM_LWSP16   = 0x0d,
+NM_BALC16   = 0x0e,
+NM_P16_4X4  = 0x0f,
+
+NM_P_GP_W   = 0x10,
+NM_P_GP_BH  = 0x11,
+NM_P_J  = 0x12,
+NM_P16C = 0x14,
+NM_LWGP16   = 0x15,
+NM_P16_LB   = 0x17,
+
+NM_P48I = 0x18,
+NM_P16_A1   = 0x1c,
+NM_LW4X4= 0x1d,
+NM_P16_LH   = 0x1f,
+
+NM_P_U12= 0x20,
+NM_P_LS_U12 = 0x21,
+NM_P_BR1= 0x22,
+NM_P16_A2   = 0x24,
+NM_SW16 = 0x25,
+NM_BEQZC16  = 0x26,
+
+NM_POOL32F  = 0x28,
+NM_P_LS_S9  = 0x29,
+NM_P_BR2= 0x2a,
+
+NM_P16_ADDU = 0x2c,
+NM_SWSP16   = 0x2d,
+NM_BNEZC16  = 0x2e,
+NM_MOVEP= 0x2f,
+
+NM_POOL32S  = 0x30,
+NM_P_BRI= 0x32,
+NM_LI16 = 0x34,
+NM_SWGP16   = 0x35,
+NM_P16_BR   = 0x36,
+
+NM_P_LUI= 0x38,
+NM_ANDI16   = 0x3c,
+NM_SW4X4= 0x3d,
+NM_MOVEPREV = 0x3f,
+};
+
+/* POOL32A instruction pool */
+enum {
+NM_POOL32A0= 0x00,
+NM_SPECIAL2= 0x01,
+NM_COP2_1  = 0x02,
+NM_UDI = 0x03,
+NM_POOL32A5= 0x05,
+NM_POOL32A7= 0x07,
+};
+
+/* P.GP.W instruction pool */
+enum {
+NM_ADDIUGP_W = 0x00,
+NM_LWGP  = 0x02,
+NM_SWGP  = 0x03,
+};
+
+/* P48I instruction pool */
+enum {
+NM_LI48= 0x00,
+NM_ADDIU48 = 0x01,
+NM_ADDIUGP48   = 0x02,
+NM_ADDIUPC48   = 0x03,
+NM_LWPC48  = 0x0b,
+NM_SWPC48  = 0x0f,
+};
+
+/* P.U12 instruction pool */
+enum {
+NM_ORI  = 0x00,
+NM_XORI = 0x01,
+NM_ANDI = 0x02,
+NM_P_SR = 0x03,
+NM_SLTI = 0x04,
+NM_SLTIU= 0x05,
+NM_SEQI = 0x06,
+NM_ADDIUNEG = 0x08,
+NM_P_SHIFT  = 0x0c,
+NM_P_ROTX   = 0x0d,
+NM_P_INS= 0x0e,
+NM_P_EXT= 0x0f,
+};
+
+/* POOL32F instruction pool */
+enum {
+NM_POOL32F_0   = 0x00,
+NM_POOL32F_3   = 0x03,
+NM_POOL32F_5   = 0x05,
+};
+
+/* POOL32S instruction pool */
+enum {
+NM_POOL32S_0   = 0x00,
+NM_POOL32S_4   = 0x04,
+};
+
+/* P.LUI instruction pool */
+enum {
+NM_LUI  = 0x00,
+NM_ALUIPC   = 0x01,
+};
+
+/* P.GP.BH instruction pool */
+enum {
+NM_LBGP  = 0x00,
+NM_SBGP  = 0x01,
+NM_LBUGP = 0x02,
+NM_ADDIUGP_B = 0x03,
+NM_P_GP_LH   = 0x04,
+NM_P_GP_SH   = 0x05,
+NM_P_GP_CP1  = 0x06,
+};
+
+/* P.LS.U12 instruction pool */
+enum {
+NM_LB= 0x00,
+NM_SB= 0x01,
+NM_LBU   = 0x02,
+NM_P_PREFU12 = 0x03,
+NM_LH= 0x04,
+NM_SH= 0x05,
+NM_LHU   = 0x06,
+NM_LWU   = 0x07,
+NM_LW= 0x08,
+NM_SW= 0x09,
+NM_LWC1  = 0x0a,
+NM_SWC1  = 0x0b,
+NM_LDC1  = 0x0e,
+NM_SDC1  = 0x0f,
+};
+
+/* P.LS.S9 instruction pool */
+enum {
+NM_P_LS_S0 = 0x00,
+NM_P_LS_S1 = 0x01,
+NM_P_LS_E0 = 0x02,
+NM_P_LS_WM = 0x04,
+NM_P_LS_UAWM   = 0x05,
+};
+
+/* P.BAL instruction pool */
+enum {
+NM_BC   = 0x00,
+NM_BALC = 0x01,
+};
+
+/* P.J instruction pool */
+enum {
+NM_JALRC= 0x00,
+NM_JALRC_HB = 0x01,
+NM_P_BALRSC = 0x08,
+};
+
+/* P.BR1 instruction pool */
+enum {
+NM_BEQC = 0x00,
+NM_P_BR3A   = 0x01,
+NM_BGEC = 0x02,
+NM_BGEUC= 0x03,
+};
+
+/* P.BR2 instruction pool */
+enum {
+NM_BNEC = 0x00,
+NM_BLTC = 0x02,
+NM_BLTUC= 0x03,
+};
+
+/* P.BRI instruction pool */
+enum {
+NM_BEQIC= 0x00,
+NM_BBEQZC   = 0x01,
+NM_BGEIC= 0x02,
+NM_BGEIUC   = 0x03,
+NM_BNEIC= 0x04,
+NM_BBNEZC   = 0x05,
+NM_BLTIC= 0x06,
+NM_BLTIUC   = 0x07,
+};
+
+/* P16.SHIFT instruction pool */
+enum {
+NM_SLL16= 0x00,

[Qemu-devel] [PATCH v10 11/65] target/mips: Add emulation of nanoMIPS 16-bit logic instructions

[Aleksandar Markovic]

From: Yongbok Kim 

Add emulation of NOT16, AND16, XOR16, OR16 instructions.

Reviewed-by: Richard Henderson 
Signed-off-by: Yongbok Kim 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 target/mips/translate.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index ce53eae..34df82e 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -16760,6 +16760,37 @@ static inline int decode_gpr_gpr4_zero(int r)
 }
 
 
+/* extraction utilities */
+
+#define NANOMIPS_EXTRACT_RD(op) ((op >> 7) & 0x7)
+#define NANOMIPS_EXTRACT_RS(op) ((op >> 4) & 0x7)
+#define NANOMIPS_EXTRACT_RS2(op) uMIPS_RS(op)
+#define NANOMIPS_EXTRACT_RS1(op) ((op >> 1) & 0x7)
+#define NANOMIPS_EXTRACT_RD5(op) ((op >> 5) & 0x1f)
+#define NANOMIPS_EXTRACT_RS5(op) (op & 0x1f)
+
+
+static void gen_pool16c_nanomips_insn(DisasContext *ctx)
+{
+int rt = decode_gpr_gpr3(NANOMIPS_EXTRACT_RD(ctx->opcode));
+int rs = decode_gpr_gpr3(NANOMIPS_EXTRACT_RS(ctx->opcode));
+
+switch (extract32(ctx->opcode, 2, 2)) {
+case NM_NOT16:
+gen_logic(ctx, OPC_NOR, rt, rs, 0);
+break;
+case NM_AND16:
+gen_logic(ctx, OPC_AND, rt, rt, rs);
+break;
+case NM_XOR16:
+gen_logic(ctx, OPC_XOR, rt, rt, rs);
+break;
+case NM_OR16:
+gen_logic(ctx, OPC_OR, rt, rt, rs);
+break;
+}
+}
+
 static int decode_nanomips_opc(CPUMIPSState *env, DisasContext *ctx)
 {
 uint32_t op;
@@ -16836,6 +16867,7 @@ static int decode_nanomips_opc(CPUMIPSState *env, 
DisasContext *ctx)
 case NM_P16C:
 switch (ctx->opcode & 1) {
 case NM_POOL16C_0:
+gen_pool16c_nanomips_insn(ctx);
 break;
 case NM_LWXS16:
 gen_ldxs(ctx, rt, rs, rd);
@@ -16910,6 +16942,12 @@ static int decode_nanomips_opc(CPUMIPSState *env, 
DisasContext *ctx)
 }
 break;
 case NM_ANDI16:
+{
+uint32_t u = extract32(ctx->opcode, 0, 4);
+u = (u == 12) ? 0xff :
+(u == 13) ? 0x : u;
+gen_logic_imm(ctx, OPC_ANDI, rt, rs, u);
+}
 break;
 case NM_P16_LB:
 offset = extract32(ctx->opcode, 0, 2);
-- 
2.7.4