date:20210115

  Hi,

> These patches were made in response to Volkers patches which dealt with
>  cleaning up my previous code:
>  https://www.mail-archive.com/qemu-devel@nongnu.org/msg766686.html
>  and thus are meant to be merged after Volker's patches.
> 
> Hopefully, this commit message isn't too daunting ;)
> 
> EDIT: Sorry about the badly formatted patch emailss, still can't get
>  a hand of how git send-email works.

Well, I got three copies of the patch series.  Guess that comes from
trying to get git send-email going ;)

Good news is the patches (tried the most recent mails) apply just fine
on top of Volkers series, so it seems you got it working in the end.

Patches are queued up now,
  Gerd

[PULL 0/1] 9p security fix 2021-01-15

2021-01-15 Thread Greg Kurz

The following changes since commit 7c79721606be11b5bc556449e5bcbc331ef6867d:

  Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210113' into 
staging (2021-01-14 09:54:29 +)

are available in the Git repository at:

  https://gitlab.com/gkurz/qemu.git tags/9p-next-2021-01-15

for you to fetch changes up to 89fbea8737e8f7b954745a1ffc4238d377055305:

  9pfs: Fully restart unreclaim loop (CVE-2021-20181) (2021-01-15 08:44:28 
+0100)


Fix for CVE-2021-20181


Greg Kurz (1):
  9pfs: Fully restart unreclaim loop (CVE-2021-20181)

 hw/9pfs/9p.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
-- 
2.26.2

RE: [PATCH 3/3] net/colo-compare: Add handler for passthrough connection

2021-01-15 Thread Zhang, Chen




> -Original Message-
> From: Lukas Straub 
> Sent: Thursday, January 14, 2021 9:45 PM
> To: Zhang, Chen 
> Cc: Jason Wang ; qemu-dev  de...@nongnu.org>; Eric Blake ; Dr. David Alan
> Gilbert ; Markus Armbruster ;
> Zhang Chen 
> Subject: Re: [PATCH 3/3] net/colo-compare: Add handler for passthrough
> connection
> 
> On Thu, 24 Dec 2020 09:09:18 +0800
> Zhang Chen  wrote:
> 
> > From: Zhang Chen 
> >
> > Currently, we just use guest's TCP/UDP source port as the key to
> > bypass certain network traffic.
> >
> > Signed-off-by: Zhang Chen 
> > ---
> >  net/colo-compare.c | 49
> > ++
> >  net/colo-compare.h |  2 ++
> >  net/net.c  | 27 +
> >  3 files changed, 78 insertions(+)
> >
> > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > 337025b44f..11a32caa9b 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -46,6 +46,9 @@ static QTAILQ_HEAD(, CompareState) net_compares =
> > static NotifierList colo_compare_notifiers =
> >  NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
> >
> > +static QLIST_HEAD(, PassthroughEntry) passthroughlist =
> > +QLIST_HEAD_INITIALIZER(passthroughlist);
> > +
> 
> Hi,
> I think this should be per colo-compare instance e.g. inside 'struct
> CompareState'.

It looks QMP and HMP also need to add colo-compare object ID to control it.
Do we need make this command more general?

Thanks
Chen

> 
> >  #define COMPARE_READ_LEN_MAX NET_BUFSIZE  #define
> MAX_QUEUE_SIZE 1024
> >
> > @@ -103,6 +106,12 @@ typedef struct SendEntry {
> >  uint8_t *buf;
> >  } SendEntry;
> >
> > +typedef struct PassthroughEntry {
> > +bool is_tcp;
> > +uint16_t port;
> > +QLIST_ENTRY(PassthroughEntry) node; } PassthroughEntry;
> > +
> >  struct CompareState {
> >  Object parent;
> >
> > @@ -247,6 +256,7 @@ static int packet_enqueue(CompareState *s, int
> mode, Connection **con)
> >  ConnectionKey key;
> >  Packet *pkt = NULL;
> >  Connection *conn;
> > +PassthroughEntry *bypass, *next;
> >  int ret;
> >
> >  if (mode == PRIMARY_IN) {
> > @@ -264,8 +274,23 @@ static int packet_enqueue(CompareState *s, int
> mode, Connection **con)
> >  pkt = NULL;
> >  return -1;
> >  }
> > +
> >  fill_connection_key(pkt, &key);
> >
> > +/* Check COLO passthrough connenction */
> > +if (!QLIST_EMPTY(&passthroughlist)) {
> > +QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> > +if (((key.ip_proto == IPPROTO_TCP) && bypass->is_tcp) ||
> > +((key.ip_proto == IPPROTO_UDP) && !bypass->is_tcp)) {
> > +if (bypass->port == key.src_port) {
> > +packet_destroy(pkt, NULL);
> > +pkt = NULL;
> > +return -1;
> > +}
> > +}
> > +}
> > +}
> > +
> >  conn = connection_get(s->connection_track_table,
> >&key,
> >&s->conn_list); @@ -1373,6 +1398,30 @@
> > static void colo_flush_packets(void *opaque, void *user_data)
> >  }
> >  }
> >
> > +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port) {
> > +PassthroughEntry *bypass = NULL;
> > +
> > +bypass = g_new0(PassthroughEntry, 1);
> > +bypass->is_tcp = is_tcp;
> > +bypass->port = port;
> > +QLIST_INSERT_HEAD(&passthroughlist, bypass, node); }
> > +
> > +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port) {
> > +PassthroughEntry *bypass = NULL, *next = NULL;
> > +
> > +if (!QLIST_EMPTY(&passthroughlist)) {
> > +QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> > +if ((bypass->is_tcp == is_tcp) && (bypass->port == port)) {
> > +QLIST_REMOVE(bypass, node);
> > +g_free(bypass);
> > +}
> > +}
> > +}
> > +}
> > +
> >  static void colo_compare_class_init(ObjectClass *oc, void *data)  {
> >  UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); diff --git
> > a/net/colo-compare.h b/net/colo-compare.h index
> 22ddd512e2..1fa026c85e
> > 100644
> > --- a/net/colo-compare.h
> > +++ b/net/colo-compare.h
> > @@ -20,5 +20,7 @@
> >  void colo_notify_compares_event(void *opaque, int event, Error
> > **errp);  void colo_compare_register_notifier(Notifier *notify);  void
> > colo_compare_unregister_notifier(Notifier *notify);
> > +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port);
> > +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port);
> >
> >  #endif /* QEMU_COLO_COMPARE_H */
> > diff --git a/net/net.c b/net/net.c
> > index eac7a92618..1f303e8309 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -55,6 +55,7 @@
> >  #include "sysemu/sysemu.h"
> >  #include "net/filter.h"
> >  #include "qapi/string-output-visitor.h"
> > +#include "net/colo-compare.h"
> >
> >  /* Net bridge is currently not supported for W32. */  #if
>

[PULL 1/1] 9pfs: Fully restart unreclaim loop (CVE-2021-20181)

2021-01-15 Thread Greg Kurz

Depending on the client activity, the server can be asked to open a huge
number of file descriptors and eventually hit RLIMIT_NOFILE. This is
currently mitigated using a reclaim logic : the server closes the file
descriptors of idle fids, based on the assumption that it will be able
to re-open them later. This assumption doesn't hold of course if the
client requests the file to be unlinked. In this case, we loop on the
entire fid list and mark all related fids as unreclaimable (the reclaim
logic will just ignore them) and, of course, we open or re-open their
file descriptors if needed since we're about to unlink the file.

This is the purpose of v9fs_mark_fids_unreclaim(). Since the actual
opening of a file can cause the coroutine to yield, another client
request could possibly add a new fid that we may want to mark as
non-reclaimable as well. The loop is thus restarted if the re-open
request was actually transmitted to the backend. This is achieved
by keeping a reference on the first fid (head) before traversing
the list.

This is wrong in several ways:
- a potential clunk request from the client could tear the first
  fid down and cause the reference to be stale. This leads to a
  use-after-free error that can be detected with ASAN, using a
  custom 9p client
- fids are added at the head of the list : restarting from the
  previous head will always miss fids added by a some other
  potential request

All these problems could be avoided if fids were being added at the
end of the list. This can be achieved with a QSIMPLEQ, but this is
probably too much change for a bug fix. For now let's keep it
simple and just restart the loop from the current head.

Fixes: CVE-2021-20181
Buglink: https://bugs.launchpad.net/qemu/+bug/1911666
Reported-by: Zero Day Initiative 
Reviewed-by: Christian Schoenebeck 
Reviewed-by: Stefano Stabellini 
Message-Id: <161064025265.1838153.15185571283519390907.st...@bahia.lan>
Signed-off-by: Greg Kurz 
---
 hw/9pfs/9p.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 94df440fc740..6026b51a1c04 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -502,9 +502,9 @@ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU 
*pdu, V9fsPath *path)
 {
 int err;
 V9fsState *s = pdu->s;
-V9fsFidState *fidp, head_fid;
+V9fsFidState *fidp;
 
-head_fid.next = s->fid_list;
+again:
 for (fidp = s->fid_list; fidp; fidp = fidp->next) {
 if (fidp->path.size != path->size) {
 continue;
@@ -524,7 +524,7 @@ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU 
*pdu, V9fsPath *path)
  * switched to the worker thread
  */
 if (err == 0) {
-fidp = &head_fid;
+goto again;
 }
 }
 }
-- 
2.26.2

RE: [PATCH 3/3] net/colo-compare: Add handler for passthrough connection

2021-01-15 Thread Zhang, Chen




> -Original Message-
> From: Lukas Straub 
> Sent: Thursday, January 14, 2021 9:51 PM
> To: Zhang, Chen 
> Cc: Jason Wang ; qemu-dev  de...@nongnu.org>; Eric Blake ; Dr. David Alan
> Gilbert ; Markus Armbruster ;
> Zhang Chen 
> Subject: Re: [PATCH 3/3] net/colo-compare: Add handler for passthrough
> connection
> 
> On Thu, 24 Dec 2020 09:09:18 +0800
> Zhang Chen  wrote:
> 
> > From: Zhang Chen 
> >
> > Currently, we just use guest's TCP/UDP source port as the key to
> > bypass certain network traffic.
> >
> > Signed-off-by: Zhang Chen 
> > ---
> >  net/colo-compare.c | 49
> > ++
> >  net/colo-compare.h |  2 ++
> >  net/net.c  | 27 +
> >  3 files changed, 78 insertions(+)
> >
> > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > 337025b44f..11a32caa9b 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -46,6 +46,9 @@ static QTAILQ_HEAD(, CompareState) net_compares =
> > static NotifierList colo_compare_notifiers =
> >  NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
> >
> > +static QLIST_HEAD(, PassthroughEntry) passthroughlist =
> > +QLIST_HEAD_INITIALIZER(passthroughlist);
> > +
> >  #define COMPARE_READ_LEN_MAX NET_BUFSIZE  #define
> MAX_QUEUE_SIZE 1024
> >
> > @@ -103,6 +106,12 @@ typedef struct SendEntry {
> >  uint8_t *buf;
> >  } SendEntry;
> >
> > +typedef struct PassthroughEntry {
> > +bool is_tcp;
> > +uint16_t port;
> > +QLIST_ENTRY(PassthroughEntry) node; } PassthroughEntry;
> > +
> >  struct CompareState {
> >  Object parent;
> >
> > @@ -247,6 +256,7 @@ static int packet_enqueue(CompareState *s, int
> mode, Connection **con)
> >  ConnectionKey key;
> >  Packet *pkt = NULL;
> >  Connection *conn;
> > +PassthroughEntry *bypass, *next;
> >  int ret;
> >
> >  if (mode == PRIMARY_IN) {
> > @@ -264,8 +274,23 @@ static int packet_enqueue(CompareState *s, int
> mode, Connection **con)
> >  pkt = NULL;
> >  return -1;
> >  }
> > +
> >  fill_connection_key(pkt, &key);
> >
> > +/* Check COLO passthrough connenction */
> > +if (!QLIST_EMPTY(&passthroughlist)) {
> > +QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> > +if (((key.ip_proto == IPPROTO_TCP) && bypass->is_tcp) ||
> > +((key.ip_proto == IPPROTO_UDP) && !bypass->is_tcp)) {
> > +if (bypass->port == key.src_port) {
> > +packet_destroy(pkt, NULL);
> > +pkt = NULL;
> > +return -1;
> > +}
> > +}
> > +}
> > +}
> > +
> >  conn = connection_get(s->connection_track_table,
> >&key,
> >&s->conn_list); @@ -1373,6 +1398,30 @@
> > static void colo_flush_packets(void *opaque, void *user_data)
> >  }
> >  }
> >
> > +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port) {
> > +PassthroughEntry *bypass = NULL;
> > +
> > +bypass = g_new0(PassthroughEntry, 1);
> > +bypass->is_tcp = is_tcp;
> > +bypass->port = port;
> > +QLIST_INSERT_HEAD(&passthroughlist, bypass, node); }
> > +
> > +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port) {
> > +PassthroughEntry *bypass = NULL, *next = NULL;
> > +
> > +if (!QLIST_EMPTY(&passthroughlist)) {
> > +QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> > +if ((bypass->is_tcp == is_tcp) && (bypass->port == port)) {
> > +QLIST_REMOVE(bypass, node);
> > +g_free(bypass);
> > +}
> > +}
> > +}
> > +}
> > +
> 
> Access to "passtroughlist" needs to be protected by a lock, as
> "packet_enqueue" is called from a different iothread.

OK, I will add the lock in next version.

Thanks
Chen

> 
> >  static void colo_compare_class_init(ObjectClass *oc, void *data)  {
> >  UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); diff --git
> > a/net/colo-compare.h b/net/colo-compare.h index
> 22ddd512e2..1fa026c85e
> > 100644
> > --- a/net/colo-compare.h
> > +++ b/net/colo-compare.h
> > @@ -20,5 +20,7 @@
> >  void colo_notify_compares_event(void *opaque, int event, Error
> > **errp);  void colo_compare_register_notifier(Notifier *notify);  void
> > colo_compare_unregister_notifier(Notifier *notify);
> > +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port);
> > +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port);
> >
> >  #endif /* QEMU_COLO_COMPARE_H */
> > diff --git a/net/net.c b/net/net.c
> > index eac7a92618..1f303e8309 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -55,6 +55,7 @@
> >  #include "sysemu/sysemu.h"
> >  #include "net/filter.h"
> >  #include "qapi/string-output-visitor.h"
> > +#include "net/colo-compare.h"
> >
> >  /* Net bridge is currently not supported for W32. */  #if
> > !defined(_WIN32) @@ -1155,12 +1156,38 @@ void
>

Re: [PATCH] qtest/npcm7xx_pwm-test: Fix memleak in pwm_qom_get

On 1/15/21 8:56 AM, Gan Qixin wrote:
> The pwm_qom_get function didn't free "response", which caused an indirect
> memory leak. So use qobject_unref() to fix it.
> 
> ASAN shows memory leak stack:
> 
> Indirect leak of 7416 byte(s) in 18000 object(s) allocated from:
> #0 0x7f96e2f79d4e in __interceptor_calloc (/lib64/libasan.so.5+0x112d4e)
> #1 0x7f96e2d98a50 in g_malloc0 (/lib64/libglib-2.0.so.0+0x55a50)
> #2 0x556313112180 in qdict_new ../qobject/qdict.c:30
> #3 0x556313115bca in parse_object ../qobject/json-parser.c:318
> #4 0x556313117810 in parse_value ../qobject/json-parser.c:546
> #5 0x556313117bda in json_parser_parse ../qobject/json-parser.c:580
> #6 0x55631310fe67 in json_message_process_token 
> ../qobject/json-streamer.c:92
> #7 0x5563131210b7 in json_lexer_feed_char ../qobject/json-lexer.c:313
> #8 0x556313121662 in json_lexer_feed ../qobject/json-lexer.c:350
> #9 0x5563131101e9 in json_message_parser_feed 
> ../qobject/json-streamer.c:121
> #10 0x5563130cb81e in qmp_fd_receive ../tests/qtest/libqtest.c:614
> #11 0x5563130cba2b in qtest_qmp_receive_dict ../tests/qtest/libqtest.c:636
> #12 0x5563130cb939 in qtest_qmp_receive ../tests/qtest/libqtest.c:624
> #13 0x5563130cbe0d in qtest_vqmp ../tests/qtest/libqtest.c:715
> #14 0x5563130cc40f in qtest_qmp ../tests/qtest/libqtest.c:756
> #15 0x5563130c5623 in pwm_qom_get ../tests/qtest/npcm7xx_pwm-test.c:180
> #16 0x5563130c595e in pwm_get_duty ../tests/qtest/npcm7xx_pwm-test.c:210
> #17 0x5563130c7529 in test_toggle ../tests/qtest/npcm7xx_pwm-test.c:447
> 
> Reported-by: Euler Robot 
> Signed-off-by: Gan Qixin 
> ---
> Cc: Havard Skinnemoen 
> Cc: Tyrone Ting 
> Cc: Thomas Huth 
> Cc: Laurent Vivier 
> ---
>  tests/qtest/npcm7xx_pwm-test.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH v4 3/3] hw/blocl/nvme: trigger async event during injecting smart warning

On 1/15/21 4:27 AM, zhenwei pi wrote:
> During smart critical warning injection by setting property from QMP
> command, also try to trigger asynchronous event.
> 
> Suggested by Keith, if a event has already been raised, there is no
> need to enqueue the duplicate event any more.
> 
> Signed-off-by: zhenwei pi 
> ---
>  hw/block/nvme.c  | 48 +---
>  include/block/nvme.h |  1 +
>  2 files changed, 42 insertions(+), 7 deletions(-)

Typo "blocl" in subject ;) No need to repost.

Re: [PATCH v4 1/3] block/nvme: introduce bit 5 for critical warning

On 1/15/21 4:27 AM, zhenwei pi wrote:
> According to NVMe spec 1.4 section

"According to NVMe spec 1.4 section 5.14.1.2"

> , introduce bit 5
> for "Persistent Memory Region has become read-only or unreliable".
> 
> Signed-off-by: zhenwei pi 
> ---
>  include/block/nvme.h | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH v3 01/10] iotests.py: Assume a couple of variables as given


14.01.2021 20:02, Max Reitz wrote:

There are a couple of environment variables that we fetch with
os.environ.get() without supplying a default.  Clearly they are required
and expected to be set by the ./check script (as evidenced by
execute_setup_common(), which checks for test_dir and
qemu_default_machine to be set, and aborts if they are not).

Using .get() this way has the disadvantage of returning an Optional[str]
type, which mypy will complain about when tests just assume these values
to be str.

Use [] instead, which raises a KeyError for environment variables that
are not set.  When this exception is raised, catch it and move the abort
code from execute_setup_common() there.

Drop the 'assert iotests.sock_dir is not None' from iotest 300, because
that sort of thing is precisely what this patch wants to prevent.

Signed-off-by: Max Reitz



Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH v3 18/19] i386: provide simple 'hv-default=on' option

2021-01-15 Thread Vitaly Kuznetsov

Igor Mammedov  writes:

> On Thu,  7 Jan 2021 16:14:49 +0100
> Vitaly Kuznetsov  wrote:
>
>> Enabling Hyper-V emulation for a Windows VM is a tiring experience as it
>> requires listing all currently supported enlightenments ("hv-*" CPU
>> features) explicitly. We do have 'hv-passthrough' mode enabling
>> everything but it can't be used in production as it prevents migration.
>> 
>> Introduce a simple 'hv-default=on' CPU flag enabling all currently supported
>> Hyper-V enlightenments. Later, when new enlightenments get implemented,
>> compat_props mechanism will be used to disable them for legacy machine types,
>> this will keep 'hv-default=on' configurations migratable.
>> 
>> Signed-off-by: Vitaly Kuznetsov 
>> ---
>>  docs/hyperv.txt   | 16 +---
>>  target/i386/cpu.c | 38 ++
>>  target/i386/cpu.h |  5 +
>>  3 files changed, 56 insertions(+), 3 deletions(-)
>> 
>> diff --git a/docs/hyperv.txt b/docs/hyperv.txt
>> index 5df00da54fc4..a54c066cab09 100644
>> --- a/docs/hyperv.txt
>> +++ b/docs/hyperv.txt
>> @@ -17,10 +17,20 @@ compatible hypervisor and use Hyper-V specific features.
>>  
>>  2. Setup
>>  =
>> -No Hyper-V enlightenments are enabled by default by either KVM or QEMU. In
>> -QEMU, individual enlightenments can be enabled through CPU flags, e.g:
>> +All currently supported Hyper-V enlightenments can be enabled by specifying
>> +'hv-default=on' CPU flag:
>>  
>> -  qemu-system-x86_64 --enable-kvm --cpu host,hv_relaxed,hv_vpindex,hv_time, 
>> ...
>> +  qemu-system-x86_64 --enable-kvm --cpu host,hv-default ...
>> +
>> +Alternatively, it is possible to do fine-grained enablement through CPU 
>> flags,
>> +e.g:
>> +
>> +  qemu-system-x86_64 --enable-kvm --cpu host,hv-relaxed,hv-vpindex,hv-time 
>> ...
>
> I'd put here not '...' but rather recommended list of flags, and update
> it every time when new feature added if necessary.
>

This is an example of fine-grained enablement, there is no point to put
all the existing flags there (hv-default is the only recommended way
now, the rest is 'expert'/'debugging').

> (not to mention that if we had it to begin with, then new 'hv-default' won't
> be necessary, I still see it as functionality duplication but I will not 
> oppose it)
>

Unfortunately, upper layer tools don't read this doc and update
themselves to enable new features when they appear. Similarly, if when
these tools use '-machine q35' they get all the new features we add
automatically, right?

>
>> +It is also possible to disable individual enlightenments from the default 
>> list,
>> +this can be used for debugging purposes:
>> +
>> +  qemu-system-x86_64 --enable-kvm --cpu host,hv-default=on,hv-evmcs=off ...
>>  
>>  Sometimes there are dependencies between enlightenments, QEMU is supposed to
>>  check that the supplied configuration is sane.
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 48007a876e32..99338de00f78 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -4552,6 +4552,24 @@ static void x86_cpuid_set_tsc_freq(Object *obj, 
>> Visitor *v, const char *name,
>>  cpu->env.tsc_khz = cpu->env.user_tsc_khz = value / 1000;
>>  }
>>  
>> +static bool x86_hv_default_get(Object *obj, Error **errp)
>> +{
>> +X86CPU *cpu = X86_CPU(obj);
>> +
>> +return cpu->hyperv_default;
>> +}
>> +
>> +static void x86_hv_default_set(Object *obj, bool value, Error **errp)
>> +{
>> +X86CPU *cpu = X86_CPU(obj);
>> +
>> +cpu->hyperv_default = value;
>> +
>> +if (value) {
>> +cpu->hyperv_features |= cpu->hyperv_default_features;
>
> s/|="/=/ please,
> i.e. no option overrides whatever was specified before to keep semantics 
> consistent.
>

Hm,

this doesn't matter for the most recent machine type as
hyperv_default_features has all the features but imagine you're running
an older machine type which doesn't have 'hv_feature'. Now your
suggestion is 

if I do:

'hv_default,hv_feature=on' I will get "hyperv_default_features | hv_feature"

but if I do

'hv_feature=on,hv_default' I will just get 'hyperv_default_features'
(as hv_default enablement will overwrite everything)

How is this consistent?

>> +}
>> +}
>> +
>>  /* Generic getter for "feature-words" and "filtered-features" properties */
>>  static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
>>const char *name, void *opaque,
>> @@ -6955,10 +6973,26 @@ static void x86_cpu_initfn(Object *obj)
>>  object_property_add_alias(obj, "pause_filter", obj, "pause-filter");
>>  object_property_add_alias(obj, "sse4_1", obj, "sse4.1");
>>  object_property_add_alias(obj, "sse4_2", obj, "sse4.2");
>> +object_property_add_alias(obj, "hv_default", obj, "hv-default");
>>  
>>  if (xcc->model) {
>>  x86_cpu_load_model(cpu, xcc->model);
>>  }
>> +
>> +/* Hyper-V features enabled with 'hv-default=on' */
>> +cpu->hyperv_default_features = BIT(HYPERV_FEAT_RELAXED) |
>> +

Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers

2021-01-15 Thread Stefan Hajnoczi

On Thu, Jan 14, 2021 at 01:24:37PM -0500, Jag Raman wrote:
> 
> 
> > On Jan 14, 2021, at 1:00 PM, Daniel P. Berrangé  wrote:
> > 
> > On Thu, Jan 14, 2021 at 12:55:58PM -0500, Jag Raman wrote:
> >> 
> >> 
> >>> On Jan 14, 2021, at 11:27 AM, Stefan Hajnoczi  wrote:
> >>> 
> >>> On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote:
>  +int qio_channel_readv_full_all(QIOChannel *ioc,
>  +   const struct iovec *iov,
>  +   size_t niov,
>  +   int **fds, size_t *nfds,
>  +   Error **errp)
>  {
>  -int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp);
>  +int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, 
>  errp);
>  
> if (ret == 0) {
>  -ret = -1;
> error_setg(errp,
>    "Unexpected end-of-file before all bytes were read");
> >>> 
> >>> qio_channel_readv_full_all_eof() can read file descriptors but no data
> >>> and return 0.
> >>> 
> >>> Here that case is converted into an error and the file descriptors
> >>> aren't closed, freed, and fds/nfds isn't cleared.
> >> 
> >> That’s a valid point. I’m wondering if the fix for this case should be in
> >> qio_channel_readv_full_all_eof(), instead of here.
> >> 
> >> qio_channel_readv_full_all_eof() should probably return error (-1) if the
> >> amount of data read does not match iov_size(). If the caller is only 
> >> expecting
> >> to read fds, and not any data, it would indicate that by setting iov to 
> >> NULL
> >> and/or setting niov=0. If the caller is setting these parameters, it means 
> >> it is
> >> expecting data.Does that sound good?
> > 
> > The API spec for the existing _eof() methods says:
> > 
> > * The function will wait for all requested data
> > * to be read, yielding from the current coroutine
> > * if required.
> > *
> > * If end-of-file occurs before any data is read,
> > * no error is reported; otherwise, if it occurs
> > * before all requested data has been read, an error
> > * will be reported.
> > 
> > 
> > IOW, return '0' is *only* valid if we've not read anything. I consider
> > file descriptors to be something.
> > 
> > IOW, qio_channel_readv_full_all_eof must only return 0, if it didn't
> > read any data and also didn't receive any file descriptors. So yeah,
> > we must return -1 in the scenario Stefan describes
> 
> That makes sense to me. Reading “fds" is something, which is different
> from our previous understanding. I thought data only meant iov, and not fds.
> 
> So the return values for qio_channel_readv_full_all_eof() would be:
>   - ‘0’ only if EOF is reached without reading any fds and data.
>   - ‘1’ if all data that the caller expects are read (even if the caller reads
> fds exclusively, without any iovs)
>   - ‘-1’ otherwise, considered as error
> 
> qio_channel_readv_full_all() would return:
>   - ‘0’ if all the data that caller expects are read
>   - ‘-1’ otherwise, considered as error
> 
> Hey Stefan,
> 
> Does this sound good to you?

The while (nlocal_iov > 0) loop only runs if the caller has requested to
read at least some data, so the fds-only case doesn't work yet.

This suggests that no current QEMU code relies on the fds-only case.
Therefore you could change the doc comment to clarify this instead of
adding support for this case to the code.

But if you would to fully support the fds-only case that would be even
better.

Stefan


signature.asc
Description: PGP signature

Re: [PATCH v2 2/7] audio: Add spaces around operator/delete redundant spaces

On 1/15/21 2:24 AM, Zhang Han wrote:
> Fix problems about spaces:
> -operator needs spaces around it, add them.
> -somespaces are redundant, remove them.
> 
> Signed-off-by: Zhang Han 
> ---
>  audio/audio_template.h | 2 +-
>  audio/coreaudio.c  | 2 +-
>  audio/dsoundaudio.c| 2 +-
>  audio/jackaudio.c  | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH v2 5/7] audio: Don't use '%#' in format strings

On 1/15/21 2:24 AM, Zhang Han wrote:
> Use '0x' prefix instead of '%#'
> 
> Signed-off-by: Zhang Han 
> ---
>  audio/dsoundaudio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH v3 09/10] iotests/129: Clean up pylint and mypy complaints


On 14.01.21 21:02, Willian Rampazzo wrote:

On Thu, Jan 14, 2021 at 2:41 PM Max Reitz  wrote:


Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/129 | 4 ++--
  tests/qemu-iotests/297 | 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index 6d21470cd7..201d9e0a0b 100755
--- a/tests/qemu-iotests/129
+++ b/tests/qemu-iotests/129
@@ -20,7 +20,6 @@

  import os
  import iotests
-import time

  class TestStopWithBlockJob(iotests.QMPTestCase):
  test_img = os.path.join(iotests.test_dir, 'test.img')
@@ -32,7 +31,8 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
  iotests.qemu_img('create', '-f', iotests.imgfmt, self.base_img, "1G")
  iotests.qemu_img('create', '-f', iotests.imgfmt, self.test_img,
   "-b", self.base_img, '-F', iotests.imgfmt)
-iotests.qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 1M 128M', 
self.test_img)
+iotests.qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 1M 128M',
+self.test_img)
  self.vm = iotests.VM()
  self.vm.add_object('throttle-group,id=tg0,x-bps-total=1024')

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index bfa26d280b..1dce1d1b1c 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -27,7 +27,7 @@ import iotests
  # TODO: Empty this list!
  SKIP_FILES = (
  '030', '040', '041', '044', '045', '055', '056', '057', '065', '093',
-'096', '118', '124', '129', '132', '136', '139', '147', '148', '149',
+'096', '118', '124', '132', '136', '139', '147', '148', '149',


Is this also part of mypy/pylint cleanup? It seems you are doing more
than that here. It would be good to have this listed in the commit
message.


Sure, why not.  Something like “And consequentially drop it from 297's 
skip list.”?


Though I think making a test pass pylint+mypy complaints basically means 
exactly to remove it from 297's skip list and then making 297 pass, so 
I’m not entirely sure it’s necessary.  But it can’t hurt, so.



Despite that,

Reviewed-by: Willian Rampazzo 


Thanks!

Max


  '151', '152', '155', '163', '165', '169', '194', '196', '199', '202',
  '203', '205', '206', '207', '208', '210', '211', '212', '213', '216',
  '218', '219', '222', '224', '228', '234', '235', '236', '237', '238',
--
2.29.2

Re: USB Gen2 passthrough not working

  Hi,

>  usb 2-3: new SuperSpeedPlus Gen 2 USB device number 3 using xhci_hcd

> localhost login: [   72.763264] usb 1-4: new low-speed USB device number 3 
> using xhci_hcd

ilibusb reports LIBUSB_SPEED_SUPER_PLUS and qemu
doesn't handle it ...

Lets treat it like superspeed for now, does that help?

--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -186,6 +186,7 @@ static const char *speed_name[] = {
 [LIBUSB_SPEED_FULL]= "12",
 [LIBUSB_SPEED_HIGH]= "480",
 [LIBUSB_SPEED_SUPER]   = "5000",
+[LIBUSB_SPEED_SUPER_PLUS] = "5000+",
 };
 
 static const unsigned int speed_map[] = {
@@ -193,6 +194,7 @@ static const unsigned int speed_map[] = {
 [LIBUSB_SPEED_FULL]= USB_SPEED_FULL,
 [LIBUSB_SPEED_HIGH]= USB_SPEED_HIGH,
 [LIBUSB_SPEED_SUPER]   = USB_SPEED_SUPER,
+[LIBUSB_SPEED_SUPER_PLUS] = USB_SPEED_SUPER,
 };
 
 static const unsigned int status_map[] = {

Re: [PATCH] hw/block/nvme: fix for non-msix machines

On 1/15/21 8:07 AM, Klaus Jensen wrote:
> On Jan 12 14:20, Philippe Mathieu-Daudé wrote:
>> On 1/12/21 1:47 PM, Klaus Jensen wrote:
>>> From: Klaus Jensen 
>>>
>>> Commit 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar()
>>> return value") had the unintended effect of breaking support on
>>> several platforms not supporting MSI-X.
>>>
>>> Still check for errors, but only report that MSI-X is unsupported
>>> instead of bailing out.
>>>

BTW maybe worth adding:

Cc: qemu-sta...@nongnu.org

So this patch get backported in the next stable release based on 5.2.

>>> Reported-by: Guenter Roeck 
>>> Fixes: 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar() 
>>> return value")
>>> Fixes: fbf2e5375e33 ("hw/block/nvme: Verify msix_vector_use() returned 
>>> value")
>>> Signed-off-by: Klaus Jensen 
>>> ---
>>>  hw/block/nvme.c | 31 ++-
>>>  1 file changed, 22 insertions(+), 9 deletions(-)
>>
>> Reviewed-by: Philippe Mathieu-Daudé 
>>
> 
> Thanks! Applied to nvme-next.
>

Re: [PATCH v2] machine: add missing doc for memory-backend option

2021-01-15 Thread Michal Privoznik


On 1/15/21 12:46 AM, Igor Mammedov wrote:

Add documentation for '-machine memory-backend' CLI option and
how to use it.

And document that x-use-canonical-path-for-ramblock-id,
is considered to be stable to make sure it won't go away by accident.

Signed-off-by: Igor Mammedov 
---
v2:
  - add doc that x-use-canonical-path-for-ramblock-id is considered stable,
(Peter Krempa )
---
  backends/hostmem.c | 10 ++
  qemu-options.hx| 28 +++-
  2 files changed, 37 insertions(+), 1 deletion(-)





@@ -96,6 +97,31 @@ SRST
  ``hmat=on|off``
  Enables or disables ACPI Heterogeneous Memory Attribute Table
  (HMAT) support. The default is off.
+
+ ``memory-backend='id'``
+An alternative to legacy ``-mem-path`` and ``mem-prealloc`` options.
+Allows to use a memory backend as main RAM.
+
+For example:
+::
+-object 
memory-backend-file,id=pc.ram,size=512M,mem-path=/hugetlbfs,prealloc=on,share=on
+-machine memory-backend=pc.ram
+-m 512M
+
+Migration compatibility note:
+a) as backend id one shall use value of 'default-ram-id', advertised by
+machine type (available via ``query-machines`` QMP command)
+b) for machine types 4.0 and older, user shall
+use ``x-use-canonical-path-for-ramblock-id=on`` backend option
+(this option must be considered stable, as if it didn't have the 'x-'
+prefix including deprecation period, as long as 4.0 and older machine
+types exists),
+if migration to/from old QEMU (<5.0) is expected.
+For example:
+::
+-object 
memory-backend-ram,id=pc.ram,size=512M,x-use-canonical-path-for-ramblock-id=on
+-machine memory-backend=pc.ram
+-m 512M


Igor, this doesn't correspond with your comment in bugzilla:

https://bugzilla.redhat.com/show_bug.cgi?id=1836043#c31

In fact, we had to turn the attribute OFF so that canonical path is not 
used. Isn't ON the default state anyway?


Michal

Re: [PATCH] hw/block/nvme: fix for non-msix machines

2021-01-15 Thread Klaus Jensen

On Jan 15 10:37, Philippe Mathieu-Daudé wrote:
> On 1/15/21 8:07 AM, Klaus Jensen wrote:
> > On Jan 12 14:20, Philippe Mathieu-Daudé wrote:
> >> On 1/12/21 1:47 PM, Klaus Jensen wrote:
> >>> From: Klaus Jensen 
> >>>
> >>> Commit 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar()
> >>> return value") had the unintended effect of breaking support on
> >>> several platforms not supporting MSI-X.
> >>>
> >>> Still check for errors, but only report that MSI-X is unsupported
> >>> instead of bailing out.
> >>>
> 
> BTW maybe worth adding:
> 
> Cc: qemu-sta...@nongnu.org
> 
> So this patch get backported in the next stable release based on 5.2.
> 

Thanks Philippe,

I messed up the bounce, but I sent it on to stable now.


signature.asc
Description: PGP signature

Re: [PATCH v2] machine: add missing doc for memory-backend option

2021-01-15 Thread Peter Maydell

On Thu, 14 Jan 2021 at 23:48, Igor Mammedov  wrote:
>
> Add documentation for '-machine memory-backend' CLI option and
> how to use it.
>
> And document that x-use-canonical-path-for-ramblock-id,
> is considered to be stable to make sure it won't go away by accident.

That's not what the x- prefix is supposed to mean.
If we have an internal constraint that we mustn't delete
the option in order to support some other must-be-stable
interface (eg migration of some machines) we can document
that in a comment, but that doesn't mean that we should
document to users that direct use of an x-prefix option
is supported as a stable interface.

Alternatively, if the option is really stable for direct
use by users then we should commit to making it so by
removing the x-.

thanks
-- PMM

[PATCHv7 2/3] arm-virt: refactor gpios creation

No functional change. Just refactor code to better
support secure and normal world gpios.

Signed-off-by: Maxim Uvarov 
---
 hw/arm/virt.c | 67 ---
 1 file changed, 47 insertions(+), 20 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 96985917d3..26bb66e8e1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -820,17 +820,43 @@ static void virt_powerdown_req(Notifier *n, void *opaque)
 }
 }
 
-static void create_gpio(const VirtMachineState *vms)
+static void create_gpio_keys(const VirtMachineState *vms,
+ DeviceState *pl061_dev,
+ uint32_t phandle)
+{
+gpio_key_dev = sysbus_create_simple("gpio-key", -1,
+qdev_get_gpio_in(pl061_dev, 3));
+
+qemu_fdt_add_subnode(vms->fdt, "/gpio-keys");
+qemu_fdt_setprop_string(vms->fdt, "/gpio-keys", "compatible", "gpio-keys");
+qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#size-cells", 0);
+qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1);
+
+qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff");
+qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff",
+"label", "GPIO Key Poweroff");
+qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code",
+  KEY_POWER);
+qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
+   "gpios", phandle, 3, 0);
+}
+
+static void create_gpio_devices(const VirtMachineState *vms, int gpio,
+MemoryRegion *mem)
 {
 char *nodename;
 DeviceState *pl061_dev;
-hwaddr base = vms->memmap[VIRT_GPIO].base;
-hwaddr size = vms->memmap[VIRT_GPIO].size;
-int irq = vms->irqmap[VIRT_GPIO];
+hwaddr base = vms->memmap[gpio].base;
+hwaddr size = vms->memmap[gpio].size;
+int irq = vms->irqmap[gpio];
 const char compat[] = "arm,pl061\0arm,primecell";
+SysBusDevice *s;
 
-pl061_dev = sysbus_create_simple("pl061", base,
- qdev_get_gpio_in(vms->gic, irq));
+pl061_dev = qdev_new("pl061");
+s = SYS_BUS_DEVICE(pl061_dev);
+sysbus_realize_and_unref(s, &error_fatal);
+memory_region_add_subregion(mem, base, sysbus_mmio_get_region(s, 0));
+sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
 
 uint32_t phandle = qemu_fdt_alloc_phandle(vms->fdt);
 nodename = g_strdup_printf("/pl061@%" PRIx64, base);
@@ -847,21 +873,22 @@ static void create_gpio(const VirtMachineState *vms)
 qemu_fdt_setprop_string(vms->fdt, nodename, "clock-names", "apb_pclk");
 qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", phandle);
 
-gpio_key_dev = sysbus_create_simple("gpio-key", -1,
-qdev_get_gpio_in(pl061_dev, 3));
-qemu_fdt_add_subnode(vms->fdt, "/gpio-keys");
-qemu_fdt_setprop_string(vms->fdt, "/gpio-keys", "compatible", "gpio-keys");
-qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#size-cells", 0);
-qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1);
+if (gpio == VIRT_GPIO) {
+qemu_fdt_setprop_string(vms->fdt, "/chosen", "stdout-path", nodename);
+} else {
+/* Mark as not usable by the normal world */
+qemu_fdt_setprop_string(vms->fdt, nodename, "status", "disabled");
+qemu_fdt_setprop_string(vms->fdt, nodename, "secure-status", "okay");
 
-qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff");
-qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff",
-"label", "GPIO Key Poweroff");
-qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code",
-  KEY_POWER);
-qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
-   "gpios", phandle, 3, 0);
+qemu_fdt_setprop_string(vms->fdt, "/secure-chosen", "stdout-path",
+nodename);
+}
 g_free(nodename);
+
+/* Child gpio devices */
+if (gpio == VIRT_GPIO) {
+create_gpio_keys(vms, pl061_dev, phandle);
+}
 }
 
 static void create_virtio_devices(const VirtMachineState *vms)
@@ -1990,7 +2017,7 @@ static void machvirt_init(MachineState *machine)
 if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) {
 vms->acpi_dev = create_acpi_ged(vms);
 } else {
-create_gpio(vms);
+create_gpio_devices(vms, VIRT_GPIO, sysmem);
 }
 
  /* connect powerdown request */
-- 
2.17.1

[PATCHv7 0/3] arm-virt: add secure pl061 for reset/power down

v7: - same as v6, but resplit patches: patch 2 no function changes and refactor
gpio setup for virt platfrom and patch 3 adds secure gpio.
v6: - 64k align gpio memory region (Andrew Jones)
- adjusted memory region to map this address in the corresponding atf patch
v5: - removed vms flag, added fdt (Andrew Jones)
- added patch3 to combine secure and non secure pl061. It has to be
more easy to review if this changes are in the separate patch.
v4: rework patches accodring to Peter Maydells comments:
- split patches on gpio-pwr driver and arm-virt integration.
- start secure gpio only from virt-6.0.
- rework qemu interface for gpio-pwr to use 2 named gpio.
- put secure gpio to secure name space.
v3: added missed include qemu/log.h for qemu_log(..
v2: replace printf with qemu_log (Philippe Mathieu-Daudé)

This patch works together with ATF patch:

https://github.com/muvarov/arm-trusted-firmware/commit/7556d07e87f755c602cd9d90359341bdd14d9d57

Previus discussion for reboot issue was here:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg757705.html
Maxim Uvarov (3):
hw: gpio: implement gpio-pwr driver for qemu reset/poweroff
arm-virt: refactor gpios creation
arm-virt: add secure pl061 for reset/power down

--
2.17.1

[PATCHv7 1/3] hw: gpio: implement gpio-pwr driver for qemu reset/poweroff

Implement gpio-pwr driver to allow reboot and poweroff machine.
This is simple driver with just 2 gpios lines. Current use case
is to reboot and poweroff virt machine in secure mode. Secure
pl066 gpio chip is needed for that.

Signed-off-by: Maxim Uvarov 
Reviewed-by: Hao Wu 
---
 hw/gpio/Kconfig |  3 ++
 hw/gpio/gpio_pwr.c  | 70 +
 hw/gpio/meson.build |  1 +
 3 files changed, 74 insertions(+)
 create mode 100644 hw/gpio/gpio_pwr.c

diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
index b6fdaa2586..f0e7405f6e 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -8,5 +8,8 @@ config PL061
 config GPIO_KEY
 bool
 
+config GPIO_PWR
+bool
+
 config SIFIVE_GPIO
 bool
diff --git a/hw/gpio/gpio_pwr.c b/hw/gpio/gpio_pwr.c
new file mode 100644
index 00..8ed8d5d24f
--- /dev/null
+++ b/hw/gpio/gpio_pwr.c
@@ -0,0 +1,70 @@
+/*
+ * GPIO qemu power controller
+ *
+ * Copyright (c) 2020 Linaro Limited
+ *
+ * Author: Maxim Uvarov 
+ *
+ * Virtual gpio driver which can be used on top of pl061
+ * to reboot and shutdown qemu virtual machine. One of use
+ * case is gpio driver for secure world application (ARM
+ * Trusted Firmware.).
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+/*
+ * QEMU interface:
+ * two named input GPIO lines:
+ *   'reset' : when asserted, trigger system reset
+ *   'shutdown' : when asserted, trigger system shutdown
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "sysemu/runstate.h"
+
+#define TYPE_GPIOPWR "gpio-pwr"
+OBJECT_DECLARE_SIMPLE_TYPE(GPIO_PWR_State, GPIOPWR)
+
+struct GPIO_PWR_State {
+SysBusDevice parent_obj;
+};
+
+static void gpio_pwr_reset(void *opaque, int n, int level)
+{
+if (!level) {
+qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+}
+}
+
+static void gpio_pwr_shutdown(void *opaque, int n, int level)
+{
+if (!level) {
+qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+}
+}
+
+static void gpio_pwr_init(Object *obj)
+{
+DeviceState *dev = DEVICE(obj);
+
+qdev_init_gpio_in_named(dev, gpio_pwr_reset, "reset", 1);
+qdev_init_gpio_in_named(dev, gpio_pwr_shutdown, "shutdown", 1);
+}
+
+static const TypeInfo gpio_pwr_info = {
+.name  = TYPE_GPIOPWR,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(GPIO_PWR_State),
+.instance_init = gpio_pwr_init,
+};
+
+static void gpio_pwr_register_types(void)
+{
+type_register_static(&gpio_pwr_info);
+}
+
+type_init(gpio_pwr_register_types)
diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
index 5c0a7d7b95..79568f00ce 100644
--- a/hw/gpio/meson.build
+++ b/hw/gpio/meson.build
@@ -1,5 +1,6 @@
 softmmu_ss.add(when: 'CONFIG_E500', if_true: files('mpc8xxx.c'))
 softmmu_ss.add(when: 'CONFIG_GPIO_KEY', if_true: files('gpio_key.c'))
+softmmu_ss.add(when: 'CONFIG_GPIO_PWR', if_true: files('gpio_pwr.c'))
 softmmu_ss.add(when: 'CONFIG_MAX7310', if_true: files('max7310.c'))
 softmmu_ss.add(when: 'CONFIG_PL061', if_true: files('pl061.c'))
 softmmu_ss.add(when: 'CONFIG_PUV3', if_true: files('puv3_gpio.c'))
-- 
2.17.1

[PATCHv7 3/3] arm-virt: add secure pl061 for reset/power down

Add secure pl061 for reset/power down machine from
the secure world (Arm Trusted Firmware). Connect it
with gpio-pwr driver.

Signed-off-by: Maxim Uvarov 
---
 hw/arm/Kconfig|  1 +
 hw/arm/virt.c | 50 +++
 include/hw/arm/virt.h |  2 ++
 3 files changed, 53 insertions(+)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 0a242e4c5d..13cc42dcc8 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -17,6 +17,7 @@ config ARM_VIRT
 select PL011 # UART
 select PL031 # RTC
 select PL061 # GPIO
+select GPIO_PWR
 select PLATFORM_BUS
 select SMBIOS
 select VIRTIO_MMIO
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 26bb66e8e1..436ae894c9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -153,6 +153,7 @@ static const MemMapEntry base_memmap[] = {
 [VIRT_ACPI_GED] =   { 0x0908, ACPI_GED_EVT_SEL_LEN },
 [VIRT_NVDIMM_ACPI] ={ 0x0909, NVDIMM_ACPI_IO_LEN},
 [VIRT_PVTIME] = { 0x090a, 0x0001 },
+[VIRT_SECURE_GPIO] ={ 0x090b, 0x1000 },
 [VIRT_MMIO] =   { 0x0a00, 0x0200 },
 /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
 [VIRT_PLATFORM_BUS] =   { 0x0c00, 0x0200 },
@@ -841,6 +842,46 @@ static void create_gpio_keys(const VirtMachineState *vms,
"gpios", phandle, 3, 0);
 }
 
+#define ATF_GPIO_POWEROFF 3
+#define ATF_GPIO_REBOOT   4
+
+static void create_gpio_pwr(const VirtMachineState *vms,
+DeviceState *pl061_dev,
+uint32_t phandle)
+{
+DeviceState *gpio_pwr_dev;
+
+/* gpio-pwr */
+gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL);
+
+/* connect secure pl061 to gpio-pwr */
+qdev_connect_gpio_out(pl061_dev, ATF_GPIO_POWEROFF,
+  qdev_get_gpio_in_named(gpio_pwr_dev, "reset", 0));
+qdev_connect_gpio_out(pl061_dev, ATF_GPIO_REBOOT,
+  qdev_get_gpio_in_named(gpio_pwr_dev, "shutdown", 0));
+
+qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr");
+qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr", "compatible", "gpio-pwr");
+qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#size-cells", 0);
+qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#address-cells", 1);
+
+qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/poweroff");
+qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/poweroff",
+"label", "GPIO PWR Poweroff");
+qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/poweroff", "code",
+  ATF_GPIO_POWEROFF);
+qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/poweroff",
+   "gpios", phandle, 3, 0);
+
+qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/reboot");
+qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/reboot",
+"label", "GPIO PWR Reboot");
+qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/reboot", "code",
+  ATF_GPIO_REBOOT);
+qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/reboot",
+   "gpios", phandle, 3, 0);
+}
+
 static void create_gpio_devices(const VirtMachineState *vms, int gpio,
 MemoryRegion *mem)
 {
@@ -888,6 +929,8 @@ static void create_gpio_devices(const VirtMachineState 
*vms, int gpio,
 /* Child gpio devices */
 if (gpio == VIRT_GPIO) {
 create_gpio_keys(vms, pl061_dev, phandle);
+} else {
+create_gpio_pwr(vms, pl061_dev, phandle);
 }
 }
 
@@ -2020,6 +2063,10 @@ static void machvirt_init(MachineState *machine)
 create_gpio_devices(vms, VIRT_GPIO, sysmem);
 }
 
+if (vms->secure && !vmc->no_secure_gpio) {
+create_gpio_devices(vms, VIRT_SECURE_GPIO, secure_sysmem);
+}
+
  /* connect powerdown request */
  vms->powerdown_notifier.notify = virt_powerdown_req;
  qemu_register_powerdown_notifier(&vms->powerdown_notifier);
@@ -2635,8 +2682,11 @@ DEFINE_VIRT_MACHINE_AS_LATEST(6, 0)
 
 static void virt_machine_5_2_options(MachineClass *mc)
 {
+VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
 virt_machine_6_0_options(mc);
 compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len);
+vmc->no_secure_gpio = true;
 }
 DEFINE_VIRT_MACHINE(5, 2)
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index abf54fab49..6f6c85ffcf 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -81,6 +81,7 @@ enum {
 VIRT_GPIO,
 VIRT_SECURE_UART,
 VIRT_SECURE_MEM,
+VIRT_SECURE_GPIO,
 VIRT_PCDIMM_ACPI,
 VIRT_ACPI_GED,
 VIRT_NVDIMM_ACPI,
@@ -127,6 +128,7 @@ struct VirtMachineClass {
 bool kvm_no_adjvtime;
 bool no_kvm_steal_time;
 bool acpi_expose_flash;
+bool no_secure_gpio;
 };
 
 struct VirtMachineState {
-- 
2.17.1

Re: [PATCH] hw/block/nvme: fix for non-msix machines

On 1/15/21 10:46 AM, Klaus Jensen wrote:
> On Jan 15 10:37, Philippe Mathieu-Daudé wrote:
>> On 1/15/21 8:07 AM, Klaus Jensen wrote:
>>> On Jan 12 14:20, Philippe Mathieu-Daudé wrote:
 On 1/12/21 1:47 PM, Klaus Jensen wrote:
> From: Klaus Jensen 
>
> Commit 1c0c2163aa08 ("hw/block/nvme: verify msix_init_exclusive_bar()
> return value") had the unintended effect of breaking support on
> several platforms not supporting MSI-X.
>
> Still check for errors, but only report that MSI-X is unsupported
> instead of bailing out.
>
>>
>> BTW maybe worth adding:
>>
>> Cc: qemu-sta...@nongnu.org
>>
>> So this patch get backported in the next stable release based on 5.2.
>>
> 
> Thanks Philippe,
> 
> I messed up the bounce, but I sent it on to stable now.

I meant in the patch description itself before the other tags
(qemu-stable scan for commits with the "Cc: qemu-sta...@nongnu.org"
tag).

Re: [PATCH 0/2] net/eth: Fix stack-buffer-overflow in _eth_get_rss_ex_dst_addr()

2021-01-15 Thread Miroslav Rezanina

On Thu, Jan 14, 2021 at 03:50:39PM +0100, Philippe Mathieu-DaudÃ© wrote:
> I had a look at the patch from Miroslav trying to silence a
> compiler warning which in fact is a nasty bug. Here is a fix.
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg772735.html
> 
> Philippe Mathieu-DaudÃ© (2):
>   net/eth: Simplify _eth_get_rss_ex_dst_addr()
>   net/eth: Fix stack-buffer-overflow in _eth_get_rss_ex_dst_addr()
> 
>  net/eth.c | 37 +
>  1 file changed, 17 insertions(+), 20 deletions(-)
> 
> -- 
> 2.26.2
> 
> 
> 

With qtest chunk included:

Reviewed-by: Miroslav Rezanina

[PULL 01/11] ui/gtk: don't try to redefine SI prefixes

From: Volker Rümelin 

Redefining SI prefixes is always wrong. 1s has per definition
1000ms. Remove the misnamed named constant and replace it with
a comment explaining the frequency to period conversion in two
simple steps. Now you can cancel out the unit mHz in the comment
with the implicit unit mHz in refresh_rate_millihz and see why
the implicit unit ms for update_interval remains.

Signed-off-by: Volker Rümelin 
Message-Id: <20201213165724.13418-1-vr_q...@t-online.de>
Signed-off-by: Gerd Hoffmann 
---
 include/ui/gtk.h | 2 --
 ui/gtk.c | 3 ++-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index eaeb450f913e..80851fb4c7e1 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -24,8 +24,6 @@
 #include "ui/egl-context.h"
 #endif
 
-#define MILLISEC_PER_SEC 100
-
 typedef struct GtkDisplayState GtkDisplayState;
 
 typedef struct VirtualGfxConsole {
diff --git a/ui/gtk.c b/ui/gtk.c
index e8474456df88..a83c8c3785e1 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -798,7 +798,8 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t 
*cr, void *opaque)
 refresh_rate_millihz = gd_refresh_rate_millihz(vc->window ?
vc->window : s->window);
 if (refresh_rate_millihz) {
-vc->gfx.dcl.update_interval = MILLISEC_PER_SEC / refresh_rate_millihz;
+/* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */
+vc->gfx.dcl.update_interval = 1000 * 1000 / refresh_rate_millihz;
 }
 
 fbw = surface_width(vc->gfx.ds);
-- 
2.29.2

[PULL 10/11] vnc: move initialization to framebuffer_update_request

qemu sends various state info like current cursor shape to newly connected
clients in response to a set_encoding message.  This is not correct according
to the rfb spec.  Send that information in response to a full (incremental=0)
framebuffer update request instead.  Also send the resize information
unconditionally, not only in case of an actual server-side change.

This makes the qemu vnc server conform to the spec and allows clients to
request the complete vnc server state without reconnect.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Daniel P. Berrangé 
Message-id: 20210112134120.2031837-3-kra...@redhat.com
---
 ui/vnc.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 0f01481cac57..b4e98cf647f5 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -661,10 +661,6 @@ static void vnc_desktop_resize(VncState *vs)
 if (vs->ioc == NULL || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
 return;
 }
-if (vs->client_width == pixman_image_get_width(vs->vd->server) &&
-vs->client_height == pixman_image_get_height(vs->vd->server)) {
-return;
-}
 
 assert(pixman_image_get_width(vs->vd->server) < 65536 &&
pixman_image_get_width(vs->vd->server) >= 0);
@@ -2014,6 +2010,10 @@ static void framebuffer_update_request(VncState *vs, int 
incremental,
 } else {
 vs->update = VNC_STATE_UPDATE_FORCE;
 vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h);
+vnc_colordepth(vs);
+vnc_desktop_resize(vs);
+vnc_led_state_change(vs);
+vnc_cursor_define(vs);
 }
 }
 
@@ -2154,10 +2154,7 @@ static void set_encodings(VncState *vs, int32_t 
*encodings, size_t n_encodings)
 break;
 }
 }
-vnc_desktop_resize(vs);
 check_pointer_type_change(&vs->mouse_mode_notifier, NULL);
-vnc_led_state_change(vs);
-vnc_cursor_define(vs);
 }
 
 static void set_pixel_conversion(VncState *vs)
-- 
2.29.2

[PULL 02/11] ui/gtk: rename variable window to widget

From: Volker Rümelin 

The type of the variable window is GtkWidget. Rename the variable
from window to widget, because windows and widgets are different
things.

Signed-off-by: Volker Rümelin 
Message-Id: <20201213165724.13418-2-vr_q...@t-online.de>
Signed-off-by: Gerd Hoffmann 
---
 ui/gtk.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index a83c8c3785e1..439c1e949fd9 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -752,13 +752,13 @@ static void gd_resize_event(GtkGLArea *area,
  * If available, return the refresh rate of the display in milli-Hertz,
  * else return 0.
  */
-static int gd_refresh_rate_millihz(GtkWidget *window)
+static int gd_refresh_rate_millihz(GtkWidget *widget)
 {
 #ifdef GDK_VERSION_3_22
-GdkWindow *win = gtk_widget_get_window(window);
+GdkWindow *win = gtk_widget_get_window(widget);
 
 if (win) {
-GdkDisplay *dpy = gtk_widget_get_display(window);
+GdkDisplay *dpy = gtk_widget_get_display(widget);
 GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
 
 return gdk_monitor_get_refresh_rate(monitor);
-- 
2.29.2

[PULL 07/11] ui: add support for remote power control to VNC server

From: Daniel P. Berrangé 

The "XVP" (Xen VNC Proxy) extension defines a mechanism for a VNC client
to issue power control requests to trigger graceful shutdown, reboot, or
hard reset.

This option is not enabled by default, since we cannot assume that users
with VNC access implicitly have administrator access to the guest OS.

Thus is it enabled with a boolean "power-control" option e.g.

   -vnc :1,power-control=on

While, QEMU can easily support shutdown and reset, there's no easy way
to wire up reboot support at this time. In theory it could be done by
issuing a shutdown, followed by a reset, but there's no convenient
wiring for such a pairing in QEMU. It also isn't possible to have the
VNC server directly talk to QEMU guest agent, since the agent chardev is
typically owned by an external mgmt app.

Signed-off-by: Daniel P. Berrangé 

[ kraxel: rebase to master  ]
[ kraxel: add missing break ]

Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.h| 13 +++
 ui/vnc.c| 59 +
 qemu-options.hx |  4 
 3 files changed, 76 insertions(+)

diff --git a/ui/vnc.h b/ui/vnc.h
index c8d3ad9ec496..5feeef9df08c 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -176,6 +176,7 @@ struct VncDisplay
 int ws_subauth; /* Used by websockets */
 bool lossy;
 bool non_adaptive;
+bool power_control;
 QCryptoTLSCreds *tlscreds;
 QAuthZ *tlsauthz;
 char *tlsauthzid;
@@ -412,6 +413,7 @@ enum {
 #define VNC_ENCODING_TIGHT_PNG0xFEFC /* -260 */
 #define VNC_ENCODING_LED_STATE0XFEFB /* -261 */
 #define VNC_ENCODING_DESKTOP_RESIZE_EXT   0XFECC /* -308 */
+#define VNC_ENCODING_XVP  0XFECB /* -309 */
 #define VNC_ENCODING_ALPHA_CURSOR 0XFEC6 /* -314 */
 #define VNC_ENCODING_WMVi 0x574D5669
 
@@ -453,6 +455,7 @@ enum VncFeatures {
 VNC_FEATURE_ZRLE,
 VNC_FEATURE_ZYWRLE,
 VNC_FEATURE_LED_STATE,
+VNC_FEATURE_XVP,
 };
 
 #define VNC_FEATURE_RESIZE_MASK  (1 << VNC_FEATURE_RESIZE)
@@ -467,6 +470,7 @@ enum VncFeatures {
 #define VNC_FEATURE_ZRLE_MASK(1 << VNC_FEATURE_ZRLE)
 #define VNC_FEATURE_ZYWRLE_MASK  (1 << VNC_FEATURE_ZYWRLE)
 #define VNC_FEATURE_LED_STATE_MASK   (1 << VNC_FEATURE_LED_STATE)
+#define VNC_FEATURE_XVP_MASK (1 << VNC_FEATURE_XVP)
 
 
 /* Client -> Server message IDs */
@@ -519,6 +523,15 @@ enum VncFeatures {
 #define VNC_MSG_SERVER_QEMU_AUDIO_BEGIN   1
 #define VNC_MSG_SERVER_QEMU_AUDIO_DATA2
 
+/* XVP server -> client status code */
+#define VNC_XVP_CODE_FAIL 0
+#define VNC_XVP_CODE_INIT 1
+
+/* XVP client -> server action request  */
+#define VNC_XVP_ACTION_SHUTDOWN 2
+#define VNC_XVP_ACTION_REBOOT 3
+#define VNC_XVP_ACTION_RESET 4
+
 
 /*
  *
diff --git a/ui/vnc.c b/ui/vnc.c
index 69e92b1ef361..a0bf750767a2 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -30,6 +30,7 @@
 #include "trace.h"
 #include "hw/qdev-core.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/runstate.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
@@ -2042,6 +2043,17 @@ static void send_ext_audio_ack(VncState *vs)
 vnc_flush(vs);
 }
 
+static void send_xvp_message(VncState *vs, int code)
+{
+vnc_lock_output(vs);
+vnc_write_u8(vs, VNC_MSG_SERVER_XVP);
+vnc_write_u8(vs, 0); /* pad */
+vnc_write_u8(vs, 1); /* version */
+vnc_write_u8(vs, code);
+vnc_unlock_output(vs);
+vnc_flush(vs);
+}
+
 static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
 {
 int i;
@@ -2121,6 +2133,12 @@ static void set_encodings(VncState *vs, int32_t 
*encodings, size_t n_encodings)
 case VNC_ENCODING_LED_STATE:
 vs->features |= VNC_FEATURE_LED_STATE_MASK;
 break;
+case VNC_ENCODING_XVP:
+if (vs->vd->power_control) {
+vs->features |= VNC_FEATURE_XVP;
+send_xvp_message(vs, VNC_XVP_CODE_INIT);
+}
+break;
 case VNC_ENCODING_COMPRESSLEVEL0 ... VNC_ENCODING_COMPRESSLEVEL0 + 9:
 vs->tight->compression = (enc & 0x0F);
 break;
@@ -2353,6 +2371,42 @@ static int protocol_client_msg(VncState *vs, uint8_t 
*data, size_t len)
 
 client_cut_text(vs, read_u32(data, 4), data + 8);
 break;
+case VNC_MSG_CLIENT_XVP:
+if (!(vs->features & VNC_FEATURE_XVP)) {
+error_report("vnc: xvp client message while disabled");
+vnc_client_error(vs);
+break;
+}
+if (len == 1) {
+return 4;
+}
+if (len == 4) {
+uint8_t version = read_u8(data, 2);
+uint8_t action = read_u8(data, 3);
+
+if (version != 1) {
+error_report("vnc: xvp client message version %d != 1",
+ versi

[PULL 00/11] Ui 20210115 patches

The following changes since commit 45240eed4f064576d589ea60ebadf3c11d7ab891:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-yank-2021-01-13' int=
o staging (2021-01-13 14:19:24 +)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/ui-20210115-pull-request

for you to fetch changes up to 763deea7e906321f8ba048c359f168f60d51c14e:

  vnc: add support for extended desktop resize (2021-01-15 11:22:43 +0100)


ui/gtk: refresh rate fixes.
ui/vnc: add support for desktop resize and power contol.
ui/vnc: misc bugfixes.



Alex Chen (1):
  vnc: Fix a memleak in vnc_display_connect()

Daniel P. Berrang=C3=A9 (1):
  ui: add support for remote power control to VNC server

Gerd Hoffmann (3):
  vnc: move check into vnc_cursor_define
  vnc: move initialization to framebuffer_update_request
  vnc: add support for extended desktop resize

Nikola Pavlica (2):
  ui/gtk: expose gd_monitor_update_interval
  ui/gtk: update monitor interval on egl displays

Volker R=C3=BCmelin (3):
  ui/gtk: don't try to redefine SI prefixes
  ui/gtk: rename variable window to widget
  ui/gtk: limit virtual console max update interval

Zihao Chang (1):
  vnc: fix unfinalized tlscreds for VncDisplay

 include/ui/gtk.h |   3 +-
 ui/vnc.h |  15 +
 ui/gtk-egl.c |   3 +
 ui/gtk.c |  25 
 ui/vnc.c | 147 ++-
 qemu-options.hx  |   4 ++
 6 files changed, 169 insertions(+), 28 deletions(-)

--=20
2.29.2

[PULL 03/11] ui/gtk: limit virtual console max update interval

From: Volker Rümelin 

Limit the virtual console maximum update interval to
GUI_REFRESH_INTERVAL_DEFAULT. This papers over a integer
overflow bug in gtk3 on Windows where the reported monitor
refresh frequency can be much smaller than the real refresh
frequency.

The gtk bug report can be found here:
https://gitlab.gnome.org/GNOME/gtk/-/issues/3394

On my Windows 10 system gtk reports a monitor refresh rate of
1.511Hz instead of 60.031Hz and slows down the screen update
rate in qemu to a crawl. Provided you are affected by the gtk
bug on Windows, these are the steps to reproduce the issue:

Start qemu with -display gtk and activate all qemu virtual
consoles and notice the reduced qemu refresh rate. Activating
all virtual consoles is necessary, because gui_update() in
ui/console.c uses the minimum of all display change listeners
update interval and not yet activated virtual consoles report
the default update interval (30ms).

Signed-off-by: Volker Rümelin 
Message-Id: <20201213165724.13418-3-vr_q...@t-online.de>
Signed-off-by: Gerd Hoffmann 
---
 ui/gtk.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 439c1e949fd9..d2004a4dc162 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -749,10 +749,10 @@ static void gd_resize_event(GtkGLArea *area,
 #endif
 
 /*
- * If available, return the refresh rate of the display in milli-Hertz,
- * else return 0.
+ * If available, return the update interval of the monitor in ms,
+ * else return 0 (the default update interval).
  */
-static int gd_refresh_rate_millihz(GtkWidget *widget)
+static int gd_monitor_update_interval(GtkWidget *widget)
 {
 #ifdef GDK_VERSION_3_22
 GdkWindow *win = gtk_widget_get_window(widget);
@@ -760,8 +760,13 @@ static int gd_refresh_rate_millihz(GtkWidget *widget)
 if (win) {
 GdkDisplay *dpy = gtk_widget_get_display(widget);
 GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
+int refresh_rate = gdk_monitor_get_refresh_rate(monitor); /* [mHz] */
 
-return gdk_monitor_get_refresh_rate(monitor);
+if (refresh_rate) {
+/* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */
+return MIN(1000 * 1000 / refresh_rate,
+   GUI_REFRESH_INTERVAL_DEFAULT);
+}
 }
 #endif
 return 0;
@@ -774,7 +779,6 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t 
*cr, void *opaque)
 int mx, my;
 int ww, wh;
 int fbw, fbh;
-int refresh_rate_millihz;
 
 #if defined(CONFIG_OPENGL)
 if (vc->gfx.gls) {
@@ -795,12 +799,8 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t 
*cr, void *opaque)
 return FALSE;
 }
 
-refresh_rate_millihz = gd_refresh_rate_millihz(vc->window ?
-   vc->window : s->window);
-if (refresh_rate_millihz) {
-/* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */
-vc->gfx.dcl.update_interval = 1000 * 1000 / refresh_rate_millihz;
-}
+vc->gfx.dcl.update_interval =
+gd_monitor_update_interval(vc->window ? vc->window : s->window);
 
 fbw = surface_width(vc->gfx.ds);
 fbh = surface_height(vc->gfx.ds);
-- 
2.29.2

[PULL 11/11] vnc: add support for extended desktop resize

The extended desktop resize encoding adds support for (a) clients
sending resize requests to the server, and (b) multihead support.

This patch implements (a).  All resize requests are rejected by qemu.
Qemu can't resize the framebuffer on its own, this is in the hands of
the guest, so all qemu can do is forward the request to the guest.
Should the guest actually resize the framebuffer we can notify the vnc
client later with a separate message.

This requires support in the display device.  Works with virtio-gpu.

https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#extendeddesktopsize-pseudo-encoding

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Daniel P. Berrangé 
Message-id: 20210112134120.2031837-4-kra...@redhat.com
---
 ui/vnc.h |  2 ++
 ui/vnc.c | 64 +++-
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/ui/vnc.h b/ui/vnc.h
index 5feeef9df08c..116463d5f099 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -444,6 +444,7 @@ enum {
 
 enum VncFeatures {
 VNC_FEATURE_RESIZE,
+VNC_FEATURE_RESIZE_EXT,
 VNC_FEATURE_HEXTILE,
 VNC_FEATURE_POINTER_TYPE_CHANGE,
 VNC_FEATURE_WMVI,
@@ -459,6 +460,7 @@ enum VncFeatures {
 };
 
 #define VNC_FEATURE_RESIZE_MASK  (1 << VNC_FEATURE_RESIZE)
+#define VNC_FEATURE_RESIZE_EXT_MASK  (1 << VNC_FEATURE_RESIZE_EXT)
 #define VNC_FEATURE_HEXTILE_MASK (1 << VNC_FEATURE_HEXTILE)
 #define VNC_FEATURE_POINTER_TYPE_CHANGE_MASK (1 << 
VNC_FEATURE_POINTER_TYPE_CHANGE)
 #define VNC_FEATURE_WMVI_MASK(1 << VNC_FEATURE_WMVI)
diff --git a/ui/vnc.c b/ui/vnc.c
index b4e98cf647f5..d429bfee5a65 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -655,10 +655,35 @@ void vnc_framebuffer_update(VncState *vs, int x, int y, 
int w, int h,
 vnc_write_s32(vs, encoding);
 }
 
+static void vnc_desktop_resize_ext(VncState *vs, int reject_reason)
+{
+vnc_lock_output(vs);
+vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
+vnc_write_u8(vs, 0);
+vnc_write_u16(vs, 1); /* number of rects */
+vnc_framebuffer_update(vs,
+   reject_reason ? 1 : 0,
+   reject_reason,
+   vs->client_width, vs->client_height,
+   VNC_ENCODING_DESKTOP_RESIZE_EXT);
+vnc_write_u8(vs, 1);  /* number of screens */
+vnc_write_u8(vs, 0);  /* padding */
+vnc_write_u8(vs, 0);  /* padding */
+vnc_write_u8(vs, 0);  /* padding */
+vnc_write_u32(vs, 0); /* screen id */
+vnc_write_u16(vs, 0); /* screen x-pos */
+vnc_write_u16(vs, 0); /* screen y-pos */
+vnc_write_u16(vs, vs->client_width);
+vnc_write_u16(vs, vs->client_height);
+vnc_write_u32(vs, 0); /* screen flags */
+vnc_unlock_output(vs);
+vnc_flush(vs);
+}
 
 static void vnc_desktop_resize(VncState *vs)
 {
-if (vs->ioc == NULL || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
+if (vs->ioc == NULL || (!vnc_has_feature(vs, VNC_FEATURE_RESIZE) &&
+!vnc_has_feature(vs, VNC_FEATURE_RESIZE_EXT))) {
 return;
 }
 
@@ -668,6 +693,12 @@ static void vnc_desktop_resize(VncState *vs)
pixman_image_get_height(vs->vd->server) >= 0);
 vs->client_width = pixman_image_get_width(vs->vd->server);
 vs->client_height = pixman_image_get_height(vs->vd->server);
+
+if (vnc_has_feature(vs, VNC_FEATURE_RESIZE_EXT)) {
+vnc_desktop_resize_ext(vs, 0);
+return;
+}
+
 vnc_lock_output(vs);
 vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
 vnc_write_u8(vs, 0);
@@ -2114,6 +2145,9 @@ static void set_encodings(VncState *vs, int32_t 
*encodings, size_t n_encodings)
 case VNC_ENCODING_DESKTOPRESIZE:
 vs->features |= VNC_FEATURE_RESIZE_MASK;
 break;
+case VNC_ENCODING_DESKTOP_RESIZE_EXT:
+vs->features |= VNC_FEATURE_RESIZE_EXT_MASK;
+break;
 case VNC_ENCODING_POINTER_TYPE_CHANGE:
 vs->features |= VNC_FEATURE_POINTER_TYPE_CHANGE_MASK;
 break;
@@ -2474,6 +2508,34 @@ static int protocol_client_msg(VncState *vs, uint8_t 
*data, size_t len)
 break;
 }
 break;
+case VNC_MSG_CLIENT_SET_DESKTOP_SIZE:
+{
+size_t size;
+uint8_t screens;
+
+if (len < 8) {
+return 8;
+}
+
+screens = read_u8(data, 6);
+size= 8 + screens * 16;
+if (len < size) {
+return size;
+}
+
+if (dpy_ui_info_supported(vs->vd->dcl.con)) {
+QemuUIInfo info;
+memset(&info, 0, sizeof(info));
+info.width = read_u16(data, 2);
+info.height = read_u16(data, 4);
+dpy_set_ui_info(vs->vd->dcl.con, &info);
+vnc_desktop_resize_ext(vs, 4 /* Request forwarded */);
+} else {
+vnc_desktop_resize_ext(vs, 3 /* Invalid screen layout */);
+}
+
+break;
+}
 default

Re: [PATCH v4 04/11] vfio: Support for RamDiscardMgr in the !vIOMMU case

2021-01-15 Thread David Hildenbrand

On 14.01.21 16:57, David Hildenbrand wrote:
> On 14.01.21 16:54, David Hildenbrand wrote:
>> On 14.01.21 00:27, Alex Williamson wrote:
>>> On Thu,  7 Jan 2021 14:34:16 +0100
>>> David Hildenbrand  wrote:
>>>
 Implement support for RamDiscardMgr, to prepare for virtio-mem
 support. Instead of mapping the whole memory section, we only map
 "populated" parts and update the mapping when notified about
 discarding/population of memory via the RamDiscardListener. Similarly, when
 syncing the dirty bitmaps, sync only the actually mapped (populated) parts
 by replaying via the notifier.

 Using virtio-mem with vfio is still blocked via
 ram_block_discard_disable()/ram_block_discard_require() after this patch.

 Cc: Paolo Bonzini 
 Cc: "Michael S. Tsirkin" 
 Cc: Alex Williamson 
 Cc: Dr. David Alan Gilbert 
 Cc: Igor Mammedov 
 Cc: Pankaj Gupta 
 Cc: Peter Xu 
 Cc: Auger Eric 
 Cc: Wei Yang 
 Cc: teawater 
 Cc: Marek Kedzierski 
 Signed-off-by: David Hildenbrand 
 ---
  hw/vfio/common.c  | 200 ++
  include/hw/vfio/vfio-common.h |  12 ++
  2 files changed, 212 insertions(+)

 diff --git a/hw/vfio/common.c b/hw/vfio/common.c
 index 6ff1daa763..2bd219cf1d 100644
 --- a/hw/vfio/common.c
 +++ b/hw/vfio/common.c
 @@ -654,6 +654,136 @@ out:
  rcu_read_unlock();
  }
  
 +static void vfio_ram_discard_notify_discard(RamDiscardListener *rdl,
 +const MemoryRegion *mr,
 +ram_addr_t offset, ram_addr_t 
 size)
 +{
 +VFIORamDiscardListener *vrdl = container_of(rdl, 
 VFIORamDiscardListener,
 +listener);
 +const hwaddr mr_start = MAX(offset, vrdl->offset_within_region);
 +const hwaddr mr_end = MIN(offset + size,
 +  vrdl->offset_within_region + vrdl->size);
 +const hwaddr iova = mr_start - vrdl->offset_within_region +
 +vrdl->offset_within_address_space;
 +int ret;
 +
 +if (mr_start >= mr_end) {
 +return;
 +}
 +
 +/* Unmap with a single call. */
 +ret = vfio_dma_unmap(vrdl->container, iova, mr_end - mr_start, NULL);
 +if (ret) {
 +error_report("%s: vfio_dma_unmap() failed: %s", __func__,
 + strerror(-ret));
 +}
 +}
 +
 +static int vfio_ram_discard_notify_populate(RamDiscardListener *rdl,
 +const MemoryRegion *mr,
 +ram_addr_t offset, ram_addr_t 
 size)
 +{
 +VFIORamDiscardListener *vrdl = container_of(rdl, 
 VFIORamDiscardListener,
 +listener);
 +const hwaddr mr_end = MIN(offset + size,
 +  vrdl->offset_within_region + vrdl->size);
 +hwaddr mr_start = MAX(offset, vrdl->offset_within_region);
 +hwaddr mr_next, iova;
 +void *vaddr;
 +int ret;
 +
 +/*
 + * Map in (aligned within memory region) minimum granularity, so we 
 can
 + * unmap in minimum granularity later.
 + */
 +for (; mr_start < mr_end; mr_start = mr_next) {
 +mr_next = ROUND_UP(mr_start + 1, vrdl->granularity);
 +mr_next = MIN(mr_next, mr_end);
 +
 +iova = mr_start - vrdl->offset_within_region +
 +   vrdl->offset_within_address_space;
 +vaddr = memory_region_get_ram_ptr(vrdl->mr) + mr_start;
 +
 +ret = vfio_dma_map(vrdl->container, iova, mr_next - mr_start,
 +   vaddr, mr->readonly);
 +if (ret) {
 +/* Rollback */
 +vfio_ram_discard_notify_discard(rdl, mr, offset, size);
 +return ret;
 +}
 +}
 +return 0;
 +}
 +
 +static void vfio_ram_discard_notify_discard_all(RamDiscardListener *rdl,
 +const MemoryRegion *mr)
 +{
 +VFIORamDiscardListener *vrdl = container_of(rdl, 
 VFIORamDiscardListener,
 +listener);
 +int ret;
 +
 +/* Unmap with a single call. */
 +ret = vfio_dma_unmap(vrdl->container, 
 vrdl->offset_within_address_space,
 + vrdl->size, NULL);
 +if (ret) {
 +error_report("%s: vfio_dma_unmap() failed: %s", __func__,
 + strerror(-ret));
 +}
 +}
 +
 +static void vfio_register_ram_discard_notifier(VFIOContainer *container,
 +   MemoryRegio

[PULL 04/11] ui/gtk: expose gd_monitor_update_interval

From: Nikola Pavlica 

The gd_egl_refresh function, as the name suggests, is responsible for
refreshing displays when using EGL graphics with QEMU's GTK UI. This is
a perfect candidate for a function to update the refresh rate in.

Since gd_monitor_update_interval is inaccessible from the gd_egl_refresh
function, we need to expose/globalize it in the include/ui/gtk.h file.

Signed-off-by: Nikola Pavlica 
Message-Id: <20210114140153.301473-2-pavlica.nik...@gmail.com>
Signed-off-by: Gerd Hoffmann 
---
 include/ui/gtk.h | 1 +
 ui/gtk.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 80851fb4c7e1..3f395d7f943b 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -86,6 +86,7 @@ extern bool gtk_use_gl_area;
 
 /* ui/gtk.c */
 void gd_update_windowsize(VirtualConsole *vc);
+int gd_monitor_update_interval(GtkWidget *widget);
 
 /* ui/gtk-egl.c */
 void gd_egl_init(VirtualConsole *vc);
diff --git a/ui/gtk.c b/ui/gtk.c
index d2004a4dc162..26665cd2e657 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -752,7 +752,7 @@ static void gd_resize_event(GtkGLArea *area,
  * If available, return the update interval of the monitor in ms,
  * else return 0 (the default update interval).
  */
-static int gd_monitor_update_interval(GtkWidget *widget)
+int gd_monitor_update_interval(GtkWidget *widget)
 {
 #ifdef GDK_VERSION_3_22
 GdkWindow *win = gtk_widget_get_window(widget);
-- 
2.29.2

[PULL 06/11] vnc: fix unfinalized tlscreds for VncDisplay

From: Zihao Chang 

In vnc_display_open(), if tls-creds is enabled, do object_ref(object
ref 1->2) for tls-creds. While in vnc_display_close(), object_unparent
sets object ref to 1(2->1) and  unparent the object for root.
Problem:
1. the object can not be found from the objects_root, while the object
is not finalized.
2. the qemu_opts of tls-creds(id: creds0) is not deleted, so new tls
object with the same id(creds0) can not be delete & add.

Signed-off-by: Zihao Chang 
Reviewed-by: Daniel P. Berrangé 
Message-Id: <2021031911.805-1-changzih...@huawei.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 7452ac7df2ce..69e92b1ef361 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3234,7 +3234,7 @@ static void vnc_display_close(VncDisplay *vd)
 vd->auth = VNC_AUTH_INVALID;
 vd->subauth = VNC_AUTH_INVALID;
 if (vd->tlscreds) {
-object_unparent(OBJECT(vd->tlscreds));
+object_unref(OBJECT(vd->tlscreds));
 vd->tlscreds = NULL;
 }
 if (vd->tlsauthz) {
-- 
2.29.2

[PULL 08/11] vnc: Fix a memleak in vnc_display_connect()

From: Alex Chen 

Free the 'sioc' when the qio_channel_socket_connect_sync() fails.

Reported-by: Euler Robot 
Signed-off-by: Alex Chen 
Reviewed-by: Li Qiang 
Reviewed-by: Laurent Vivier 
Message-Id: <20201126065702.35095-1-alex.c...@huawei.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/vnc.c b/ui/vnc.c
index a0bf750767a2..d4854d351bac 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3805,6 +3805,7 @@ static int vnc_display_connect(VncDisplay *vd,
 sioc = qio_channel_socket_new();
 qio_channel_set_name(QIO_CHANNEL(sioc), "vnc-reverse");
 if (qio_channel_socket_connect_sync(sioc, saddr[0], errp) < 0) {
+object_unref(OBJECT(sioc));
 return -1;
 }
 vnc_connect(vd, sioc, false, false);
-- 
2.29.2

[PULL 09/11] vnc: move check into vnc_cursor_define

Move the check whenever a cursor exists into the vnc_cursor_define()
function so callers don't have to do it.

Suggested-by: Marc-André Lureau 
Signed-off-by: Gerd Hoffmann 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Daniel P. Berrangé 
Message-id: 20210112134120.2031837-2-kra...@redhat.com
---
 ui/vnc.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index d4854d351bac..0f01481cac57 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -793,9 +793,7 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
 QTAILQ_FOREACH(vs, &vd->clients, next) {
 vnc_colordepth(vs);
 vnc_desktop_resize(vs);
-if (vs->vd->cursor) {
-vnc_cursor_define(vs);
-}
+vnc_cursor_define(vs);
 memset(vs->dirty, 0x00, sizeof(vs->dirty));
 vnc_set_area_dirty(vs->dirty, vd, 0, 0,
vnc_width(vd),
@@ -929,6 +927,10 @@ static int vnc_cursor_define(VncState *vs)
 QEMUCursor *c = vs->vd->cursor;
 int isize;
 
+if (!vs->vd->cursor) {
+return -1;
+}
+
 if (vnc_has_feature(vs, VNC_FEATURE_ALPHA_CURSOR)) {
 vnc_lock_output(vs);
 vnc_write_u8(vs,  VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
@@ -2155,9 +2157,7 @@ static void set_encodings(VncState *vs, int32_t 
*encodings, size_t n_encodings)
 vnc_desktop_resize(vs);
 check_pointer_type_change(&vs->mouse_mode_notifier, NULL);
 vnc_led_state_change(vs);
-if (vs->vd->cursor) {
-vnc_cursor_define(vs);
-}
+vnc_cursor_define(vs);
 }
 
 static void set_pixel_conversion(VncState *vs)
-- 
2.29.2

[PULL 05/11] ui/gtk: update monitor interval on egl displays

From: Nikola Pavlica 

When running QEMU's GTK UI without EGL or OGL, the
gd_monitor_update_interval function gets executed and the display refresh
rate gets updated accordingly. However, when using EGL or just regular
OGL, the function never gets executed.

Which is why I decided that the function should be in gd_egl_refresh
where the display output gets updated, in the same vain as how it's done
for normal GTK UIs (aka. those without EGL) - in it's display refresh
function.

Since the gd_monitor_update_interval function now is exposed, we are
going to use it to update the refresh rate.

Signed-off-by: Nikola Pavlica 
Message-Id: <20210114140153.301473-3-pavlica.nik...@gmail.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/gtk-egl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 99231a3597f5..71c3d698b400 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -113,6 +113,9 @@ void gd_egl_refresh(DisplayChangeListener *dcl)
 {
 VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 
+vc->gfx.dcl.update_interval = gd_monitor_update_interval(
+vc->window ? vc->window : vc->gfx.drawing_area);
+
 if (!vc->gfx.esurface) {
 gd_egl_init(vc);
 if (!vc->gfx.esurface) {
-- 
2.29.2

Re: [PATCH] fuzz: Add virtio-9p configurations for fuzzing

2021-01-15 Thread Darren Kenny

Hi Alex,

On Thursday, 2021-01-14 at 17:17:48 -05, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov 

In general this look good, so:

Reviewed-by: Darren Kenny 

but I do have a question below...

> ---
>  tests/qtest/fuzz/generic_fuzz_configs.h | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h 
> b/tests/qtest/fuzz/generic_fuzz_configs.h
> index 7fed035345..ffdb590c58 100644
> --- a/tests/qtest/fuzz/generic_fuzz_configs.h
> +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
> @@ -59,6 +59,18 @@ const generic_fuzz_config predefined_configs[] = {
>  .name = "virtio-mouse",
>  .args = "-machine q35 -nodefaults -device virtio-mouse",
>  .objects = "virtio*",
> +},{
> +.name = "virtio-9p",
> +.args = "-machine q35 -nodefaults "
> +"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
> +"-fsdev local,id=hshare,path=/tmp/,security_model=none",
> +.objects = "virtio*",

I wonder about the use of "/tmp" rather than maybe some generated name
using mkdtemp() - I also realise that the ability to generate this and
plug it in here probably doesn't exist either, hence not holding you to
it for this patch. Also the fact that in OSS-Fuzz this is run in limited
containers.

Have you observed any changes to "/tmp" while this is running? My
concerns may be unfounded since I don't really know what state things
are in while this is executed with no running OS.

Thanks,

Darren.

Re: [PATCH RFC 0/2] Add debug interface to kick/call on purpose

2021-01-15 Thread Daniel P . Berrangé

On Thu, Jan 14, 2021 at 04:27:28PM -0800, Dongli Zhang wrote:
> The virtio device/driver (e.g., vhost-scsi and indeed any device including
> e1000e) may hang due to the lost of IRQ or the lost of doorbell register
> kick, e.g.,
> 
> https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg01711.html
> 
> The virtio-net was in trouble in above link because the 'kick' was not
> taking effect (missed).
> 
> This RFC adds a new debug interface 'DeviceEvent' to DeviceClass to help
> narrow down if the issue is due to lost of irq/kick. So far the new
> interface handles only two events: 'call' and 'kick'. Any device (e.g.,
> e1000e or vhost-scsi) may implement (e.g., via eventfd, MSI-X or legacy
> IRQ).
> 
> The 'call' is to inject irq on purpose by admin for a specific device (e.g.,
> vhost-scsi) from QEMU/host to VM, while the 'kick' is to kick the doorbell
> on purpose by admin at QEMU/host side for a specific device.

I'm really not convinced that we want to give admins the direct ability to
poke at internals of devices in a running QEMU. It feels like there is way
too much potential for the admin to make a situation far worse by doing
the wrong thing here, and people dealing with support tickets will have
no idea that the admin has been poking internals of the device and broken
it by doing something wrong.

You pointed to bug that hit where this could conceivably be useful, but
that's a one time issue and should not a common occurrance that justifies
making an official public API to poke at devices forever more IMHO.

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: [PATCH v2] machine: add missing doc for memory-backend option

2021-01-15 Thread Peter Krempa

On Fri, Jan 15, 2021 at 10:02:04 +, Peter Maydell wrote:
> On Thu, 14 Jan 2021 at 23:48, Igor Mammedov  wrote:
> >
> > Add documentation for '-machine memory-backend' CLI option and
> > how to use it.
> >
> > And document that x-use-canonical-path-for-ramblock-id,
> > is considered to be stable to make sure it won't go away by accident.
> 
> That's not what the x- prefix is supposed to mean.

This is the exact reason I was asking on behalf of the libvirt team for
adding such statement if we were to use it. We want guarantee that it's
considered stable since without that it will not be accepted into
libvirt.

> If we have an internal constraint that we mustn't delete
> the option in order to support some other must-be-stable
> interface (eg migration of some machines) we can document
> that in a comment, but that doesn't mean that we should
> document to users that direct use of an x-prefix option
> is supported as a stable interface.

AFAIK the issue is that with the new approach to configure system memory
(via a memory-backend=id, since the old one was deprecated) migration
fails from/to older qemus ...

> Alternatively, if the option is really stable for direct
> use by users then we should commit to making it so by
> removing the x-.

... thus the idea behind keeping this interface as is is it also fixes
the migration compatibility for qemu 5.0/5.1/5.2 which were already
released.

Removing the 'x-' will fix it only starting with qemu-6.0 and any
downstream which backports the removal of the prefix.

Obviously not using 'x-' prefixed options is strongly preferred in
libvirt.

Re: [PATCH v2] machine: add missing doc for memory-backend option

2021-01-15 Thread Peter Krempa

On Fri, Jan 15, 2021 at 11:43:10 +0100, Peter Krempa wrote:
> On Fri, Jan 15, 2021 at 10:02:04 +, Peter Maydell wrote:
> > On Thu, 14 Jan 2021 at 23:48, Igor Mammedov  wrote:

[...]

> Removing the 'x-' will fix it only starting with qemu-6.0 and any
> downstream which backports the removal of the prefix.
> 
> Obviously not using 'x-' prefixed options is strongly preferred in
> libvirt. 

For reference, here are the relevant threads on libvir-list:

v1:
https://www.redhat.com/archives/libvir-list/2021-January/msg00601.html
v2:
https://www.redhat.com/archives/libvir-list/2021-January/msg00684.html

And my objection to use 'x-' prefixed features without being guaranteed
that it's considered stable (note that the premise is that it's supposed
to fix already-released qemu versions thus not completely refusing using
the existing naming)
https://www.redhat.com/archives/libvir-list/2021-January/msg00606.html
https://www.redhat.com/archives/libvir-list/2021-January/msg00699.html

Re: [PATCH v3 03/10] iotests: Move try_remove to iotests.py


14.01.2021 20:02, Max Reitz wrote:

Signed-off-by: Max Reitz
Reviewed-by: Eric Blake


Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH v3 06/10] iotests/129: Use throttle node


14.01.2021 20:03, Max Reitz wrote:

Throttling on the BB has not affected block jobs in a while, so it is
possible that one of the jobs in 129 finishes before the VM is stopped.
We can fix that by running the job from a throttle node.

Signed-off-by: Max Reitz
Reviewed-by: Eric Blake


you forget my :)
Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH v6 08/11] iotests: add testenv.py

2021-01-15 Thread Kevin Wolf

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Add TestEnv class, which will handle test environment in a new python
> iotests running framework.
> 
> Difference with current ./check interface:
> - -v (verbose) option dropped, as it is unused
> 
> - -xdiff option is dropped, until somebody complains that it is needed
> - same for -n option
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  tests/qemu-iotests/testenv.py | 328 ++
>  1 file changed, 328 insertions(+)
>  create mode 100755 tests/qemu-iotests/testenv.py
> 
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> new file mode 100755
> index 00..ecaf76fb85
> --- /dev/null
> +++ b/tests/qemu-iotests/testenv.py
> @@ -0,0 +1,328 @@
> +#!/usr/bin/env python3
> +#
> +# Parse command line options to manage test environment variables.
> +#
> +# Copyright (c) 2020 Virtuozzo International GmbH
> +#
> +# 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 .
> +#
> +
> +import os
> +import sys
> +import tempfile
> +from pathlib import Path
> +import shutil
> +import collections
> +import subprocess
> +import argparse
> +from typing import List, Dict
> +
> +
> +def get_default_machine(qemu_prog: str) -> str:
> +outp = subprocess.run([qemu_prog, '-machine', 'help'], check=True,
> +  text=True, stdout=subprocess.PIPE).stdout
> +
> +machines = outp.split('\n')
> +default_machine = next(m for m in machines if m.endswith(' (default)'))
> +default_machine = default_machine.split(' ', 1)[0]
> +
> +alias_suf = ' (alias of {})'.format(default_machine)
> +alias = next((m for m in machines if m.endswith(alias_suf)), None)
> +if alias is not None:
> +default_machine = alias.split(' ', 1)[0]
> +
> +return default_machine
> +
> +
> +class TestEnv:
> +"""
> +Manage system environment for running tests
> +
> +The following variables are supported/provided. They are represented by
> +lower-cased TestEnv attributes.
> +"""
> +env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
> + 'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
> + 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
> + 'SOCKET_SCM_HELPER', 'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
> + 'QEMU_IO_OPTIONS', 'QEMU_NBD_OPTIONS', 'IMGOPTS',
> + 'IMGFMT', 'IMGPROTO', 'AIOMODE', 'CACHEMODE',
> + 'VALGRIND_QEMU', 'CACHEMODE_IS_DEFAULT', 
> 'IMGFMT_GENERIC',
> + 'IMGOPTSSYNTAX', 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE']
> +
> +def get_env(self) -> Dict[str, str]:
> +env = {}
> +for v in self.env_variables:
> +val = getattr(self, v.lower(), None)
> +if val is not None:
> +env[v] = val
> +
> +return env
> +
> +_argparser = None
> +@classmethod
> +def get_argparser(cls) -> argparse.ArgumentParser:
> +if cls._argparser is not None:
> +return cls._argparser
> +
> +p = argparse.ArgumentParser(description="= test environment options 
> =",
> +add_help=False, usage=argparse.SUPPRESS)
> +
> +p.add_argument('-d', dest='debug', action='store_true', help='debug')
> +p.add_argument('-misalign', action='store_true',
> +   help='misalign memory allocations')
> +
> +p.set_defaults(imgfmt='raw', imgproto='file')
> +
> +format_list = ['raw', 'bochs', 'cloop', 'parallels', 'qcow', 'qcow2',
> +   'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 'dmg']
> +g = p.add_argument_group(
> +'image format options',
> +'The following options sets IMGFMT environment variable. '

s/sets/set the/

> +'At most one chose is allowed, default is "raw"')

s/chose/choice/

> +g = g.add_mutually_exclusive_group()
> +for fmt in format_list:
> +g.add_argument('-' + fmt, dest='imgfmt', action='store_const',
> +   const=fmt)
> +
> +protocol_list = ['file', 'rbd', 'sheepdoc', 'nbd', 'ssh', 'nfs',
> + 'fuse']
> +g = p.add_argument_group(
> +'image protocol options',
> +'T

Re: [PATCH v3 09/10] iotests/129: Clean up pylint and mypy complaints


14.01.2021 20:03, Max Reitz wrote:

Signed-off-by: Max Reitz



Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH v3 02/10] iotests/297: Rewrite in Python and extend reach


14.01.2021 20:02, Max Reitz wrote:

Instead of checking iotests.py only, check all Python files in the
qemu-iotests/ directory.  Of course, most of them do not pass, so there
is an extensive skip list for now.  (The only files that do pass are
209, 254, 283, and iotests.py.)

(Alternatively, we could have the opposite, i.e. an explicit list of
files that we do want to check, but I think it is better to check files
by default.)

Unless started in debug mode (./check -d), the output has no information
on which files are tested, so we will not have a problem e.g. with
backports, where some files may be missing when compared to upstream.

Besides the technical rewrite, some more things are changed:

- For the pylint invocation, PYTHONPATH is adjusted.  This mirrors
   setting MYPYPATH for mypy.

- Also, MYPYPATH is now derived from PYTHONPATH, so that we include
   paths set by the environment.  Maybe at some point we want to let the
   check script add '../../python/' to PYTHONPATH so that iotests.py does
   not need to do that.

- Passing --notes=FIXME,XXX to pylint suppresses warnings for TODO
   comments.  TODO is fine, we do not need 297 to complain about such
   comments.

- The "Success" line from mypy's output is suppressed, because (A) it
   does not add useful information, and (B) it would leak information
   about the files having been tested to the reference output, which we
   decidedly do not want.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/297 | 109 +
  tests/qemu-iotests/297.out |   5 +-
  2 files changed, 89 insertions(+), 25 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 5c5420712b..bfa26d280b 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/usr/bin/env python3
  #
  # Copyright (C) 2020 Red Hat, Inc.
  #
@@ -15,30 +15,95 @@
  # You should have received a copy of the GNU General Public License
  # along with this program.  If not, see .
  
-seq=$(basename $0)

-echo "QA output created by $seq"
+import os
+import re
+import shutil
+import subprocess
+import sys
  
-status=1	# failure is the default!

+import iotests
  
-# get standard environment

-. ./common.rc
  
-if ! type -p "pylint-3" > /dev/null; then

-_notrun "pylint-3 not found"
-fi
-if ! type -p "mypy" > /dev/null; then
-_notrun "mypy not found"
-fi
+# TODO: Empty this list!
+SKIP_FILES = (
+'030', '040', '041', '044', '045', '055', '056', '057', '065', '093',
+'096', '118', '124', '129', '132', '136', '139', '147', '148', '149',
+'151', '152', '155', '163', '165', '169', '194', '196', '199', '202',
+'203', '205', '206', '207', '208', '210', '211', '212', '213', '216',
+'218', '219', '222', '224', '228', '234', '235', '236', '237', '238',
+'240', '242', '245', '246', '248', '255', '256', '257', '258', '260',
+'262', '264', '266', '274', '277', '280', '281', '295', '296', '298',
+'299', '300', '302', '303', '304', '307',
+'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py'
+)
  
-pylint-3 --score=n iotests.py
  
-MYPYPATH=../../python/ mypy --warn-unused-configs --disallow-subclassing-any \

---disallow-any-generics --disallow-incomplete-defs \
---disallow-untyped-decorators --no-implicit-optional \
---warn-redundant-casts --warn-unused-ignores \
---no-implicit-reexport iotests.py
+def is_python_file(filename):
+if not os.path.isfile(filename):
+return False
  
-# success, all done

-echo "*** done"
-rm -f $seq.full
-status=0
+if filename.endswith('.py'):
+return True
+
+with open(filename) as f:
+try:
+first_line = f.readline()
+return re.match('^#!.*python', first_line) is not None
+except UnicodeDecodeError: # Ignore binary files


PEP8 asks for two spaces before inline comment


+return False
+


and two empty lines here

(good ALE vim plugin runs flake8, mypy and pylint asynchronously for me and 
shows these things)


+def run_linters():
+files = [filename for filename in (set(os.listdir('.')) - set(SKIP_FILES))
+ if is_python_file(filename)]
+
+iotests.logger.debug('Files to be checked:')
+iotests.logger.debug(', '.join(sorted(files)))


O, good.


+
+print('=== pylint ===')
+sys.stdout.flush()


Should not hurt.. But why? Should be flushed anyway as print will put a '\n'.. 
And why you care about it at all?


+
+# Todo notes are fine, but fixme's or xxx's should probably just be
+# fixed (in tests, at least)
+env = os.environ
+try:
+env['PYTHONPATH'] += ':../../python/'
+except KeyError:
+env['PYTHONPATH'] = '../../python/'
+subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files),
+   env=env, check=False)


as I understand, you don't need env argument, as os.environ

Re: [PATCH v3 10/10] iotests/300: Clean up pylint and mypy complaints


14.01.2021 20:03, Max Reitz wrote:

Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/297 |  2 +-
  tests/qemu-iotests/300 | 18 +++---
  2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 1dce1d1b1c..03d8604538 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -33,7 +33,7 @@ SKIP_FILES = (
  '218', '219', '222', '224', '228', '234', '235', '236', '237', '238',
  '240', '242', '245', '246', '248', '255', '256', '257', '258', '260',
  '262', '264', '266', '274', '277', '280', '281', '295', '296', '298',
-'299', '300', '302', '303', '304', '307',
+'299', '302', '303', '304', '307',
  'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py'
  )
  
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300

index b864a21d5e..64d388a8fa 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -22,7 +22,11 @@ import os
  import random
  import re
  from typing import Dict, List, Optional, Union
+
  import iotests
+
+# Import qemu after iotests.py has amended the PYTHONPATH


you mean os.path, exactly,

 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))

yes?


+# pylint: disable=wrong-import-order
  import qemu
  
  BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]

@@ -110,10 +114,14 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
  If @msg is None, check that there has not been any error.
  """
  self.vm_b.shutdown()
+
+log = self.vm_b.get_log()
+assert log is not None # Loaded after shutdown
+
  if msg is None:
-self.assertNotIn('qemu-system-', self.vm_b.get_log())
+self.assertNotIn('qemu-system-', log)
  else:
-self.assertIn(msg, self.vm_b.get_log())
+self.assertIn(msg, log)
  
  @staticmethod

  def mapping(node_name: str, node_alias: str,
@@ -445,9 +453,13 @@ class 
TestBlockBitmapMappingErrors(TestDirtyBitmapMigration):
  
  # Check for the error in the source's log

  self.vm_a.shutdown()
+
+log = self.vm_a.get_log()
+assert log is not None # Loaded after shutdown
+
  self.assertIn(f"Cannot migrate bitmap '{name}' on node "
f"'{self.src_node_name}': Name is longer than 255 
bytes",
-  self.vm_a.get_log())
+  log)
  
  # Expect abnormal shutdown of the destination VM because of

  # the failed migration



Your new comments are the only PEP8 complains in the 300 iotest:

flake8 300
300:119:31: E261 at least two spaces before inline comment
300:458:31: E261 at least two spaces before inline comment

So, I'd fix them.

anyway:
Reviewed-by: Vladimir Sementsov-Ogievskiy 


--
Best regards,
Vladimir

Re: [PATCH for-5.2 00/10] block/export: vhost-user-blk server tests and input validation

2021-01-15 Thread Peter Maydell

On Tue, 17 Nov 2020 at 09:18, Michael S. Tsirkin  wrote:
>
> On Wed, Nov 11, 2020 at 12:43:21PM +, Stefan Hajnoczi wrote:
> > The vhost-user-blk server test was already in Michael Tsirkin's recent vhost
> > pull request, but was dropped because it exposed vhost-user regressions
> > (b7c1bd9d7848 and the Based-on tag below). Now that the vhost-user 
> > regressions
> > are fixed we can re-introduce the test case.
> >
> > This series adds missing input validation that led to a Coverity report. The
> > virtio-blk read, write, discard, and write zeroes commands need to check
> > sector/byte ranges and other inputs. This solves the issue Peter Maydell 
> > raised
> > in "[PATCH for-5.2] block/export/vhost-user-blk-server.c: Avoid potential
> > integer overflow".
> >
> > Merging just the input validation patches would be possible too, but I 
> > prefer
> > to merge the corresponding tests so the code is exercised by the CI.
>
>
> Peter reports this hanging for him so I dropped this for now.
> Pls work with him to resolve, and resubmit.
> If possible let's defer the addition of new tests and just fix existing
> ones for 5.2.

Ping! This series was trying to fix a Coverity error -- it got dropped
for 5.2 but now 6.0 is open what are the plans for it?

thanks
-- PMM

Re: [PATCH v11 0/5] UFFD write-tracking migration/snapshots

2021-01-15 Thread Andrey Gruzdev


Ping

On 06.01.2021 18:21, Andrey Gruzdev wrote:


This patch series is a kind of 'rethinking' of Denis Plotnikov's ideas he's
implemented in his series '[PATCH v0 0/4] migration: add background snapshot'.

Currently the only way to make (external) live VM snapshot is using existing
dirty page logging migration mechanism. The main problem is that it tends to
produce a lot of page duplicates while running VM goes on updating already
saved pages. That leads to the fact that vmstate image size is commonly several
times bigger then non-zero part of virtual machine's RSS. Time required to
converge RAM migration and the size of snapshot image severely depend on the
guest memory write rate, sometimes resulting in unacceptably long snapshot
creation time and huge image size.

This series propose a way to solve the aforementioned problems. This is done
by using different RAM migration mechanism based on UFFD write protection
management introduced in v5.7 kernel. The migration strategy is to 'freeze'
guest RAM content using write-protection and iteratively release protection
for memory ranges that have already been saved to the migration stream.
At the same time we read in pending UFFD write fault events and save those
pages out-of-order with higher priority.

How to use:
1. Enable write-tracking migration capability
virsh qemu-monitor-command  --hmp migrate_set_capability.
track-writes-ram on

2. Start the external migration to a file
virsh qemu-monitor-command  --hmp migrate exec:'cat > ./vm_state'

3. Wait for the migration finish and check that the migration has completed.
state.


Changes v10->v11:

* 1. Updated commit messages.

Changes v9->v10:

* 1. Fixed commit message for [PATCH v9 1/5].

Changes v8->v9:

* 1. Fixed wrong cover letter subject.

Changes v7->v8:

* 1. Fixed coding style problems to pass checkpatch.

Changes v6->v7:

* 1. Fixed background snapshot on suspended guest: call 
qemu_system_wakeup_request()
*before stopping VM to make runstate transition valid.
* 2. Disabled dirty page logging and log syn when 'background-snapshot' is 
enabled.
* 3. Introduced 'userfaultfd-wrlat.py' script to analyze UFFD write fault 
latencies.

Changes v5->v6:

* 1. Consider possible hot pluggin/unpluggin of memory device - don't use static
*for write-tracking support level in migrate_query_write_tracking(), check
*each time when one tries to enable 'background-snapshot' capability.

Changes v4->v5:

* 1. Refactored util/userfaultfd.c code to support features required by 
postcopy.
* 2. Introduced checks for host kernel and guest memory backend compatibility
*to 'background-snapshot' branch in migrate_caps_check().
* 3. Switched to using trace_xxx instead of info_report()/error_report() for
*cases when error message must be hidden (probing UFFD-IO) or info may be
*really littering output if goes to stderr.
* 4  Added RCU_READ_LOCK_GUARDs to the code dealing with RAM block list.
* 5. Added memory_region_ref() for each RAM block being wr-protected.
* 6. Reused qemu_ram_block_from_host() instead of custom RAM block lookup 
routine.
* 7. Refused from using specific hwaddr/ram_addr_t in favour of void */uint64_t.
* 8. Currently dropped 'linear-scan-rate-limiting' patch. The reason is that
*that choosen criteria for high-latency fault detection (i.e. timestamp of
*UFFD event fetch) is not representative enough for this task.
*At the moment it looks somehow like premature optimization effort.
* 8. Dropped some unnecessary/unused code.

Andrey Gruzdev (5):
   migration: introduce 'background-snapshot' migration capability
   migration: introduce UFFD-WP low-level interface helpers
   migration: support UFFD write fault processing in ram_save_iterate()
   migration: implementation of background snapshot thread
   migration: introduce 'userfaultfd-wrlat.py' script

  include/exec/memory.h|   8 +
  include/qemu/userfaultfd.h   |  35 
  migration/migration.c| 365 ++-
  migration/migration.h|   4 +
  migration/ram.c  | 288 ++-
  migration/ram.h  |   6 +
  migration/savevm.c   |   1 -
  migration/savevm.h   |   2 +
  migration/trace-events   |   2 +
  qapi/migration.json  |   7 +-
  scripts/userfaultfd-wrlat.py | 148 ++
  util/meson.build |   1 +
  util/trace-events|   9 +
  util/userfaultfd.c   | 345 +
  14 files changed, 1211 insertions(+), 10 deletions(-)
  create mode 100644 include/qemu/userfaultfd.h
  create mode 100755 scripts/userfaultfd-wrlat.py
  create mode 100644 util/userfaultfd.c



--
Andrey Gruzdev, Principal Engineer
Virtuozzo GmbH  +7-903-247-6397
virtuzzo.com

Re: [PATCH] hw/misc/sifive_u_otp: handling the fails of blk_pread and blk_pwrite

2021-01-15 Thread Peter Maydell

Ping! This patch was trying to fix a Coverity issue (CID 1435959,
1435960, 1435961) -- is anybody planning to review it?

(I'm not entirely sure 'guest error' is the right warning category,
but I don't know the specifics of this device.)

thanks
-- PMM

On Wed, 4 Nov 2020 at 09:29, Green Wan  wrote:
>
> Fix code coverage issues by checking return value and handling fail case
> of blk_pread() and blk_pwrite(). Return default value 0xff if read fails.
>
> Signed-off-by: Green Wan 
> ---
>  hw/misc/sifive_u_otp.c | 31 +++
>  1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/hw/misc/sifive_u_otp.c b/hw/misc/sifive_u_otp.c
> index 60066375ab..4314727d0d 100644
> --- a/hw/misc/sifive_u_otp.c
> +++ b/hw/misc/sifive_u_otp.c
> @@ -62,8 +62,13 @@ static uint64_t sifive_u_otp_read(void *opaque, hwaddr 
> addr, unsigned int size)
>  if (s->blk) {
>  int32_t buf;
>
> -blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf,
> -  SIFIVE_U_OTP_FUSE_WORD);
> +if (blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf,
> +  SIFIVE_U_OTP_FUSE_WORD) < 0) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "read error index<%d>\n", s->pa);
> +return 0xff;
> +}
> +
>  return buf;
>  }
>
> @@ -160,8 +165,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
>
>  /* write to backend */
>  if (s->blk) {
> -blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD,
> -   &s->fuse[s->pa], SIFIVE_U_OTP_FUSE_WORD, 0);
> +if (blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD,
> +   &s->fuse[s->pa], SIFIVE_U_OTP_FUSE_WORD,
> +   0) < 0) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "write error index<%d>\n", s->pa);
> +}
>  }
>
>  /* update written bit */
> @@ -248,12 +257,18 @@ static void sifive_u_otp_reset(DeviceState *dev)
>  int index = SIFIVE_U_OTP_SERIAL_ADDR;
>
>  serial_data = s->serial;
> -blk_pwrite(s->blk, index * SIFIVE_U_OTP_FUSE_WORD,
> -   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
> +if (blk_pwrite(s->blk, index * SIFIVE_U_OTP_FUSE_WORD,
> +   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0) < 0) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "write error index<%d>\n", index);
> +}
>
>  serial_data = ~(s->serial);
> -blk_pwrite(s->blk, (index + 1) * SIFIVE_U_OTP_FUSE_WORD,
> -   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
> +if (blk_pwrite(s->blk, (index + 1) * SIFIVE_U_OTP_FUSE_WORD,
> +   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0) < 0) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "write error index<%d>\n", index + 1);
> +}
>  }
>
>  /* Initialize write-once map */
> --
> 2.17.1

[PATCH v3 11/10] iotests: add flake8 linter

pylint is good, but doesn't cover the PEP8. Let's add flake8, to be
sure that our code sutisfy PEP8. Add new linter and fix some code style
in checked files.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Hi!

Here is my small addition to Max's series, hope you like it!

Note, that this is not the first occurrence of 'flake8' in Qemu:

# git grep flake8
python/qemu/.flake8:[flake8]
scripts/qapi/.flake8:[flake8]
scripts/qapi/.flake8:extend-ignore = E722  # Prefer pylint's bare-except 
checks to flake8's


 tests/qemu-iotests/129|  6 ++-
 tests/qemu-iotests/254|  2 +-
 tests/qemu-iotests/297| 21 ++---
 tests/qemu-iotests/297.out|  1 +
 tests/qemu-iotests/300|  4 +-
 tests/qemu-iotests/iotests.py | 88 +--
 6 files changed, 106 insertions(+), 16 deletions(-)

diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index 201d9e0a0b..28ec1d 100755
--- a/tests/qemu-iotests/129
+++ b/tests/qemu-iotests/129
@@ -21,6 +21,7 @@
 import os
 import iotests
 
+
 class TestStopWithBlockJob(iotests.QMPTestCase):
 test_img = os.path.join(iotests.test_dir, 'test.img')
 target_img = os.path.join(iotests.test_dir, 'target.img')
@@ -39,8 +40,8 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
 source_drive = 'driver=throttle,' \
'node-name=source,' \
'throttle-group=tg0,' \
-  f'file.driver={iotests.imgfmt},' \
-  f'file.file.filename={self.test_img}'
+   f'file.driver={iotests.imgfmt},' \
+   f'file.file.filename={self.test_img}'
 
 self.vm.add_drive(None, source_drive)
 self.vm.launch()
@@ -97,6 +98,7 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
 
 self.do_test_stop('block-commit', device='drive0', top_node='source')
 
+
 if __name__ == '__main__':
 iotests.main(supported_fmts=["qcow2"],
  supported_protocols=["file"])
diff --git a/tests/qemu-iotests/254 b/tests/qemu-iotests/254
index 150e58be8e..798f316e36 100755
--- a/tests/qemu-iotests/254
+++ b/tests/qemu-iotests/254
@@ -74,7 +74,7 @@ log("query-block: device = {}, node-name = {}, 
dirty-bitmaps:".format(
 result['device'], result['inserted']['node-name']))
 log(result['dirty-bitmaps'], indent=2)
 log("\nbitmaps in backing image:")
-log(result['inserted']['image']['backing-image']['format-specific'] \
+log(result['inserted']['image']['backing-image']['format-specific']
 ['data']['bitmaps'], indent=2)
 
 vm.shutdown()
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 03d8604538..b79c341a3c 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -49,9 +49,10 @@ def is_python_file(filename):
 try:
 first_line = f.readline()
 return re.match('^#!.*python', first_line) is not None
-except UnicodeDecodeError: # Ignore binary files
+except UnicodeDecodeError:  # Ignore binary files
 return False
 
+
 def run_linters():
 files = [filename for filename in (set(os.listdir('.')) - set(SKIP_FILES))
  if is_python_file(filename)]
@@ -59,16 +60,22 @@ def run_linters():
 iotests.logger.debug('Files to be checked:')
 iotests.logger.debug(', '.join(sorted(files)))
 
-print('=== pylint ===')
-sys.stdout.flush()
-
-# Todo notes are fine, but fixme's or xxx's should probably just be
-# fixed (in tests, at least)
 env = os.environ
 try:
 env['PYTHONPATH'] += ':../../python/'
 except KeyError:
 env['PYTHONPATH'] = '../../python/'
+
+print('=== flake8 ===')
+sys.stdout.flush()
+
+subprocess.run(('flake8', *files), env=env, check=False)
+
+print('=== pylint ===')
+sys.stdout.flush()
+
+# Todo notes are fine, but fixme's or xxx's should probably just be
+# fixed (in tests, at least)
 subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files),
env=env, check=False)
 
@@ -102,7 +109,7 @@ def run_linters():
 print(p.stdout)
 
 
-for linter in ('pylint-3', 'mypy'):
+for linter in ('flake8', 'pylint-3', 'mypy'):
 if shutil.which(linter) is None:
 iotests.notrun(f'{linter} not found')
 
diff --git a/tests/qemu-iotests/297.out b/tests/qemu-iotests/297.out
index f2e1314d10..46bf3e665d 100644
--- a/tests/qemu-iotests/297.out
+++ b/tests/qemu-iotests/297.out
@@ -1,2 +1,3 @@
+=== flake8 ===
 === pylint ===
 === mypy ===
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index 64d388a8fa..bbd227ff73 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -116,7 +116,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
 self.vm_b.shutdown()
 
 log = self.vm_b.get_log()
-assert log is not None # Loaded after shutdown
+assert log is not None  # Loaded after shutdown
 
 if msg is None:
 self.ass

Re: [PATCH v3 02/10] iotests/297: Rewrite in Python and extend reach


On 15.01.21 12:13, Vladimir Sementsov-Ogievskiy wrote:

14.01.2021 20:02, Max Reitz wrote:

Instead of checking iotests.py only, check all Python files in the
qemu-iotests/ directory.  Of course, most of them do not pass, so there
is an extensive skip list for now.  (The only files that do pass are
209, 254, 283, and iotests.py.)

(Alternatively, we could have the opposite, i.e. an explicit list of
files that we do want to check, but I think it is better to check files
by default.)

Unless started in debug mode (./check -d), the output has no information
on which files are tested, so we will not have a problem e.g. with
backports, where some files may be missing when compared to upstream.

Besides the technical rewrite, some more things are changed:

- For the pylint invocation, PYTHONPATH is adjusted.  This mirrors
   setting MYPYPATH for mypy.

- Also, MYPYPATH is now derived from PYTHONPATH, so that we include
   paths set by the environment.  Maybe at some point we want to let the
   check script add '../../python/' to PYTHONPATH so that iotests.py does
   not need to do that.

- Passing --notes=FIXME,XXX to pylint suppresses warnings for TODO
   comments.  TODO is fine, we do not need 297 to complain about such
   comments.

- The "Success" line from mypy's output is suppressed, because (A) it
   does not add useful information, and (B) it would leak information
   about the files having been tested to the reference output, which we
   decidedly do not want.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/297 | 109 +
  tests/qemu-iotests/297.out |   5 +-
  2 files changed, 89 insertions(+), 25 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 5c5420712b..bfa26d280b 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/usr/bin/env python3
  #
  # Copyright (C) 2020 Red Hat, Inc.
  #
@@ -15,30 +15,95 @@
  # You should have received a copy of the GNU General Public License
  # along with this program.  If not, see .
-seq=$(basename $0)
-echo "QA output created by $seq"
+import os
+import re
+import shutil
+import subprocess
+import sys
-status=1    # failure is the default!
+import iotests
-# get standard environment
-. ./common.rc
-if ! type -p "pylint-3" > /dev/null; then
-    _notrun "pylint-3 not found"
-fi
-if ! type -p "mypy" > /dev/null; then
-    _notrun "mypy not found"
-fi
+# TODO: Empty this list!
+SKIP_FILES = (
+    '030', '040', '041', '044', '045', '055', '056', '057', '065', 
'093',
+    '096', '118', '124', '129', '132', '136', '139', '147', '148', 
'149',
+    '151', '152', '155', '163', '165', '169', '194', '196', '199', 
'202',
+    '203', '205', '206', '207', '208', '210', '211', '212', '213', 
'216',
+    '218', '219', '222', '224', '228', '234', '235', '236', '237', 
'238',
+    '240', '242', '245', '246', '248', '255', '256', '257', '258', 
'260',
+    '262', '264', '266', '274', '277', '280', '281', '295', '296', 
'298',

+    '299', '300', '302', '303', '304', '307',
+    'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py'
+)
-pylint-3 --score=n iotests.py
-MYPYPATH=../../python/ mypy --warn-unused-configs 
--disallow-subclassing-any \

-    --disallow-any-generics --disallow-incomplete-defs \
-    --disallow-untyped-decorators --no-implicit-optional \
-    --warn-redundant-casts --warn-unused-ignores \
-    --no-implicit-reexport iotests.py
+def is_python_file(filename):
+    if not os.path.isfile(filename):
+    return False
-# success, all done
-echo "*** done"
-rm -f $seq.full
-status=0
+    if filename.endswith('.py'):
+    return True
+
+    with open(filename) as f:
+    try:
+    first_line = f.readline()
+    return re.match('^#!.*python', first_line) is not None
+    except UnicodeDecodeError: # Ignore binary files


PEP8 asks for two spaces before inline comment


Wow.  I really hate PEP8.


+    return False
+


and two empty lines here

(good ALE vim plugin runs flake8, mypy and pylint asynchronously for me 
and shows these things)


I’d like to argue that that isn’t good, but I need to quench my anger. 
Perhaps one day I can quench it sufficiently to install ALE myself.



+def run_linters():
+    files = [filename for filename in (set(os.listdir('.')) - 
set(SKIP_FILES))

+ if is_python_file(filename)]
+
+    iotests.logger.debug('Files to be checked:')
+    iotests.logger.debug(', '.join(sorted(files)))


O, good.


+
+    print('=== pylint ===')
+    sys.stdout.flush()


Should not hurt.. But why? Should be flushed anyway as print will put a 
'\n'.. And why you care about it at all?


When pylint fails, I can see its result above the '=== pylint ===' line, 
even though its output is on stdout.  I suspect the Python output indeed 
isn’t flushed on \n.


(Test it by dropping the flush(), and then also

Re: [PATCH v3 06/10] iotests/129: Use throttle node


On 15.01.21 12:16, Vladimir Sementsov-Ogievskiy wrote:

14.01.2021 20:03, Max Reitz wrote:

Throttling on the BB has not affected block jobs in a while, so it is
possible that one of the jobs in 129 finishes before the VM is stopped.
We can fix that by running the job from a throttle node.

Signed-off-by: Max Reitz
Reviewed-by: Eric Blake


you forget my :)
Reviewed-by: Vladimir Sementsov-Ogievskiy 


Sorry O:)

Re: [PATCH v3 10/10] iotests/300: Clean up pylint and mypy complaints


On 15.01.21 12:30, Vladimir Sementsov-Ogievskiy wrote:

14.01.2021 20:03, Max Reitz wrote:

Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/297 |  2 +-
  tests/qemu-iotests/300 | 18 +++---
  2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 1dce1d1b1c..03d8604538 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -33,7 +33,7 @@ SKIP_FILES = (
  '218', '219', '222', '224', '228', '234', '235', '236', '237', 
'238',
  '240', '242', '245', '246', '248', '255', '256', '257', '258', 
'260',
  '262', '264', '266', '274', '277', '280', '281', '295', '296', 
'298',

-    '299', '300', '302', '303', '304', '307',
+    '299', '302', '303', '304', '307',



  'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py'
  )
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index b864a21d5e..64d388a8fa 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -22,7 +22,11 @@ import os
  import random
  import re
  from typing import Dict, List, Optional, Union
+
  import iotests
+
+# Import qemu after iotests.py has amended the PYTHONPATH


you mean os.path, exactly,

  sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 
'python'))


yes?


Indeed.


+# pylint: disable=wrong-import-order
  import qemu
  BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]
@@ -110,10 +114,14 @@ class 
TestDirtyBitmapMigration(iotests.QMPTestCase):

  If @msg is None, check that there has not been any error.
  """
  self.vm_b.shutdown()
+
+    log = self.vm_b.get_log()
+    assert log is not None # Loaded after shutdown
+
  if msg is None:
-    self.assertNotIn('qemu-system-', self.vm_b.get_log())
+    self.assertNotIn('qemu-system-', log)
  else:
-    self.assertIn(msg, self.vm_b.get_log())
+    self.assertIn(msg, log)
  @staticmethod
  def mapping(node_name: str, node_alias: str,
@@ -445,9 +453,13 @@ class 
TestBlockBitmapMappingErrors(TestDirtyBitmapMigration):

  # Check for the error in the source's log
  self.vm_a.shutdown()
+
+    log = self.vm_a.get_log()
+    assert log is not None # Loaded after shutdown
+
  self.assertIn(f"Cannot migrate bitmap '{name}' on node "
    f"'{self.src_node_name}': Name is longer than 
255 bytes",

-  self.vm_a.get_log())
+  log)
  # Expect abnormal shutdown of the destination VM because of
  # the failed migration



Your new comments are the only PEP8 complains in the 300 iotest:

flake8 300
300:119:31: E261 at least two spaces before inline comment
300:458:31: E261 at least two spaces before inline comment

So, I'd fix them.


Haha, sure :)


anyway:
Reviewed-by: Vladimir Sementsov-Ogievskiy 


Thanks!

Re: [PATCH v3 11/10] iotests: add flake8 linter


On 15.01.21 12:53, Vladimir Sementsov-Ogievskiy wrote:

pylint is good, but doesn't cover the PEP8. Let's add flake8, to be
sure that our code sutisfy PEP8. Add new linter and fix some code style
in checked files.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Hi!

Here is my small addition to Max's series, hope you like it!

Note, that this is not the first occurrence of 'flake8' in Qemu:

 # git grep flake8
 python/qemu/.flake8:[flake8]
 scripts/qapi/.flake8:[flake8]
 scripts/qapi/.flake8:extend-ignore = E722  # Prefer pylint's bare-except 
checks to flake8's


  tests/qemu-iotests/129|  6 ++-
  tests/qemu-iotests/254|  2 +-
  tests/qemu-iotests/297| 21 ++---
  tests/qemu-iotests/297.out|  1 +
  tests/qemu-iotests/300|  4 +-
  tests/qemu-iotests/iotests.py | 88 +--
  6 files changed, 106 insertions(+), 16 deletions(-)


Looks reasonable to me, but perhaps it should just be a dedicated 
series.  I think there’s enough in here to justify that.



diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index 201d9e0a0b..28ec1d 100755
--- a/tests/qemu-iotests/129
+++ b/tests/qemu-iotests/129
@@ -21,6 +21,7 @@
  import os
  import iotests
  
+

  class TestStopWithBlockJob(iotests.QMPTestCase):
  test_img = os.path.join(iotests.test_dir, 'test.img')
  target_img = os.path.join(iotests.test_dir, 'target.img')
@@ -39,8 +40,8 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
  source_drive = 'driver=throttle,' \
 'node-name=source,' \
 'throttle-group=tg0,' \
-  f'file.driver={iotests.imgfmt},' \
-  f'file.file.filename={self.test_img}'
+   f'file.driver={iotests.imgfmt},' \
+   f'file.file.filename={self.test_img}'


Interesting, when indenting this, I was wondering whether pylint would 
complain.  I was so glad it didn’t.  I really don’t like PEP8.


(Though I understand that style guides like PEP8 are there specifically 
so when someone like me goes “but I like this style better :(”, everyone 
else can say “but you’re objectively wrong”.  So me hating it kind of is 
its point, I guess.)


Max

[RFC PATCH 0/5] hw/block/nvme: support multi-path for ctrl/ns

Hello,

This series added support for multi-path I/O with multi-controllers and
namespace sharing.  By supporting these features, we can test Linux
kernel mpath(multi-path) code with this NVMe device.

Patches from the first to third added multi-controller support in a NVM
subsystem by adding a mpath.ctrl parameter to nvme device.  The rest of
the patches added namespace sharing support in a NVM subsystem with two
or more controllers by adding mpath.ns parameter to nvme-ns device.

Multi-path enabled in kernel with this series for two controllers with a
namespace:

  root@vm:~/work# nvme list -v
  NVM Express Subsystems

  SubsystemSubsystem-NQN
Controllers
   

 
  nvme-subsys0 nqn.2019-08.org.qemu:serial  
nvme0, nvme1

  NVM Express Controllers

  Device   SN   MN   FR 
  TxPort AddressSubsystemNamespaces
     
 -- --  
  nvme0serial   QEMU NVMe Ctrl   1.0
  pcie   :01:00.0   nvme-subsys0 nvme0n1
  nvme1serial   QEMU NVMe Ctrl   1.0
  pcie   :02:00.0   nvme-subsys0 nvme0n1

  NVM Express Namespaces

  Device   NSID Usage  Format   Controllers
    --  

  nvme0n1  1268.44  MB / 268.44  MB512   B +  0 B   nvme0, nvme1

The reason why I put 'RFC' tag to this series is mostly about the last
patch "hw/block/nvme: add namespace sharing param for mpath".  It seems
like QEMU block backing device does not support to be shared among two
or more -device(s).  It means that we just can't give same drive=
property to multiple nvme-ns devices.  This patch has just let -device
maps to -drive one-to-one(1:1), but if namespae sharing is detected and
setup by the host kernel, then a single block device will be selected
for the NVM subsystem.  I'm not sure this is a good start for this
feature, so I put the RFC tag here.

Please kindly review!

Thanks,

Minwoo Im (5):
  hw/block/nvme: add controller id parameter
  nvme: add CMIC enum value for Identify Controller
  hw/block/nvme: add multi-controller param for mpath
  nvme: add NMIC enum value for Identify Namespace
  hw/block/nvme: add namespace sharing param for mpath

 hw/block/nvme-ns.c   | 14 --
 hw/block/nvme-ns.h   |  2 ++
 hw/block/nvme.c  | 26 ++
 hw/block/nvme.h  |  2 ++
 include/block/nvme.h |  8 
 5 files changed, 50 insertions(+), 2 deletions(-)

-- 
2.17.1

[RFC PATCH 1/5] hw/block/nvme: add controller id parameter

There is Contrller ID field in Identify Controller data structure and
nvme device has never set this field, 0 by default.

Added a parameter for controller identifier in a NVM subsystem.  This is
reflected to Identify Controller data structrue of the controller.  This
parameter is helpful when a user wants to set up multi-controller in a
NVM subsystem.

Signed-off-by: Minwoo Im 
---
 hw/block/nvme.c | 10 ++
 hw/block/nvme.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index cf0fe28fe6eb..132e61c0ee7b 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -23,6 +23,7 @@
  *  max_ioqpairs=, \
  *  aerl=, aer_max_queued=, \
  *  mdts=,zoned.append_size_limit= \
+ *  cntlid=
  *  -device nvme-ns,drive=,bus=,nsid=,\
  *  zoned=
  *
@@ -50,6 +51,12 @@
  *   completion when there are no outstanding AERs. When the maximum number of
  *   enqueued events are reached, subsequent events will be dropped.
  *
+ * - `cntlid`
+ *   NVM subsystem unique controller identifier (default: 0).  This property
+ *   is used if a user wants to set up multi-controller in a NVM subsystem.
+ *   This value will be reported through Identify Controller data structure
+ *   with a field named CNTLID[79:78].
+ *
  * - `zoned.append_size_limit`
  *   The maximum I/O size in bytes that is allowed in Zone Append command.
  *   The default is 128KiB. Since internally this this value is maintained as
@@ -4275,6 +4282,8 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 
 n->bar.vs = NVME_SPEC_VER;
 n->bar.intmc = n->bar.intms = 0;
+
+id->cntlid = n->params.cntlid;
 }
 
 static void nvme_realize(PCIDevice *pci_dev, Error **errp)
@@ -4345,6 +4354,7 @@ static Property nvme_props[] = {
 DEFINE_BLOCK_PROPERTIES(NvmeCtrl, namespace.blkconf),
 DEFINE_PROP_LINK("pmrdev", NvmeCtrl, pmrdev, TYPE_MEMORY_BACKEND,
  HostMemoryBackend *),
+DEFINE_PROP_UINT32("cntlid", NvmeCtrl, params.cntlid, 0),
 DEFINE_PROP_STRING("serial", NvmeCtrl, params.serial),
 DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0),
 DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0),
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index b7fbcca39d9f..6aa9e89ac5a8 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -11,6 +11,7 @@
 
 typedef struct NvmeParams {
 char *serial;
+uint32_t cntlid;
 uint32_t num_queues; /* deprecated since 5.1 */
 uint32_t max_ioqpairs;
 uint16_t msix_qsize;
-- 
2.17.1

[RFC PATCH 4/5] nvme: add NMIC enum value for Identify Namespace

Added Namespace Multi-path I/O and Namespace Sharing Capabilities (NMIC)
field to support shared namespace from controller(s).

This field is in Identify Namespace data structure in [30].

Signed-off-by: Minwoo Im 
---
 include/block/nvme.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/block/nvme.h b/include/block/nvme.h
index e83ec1e124c6..dd7b5ba9ef4c 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -1110,6 +1110,10 @@ enum NvmeNsIdentifierType {
 NVME_NIDT_CSI   = 0x04,
 };
 
+enum NvmeNsNmic {
+NVME_NMIC_NS_SHARED = 1 << 0,
+};
+
 enum NvmeCsi {
 NVME_CSI_NVM= 0x00,
 NVME_CSI_ZONED  = 0x02,
-- 
2.17.1

[RFC PATCH 2/5] nvme: add CMIC enum value for Identify Controller

Added Controller Multi-path I/O and Namespace Sharing Capabilities
(CMIC) field to support multi-controller in the following patches.

This field is in Identify Controller data structure in [76].

Signed-off-by: Minwoo Im 
---
 include/block/nvme.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/block/nvme.h b/include/block/nvme.h
index 9494246f1f59..e83ec1e124c6 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -941,6 +941,10 @@ enum NvmeIdCtrlLpa {
 NVME_LPA_EXTENDED = 1 << 2,
 };
 
+enum NvmeIdCtrlCmic {
+NVME_CMIC_MULTI_CTRL= 1 << 1,
+};
+
 #define NVME_CTRL_SQES_MIN(sqes) ((sqes) & 0xf)
 #define NVME_CTRL_SQES_MAX(sqes) (((sqes) >> 4) & 0xf)
 #define NVME_CTRL_CQES_MIN(cqes) ((cqes) & 0xf)
-- 
2.17.1

[RFC PATCH 3/5] hw/block/nvme: add multi-controller param for mpath

Added mpath.ctrl parameter to set multi-controller bit[1] in CMIC field
in Identify Controller data structure.  It will indicate that a NVM
subsystem can have two or more controllers in the subsystem.

To set up multi-controller in a NVM subsystem, user needs to give same
serial parameter to the controllers (-device), but different cntlid
parameter to each controllers (-device).

Example:

  -device nvme,ctrlid=0,serial=foo,...
  -device nvme,ctrlid=1,serial=foo,...

The example above prepares two different controllers in a NVM subsystem
with the same serial which leads to same subsystem NQN.

Signed-off-by: Minwoo Im 
---
 hw/block/nvme.c | 10 ++
 hw/block/nvme.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 132e61c0ee7b..50b349cf9ea3 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -57,6 +57,11 @@
  *   This value will be reported through Identify Controller data structure
  *   with a field named CNTLID[79:78].
  *
+ * - `mpath.ctrl`
+ *   Multi-path I/O with multi-controller (default: false). A NVM subsystem
+ *   can hold two or more controllers.  This will be reflected to Identify
+ *   Controller data structure CMIC[76] field.
+ *
  * - `zoned.append_size_limit`
  *   The maximum I/O size in bytes that is allowed in Zone Append command.
  *   The default is 128KiB. Since internally this this value is maintained as
@@ -4284,6 +4289,10 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 n->bar.intmc = n->bar.intms = 0;
 
 id->cntlid = n->params.cntlid;
+
+if (n->params.mpath_ctrl) {
+id->cmic |= NVME_CMIC_MULTI_CTRL;
+}
 }
 
 static void nvme_realize(PCIDevice *pci_dev, Error **errp)
@@ -4364,6 +4373,7 @@ static Property nvme_props[] = {
 DEFINE_PROP_UINT32("aer_max_queued", NvmeCtrl, params.aer_max_queued, 64),
 DEFINE_PROP_UINT8("mdts", NvmeCtrl, params.mdts, 7),
 DEFINE_PROP_BOOL("use-intel-id", NvmeCtrl, params.use_intel_id, false),
+DEFINE_PROP_BOOL("mpath.ctrl", NvmeCtrl, params.mpath_ctrl, false),
 DEFINE_PROP_SIZE32("zoned.append_size_limit", NvmeCtrl, params.zasl_bs,
NVME_DEFAULT_MAX_ZA_SIZE),
 DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 6aa9e89ac5a8..73c9c2cff247 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -21,6 +21,7 @@ typedef struct NvmeParams {
 uint8_t  mdts;
 bool use_intel_id;
 uint32_t zasl_bs;
+bool mpath_ctrl;
 } NvmeParams;
 
 typedef struct NvmeAsyncEvent {
-- 
2.17.1

[RFC PATCH 5/5] hw/block/nvme: add namespace sharing param for mpath

Added mpath.ns parameter to set multi-namespace bit[0] in NMIC field in
Identify Namespace data structure.  It will indicate that the namespace
can be shared by two or more controllers.

Example:

  To share the namespace between two controllers in a NVM subsystem,
first multi-controllers should be prepared with the same serial:

  -device nvme,id=nvme0,ctrlid=0,serial=foo,...
  -device nvme,id=nvme1,ctrlid=1,serial=foo,...

Then, we can prepare namespace device with mpath.ns enabled.  Also, we
must give the same UUID for those two devices with the same Namespace
ID.

  -device nvme-ns,uuid=,bus=nvme0,nsid=1,...
  -device nvme-ns,uuid=,bus=nvme1,nsid=1,...

Signed-off-by: Minwoo Im 
---
 hw/block/nvme-ns.c | 14 --
 hw/block/nvme-ns.h |  2 ++
 hw/block/nvme.c|  6 ++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index 274eaf61b721..cedacda9ebab 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -32,13 +32,18 @@
 
 #define MIN_DISCARD_GRANULARITY (4 * KiB)
 
-static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
+static int nvme_ns_init(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
 {
 BlockDriverInfo bdi;
 NvmeIdNs *id_ns = &ns->id_ns;
 int lba_index = NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas);
 int npdg;
 
+if (!n->params.mpath_ctrl && ns->params.mpath_ns) {
+error_setg(errp, "mpath.ctrl must be enabled for ns sharing");
+return -1;
+}
+
 ns->id_ns.dlfeat = 0x9;
 
 id_ns->lbaf[lba_index].ds = 31 - clz32(ns->blkconf.logical_block_size);
@@ -63,6 +68,10 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
 
 id_ns->npda = id_ns->npdg = npdg - 1;
 
+if (ns->params.mpath_ns) {
+id_ns->nmic |= NVME_NMIC_NS_SHARED;
+}
+
 return 0;
 }
 
@@ -314,7 +323,7 @@ int nvme_ns_setup(NvmeCtrl *n, NvmeNamespace *ns, Error 
**errp)
 return -1;
 }
 
-if (nvme_ns_init(ns, errp)) {
+if (nvme_ns_init(n, ns, errp)) {
 return -1;
 }
 if (ns->params.zoned) {
@@ -371,6 +380,7 @@ static Property nvme_ns_props[] = {
 DEFINE_BLOCK_PROPERTIES(NvmeNamespace, blkconf),
 DEFINE_PROP_UINT32("nsid", NvmeNamespace, params.nsid, 0),
 DEFINE_PROP_UUID("uuid", NvmeNamespace, params.uuid),
+DEFINE_PROP_BOOL("mpath.ns", NvmeNamespace, params.mpath_ns, false),
 DEFINE_PROP_BOOL("zoned", NvmeNamespace, params.zoned, false),
 DEFINE_PROP_SIZE("zoned.zone_size", NvmeNamespace, params.zone_size_bs,
  NVME_DEFAULT_ZONE_SIZE),
diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
index f8f3c28c360b..d1f2518f7210 100644
--- a/hw/block/nvme-ns.h
+++ b/hw/block/nvme-ns.h
@@ -29,6 +29,8 @@ typedef struct NvmeNamespaceParams {
 uint32_t nsid;
 QemuUUID uuid;
 
+bool mpath_ns;
+
 bool zoned;
 bool cross_zone_read;
 uint64_t zone_size_bs;
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 50b349cf9ea3..e4aa15345c12 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -62,6 +62,12 @@
  *   can hold two or more controllers.  This will be reflected to Identify
  *   Controller data structure CMIC[76] field.
  *
+ * Setting `mpath.ctrl` to true enables the following properties to configure
+ * multi-path for a namespace:
+ * mpath.ns=
+ * If set to true, namespace sharing in a NVM subsystem is enabled.
+ * This will be reflected to Identify Namespace data structure.
+ *
  * - `zoned.append_size_limit`
  *   The maximum I/O size in bytes that is allowed in Zone Append command.
  *   The default is 128KiB. Since internally this this value is maintained as
-- 
2.17.1

Re: [PATCH v6 08/11] iotests: add testenv.py


15.01.2021 14:18, Kevin Wolf wrote:

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/qemu-iotests/testenv.py | 328 ++
  1 file changed, 328 insertions(+)
  create mode 100755 tests/qemu-iotests/testenv.py

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
new file mode 100755
index 00..ecaf76fb85
--- /dev/null
+++ b/tests/qemu-iotests/testenv.py
@@ -0,0 +1,328 @@
+#!/usr/bin/env python3
+#
+# Parse command line options to manage test environment variables.
+#
+# Copyright (c) 2020 Virtuozzo International GmbH
+#
+# 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 .
+#
+
+import os
+import sys
+import tempfile
+from pathlib import Path
+import shutil
+import collections
+import subprocess
+import argparse
+from typing import List, Dict
+
+
+def get_default_machine(qemu_prog: str) -> str:
+outp = subprocess.run([qemu_prog, '-machine', 'help'], check=True,
+  text=True, stdout=subprocess.PIPE).stdout
+
+machines = outp.split('\n')
+default_machine = next(m for m in machines if m.endswith(' (default)'))
+default_machine = default_machine.split(' ', 1)[0]
+
+alias_suf = ' (alias of {})'.format(default_machine)
+alias = next((m for m in machines if m.endswith(alias_suf)), None)
+if alias is not None:
+default_machine = alias.split(' ', 1)[0]
+
+return default_machine
+
+
+class TestEnv:
+"""
+Manage system environment for running tests
+
+The following variables are supported/provided. They are represented by
+lower-cased TestEnv attributes.
+"""
+env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
+ 'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
+ 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
+ 'SOCKET_SCM_HELPER', 'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
+ 'QEMU_IO_OPTIONS', 'QEMU_NBD_OPTIONS', 'IMGOPTS',
+ 'IMGFMT', 'IMGPROTO', 'AIOMODE', 'CACHEMODE',
+ 'VALGRIND_QEMU', 'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC',
+ 'IMGOPTSSYNTAX', 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE']
+
+def get_env(self) -> Dict[str, str]:
+env = {}
+for v in self.env_variables:
+val = getattr(self, v.lower(), None)
+if val is not None:
+env[v] = val
+
+return env
+
+_argparser = None
+@classmethod
+def get_argparser(cls) -> argparse.ArgumentParser:
+if cls._argparser is not None:
+return cls._argparser
+
+p = argparse.ArgumentParser(description="= test environment options =",
+add_help=False, usage=argparse.SUPPRESS)
+
+p.add_argument('-d', dest='debug', action='store_true', help='debug')
+p.add_argument('-misalign', action='store_true',
+   help='misalign memory allocations')
+
+p.set_defaults(imgfmt='raw', imgproto='file')
+
+format_list = ['raw', 'bochs', 'cloop', 'parallels', 'qcow', 'qcow2',
+   'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 'dmg']
+g = p.add_argument_group(
+'image format options',
+'The following options sets IMGFMT environment variable. '


s/sets/set the/


+'At most one chose is allowed, default is "raw"')


s/chose/choice/


+g = g.add_mutually_exclusive_group()
+for fmt in format_list:
+g.add_argument('-' + fmt, dest='imgfmt', action='store_const',
+   const=fmt)
+
+protocol_list = ['file', 'rbd', 'sheepdoc', 'nbd', 'ssh', 'nfs',
+ 'fuse']
+g = p.add_argument_group(
+'image protocol options',
+'The following options sets IMGPROTO environment variably. '
+'At most one chose is allowed, default is "file"')


Same as above, but also s/variably/variable/.

Do we consider these environment var

[PATCH] hw/block/nvme: error if drive less than a zone size

If a user gives backing device file less than a single zone size, the
namespace capacity will be reported to 0 and the kerenl will fail to
allocate namespace silently.

This patch errors in case that num_zones are 0 which is backing device
is less than a single zone size.

Signed-off-by: Minwoo Im 
---
 hw/block/nvme-ns.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index 274eaf61b721..98690d030379 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -137,6 +137,13 @@ static int nvme_ns_zoned_check_calc_geometry(NvmeNamespace 
*ns, Error **errp)
 ns->num_zones = ns->size / lbasz / ns->zone_size;
 
 /* Do a few more sanity checks of ZNS properties */
+if (!ns->num_zones) {
+error_setg(errp,
+   "num_zone is 0, drive must be larger than a zone %luB",
+   zone_size);
+return -1;
+}
+
 if (ns->params.max_open_zones > ns->num_zones) {
 error_setg(errp,
"max_open_zones value %u exceeds the number of zones %u",
-- 
2.17.1

Re: [PATCH] fuzz: Add virtio-9p configurations for fuzzing

2021-01-15 Thread Greg Kurz

On Thu, 14 Jan 2021 17:17:48 -0500
Alexander Bulekov  wrote:

> Signed-off-by: Alexander Bulekov 
> ---

No changelog at all ? 

>  tests/qtest/fuzz/generic_fuzz_configs.h | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h 
> b/tests/qtest/fuzz/generic_fuzz_configs.h
> index 7fed035345..ffdb590c58 100644
> --- a/tests/qtest/fuzz/generic_fuzz_configs.h
> +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
> @@ -59,6 +59,18 @@ const generic_fuzz_config predefined_configs[] = {
>  .name = "virtio-mouse",
>  .args = "-machine q35 -nodefaults -device virtio-mouse",
>  .objects = "virtio*",
> +},{
> +.name = "virtio-9p",
> +.args = "-machine q35 -nodefaults "
> +"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
> +"-fsdev local,id=hshare,path=/tmp/,security_model=none",

Sharing a general purpose directory like "/tmp" is definitely not a
recommended practice. This is typically the kind of thing that I'd
like to see documented in the changelog to help me understand ;-)

What operations does the fuzz test perform on the device ?

> +.objects = "virtio*",
> +},{
> +.name = "virtio-9p-synth",
> +.args = "-machine q35 -nodefaults "
> +"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
> +"-fsdev synth,id=hshare",
> +.objects = "virtio*",

Not sure this is super useful since the only known use case for
the synth fsdev driver is running the virtio-9p qtest, but
it looks fine anyway.

>  },{
>  .name = "e1000",
>  .args = "-M q35 -nodefaults "

Re: [PATCH 0/9] hw/block: m25p80: Fix the mess of dummy bytes needed for fast read commands

2021-01-15 Thread Francisco Iglesias

Hi Bin,

On [2021 Jan 15] Fri 10:07:52, Bin Meng wrote:
> Hi Francisco,
> 
> On Fri, Jan 15, 2021 at 2:13 AM Francisco Iglesias
>  wrote:
> >
> > Hi Bin,
> >
> > On [2021 Jan 14] Thu 23:08:53, Bin Meng wrote:
> > > From: Bin Meng 
> > >
> > > The m25p80 model uses s->needed_bytes to indicate how many follow-up
> > > bytes are expected to be received after it receives a command. For
> > > example, depending on the address mode, either 3-byte address or
> > > 4-byte address is needed.
> > >
> > > For fast read family commands, some dummy cycles are required after
> > > sending the address bytes, and the dummy cycles need to be counted
> > > in s->needed_bytes. This is where the mess began.
> > >
> > > As the variable name (needed_bytes) indicates, the unit is in byte.
> > > It is not in bit, or cycle. However for some reason the model has
> > > been using the number of dummy cycles for s->needed_bytes. The right
> > > approach is to convert the number of dummy cycles to bytes based on
> > > the SPI protocol, for example, 6 dummy cycles for the Fast Read Quad
> > > I/O (EBh) should be converted to 3 bytes per the formula (6 * 4 / 8).
> >
> > While not being the original implementor I must assume that above solution 
> > was
> > considered but not chosen by the developers due to it is inaccuracy (it
> > wouldn't be possible to model exacly 6 dummy cycles, only a multiple of 8,
> > meaning that if the controller is wrongly programmed to generate 7 the error
> > wouldn't be caught and the controller will still be considered "correct"). 
> > Now
> > that we have this detail in the implementation I'm in favor of keeping it, 
> > this
> > also because the detail is already in use for catching exactly above error.
> >
> 
> I found no clue from the commit message that my proposed solution here
> was ever considered, otherwise all SPI controller models supporting
> software generation should have been found out seriously broken long
> time ago!


The controllers you are referring to might lack support for commands requiring
dummy clock cycles but I really hope they work with the other commands? If so I
don't think it is fair to call them 'seriously broken' (and else we should
probably let the maintainers know about it). Most likely the lack of support
for the commands is because no request has been made for them. Also there is
one controller that has support.


> 
> The issue you pointed out that we require the total number of dummy
> bits should be multiple of 8 is true, that's why I added the
> unimplemented log message in this series (patch 2/3/4) to warn users
> if this expectation is not met. However this will not cause any issue
> when running U-Boot or Linux, because both spi-nor drivers expect the
> same assumption as we do here.
> 
> See U-Boot spi_nor_read_data() and Linux spi_nor_spimem_read_data(),
> there is a logic to calculate the dummy bytes needed for fast read
> command:
> 
> /* convert the dummy cycles to the number of bytes */
> op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
> 
> Note the default dummy cycles configuration for all flashes I have
> looked into as of today, meets the multiple of 8 assumption. On some
> flashes the dummy cycle number is configurable, and if it's been
> configured to be an odd value, it would not work on U-Boot/Linux in
> the first place.
> 
> > >
> > > Things get complicated when interacting with different SPI or QSPI
> > > flash controllers. There are major two cases:
> > >
> > > - Dummy bytes prepared by drivers, and wrote to the controller fifo.
> > >   For such case, driver will calculate the correct number of dummy
> > >   bytes and write them into the tx fifo. Fixing the m25p80 model will
> > >   fix flashes working with such controllers.
> >
> > Above can be fixed while still keeping the detailed dummy cycle 
> > implementation
> > inside m25p80. Perhaps one of the following could be looked into: 
> > configurating
> > the amount, letting the spi ctrl fetch the amount from m25p80 or by 
> > inheriting
> > some functionality handling this in the SPI controller. Or a mixture of 
> > above.
> 
> Please send patches to explain this in detail how this is going to
> work. I am open to all possible solutions.

In that case I suggest that you instead try with a device property
'model_dummy_bytes' used to select to convert the accurate dummy clock cycle
count to dummy bytes inside m25p80. Below is an example on how to modify the
decode_fast_read_cmd function (the other commands requiring dummy clock cycles
can follow a similar pattern). This way the fifo mode will be able to work the
way you desire while also keeping the current functionality intact. Suddenly
removing functionality (features) will take users by surprise. 


static void decode_fast_read_cmd(Flash *s)
{
uint8_t dummy_clk_cycles = 0;
uint8_t extra_bytes;

s->needed_bytes = get_addr_length(s);

/* Obtain the number of dummy clock cycles needed */
switch (get_man(s))

Re: [PATCH v3 02/10] iotests/297: Rewrite in Python and extend reach


15.01.2021 14:57, Max Reitz wrote:

On 15.01.21 12:13, Vladimir Sementsov-Ogievskiy wrote:

14.01.2021 20:02, Max Reitz wrote:

Instead of checking iotests.py only, check all Python files in the
qemu-iotests/ directory.  Of course, most of them do not pass, so there
is an extensive skip list for now.  (The only files that do pass are
209, 254, 283, and iotests.py.)

(Alternatively, we could have the opposite, i.e. an explicit list of
files that we do want to check, but I think it is better to check files
by default.)

Unless started in debug mode (./check -d), the output has no information
on which files are tested, so we will not have a problem e.g. with
backports, where some files may be missing when compared to upstream.

Besides the technical rewrite, some more things are changed:

- For the pylint invocation, PYTHONPATH is adjusted.  This mirrors
   setting MYPYPATH for mypy.

- Also, MYPYPATH is now derived from PYTHONPATH, so that we include
   paths set by the environment.  Maybe at some point we want to let the
   check script add '../../python/' to PYTHONPATH so that iotests.py does
   not need to do that.

- Passing --notes=FIXME,XXX to pylint suppresses warnings for TODO
   comments.  TODO is fine, we do not need 297 to complain about such
   comments.

- The "Success" line from mypy's output is suppressed, because (A) it
   does not add useful information, and (B) it would leak information
   about the files having been tested to the reference output, which we
   decidedly do not want.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/297 | 109 +
  tests/qemu-iotests/297.out |   5 +-
  2 files changed, 89 insertions(+), 25 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 5c5420712b..bfa26d280b 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/usr/bin/env python3
  #
  # Copyright (C) 2020 Red Hat, Inc.
  #
@@ -15,30 +15,95 @@
  # You should have received a copy of the GNU General Public License
  # along with this program.  If not, see .
-seq=$(basename $0)
-echo "QA output created by $seq"
+import os
+import re
+import shutil
+import subprocess
+import sys
-status=1    # failure is the default!
+import iotests
-# get standard environment
-. ./common.rc
-if ! type -p "pylint-3" > /dev/null; then
-    _notrun "pylint-3 not found"
-fi
-if ! type -p "mypy" > /dev/null; then
-    _notrun "mypy not found"
-fi
+# TODO: Empty this list!
+SKIP_FILES = (
+    '030', '040', '041', '044', '045', '055', '056', '057', '065', '093',
+    '096', '118', '124', '129', '132', '136', '139', '147', '148', '149',
+    '151', '152', '155', '163', '165', '169', '194', '196', '199', '202',
+    '203', '205', '206', '207', '208', '210', '211', '212', '213', '216',
+    '218', '219', '222', '224', '228', '234', '235', '236', '237', '238',
+    '240', '242', '245', '246', '248', '255', '256', '257', '258', '260',
+    '262', '264', '266', '274', '277', '280', '281', '295', '296', '298',
+    '299', '300', '302', '303', '304', '307',
+    'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py'
+)
-pylint-3 --score=n iotests.py
-MYPYPATH=../../python/ mypy --warn-unused-configs --disallow-subclassing-any \
-    --disallow-any-generics --disallow-incomplete-defs \
-    --disallow-untyped-decorators --no-implicit-optional \
-    --warn-redundant-casts --warn-unused-ignores \
-    --no-implicit-reexport iotests.py
+def is_python_file(filename):
+    if not os.path.isfile(filename):
+    return False
-# success, all done
-echo "*** done"
-rm -f $seq.full
-status=0
+    if filename.endswith('.py'):
+    return True
+
+    with open(filename) as f:
+    try:
+    first_line = f.readline()
+    return re.match('^#!.*python', first_line) is not None
+    except UnicodeDecodeError: # Ignore binary files


PEP8 asks for two spaces before inline comment


Wow.  I really hate PEP8.


Wow, it's unexpected :) I like it since first meet..




+    return False
+


and two empty lines here

(good ALE vim plugin runs flake8, mypy and pylint asynchronously for me and 
shows these things)


I’d like to argue that that isn’t good, but I need to quench my anger. Perhaps 
one day I can quench it sufficiently to install ALE myself.


+def run_linters():
+    files = [filename for filename in (set(os.listdir('.')) - set(SKIP_FILES))
+ if is_python_file(filename)]
+
+    iotests.logger.debug('Files to be checked:')
+    iotests.logger.debug(', '.join(sorted(files)))


O, good.


+
+    print('=== pylint ===')
+    sys.stdout.flush()


Should not hurt.. But why? Should be flushed anyway as print will put a '\n'.. 
And why you care about it at all?


When pylint fails, I can see its result above the '=== pylint ===' line, even 
though its output is on stdout.  I suspect the Python output

Re: [PATCH v6 08/11] iotests: add testenv.py

2021-01-15 Thread Kevin Wolf

Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 15.01.2021 14:18, Kevin Wolf wrote:
> > Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > Add TestEnv class, which will handle test environment in a new python
> > > iotests running framework.
> > > 
> > > Difference with current ./check interface:
> > > - -v (verbose) option dropped, as it is unused
> > > 
> > > - -xdiff option is dropped, until somebody complains that it is needed
> > > - same for -n option
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > > ---
> > >   tests/qemu-iotests/testenv.py | 328 ++
> > >   1 file changed, 328 insertions(+)
> > >   create mode 100755 tests/qemu-iotests/testenv.py
> > > 
> > > diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> > > new file mode 100755
> > > index 00..ecaf76fb85
> > > --- /dev/null
> > > +++ b/tests/qemu-iotests/testenv.py
> > > @@ -0,0 +1,328 @@
> > > +#!/usr/bin/env python3
> > > +#
> > > +# Parse command line options to manage test environment variables.
> > > +#
> > > +# Copyright (c) 2020 Virtuozzo International GmbH
> > > +#
> > > +# 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 .
> > > +#
> > > +
> > > +import os
> > > +import sys
> > > +import tempfile
> > > +from pathlib import Path
> > > +import shutil
> > > +import collections
> > > +import subprocess
> > > +import argparse
> > > +from typing import List, Dict
> > > +
> > > +
> > > +def get_default_machine(qemu_prog: str) -> str:
> > > +outp = subprocess.run([qemu_prog, '-machine', 'help'], check=True,
> > > +  text=True, stdout=subprocess.PIPE).stdout
> > > +
> > > +machines = outp.split('\n')
> > > +default_machine = next(m for m in machines if m.endswith(' 
> > > (default)'))
> > > +default_machine = default_machine.split(' ', 1)[0]
> > > +
> > > +alias_suf = ' (alias of {})'.format(default_machine)
> > > +alias = next((m for m in machines if m.endswith(alias_suf)), None)
> > > +if alias is not None:
> > > +default_machine = alias.split(' ', 1)[0]
> > > +
> > > +return default_machine
> > > +
> > > +
> > > +class TestEnv:
> > > +"""
> > > +Manage system environment for running tests
> > > +
> > > +The following variables are supported/provided. They are represented 
> > > by
> > > +lower-cased TestEnv attributes.
> > > +"""
> > > +env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 
> > > 'SAMPLE_IMG_DIR',
> > > + 'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 
> > > 'QEMU_IMG_PROG',
> > > + 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
> > > + 'SOCKET_SCM_HELPER', 'QEMU_OPTIONS', 
> > > 'QEMU_IMG_OPTIONS',
> > > + 'QEMU_IO_OPTIONS', 'QEMU_NBD_OPTIONS', 'IMGOPTS',
> > > + 'IMGFMT', 'IMGPROTO', 'AIOMODE', 'CACHEMODE',
> > > + 'VALGRIND_QEMU', 'CACHEMODE_IS_DEFAULT', 
> > > 'IMGFMT_GENERIC',
> > > + 'IMGOPTSSYNTAX', 'IMGKEYSECRET', 
> > > 'QEMU_DEFAULT_MACHINE']
> > > +
> > > +def get_env(self) -> Dict[str, str]:
> > > +env = {}
> > > +for v in self.env_variables:
> > > +val = getattr(self, v.lower(), None)
> > > +if val is not None:
> > > +env[v] = val
> > > +
> > > +return env
> > > +
> > > +_argparser = None
> > > +@classmethod
> > > +def get_argparser(cls) -> argparse.ArgumentParser:
> > > +if cls._argparser is not None:
> > > +return cls._argparser
> > > +
> > > +p = argparse.ArgumentParser(description="= test environment 
> > > options =",
> > > +add_help=False, 
> > > usage=argparse.SUPPRESS)
> > > +
> > > +p.add_argument('-d', dest='debug', action='store_true', 
> > > help='debug')
> > > +p.add_argument('-misalign', action='store_true',
> > > +   help='misalign memory allocations')
> > > +
> > > +p.set_defaults(imgfmt='raw', imgproto='file')
> > > +
> > > +format_list = ['raw', 'bochs', 'cloop', 'parallels', 'qcow', 
> > > 'qcow2',
> > > +   'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 
> > > 'dmg']
> > > +g = p.add_argumen

Re: [PATCH] fuzz: Add virtio-9p configurations for fuzzing

2021-01-15 Thread Christian Schoenebeck via

On Freitag, 15. Januar 2021 13:23:08 CET Greg Kurz wrote:
> On Thu, 14 Jan 2021 17:17:48 -0500
> 
> Alexander Bulekov  wrote:
> > Signed-off-by: Alexander Bulekov 
> > ---
> 
> No changelog at all ?

Yeah, that's indeed quite short. :)

> >  tests/qtest/fuzz/generic_fuzz_configs.h | 12 
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h
> > b/tests/qtest/fuzz/generic_fuzz_configs.h index 7fed035345..ffdb590c58
> > 100644
> > --- a/tests/qtest/fuzz/generic_fuzz_configs.h
> > +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
> > @@ -59,6 +59,18 @@ const generic_fuzz_config predefined_configs[] = {
> > 
> >  .name = "virtio-mouse",
> >  .args = "-machine q35 -nodefaults -device virtio-mouse",
> >  .objects = "virtio*",
> > 
> > +},{
> > +.name = "virtio-9p",
> > +.args = "-machine q35 -nodefaults "
> > +"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
> > +"-fsdev local,id=hshare,path=/tmp/,security_model=none",
> 
> Sharing a general purpose directory like "/tmp" is definitely not a
> recommended practice. This is typically the kind of thing that I'd
> like to see documented in the changelog to help me understand ;-)

For the (non fuzzing) 9p 'local' tests Peter wanted either an auto generated 
(e.g. mkdtemp()) or at least a hard coded test path that allows a person to 
easily identify where the data came from. So I guess that applies to fuzzing 
as well, i.e. something like "/tmp/qemu-fuzz/9pfs/" at least.

Also note that the existing (non fuzzing) 9p 'local' tests auto generate  
individual test pathes with mkdtemp() already. This was necessary there, 
because tests are often run by "make -jN ..." in which case tests were 
accessing concurrently the same single test directory before. Probably less of 
a problem here, but you might consider using the same approach that
virtio-9p-test.c already does.

Also note that 'security_model=none' is maybe Ok as a starting point for 
fuzzing, but a realistic 9p config is rather 'security_model=xattr', because 
'security_model=none' requires the qemu process to be run as root to avoid 
permission denied errors with any minor operation. I would expect these 
fuzzing tests to mostly error out with permission denied errors early instead 
of entering relevant execution pathes.

> What operations does the fuzz test perform on the device ?
> 
> > +.objects = "virtio*",
> > +},{
> > +.name = "virtio-9p-synth",
> > +.args = "-machine q35 -nodefaults "
> > +"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
> > +"-fsdev synth,id=hshare",
> > +.objects = "virtio*",
> 
> Not sure this is super useful since the only known use case for
> the synth fsdev driver is running the virtio-9p qtest, but
> it looks fine anyway.

Yeah, that's ok. Maybe it raises the chance to enter some execution pathes 
here and there. So I would keep the 'synth' driver config.

> 
> >  },{
> >  
> >  .name = "e1000",
> >  .args = "-M q35 -nodefaults "

Nice to see fuzzing coming for 9p BTW!

Best regards,
Christian Schoenebeck

Re: [PATCH v7 03/13] sev: Remove false abstraction of flash encryption

On Thu, 14 Jan 2021 10:58:01 +1100
David Gibson  wrote:

> When AMD's SEV memory encryption is in use, flash memory banks (which are
> initialed by pc_system_flash_map()) need to be encrypted with the guest's
> key, so that the guest can read them.
> 
> That's abstracted via the kvm_memcrypt_encrypt_data() callback in the KVM
> state.. except, that it doesn't really abstract much at all.
> 
> For starters, the only called is in code specific to the 'pc' family of

s/called/call site/

> machine types, so it's obviously specific to those and to x86 to begin
> with.  But it makes a bunch of further assumptions that need not be true
> about an arbitrary confidential guest system based on memory encryption,
> let alone one based on other mechanisms:
> 
>  * it assumes that the flash memory is defined to be encrypted with the
>guest key, rather than being shared with hypervisor
>  * it assumes that that hypervisor has some mechanism to encrypt data into
>the guest, even though it can't decrypt it out, since that's the whole
>point
>  * the interface assumes that this encrypt can be done in place, which
>implies that the hypervisor can write into a confidential guests's
>memory, even if what it writes isn't meaningful
> 
> So really, this "abstraction" is actually pretty specific to the way SEV
> works.  So, this patch removes it and instead has the PC flash
> initialization code call into a SEV specific callback.
> 
> Signed-off-by: David Gibson 
> ---
>  accel/kvm/kvm-all.c| 31 ++-
>  accel/kvm/sev-stub.c   |  9 ++---
>  accel/stubs/kvm-stub.c | 10 --
>  hw/i386/pc_sysfw.c | 17 ++---
>  include/sysemu/kvm.h   | 16 
>  include/sysemu/sev.h   |  4 ++--
>  target/i386/sev-stub.c |  5 +
>  target/i386/sev.c  | 24 ++--
>  8 files changed, 31 insertions(+), 85 deletions(-)

Reviewed-by: Cornelia Huck

Re: [PATCH v7 04/13] confidential guest support: Move side effect out of machine_set_memory_encryption()

On Thu, 14 Jan 2021 10:58:02 +1100
David Gibson  wrote:

> When the "memory-encryption" property is set, we also disable KSM
> merging for the guest, since it won't accomplish anything.
> 
> We want that, but doing it in the property set function itself is
> thereoretically incorrect, in the unlikely event of some configuration
> environment that set the property then cleared it again before
> constructing the guest.
> 
> More importantly, it makes some other cleanups we want more difficult.
> So, instead move this logic to machine_run_board_init() conditional on
> the final value of the property.
> 
> Signed-off-by: David Gibson 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Greg Kurz 
> ---
>  hw/core/machine.c | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)

Reviewed-by: Cornelia Huck

[RFC PATCH 1/2] block: Allow changing bs->file on reopen

2021-01-15 Thread Alberto Garcia

When the x-blockdev-reopen was added it allowed reconfiguring the
graph by replacing backing files, but changing the 'file' option was
forbidden. Because of this restriction some operations are not
possible, notably inserting and removing block filters.

This patch adds support for replacing the 'file' option. This is
similar to replacing the backing file and the user is likewise
responsible for the correctness of the resulting graph, otherwise this
can lead to data corruption.

Signed-off-by: Alberto Garcia 
---
 include/block/block.h  |  1 +
 block.c| 61 ++
 tests/qemu-iotests/245 |  7 ++---
 3 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 82271d9ccd..6dd687a69e 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -196,6 +196,7 @@ typedef struct BDRVReopenState {
 bool backing_missing;
 bool replace_backing_bs;  /* new_backing_bs is ignored if this is false */
 BlockDriverState *old_backing_bs; /* keep pointer for permissions update */
+BlockDriverState *old_file_bs;/* keep pointer for permissions update */
 uint64_t perm, shared_perm;
 QDict *options;
 QDict *explicit_options;
diff --git a/block.c b/block.c
index 576b145cbf..114788e58e 100644
--- a/block.c
+++ b/block.c
@@ -3978,6 +3978,10 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, 
Error **errp)
 refresh_list = bdrv_topological_dfs(refresh_list, found,
 state->old_backing_bs);
 }
+if (state->old_file_bs) {
+refresh_list = bdrv_topological_dfs(refresh_list, found,
+state->old_file_bs);
+}
 }
 
 ret = bdrv_list_refresh_perms(refresh_list, bs_queue, &tran, errp);
@@ -4196,6 +4200,57 @@ static int bdrv_reopen_parse_backing(BDRVReopenState 
*reopen_state,
 return 0;
 }
 
+static int bdrv_reopen_parse_file(BDRVReopenState *reopen_state,
+  GSList **tran,
+  Error **errp)
+{
+BlockDriverState *bs = reopen_state->bs;
+BlockDriverState *new_file_bs;
+QObject *value;
+const char *str;
+
+value = qdict_get(reopen_state->options, "file");
+if (value == NULL) {
+return 0;
+}
+
+/* The 'file' option only allows strings */
+assert(qobject_type(value) == QTYPE_QSTRING);
+
+str = qobject_get_try_str(value);
+new_file_bs = bdrv_lookup_bs(NULL, str, errp);
+if (new_file_bs == NULL) {
+return -EINVAL;
+} else if (bdrv_recurse_has_child(new_file_bs, bs)) {
+error_setg(errp, "Making '%s' a file of '%s' "
+   "would create a cycle", str, bs->node_name);
+return -EINVAL;
+}
+
+assert(bs->file && bs->file->bs);
+
+/* If 'file' points to the current child then there's nothing to do */
+if (bs->file->bs == new_file_bs) {
+return 0;
+}
+
+/* Check AioContext compatibility */
+if (!bdrv_reopen_can_attach(bs, bs->file, new_file_bs, errp)) {
+return -EINVAL;
+}
+
+/* At the moment only backing links are frozen */
+assert(!bs->file->frozen);
+
+/* Store the old file bs because we'll need to refresh its permissions */
+reopen_state->old_file_bs = bs->file->bs;
+
+/* And finally replace the child */
+bdrv_replace_child(bs->file, new_file_bs, tran);
+
+return 0;
+}
+
 /*
  * Prepares a BlockDriverState for reopen. All changes are staged in the
  * 'opaque' field of the BDRVReopenState, which is used and allocated by
@@ -4347,6 +4402,12 @@ static int bdrv_reopen_prepare(BDRVReopenState 
*reopen_state,
 }
 qdict_del(reopen_state->options, "backing");
 
+ret = bdrv_reopen_parse_file(reopen_state, set_backings_tran, errp);
+if (ret < 0) {
+goto error;
+}
+qdict_del(reopen_state->options, "file");
+
 /* Options that are not handled are only okay if they are unchanged
  * compared to the old state. It is expected that some options are only
  * used for the initial open, but not reopen (e.g. filename) */
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index e60c8326d3..f9d68b3958 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -145,8 +145,8 @@ class TestBlockdevReopen(iotests.QMPTestCase):
 self.reopen(opts, {'driver': 'raw'}, "Cannot change the option 
'driver'")
 self.reopen(opts, {'driver': ''}, "Invalid parameter ''")
 self.reopen(opts, {'driver': None}, "Invalid parameter type for 
'driver', expected: string")
-self.reopen(opts, {'file': 'not-found'}, "Cannot change the option 
'file'")
-self.reopen(opts, {'file': ''}, "Cannot change the option 'file'")
+self.reopen(opts, {'file': 'not-found'}, "Cannot find device= nor 
node_name=not-found")
+self.reopen(opts, {'file': ''}, "Cannot find devi

[RFC PATCH 0/2] Allow changing bs->file on reopen

2021-01-15 Thread Alberto Garcia

Hi,

during the past months we talked about making x-blockdev-reopen stable
API, and one of the missing things was having support for changing
bs->file. See here for the discusssion (I can't find the message from
Kashyap that started the thread in the web archives):

   https://lists.gnu.org/archive/html/qemu-block/2020-10/msg00922.html

I was testing this and one of the problems that I found was that
removing a filter node using this command is tricky because of the
permission system, see here for details:

   https://lists.gnu.org/archive/html/qemu-block/2020-12/msg00092.html

The good news is that Vladimir posted a set of patches that changes
the way that permissions are updated on reopen:

   https://lists.gnu.org/archive/html/qemu-block/2020-11/msg00745.html

I was testing if this would be useful to solve the problem that I
mentioned earlier and it seems to be the case so I wrote a patch to
add support for changing bs->file, along with a couple of test cases.

This is still an RFC but you can see the idea.

These patches apply on top of Vladimir's branch:

git: https://src.openvz.org/scm/~vsementsov/qemu.git
tag: up-block-topologic-perm-v2

Opinions are very welcome!

Berto

Alberto Garcia (2):
  block: Allow changing bs->file on reopen
  iotests: Update 245 to support replacing files with x-blockdev-reopen

 include/block/block.h  |  1 +
 block.c| 61 ++
 tests/qemu-iotests/245 | 61 +++---
 tests/qemu-iotests/245.out |  4 +--
 4 files changed, 121 insertions(+), 6 deletions(-)

-- 
2.20.1

[RFC PATCH 2/2] iotests: Update 245 to support replacing files with x-blockdev-reopen

2021-01-15 Thread Alberto Garcia

Signed-off-by: Alberto Garcia 
---
 tests/qemu-iotests/245 | 54 +-
 tests/qemu-iotests/245.out |  4 +--
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index f9d68b3958..bad6911f0c 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -78,7 +78,7 @@ class TestBlockdevReopen(iotests.QMPTestCase):
 for line in log.split("\n"):
 if line.startswith("Pattern verification failed"):
 raise Exception("%s (command #%d)" % (line, found))
-if re.match("read .*/.* bytes at offset", line):
+if re.match("(read|wrote) .*/.* bytes at offset", line):
 found += 1
 self.assertEqual(found, self.total_io_cmds,
  "Expected output of %d qemu-io commands, found %d" %
@@ -536,6 +536,58 @@ class TestBlockdevReopen(iotests.QMPTestCase):
 result = self.vm.qmp('blockdev-del', conv_keys = True, node_name = 
'bv')
 self.assert_qmp(result, 'return', {})
 
+def test_replace_file(self):
+qemu_img('create', '-f', 'raw', hd_path[0], '10k')
+qemu_img('create', '-f', 'raw', hd_path[1], '10k')
+
+hd0_opts = {'driver': 'file',
+'node-name': 'hd0-file',
+'filename':  hd_path[0] }
+hd1_opts = {'driver': 'file',
+'node-name': 'hd1-file',
+'filename':  hd_path[1] }
+
+opts = {'driver': 'raw', 'node-name': 'hd', 'file': 'hd0-file'}
+
+result = self.vm.qmp('blockdev-add', conv_keys = False, **hd0_opts)
+self.assert_qmp(result, 'return', {})
+result = self.vm.qmp('blockdev-add', conv_keys = False, **hd1_opts)
+self.assert_qmp(result, 'return', {})
+result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
+self.assert_qmp(result, 'return', {})
+
+self.run_qemu_io("hd", "read  -P 0 0 10k")
+self.run_qemu_io("hd", "write -P 0xa0 0 10k")
+
+self.reopen(opts, {'file': 'hd1-file'})
+self.run_qemu_io("hd", "read  -P 0 0 10k")
+self.run_qemu_io("hd", "write -P 0xa1 0 10k")
+
+self.reopen(opts, {'file': 'hd0-file'})
+self.run_qemu_io("hd", "read  -P 0xa0 0 10k")
+
+self.reopen(opts, {'file': 'hd1-file'})
+self.run_qemu_io("hd", "read  -P 0xa1 0 10k")
+
+def test_insert_throttle_filter(self):
+hd0_opts = hd_opts(0)
+result = self.vm.qmp('blockdev-add', conv_keys = False, **hd0_opts)
+self.assert_qmp(result, 'return', {})
+
+opts = { 'qom-type': 'throttle-group', 'id': 'group0',
+ 'props': { 'limits': { 'iops-total': 1000 } } }
+result = self.vm.qmp('object-add', conv_keys = False, **opts)
+self.assert_qmp(result, 'return', {})
+
+opts = { 'driver': 'throttle', 'node-name': 'throttle0',
+ 'throttle-group': 'group0', 'file': 'hd0-file' }
+result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
+self.assert_qmp(result, 'return', {})
+
+self.reopen(hd0_opts, {'file': 'throttle0'})
+
+self.reopen(hd0_opts, {'file': 'hd0-file'})
+
 # Misc reopen tests with different block drivers
 @iotests.skip_if_unsupported(['quorum', 'throttle'])
 def test_misc_drivers(self):
diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out
index 4b33dcaf5c..537a2b5b63 100644
--- a/tests/qemu-iotests/245.out
+++ b/tests/qemu-iotests/245.out
@@ -10,8 +10,8 @@
 {"return": {}}
 {"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", 
"timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
 {"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, 
"type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": 
{"microseconds": "USECS", "seconds": "SECS"}}
-.
+...
 --
-Ran 21 tests
+Ran 23 tests
 
 OK
-- 
2.20.1

Re: [PATCH v7 05/13] confidential guest support: Rework the "memory-encryption" property

On Thu, 14 Jan 2021 10:58:03 +1100
David Gibson  wrote:

> Currently the "memory-encryption" property is only looked at once we
> get to kvm_init().  Although protection of guest memory from the
> hypervisor isn't something that could really ever work with TCG, it's
> not conceptually tied to the KVM accelerator.
> 
> In addition, the way the string property is resolved to an object is
> almost identical to how a QOM link property is handled.
> 
> So, create a new "confidential-guest-support" link property which sets
> this QOM interface link directly in the machine.  For compatibility we
> keep the "memory-encryption" property, but now implemented in terms of
> the new property.
> 
> Signed-off-by: David Gibson 
> Reviewed-by: Greg Kurz 
> ---
>  accel/kvm/kvm-all.c  |  5 +++--
>  accel/kvm/sev-stub.c |  5 +++--
>  hw/core/machine.c| 43 +--
>  include/hw/boards.h  |  2 +-
>  include/sysemu/sev.h |  2 +-
>  target/i386/sev.c| 32 ++--
>  6 files changed, 47 insertions(+), 42 deletions(-)

Reviewed-by: Cornelia Huck

[PULL 01/30] tests/docker: Remove Debian 9 remnant lines

From: Philippe Mathieu-Daudé 

Debian 9 base container has been removed in commits
e3755276d1f and c9d78b06c06. Remove the last remnants.

Fixes: e3755276d1f ("tests/docker: Remove old Debian 9 containers")
Signed-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Alex Bennée 
Reviewed-by: Thomas Huth 
Reviewed-by: Willian Rampazzo 
Message-Id: <20210107072933.3828450-1-f4...@amsat.org>
Message-Id: <20210114165730.31607-2-alex.ben...@linaro.org>

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index c254ac38d0..0779dab5b9 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -108,7 +108,6 @@ ifneq ($(HOST_ARCH),x86_64)
 DOCKER_PARTIAL_IMAGES += debian-mips-cross debian-mipsel-cross 
debian-mips64el-cross
 DOCKER_PARTIAL_IMAGES += debian-ppc64el-cross
 DOCKER_PARTIAL_IMAGES += debian-s390x-cross
-DOCKER_PARTIAL_IMAGES += debian-win32-cross debian-win64-cross
 DOCKER_PARTIAL_IMAGES += fedora travis
 endif
 
-- 
2.20.1

[PULL 02/30] Makefile: add GNU global tags support

GNU Global is another tags engine which is more like cscope in being
able to support finding both references and definitions. You will be
un-surprised to know it also integrates well with Emacs.

The main benefit of integrating it into find-src-path is it takes less
time to rebuild the database from scratch when you have a lot of build
directories under your source tree.

Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210114165730.31607-3-alex.ben...@linaro.org>

diff --git a/Makefile b/Makefile
index fb9923ff22..0c509a7704 100644
--- a/Makefile
+++ b/Makefile
@@ -253,6 +253,18 @@ ctags:
rm -f "$(SRC_PATH)/"tags
$(find-src-path) -exec ctags -f "$(SRC_PATH)/"tags --append {} +
 
+.PHONY: gtags
+gtags:
+   $(call quiet-command,   \
+   rm -f "$(SRC_PATH)/"GTAGS;  \
+   rm -f "$(SRC_PATH)/"GRTAGS; \
+   rm -f "$(SRC_PATH)/"GPATH,  \
+   "GTAGS", "Remove old $@ files")
+   $(call quiet-command,   \
+   (cd $(SRC_PATH) &&  \
+$(find-src-path) | gtags -f -),\
+   "GTAGS", "Re-index $(SRC_PATH)")
+
 .PHONY: TAGS
 TAGS:
rm -f "$(SRC_PATH)/"TAGS
@@ -279,7 +291,7 @@ help:
$(call print-help,all,Build all)
$(call print-help,dir/file.o,Build specified target only)
$(call print-help,install,Install QEMU, documentation and tools)
-   $(call print-help,ctags/TAGS,Generate tags file for editors)
+   $(call print-help,ctags/gtags/TAGS,Generate tags file for editors)
$(call print-help,cscope,Generate cscope index)
$(call print-help,sparse,Run sparse on the QEMU source)
@echo  ''
diff --git a/.gitignore b/.gitignore
index b32bca1315..75a4be0724 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,6 +7,9 @@
 cscope.*
 tags
 TAGS
+GPATH
+GRTAGS
+GTAGS
 *~
 *.ast_raw
 *.depend_raw
-- 
2.20.1

[PULL 04/30] Add newline when generating Dockerfile

From: Alessandro Di Federico 

Signed-off-by: Alessandro Di Federico 
Signed-off-by: Alex Bennée 
Reviewed-by: Willian Rampazzo 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <1610080146-14968-36-git-send-email-tsimp...@quicinc.com>
Message-Id: <20210114165730.31607-5-alex.ben...@linaro.org>

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 36b7868406..884dfeb29c 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -332,9 +332,9 @@ class Docker(object):
  (uname, uid, uname))
 
 tmp_df.write("\n")
-tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" % (checksum))
+tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s\n" % (checksum))
 for f, c in extra_files_cksum:
-tmp_df.write("LABEL com.qemu.%s-checksum=%s" % (f, c))
+tmp_df.write("LABEL com.qemu.%s-checksum=%s\n" % (f, c))
 
 tmp_df.flush()
 
-- 
2.20.1

[PULL 00/30] testing, gdbstub and semihosting

The following changes since commit 7c79721606be11b5bc556449e5bcbc331ef6867d:

  Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210113' into 
staging (2021-01-14 09:54:29 +)

are available in the Git repository at:

  https://github.com/stsquad/qemu.git tags/pull-testing-and-misc-150121-1

for you to fetch changes up to be846761ca8b5a7e2e7b7108c8c156126b799824:

  semihosting: Implement SYS_ISERROR (2021-01-15 11:12:34 +)


Testing, gdbstub and semihosting patches:

  - clean-ups to docker images
  - drop duplicate jobs from shippable
  - prettier tag generation (+gtags)
  - generate browsable source tree
  - more Travis->GitLab migrations
  - fix checkpatch to deal with commits
  - gate gdbstub tests on 8.3.1, expand tests
  - support Xfer:auxv:read gdb packet
  - better gdbstub cleanup
  - use GDB's SVE register layout
  - make arm-compat-semihosting common
  - add riscv semihosting support
  - add HEAPINFO, ELAPSED, TICKFREQ, TMPNAM and ISERROR to semihosting


Alessandro Di Federico (1):
  Add newline when generating Dockerfile

Alex Bennée (16):
  Makefile: add GNU global tags support
  Makefile: wrap ctags in quiet-command calls
  Makefile: wrap etags in quiet-command calls
  Makefile: wrap cscope in quiet-command calls
  docker: expand debian-amd64 image to include tag tools
  gitlab: move docs and tools build across from Travis
  gitlab: migrate the minimal tools and unit tests from Travis
  scripts/checkpatch.pl: fix git-show invocation to include diffstat
  test/guest-debug: echo QEMU command as well
  configure: gate our use of GDB to 8.3.1 or above
  Revert "tests/tcg/multiarch/Makefile.target: Disable run-gdbstub-sha1 
test"
  gdbstub: implement a softmmu based test
  gdbstub: drop CPUEnv from gdb_exit()
  gdbstub: drop gdbserver_cleanup in favour of gdb_exit
  gdbstub: ensure we clean-up when terminated
  target/arm: use official org.gnu.gdb.aarch64.sve layout for registers

Keith Packard (8):
  semihosting: Move ARM semihosting code to shared directories
  semihosting: Change common-semi API to be architecture-independent
  semihosting: Change internal common-semi interfaces to use CPUState *
  semihosting: Support SYS_HEAPINFO when env->boot_info is not set
  riscv: Add semihosting support
  semihosting: Implement SYS_ELAPSED and SYS_TICKFREQ
  semihosting: Implement SYS_TMPNAM
  semihosting: Implement SYS_ISERROR

Kito Cheng (1):
  riscv: Add semihosting support for user mode

Lirong Yuan (1):
  gdbstub: add support to Xfer:auxv:read: packet

Lukas Straub (1):
  Fix build with new yank feature by adding stubs

Philippe Mathieu-Daudé (2):
  tests/docker: Remove Debian 9 remnant lines
  shippable.yml: Remove jobs duplicated on Gitlab-CI

 configure  |   7 +-
 Makefile   |  46 +-
 default-configs/devices/arm-softmmu.mak|   1 +
 default-configs/devices/riscv32-softmmu.mak|   2 +
 default-configs/devices/riscv64-softmmu.mak|   2 +
 default-configs/targets/aarch64-linux-user.mak |   1 +
 default-configs/targets/aarch64_be-linux-user.mak  |   1 +
 default-configs/targets/arm-linux-user.mak |   1 +
 default-configs/targets/armeb-linux-user.mak   |   1 +
 default-configs/targets/riscv32-linux-user.mak |   1 +
 default-configs/targets/riscv64-linux-user.mak |   1 +
 hw/semihosting/common-semi.h   |  39 ++
 include/exec/gdbstub.h |  14 +-
 include/qemu/timer.h   |   2 +
 linux-user/qemu.h  |   4 +-
 target/arm/cpu.h   |   8 -
 target/riscv/cpu_bits.h|   1 +
 bsd-user/syscall.c |   6 +-
 gdbstub.c  |  65 ++-
 .../arm-semi.c => hw/semihosting/arm-compat-semi.c | 525 ++---
 linux-user/aarch64/cpu_loop.c  |   3 +-
 linux-user/arm/cpu_loop.c  |   3 +-
 linux-user/exit.c  |   2 +-
 linux-user/riscv/cpu_loop.c|   5 +
 linux-user/{arm => }/semihost.c|   8 +-
 softmmu/runstate.c |   2 +-
 stubs/yank.c   |  29 ++
 target/arm/gdbstub.c   |  75 ++-
 target/arm/helper.c|   7 +-
 target/arm/m_helper.c  |   7 +-
 target/m68k/m68k-semi.c|   2 +-
 target/nios2/nios2-semi.c  |   2 +-
 target/riscv/cpu_helper.c  |  10 +
 targ

[PULL 03/30] shippable.yml: Remove jobs duplicated on Gitlab-CI

From: Philippe Mathieu-Daudé 

The following jobs are duplicated on Gitlab-CI since commit
6bcb5fc0f7a ("gitlab-ci: Add cross-compiling build tests"):

- IMAGE=debian-armel-cross

  TARGET_LIST=arm-softmmu   -> cross-armel-system
  TARGET_LIST=arm-linux-user-> cross-armel-user
  TARGET_LIST=armeb-linux-user  -> cross-armel-user

- IMAGE=debian-armhf-cross

  TARGET_LIST=arm-softmmu   -> cross-armhf-system
  TARGET_LIST=arm-linux-user-> cross-armhf-user
  TARGET_LIST=armeb-linux-user  -> cross-armhf-user

- IMAGE=debian-arm64-cross

  TARGET_LIST=aarch64-softmmu   -> cross-arm64-system
  TARGET_LIST=aarch64-linux-user-> cross-arm64-user

- IMAGE=debian-s390x-cross

  TARGET_LIST=s390x-softmmu -> cross-s390x-system
  TARGET_LIST=s390x-linux-user  -> cross-s390x-user

- IMAGE=debian-mips-cross

  TARGET_LIST=mipsel-linux-user -> cross-mips-user

- IMAGE=debian-mips64el-cross

  TARGET_LIST=mips64el-softmmu  -> cross-mips64el-system
  TARGET_LIST=mips64el-linux-user   -> cross-mips64el-user

- IMAGE=debian-ppc64el-cross

  TARGET_LIST=ppc64-softmmu -> cross-ppc64el-system
  TARGET_LIST=ppc64-linux-user  -> cross-ppc64el-user
  TARGET_LIST=ppc64abi32-linux-user -> cross-ppc64el-user

Remove them from Shippable CI.

Signed-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Alex Bennée 
Acked-by: Alex Bennée 
Message-Id: <20210108145103.269353-1-f4...@amsat.org>
Message-Id: <20210114165730.31607-4-alex.ben...@linaro.org>

diff --git a/.shippable.yml b/.shippable.yml
index 14350e6de8..97bfa2a0f3 100644
--- a/.shippable.yml
+++ b/.shippable.yml
@@ -7,20 +7,8 @@ env:
   matrix:
 - IMAGE=debian-amd64
   TARGET_LIST=x86_64-softmmu,x86_64-linux-user
-- IMAGE=debian-armel-cross
-  TARGET_LIST=arm-softmmu,arm-linux-user,armeb-linux-user
-- IMAGE=debian-armhf-cross
-  TARGET_LIST=arm-softmmu,arm-linux-user,armeb-linux-user
-- IMAGE=debian-arm64-cross
-  TARGET_LIST=aarch64-softmmu,aarch64-linux-user
-- IMAGE=debian-s390x-cross
-  TARGET_LIST=s390x-softmmu,s390x-linux-user
 - IMAGE=debian-mips-cross
-  TARGET_LIST=mips-softmmu,mipsel-linux-user
-- IMAGE=debian-mips64el-cross
-  TARGET_LIST=mips64el-softmmu,mips64el-linux-user
-- IMAGE=debian-ppc64el-cross
-  TARGET_LIST=ppc64-softmmu,ppc64-linux-user,ppc64abi32-linux-user
+  TARGET_LIST=mips-softmmu
 build:
   pre_ci_boot:
 image_name: registry.gitlab.com/qemu-project/qemu/qemu/${IMAGE}
-- 
2.20.1

Re: [PATCH v6 08/11] iotests: add testenv.py


15.01.2021 15:45, Kevin Wolf wrote:

Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:

15.01.2021 14:18, Kevin Wolf wrote:

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
   tests/qemu-iotests/testenv.py | 328 ++
   1 file changed, 328 insertions(+)
   create mode 100755 tests/qemu-iotests/testenv.py



[..]


+def init_binaries(self):
+"""Init binary path variables:
+ PYTHON (for bash tests)
+ QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
+ SOCKET_SCM_HELPER
+"""
+self.python = '/usr/bin/python3 -B'


This doesn't look right, we need to respect the Python binary set in
configure (which I think we get from common.env)


Oh, I missed the change. Then I should just drop this self.python.


Do we still get the value from elsewhere or do we need to manually parse
common.env?


Hmm.. Good question. We have either parse common.env, and still create 
self.python variable.

Or drop it, and include common.env directly to bash tests. For this we'll need 
to export

BUILD_IOTESTS, and do
 . $BUILD_IOTESTS/common.env

in common.rc..







+def root(*names):
+return os.path.join(self.build_root, *names)
+
+arch = os.uname().machine
+if 'ppc64' in arch:
+arch = 'ppc64'
+
+self.qemu_prog = os.getenv('QEMU_PROG', root(f'qemu-system-{arch}'))
+self.qemu_img_prog = os.getenv('QEMU_IMG_PROG', root('qemu-img'))
+self.qemu_io_prog = os.getenv('QEMU_IO_PROG', root('qemu-io'))
+self.qemu_nbd_prog = os.getenv('QEMU_NBD_PROG', root('qemu-nbd'))
+self.qsd_prog = os.getenv('QSD_PROG', root('storage-daemon',
+   'qemu-storage-daemon'))
+
+for b in [self.qemu_img_prog, self.qemu_io_prog, self.qemu_nbd_prog,
+  self.qemu_prog, self.qsd_prog]:
+if not os.path.exists(b):
+exit('Not such file: ' + b)
+if not os.access(b, os.X_OK):
+exit('Not executable: ' + b)
+
+helper_path = os.path.join(self.build_iotests, 'socket_scm_helper')
+if os.access(helper_path, os.X_OK):
+self.socket_scm_helper = helper_path  # SOCKET_SCM_HELPER
+
+def __init__(self, argv: List[str]) -> None:
+"""Parse args and environment"""
+
+# Initialize generic paths: build_root, build_iotests, source_iotests,
+# which are needed to initialize some environment variables. They are
+# used by init_*() functions as well.
+
+
+if os.path.islink(sys.argv[0]):
+# called from the build tree
+self.source_iotests = os.path.dirname(os.readlink(sys.argv[0]))
+self.build_iotests = os.path.dirname(os.path.abspath(sys.argv[0]))
+else:
+# called from the source tree
+self.source_iotests = os.getcwd()
+self.build_iotests = self.source_iotests
+
+self.build_root = os.path.join(self.build_iotests, '..', '..')
+
+self.init_handle_argv(argv)
+self.init_directories()
+self.init_binaries()
+
+# QEMU_OPTIONS
+self.qemu_options = '-nodefaults -display none -accel qtest'
+machine_map = (
+(('arm', 'aarch64'), 'virt'),


How does this work? Won't we check for "qemu-system-('arm', 'aarch64')"
below, which we'll never find?


Hmm. I just took existing logic from check:

[..]
   case "$QEMU_PROG" in
   *qemu-system-arm|*qemu-system-aarch64)
   export QEMU_OPTIONS="$QEMU_OPTIONS -machine virt"
   ;;
[..]


What I mean is that the code below doesn't look like it's prepared to
interpret a tuple like ('arm', 'aarch64'), it expects a single string:




+('avr', 'mega2560'),
+('rx', 'gdbsim-r5f562n8'),
+('tricore', 'tricore_testboard')
+)
+for suffix, machine in machine_map:
+if self.qemu_prog.endswith(f'qemu-system-{suffix}'):


Here we get effectively:

 suffix: Tuple[str, str] = ('arm', 'aarch64')

The formatted string uses str(suffix), which makes the result
"qemu-system-('arm', 'aarch64')".

Or am I misunderstanding something here?


Ah, you are right:) Will fix. I interpreted your

  Won't we check for "qemu-system-('arm', 'aarch64')"

as
  
  Won't we check for "qemu-system-arm" or "qemu-system-aarch64"




--
Best regards,
Vladimir

Re: [PATCH] cirrus: don't run full qtest on macOS

2021-01-15 Thread Markus Armbruster

Mark Cave-Ayland  writes:

> On 06/01/2021 13:36, Daniel P. Berrangé wrote:
>
>>> The Cirrus CI macOS build hosts have exhibited a serious performance
>>> degradation in recent months. For example the "qom-test" qtest takes
>>> over an hour for only the qemu-system-aarch64 binary. This is as much
>>> 20-40 times slower than other environments. The other qtests all show
>>> similar performance degradation, as do many of the unit tests.
>>>
>>> This does not appear related to QEMU code changes, since older git
>>> commits which were known to fully complete in less than 1 hour on
>>> Cirrus CI now also show similar bad performance. Either Cirrus CI
>>> performance has degraded, or an change in its environment has exposed
>>> a latent bug widely affecting all of QEMU. Debugging the qom-test
>>> showed no easily identified large bottleneck - every step of the
>>> test execution was simply slower.
>> It appears I might be mistaken here. On IRC it was reported that
>> going back furrther to v5.1.0 shows good performance in Cirrus
>> still.
>> I had only gone back as far as
>> 2a5a79d1b57280edd72193f6031de3feb682154e
>> which I thought was fast originally.
>> So somewhere between v5.1.0 and 2a5a79 we apparently regressed.
>
> I tested a few macos cirrus-ci builds after the meson conversion and
> found that they were working fine, so whatever is affecting the macos
> build must be related to a QEMU change.
>
> A full bisect proved to be too tricky due to the instability of the
> tree at that point in time, however reading through "git log" and
> attempting some builds at merges I thought might be related I
> discovered that the slowness was introduced by this PR:
>
>
> commit b7092cda1b36ce687e65ab1831346f9529b781b8
> Merge: 497d415d76 eb94b81a94
> Author: Peter Maydell 
> Date:   Fri Oct 9 13:20:46 2020 +0100
>
> Merge remote-tracking branch
> 'remotes/armbru/tags/pull-monitor-2020-10-09' into staging
>
> Monitor patches for 2020-10-09
[...]
>
> Fortunately that PR could be bisected and that led me this commit:
>
>
> 9ce44e2ce267caf5559904a201aa1986b0a8326b is the first bad commit
> commit 9ce44e2ce267caf5559904a201aa1986b0a8326b
> Author: Kevin Wolf 
> Date:   Mon Oct 5 17:58:50 2020 +0200
>
> qmp: Move dispatcher to a coroutine
>
> This moves the QMP dispatcher to a coroutine and runs all QMP command
> handlers that declare 'coroutine': true in coroutine context so they
> can avoid blocking the main loop while doing I/O or waiting for other
> events.
>
> For commands that are not declared safe to run in a coroutine, the
> dispatcher drops out of coroutine context by calling the QMP command
> handler from a bottom half.
>
> Signed-off-by: Kevin Wolf 
> Reviewed-by: Markus Armbruster 
> Message-Id: <20201005155855.256490-10-kw...@redhat.com>
> Reviewed-by: Markus Armbruster 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Markus Armbruster 
>
>
> Given that Peter can run the tests manually, I'm not exactly sure why
> the cirrus-ci environment is different - all I can think of is that it
> could be related to running in a headless terminal.
>
> For reference running cirrus-ci on the last good commit 04f22362f1
> "qapi: Add a 'coroutine' flag for commands" gave me a total runtime of
> 35 mins.

Let me explain what the patch does, in the hope of helping somebody else
find out why it behaves badly on MacOS.  Unfortunately, a proper
explanation requires quite some context.  Bear with me.

In the beginning, there was just one monitor, and it ran entirely in the
main loop (callback when input is available).  To keep the main loop
going, monitor commands better complete quickly.

Then we got multiple monitors.  Same story, just multiple input streams,
each with a callback.

We also got additional threads.  When I say "main loop", I mean the main
thread's main loop.

"Must complete quickly" means no blocking I/O and such.  Writing code
that way is somewhere between hard and impractical.  Much code called by
monitor commands wasn't written that way.

When a monitor command blocks, the main loop blocks, and that means no
more monitor commands can run, not even on other monitors.

"Doctor, doctor, running code in the main loop hurts".  Sadly, the
doctor's recommended remedy "don't do that then" is really hard to
apply: a lot of code has been written assuming "running in the main
loop, with the big QEMU lock held".

The first small step towards it was taken to enable the "out-of-band"
feature.  We moved the QMP monitor proper out of the main loop into a
monitor I/O thread.  The monitor commands get funneled to the main loop.
Instead of the main loop calling the monitor when a file descriptor has
input, it now calls the command dispatcher when a funnel queue has a
command.

Why bother?  Because now we can thread execute special "out-of-band"
commands right away, in the I/O thread, regardless of how badly the main
loop is.  Peter Xu wanted this badly enough for

[PULL 17/30] gdbstub: add support to Xfer:auxv:read: packet

From: Lirong Yuan 

This allows gdb to access the target’s auxiliary vector,
which can be helpful for telling system libraries important details
about the hardware, operating system, and process.

Signed-off-by: Lirong Yuan 
[AJB: minor tweaks to test case, update MAINTAINERS]
Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Message-Id: <20200730193932.3654677-1-yua...@google.com>
Message-Id: <20210108224256.2321-7-alex.ben...@linaro.org>

diff --git a/gdbstub.c b/gdbstub.c
index d99bc0bf2e..15d3a8e1f5 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2172,6 +2172,12 @@ static void handle_query_supported(GdbCmdContext 
*gdb_ctx, void *user_ctx)
 ";ReverseStep+;ReverseContinue+");
 }
 
+#ifdef CONFIG_USER_ONLY
+if (gdbserver_state.c_cpu->opaque) {
+g_string_append(gdbserver_state.str_buf, ";qXfer:auxv:read+");
+}
+#endif
+
 if (gdb_ctx->num_params &&
 strstr(gdb_ctx->params[0].data, "multiprocess+")) {
 gdbserver_state.multiprocess = true;
@@ -2233,6 +2239,46 @@ static void handle_query_xfer_features(GdbCmdContext 
*gdb_ctx, void *user_ctx)
   gdbserver_state.str_buf->len, true);
 }
 
+#ifdef CONFIG_USER_ONLY
+static void handle_query_xfer_auxv(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+TaskState *ts;
+unsigned long offset, len, saved_auxv, auxv_len;
+const char *mem;
+
+if (gdb_ctx->num_params < 2) {
+put_packet("E22");
+return;
+}
+
+offset = gdb_ctx->params[0].val_ul;
+len = gdb_ctx->params[1].val_ul;
+ts = gdbserver_state.c_cpu->opaque;
+saved_auxv = ts->info->saved_auxv;
+auxv_len = ts->info->auxv_len;
+mem = (const char *)(saved_auxv + offset);
+if (offset > auxv_len) {
+put_packet("E00");
+return;
+}
+
+if (len > (MAX_PACKET_LENGTH - 5) / 2) {
+len = (MAX_PACKET_LENGTH - 5) / 2;
+}
+
+if (len < auxv_len - offset) {
+g_string_assign(gdbserver_state.str_buf, "m");
+memtox(gdbserver_state.str_buf, mem, len);
+} else {
+g_string_assign(gdbserver_state.str_buf, "l");
+memtox(gdbserver_state.str_buf, mem, auxv_len - offset);
+}
+
+put_packet_binary(gdbserver_state.str_buf->str,
+  gdbserver_state.str_buf->len, true);
+}
+#endif
+
 static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
 put_packet(GDB_ATTACHED);
@@ -2338,6 +2384,14 @@ static GdbCmdParseEntry gdb_gen_query_table[] = {
 .cmd_startswith = 1,
 .schema = "s:l,l0"
 },
+#ifdef CONFIG_USER_ONLY
+{
+.handler = handle_query_xfer_auxv,
+.cmd = "Xfer:auxv:read::",
+.cmd_startswith = 1,
+.schema = "l,l0"
+},
+#endif
 {
 .handler = handle_query_attached,
 .cmd = "Attached:",
diff --git a/MAINTAINERS b/MAINTAINERS
index 07e4851aa4..3216387521 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2322,6 +2322,7 @@ R: Philippe Mathieu-Daudé 
 S: Maintained
 F: gdbstub*
 F: gdb-xml/
+F: tests/tcg/multiarch/gdbstub/
 
 Memory API
 M: Paolo Bonzini 
diff --git a/tests/tcg/multiarch/Makefile.target 
b/tests/tcg/multiarch/Makefile.target
index cb49cc9ccb..1dd0f64d23 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -55,6 +55,15 @@ run-gdbstub-sha1: sha1
"basic gdbstub support")
 
 EXTRA_RUNS += run-gdbstub-sha1
+
+run-gdbstub-qxfer-auxv-read: sha1
+   $(call run-test, $@, $(GDB_SCRIPT) \
+   --gdb $(HAVE_GDB_BIN) \
+   --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+   --bin $< --test 
$(MULTIARCH_SRC)/gdbstub/test-qxfer-auxv-read.py, \
+   "basic gdbstub qXfer:auxv:read support")
+
+EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read
 endif
 
 
diff --git a/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py 
b/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
new file mode 100644
index 00..d91e8fdf19
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
@@ -0,0 +1,57 @@
+from __future__ import print_function
+#
+# Test auxiliary vector is loaded via gdbstub
+#
+# This is launched via tests/guest-debug/run-test.py
+#
+
+import gdb
+import sys
+
+failcount = 0
+
+def report(cond, msg):
+"Report success/fail of test"
+if cond:
+print ("PASS: %s" % (msg))
+else:
+print ("FAIL: %s" % (msg))
+global failcount
+failcount += 1
+
+def run_test():
+"Run through the tests one by one"
+
+auxv = gdb.execute("info auxv", False, True)
+report(isinstance(auxv, str), "Fetched auxv from inferior")
+report(auxv.find("sha1"), "Found test binary name in auxv")
+
+#
+# This runs as the script it sourced (via -x, via run-test.py)
+#
+try:
+inferior = gdb.selected_inferior()
+arch = inferior.architecture()
+print("ATTACHED: %s" % arch.name())
+except (gdb.error, AttributeError):
+print("SKIPPING (not connected)", file=sys.stderr)
+exit(0)
+
+if g

[PULL 06/30] Makefile: wrap etags in quiet-command calls

For prettier output.

Signed-off-by: Alex Bennée 
Reviewed-by: Willian Rampazzo 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210114165730.31607-7-alex.ben...@linaro.org>

diff --git a/Makefile b/Makefile
index bbab640b31..f7e9eb9f08 100644
--- a/Makefile
+++ b/Makefile
@@ -272,8 +272,13 @@ gtags:
 
 .PHONY: TAGS
 TAGS:
-   rm -f "$(SRC_PATH)/"TAGS
-   $(find-src-path) -exec etags -f "$(SRC_PATH)/"TAGS --append {} +
+   $(call quiet-command,   \
+   rm -f "$(SRC_PATH)/"TAGS,   \
+   "TAGS", "Remove old $@")
+   $(call quiet-command,   \
+   $(find-src-path) -exec etags\
+   -f "$(SRC_PATH)/"TAGS --append {} +,\
+   "TAGS", "Re-index $(SRC_PATH)")
 
 .PHONY: cscope
 cscope:
-- 
2.20.1

[PULL 25/30] semihosting: Support SYS_HEAPINFO when env->boot_info is not set

From: Keith Packard 

env->boot_info is only set in some ARM startup paths, so we cannot
rely on it to support the SYS_HEAPINFO semihosting function. When not
available, fallback to finding a RAM memory region containing the
current stack and use the base of that.

Signed-off-by: Keith Packard 
Signed-off-by: Alex Bennée 
Message-Id: <20210107170717.2098982-5-kei...@keithp.com>
Message-Id: <20210108224256.2321-16-alex.ben...@linaro.org>

diff --git a/hw/semihosting/arm-compat-semi.c b/hw/semihosting/arm-compat-semi.c
index ac1271545e..293791f721 100644
--- a/hw/semihosting/arm-compat-semi.c
+++ b/hw/semihosting/arm-compat-semi.c
@@ -137,6 +137,36 @@ typedef struct GuestFD {
 
 static GArray *guestfd_array;
 
+#ifndef CONFIG_USER_ONLY
+#include "exec/address-spaces.h"
+/*
+ * Find the base of a RAM region containing the specified address
+ */
+static inline hwaddr
+common_semi_find_region_base(hwaddr addr)
+{
+MemoryRegion *subregion;
+
+/*
+ * Find the chunk of R/W memory containing the address.  This is
+ * used for the SYS_HEAPINFO semihosting call, which should
+ * probably be using information from the loaded application.
+ */
+QTAILQ_FOREACH(subregion, &get_system_memory()->subregions,
+   subregions_link) {
+if (subregion->ram && !subregion->readonly) {
+Int128 top128 = int128_add(int128_make64(subregion->addr),
+   subregion->size);
+Int128 addr128 = int128_make64(addr);
+if (subregion->addr <= addr && int128_lt(addr128, top128)) {
+return subregion->addr;
+}
+}
+}
+return 0;
+}
+#endif
+
 #ifdef TARGET_ARM
 static inline target_ulong
 common_semi_arg(CPUState *cs, int argno)
@@ -175,7 +205,18 @@ common_semi_rambase(CPUState *cs)
 {
 CPUArchState *env = cs->env_ptr;
 const struct arm_boot_info *info = env->boot_info;
-return info->loader_start;
+target_ulong sp;
+
+if (info) {
+return info->loader_start;
+}
+
+if (is_a64(env)) {
+sp = env->xregs[31];
+} else {
+sp = env->regs[13];
+}
+return common_semi_find_region_base(sp);
 }
 #endif
 
-- 
2.20.1

[PULL 16/30] gdbstub: implement a softmmu based test

This adds a new tests that allows us to test softmmu only features
including watchpoints. To do achieve this we need to:

  - add _exit: labels to the boot codes
  - write a memory.py test case
  - plumb the test case into the build system
  - tweak the run_test script to:
- re-direct output when asked
- use socket based connection for all tests
- add a small pause before connection

Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210108224256.2321-6-alex.ben...@linaro.org>

diff --git a/tests/guest-debug/run-test.py b/tests/guest-debug/run-test.py
index 0c4f5c3808..8b91ff95af 100755
--- a/tests/guest-debug/run-test.py
+++ b/tests/guest-debug/run-test.py
@@ -16,6 +16,7 @@ import subprocess
 import shutil
 import shlex
 import os
+from time import sleep
 from tempfile import TemporaryDirectory
 
 def get_args():
@@ -27,10 +28,21 @@ def get_args():
 required=True)
 parser.add_argument("--test", help="GDB test script",
 required=True)
-parser.add_argument("--gdb", help="The gdb binary to use", default=None)
+parser.add_argument("--gdb", help="The gdb binary to use",
+default=None)
+parser.add_argument("--output", help="A file to redirect output to")
 
 return parser.parse_args()
 
+
+def log(output, msg):
+if output:
+output.write(msg + "\n")
+output.flush()
+else:
+print(msg)
+
+
 if __name__ == '__main__':
 args = get_args()
 
@@ -42,18 +54,25 @@ if __name__ == '__main__':
 if not args.gdb:
 print("We need gdb to run the test")
 exit(-1)
+if args.output:
+output = open(args.output, "w")
+else:
+output = None
 
 socket_dir = TemporaryDirectory("qemu-gdbstub")
 socket_name = os.path.join(socket_dir.name, "gdbstub.socket")
 
 # Launch QEMU with binary
 if "system" in args.qemu:
-cmd = "%s %s %s -s -S" % (args.qemu, args.qargs, args.binary)
+cmd = "%s %s %s -gdb unix:path=%s,server" % (args.qemu,
+ args.qargs,
+ args.binary,
+ socket_name)
 else:
 cmd = "%s %s -g %s %s" % (args.qemu, args.qargs, socket_name,
   args.binary)
 
-print("QEMU CMD: %s" % (cmd))
+log(output, "QEMU CMD: %s" % (cmd))
 inferior = subprocess.Popen(shlex.split(cmd))
 
 # Now launch gdb with our test and collect the result
@@ -63,16 +82,15 @@ if __name__ == '__main__':
 # disable prompts in case of crash
 gdb_cmd += " -ex 'set confirm off'"
 # connect to remote
-if "system" in args.qemu:
-gdb_cmd += " -ex 'target remote localhost:1234'"
-else:
-gdb_cmd += " -ex 'target remote %s'" % (socket_name)
+gdb_cmd += " -ex 'target remote %s'" % (socket_name)
 # finally the test script itself
 gdb_cmd += " -x %s" % (args.test)
 
-print("GDB CMD: %s" % (gdb_cmd))
 
-result = subprocess.call(gdb_cmd, shell=True);
+sleep(1)
+log(output, "GDB CMD: %s" % (gdb_cmd))
+
+result = subprocess.call(gdb_cmd, shell=True, stdout=output)
 
 # A negative result is the result of an internal gdb failure like
 # a crash. We force a return of 0 so we don't fail the test on
diff --git a/tests/tcg/aarch64/Makefile.softmmu-target 
b/tests/tcg/aarch64/Makefile.softmmu-target
index 1057a8ac49..a7286ac295 100644
--- a/tests/tcg/aarch64/Makefile.softmmu-target
+++ b/tests/tcg/aarch64/Makefile.softmmu-target
@@ -15,6 +15,7 @@ CRT_PATH=$(AARCH64_SYSTEM_SRC)
 LINK_SCRIPT=$(AARCH64_SYSTEM_SRC)/kernel.ld
 LDFLAGS=-Wl,-T$(LINK_SCRIPT)
 TESTS+=$(AARCH64_TESTS) $(MULTIARCH_TESTS)
+EXTRA_RUNS+=$(MULTIARCH_RUNS)
 CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC)
 LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
 
diff --git a/tests/tcg/aarch64/system/boot.S b/tests/tcg/aarch64/system/boot.S
index b14e94f332..e190b1efa6 100644
--- a/tests/tcg/aarch64/system/boot.S
+++ b/tests/tcg/aarch64/system/boot.S
@@ -197,6 +197,7 @@ __start:
bl  main
 
/* pass return value to sys exit */
+_exit:
movx1, x0
ldrx0, =0x20026 /* ADP_Stopped_ApplicationExit */
stpx0, x1, [sp, #-16]!
diff --git a/tests/tcg/i386/Makefile.softmmu-target 
b/tests/tcg/i386/Makefile.softmmu-target
index 1c8790eecd..5266f2335a 100644
--- a/tests/tcg/i386/Makefile.softmmu-target
+++ b/tests/tcg/i386/Makefile.softmmu-target
@@ -19,6 +19,7 @@ CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC)
 LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
 
 TESTS+=$(MULTIARCH_TESTS)
+EXTRA_RUNS+=$(MULTIARCH_RUNS)
 
 # building head blobs
 .PRECIOUS: $(CRT_OBJS)
diff --git a/tests/tcg/i386/system/boot.S b/tests/tcg/i386/system/boot.S
index 90aa174908..794c2cb0ad 100644
--- a/tests/tcg/i386/system/boot.S
+++ b/tests/tcg/i386/system/boot.S
@@ -76,7 +76,

[PULL 05/30] Makefile: wrap ctags in quiet-command calls

For prettier output.

Signed-off-by: Alex Bennée 
Reviewed-by: Willian Rampazzo 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210114165730.31607-6-alex.ben...@linaro.org>

diff --git a/Makefile b/Makefile
index 0c509a7704..bbab640b31 100644
--- a/Makefile
+++ b/Makefile
@@ -250,8 +250,13 @@ find-src-path = find "$(SRC_PATH)/" -path 
"$(SRC_PATH)/meson" -prune -o \( -name
 
 .PHONY: ctags
 ctags:
-   rm -f "$(SRC_PATH)/"tags
-   $(find-src-path) -exec ctags -f "$(SRC_PATH)/"tags --append {} +
+   $(call quiet-command,   \
+   rm -f "$(SRC_PATH)/"tags,   \
+   "CTAGS", "Remove old tags")
+   $(call quiet-command, \
+   $(find-src-path) -exec ctags\
+   -f "$(SRC_PATH)/"tags --append {} +,\
+   "CTAGS", "Re-index $(SRC_PATH)")
 
 .PHONY: gtags
 gtags:
-- 
2.20.1

[PULL 21/30] target/arm: use official org.gnu.gdb.aarch64.sve layout for registers

While GDB can work with any XML description given to it there is
special handling for SVE registers on the GDB side which makes the
users life a little better. The changes aren't that major and all the
registers save the $vg reported the same. All that changes is:

  - report org.gnu.gdb.aarch64.sve
  - use gdb nomenclature for names and types
  - minor re-ordering of the types to match reference
  - re-enable ieee_half (as we know gdb supports it now)
  - $vg is now a 64 bit int
  - check $vN and $zN aliasing in test

Signed-off-by: Alex Bennée 
Reviewed-by: Luis Machado 
Message-Id: <20210108224256.2321-11-alex.ben...@linaro.org>

diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 866595b4f1..a8fff2a3d0 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -195,22 +195,17 @@ static const struct TypeSize vec_lanes[] = {
 { "uint128", 128, 'q', 'u' },
 { "int128", 128, 'q', 's' },
 /* 64 bit */
+{ "ieee_double", 64, 'd', 'f' },
 { "uint64", 64, 'd', 'u' },
 { "int64", 64, 'd', 's' },
-{ "ieee_double", 64, 'd', 'f' },
 /* 32 bit */
+{ "ieee_single", 32, 's', 'f' },
 { "uint32", 32, 's', 'u' },
 { "int32", 32, 's', 's' },
-{ "ieee_single", 32, 's', 'f' },
 /* 16 bit */
+{ "ieee_half", 16, 'h', 'f' },
 { "uint16", 16, 'h', 'u' },
 { "int16", 16, 'h', 's' },
-/*
- * TODO: currently there is no reliable way of telling
- * if the remote gdb actually understands ieee_half so
- * we don't expose it in the target description for now.
- * { "ieee_half", 16, 'h', 'f' },
- */
 /* bytes */
 { "uint8", 8, 'b', 'u' },
 { "int8", 8, 'b', 's' },
@@ -223,17 +218,16 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
 GString *s = g_string_new(NULL);
 DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
 g_autoptr(GString) ts = g_string_new("");
-int i, bits, reg_width = (cpu->sve_max_vq * 128);
+int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
 info->num = 0;
 g_string_printf(s, "");
 g_string_append_printf(s, "");
-g_string_append_printf(s, "");
+g_string_append_printf(s, "");
 
 /* First define types and totals in a whole VL */
 for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
 int count = reg_width / vec_lanes[i].size;
-g_string_printf(ts, "vq%d%c%c", count,
-vec_lanes[i].sz, vec_lanes[i].suffix);
+g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
 g_string_append_printf(s,
"",
ts->str, vec_lanes[i].gdb_type, count);
@@ -243,39 +237,37 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
  * signed and potentially float versions of each size from 128 to
  * 8 bits.
  */
-for (bits = 128; bits >= 8; bits /= 2) {
-int count = reg_width / bits;
-g_string_append_printf(s, "", count);
-for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
-if (vec_lanes[i].size == bits) {
-g_string_append_printf(s, "",
-   vec_lanes[i].suffix,
-   count,
-   vec_lanes[i].sz, vec_lanes[i].suffix);
+for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+const char suf[] = { 'q', 'd', 's', 'h', 'b' };
+g_string_append_printf(s, "", suf[i]);
+for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
+if (vec_lanes[j].size == bits) {
+g_string_append_printf(s, "",
+   vec_lanes[j].suffix,
+   vec_lanes[j].sz, vec_lanes[j].suffix);
 }
 }
 g_string_append(s, "");
 }
 /* And now the final union of unions */
-g_string_append(s, "");
-for (bits = 128; bits >= 8; bits /= 2) {
-int count = reg_width / bits;
-for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
-if (vec_lanes[i].size == bits) {
-g_string_append_printf(s, "",
-   vec_lanes[i].sz, count);
-break;
-}
-}
+g_string_append(s, "");
+for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+const char suf[] = { 'q', 'd', 's', 'h', 'b' };
+g_string_append_printf(s, "",
+   suf[i], suf[i]);
 }
 g_string_append(s, "");
 
+/* Finally the sve prefix type */
+g_string_append_printf(s,
+   "",
+   reg_width / 8);
+
 /* Then define each register in parts for each vq */
 for (i = 0; i < 32; i++) {
 g_string_append_printf(s,
"",
+   " regnum=\"%d\" type=\"svev\"/>",
i, reg_width, base_reg++);
 info->num++;
 }
@@ -287,31 +279,22 @@

[PULL 18/30] gdbstub: drop CPUEnv from gdb_exit()

gdb_exit() has never needed anything from env and I doubt we are going
to start now.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210108224256.2321-8-alex.ben...@linaro.org>

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 94d8f83e92..492db0f512 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -46,7 +46,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char 
*fmt, ...);
 void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va);
 int use_gdb_syscalls(void);
 void gdb_set_stop_cpu(CPUState *cpu);
-void gdb_exit(CPUArchState *, int);
+void gdb_exit(int);
 #ifdef CONFIG_USER_ONLY
 /**
  * gdb_handlesig: yield control to gdb
diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
index d38ec7a162..adc3d21b54 100644
--- a/bsd-user/syscall.c
+++ b/bsd-user/syscall.c
@@ -333,7 +333,7 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 #ifdef CONFIG_GPROF
 _mcleanup();
 #endif
-gdb_exit(cpu_env, arg1);
+gdb_exit(arg1);
 qemu_plugin_atexit_cb();
 /* XXX: should free thread stack and CPU env */
 _exit(arg1);
@@ -435,7 +435,7 @@ abi_long do_netbsd_syscall(void *cpu_env, int num, abi_long 
arg1,
 #ifdef CONFIG_GPROF
 _mcleanup();
 #endif
-gdb_exit(cpu_env, arg1);
+gdb_exit(arg1);
 qemu_plugin_atexit_cb();
 /* XXX: should free thread stack and CPU env */
 _exit(arg1);
@@ -514,7 +514,7 @@ abi_long do_openbsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 #ifdef CONFIG_GPROF
 _mcleanup();
 #endif
-gdb_exit(cpu_env, arg1);
+gdb_exit(arg1);
 qemu_plugin_atexit_cb();
 /* XXX: should free thread stack and CPU env */
 _exit(arg1);
diff --git a/gdbstub.c b/gdbstub.c
index 15d3a8e1f5..afa553e8fc 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3068,7 +3068,7 @@ static void gdb_read_byte(uint8_t ch)
 }
 
 /* Tell the remote gdb that the process has exited.  */
-void gdb_exit(CPUArchState *env, int code)
+void gdb_exit(int code)
 {
   char buf[4];
 
diff --git a/linux-user/exit.c b/linux-user/exit.c
index 1594015444..70b344048c 100644
--- a/linux-user/exit.c
+++ b/linux-user/exit.c
@@ -34,6 +34,6 @@ void preexit_cleanup(CPUArchState *env, int code)
 #ifdef CONFIG_GCOV
 __gcov_dump();
 #endif
-gdb_exit(env, code);
+gdb_exit(code);
 qemu_plugin_atexit_cb();
 }
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index f7b7bff522..93360e28c7 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -1101,7 +1101,7 @@ target_ulong do_arm_semihosting(CPUARMState *env)
  */
 ret = (args == ADP_Stopped_ApplicationExit) ? 0 : 1;
 }
-gdb_exit(env, ret);
+gdb_exit(ret);
 exit(ret);
 case TARGET_SYS_SYNCCACHE:
 /*
diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
index 27600e0cc0..d919245e4f 100644
--- a/target/m68k/m68k-semi.c
+++ b/target/m68k/m68k-semi.c
@@ -195,7 +195,7 @@ void do_m68k_semihosting(CPUM68KState *env, int nr)
 args = env->dregs[1];
 switch (nr) {
 case HOSTED_EXIT:
-gdb_exit(env, env->dregs[0]);
+gdb_exit(env->dregs[0]);
 exit(env->dregs[0]);
 case HOSTED_OPEN:
 GET_ARG(0);
diff --git a/target/nios2/nios2-semi.c b/target/nios2/nios2-semi.c
index d7a80dd303..e508b2fafc 100644
--- a/target/nios2/nios2-semi.c
+++ b/target/nios2/nios2-semi.c
@@ -215,7 +215,7 @@ void do_nios2_semihosting(CPUNios2State *env)
 args = env->regs[R_ARG1];
 switch (nr) {
 case HOSTED_EXIT:
-gdb_exit(env, env->regs[R_ARG0]);
+gdb_exit(env->regs[R_ARG0]);
 exit(env->regs[R_ARG0]);
 case HOSTED_OPEN:
 GET_ARG(0);
-- 
2.20.1

[PULL 10/30] audio: break generic buffer dependency on mixing-engine

From: Volker Rümelin 

Break the unnecessary dependency of the generic buffer management
code on mixing-engine. This is required for the next patch.

Signed-off-by: Volker Rümelin 
Message-id: 9315afe5-5958-c0b4-ea1e-14769511a...@t-online.de
Message-Id: <20210110100239.27588-10-vr_q...@t-online.de>
Signed-off-by: Gerd Hoffmann 
---
 audio/audio.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 480b3cce1ffb..22d769db0c99 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1388,9 +1388,8 @@ void audio_run(AudioState *s, const char *msg)
 void audio_generic_run_buffer_in(HWVoiceIn *hw)
 {
 if (unlikely(!hw->buf_emul)) {
-size_t calc_size = hw->conv_buf->size * hw->info.bytes_per_frame;
-hw->buf_emul = g_malloc(calc_size);
-hw->size_emul = calc_size;
+hw->size_emul = hw->samples * hw->info.bytes_per_frame;
+hw->buf_emul = g_malloc(hw->size_emul);
 hw->pos_emul = hw->pending_emul = 0;
 }
 
@@ -1452,10 +1451,8 @@ void audio_generic_run_buffer_out(HWVoiceOut *hw)
 void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size)
 {
 if (unlikely(!hw->buf_emul)) {
-size_t calc_size = hw->mix_buf->size * hw->info.bytes_per_frame;
-
-hw->buf_emul = g_malloc(calc_size);
-hw->size_emul = calc_size;
+hw->size_emul = hw->samples * hw->info.bytes_per_frame;
+hw->buf_emul = g_malloc(hw->size_emul);
 hw->pos_emul = hw->pending_emul = 0;
 }
 
-- 
2.29.2

[PULL 30/30] semihosting: Implement SYS_ISERROR

From: Keith Packard 

Part of Semihosting for AArch32 and AArch64 Release 2.0

Signed-off-by: Keith Packard 
Signed-off-by: Alex Bennée 
Message-Id: <20210107170717.2098982-10-kei...@keithp.com>
Message-Id: <20210108224256.2321-21-alex.ben...@linaro.org>

diff --git a/hw/semihosting/arm-compat-semi.c b/hw/semihosting/arm-compat-semi.c
index a631904fb0..23c6e3edcb 100644
--- a/hw/semihosting/arm-compat-semi.c
+++ b/hw/semihosting/arm-compat-semi.c
@@ -59,6 +59,7 @@
 #define TARGET_SYS_WRITE   0x05
 #define TARGET_SYS_READ0x06
 #define TARGET_SYS_READC   0x07
+#define TARGET_SYS_ISERROR 0x08
 #define TARGET_SYS_ISTTY   0x09
 #define TARGET_SYS_SEEK0x0a
 #define TARGET_SYS_FLEN0x0c
@@ -967,6 +968,9 @@ target_ulong do_common_semihosting(CPUState *cs)
 return guestfd_fns[gf->type].readfn(cs, gf, arg1, len);
 case TARGET_SYS_READC:
 return qemu_semihosting_console_inc(cs->env_ptr);
+case TARGET_SYS_ISERROR:
+GET_ARG(0);
+return (target_long) arg0 < 0 ? 1 : 0;
 case TARGET_SYS_ISTTY:
 GET_ARG(0);
 
-- 
2.20.1

Re: [PATCH v7 07/13] confidential guest support: Introduce cgs "ready" flag

On Thu, 14 Jan 2021 10:58:05 +1100
David Gibson  wrote:

> The platform specific details of mechanisms for implementing
> confidential guest support may require setup at various points during
> initialization.  Thus, it's not really feasible to have a single cgs
> initialization hook, but instead each mechanism needs its own
> initialization calls in arch or machine specific code.
> 
> However, to make it harder to have a bug where a mechanism isn't
> properly initialized under some circumstances, we want to have a
> common place, relatively late in boot, where we verify that cgs has
> been initialized if it was requested.
> 
> This patch introduces a ready flag to the ConfidentialGuestSupport
> base type to accomplish this, which we verify just before the machine
> specific initialization function.
> 
> Signed-off-by: David Gibson 
> ---
>  hw/core/machine.c | 8 
>  include/exec/confidential-guest-support.h | 2 ++
>  target/i386/sev.c | 2 ++
>  3 files changed, 12 insertions(+)

Reviewed-by: Cornelia Huck

Re: [PATCH v6 08/11] iotests: add testenv.py

2021-01-15 Thread Kevin Wolf

Am 15.01.2021 um 14:10 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 15.01.2021 15:45, Kevin Wolf wrote:
> > Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > 15.01.2021 14:18, Kevin Wolf wrote:
> > > > Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > > > Add TestEnv class, which will handle test environment in a new python
> > > > > iotests running framework.
> > > > > 
> > > > > Difference with current ./check interface:
> > > > > - -v (verbose) option dropped, as it is unused
> > > > > 
> > > > > - -xdiff option is dropped, until somebody complains that it is needed
> > > > > - same for -n option
> > > > > 
> > > > > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > > > > ---
> > > > >tests/qemu-iotests/testenv.py | 328 
> > > > > ++
> > > > >1 file changed, 328 insertions(+)
> > > > >create mode 100755 tests/qemu-iotests/testenv.py
> > > > > 
> 
> [..]
> 
> > > > > +def init_binaries(self):
> > > > > +"""Init binary path variables:
> > > > > + PYTHON (for bash tests)
> > > > > + QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, 
> > > > > QSD_PROG
> > > > > + SOCKET_SCM_HELPER
> > > > > +"""
> > > > > +self.python = '/usr/bin/python3 -B'
> > > > 
> > > > This doesn't look right, we need to respect the Python binary set in
> > > > configure (which I think we get from common.env)
> > > 
> > > Oh, I missed the change. Then I should just drop this self.python.
> > 
> > Do we still get the value from elsewhere or do we need to manually parse
> > common.env?
> 
> Hmm.. Good question. We have either parse common.env, and still create 
> self.python variable.
> 
> Or drop it, and include common.env directly to bash tests. For this we'll 
> need to export
> 
> BUILD_IOTESTS, and do
>  . $BUILD_IOTESTS/common.env
> 
> in common.rc..

check uses it, too, for running Python test cases.

Kevin

[PULL 21/30] dsoundaudio: rename dsound_open()

From: Volker Rümelin 

Rename dsound_open() to dsound_set_cooperative_level(). The
only task of that function is to set the cooperative level for
DirectSound.

Signed-off-by: Volker Rümelin 
Message-id: 9315afe5-5958-c0b4-ea1e-14769511a...@t-online.de
Message-Id: <20210110100239.27588-21-vr_q...@t-online.de>
Signed-off-by: Gerd Hoffmann 
---
 audio/dsoundaudio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index 0fbdf770ac75..d3695f3af653 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -342,7 +342,7 @@ static void dsound_clear_sample (HWVoiceOut *hw, 
LPDIRECTSOUNDBUFFER dsb,
 dsound_unlock_out (dsb, p1, p2, blen1, blen2);
 }
 
-static int dsound_open (dsound *s)
+static int dsound_set_cooperative_level(dsound *s)
 {
 HRESULT hr;
 HWND hwnd;
@@ -673,7 +673,7 @@ static void *dsound_audio_init(Audiodev *dev)
 }
 }
 
-err = dsound_open (s);
+err = dsound_set_cooperative_level(s);
 if (err) {
 dsound_audio_fini (s);
 return NULL;
-- 
2.29.2

[PULL 07/30] Makefile: wrap cscope in quiet-command calls

For prettier output.

Signed-off-by: Alex Bennée 
Reviewed-by: Willian Rampazzo 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210114165730.31607-8-alex.ben...@linaro.org>

diff --git a/Makefile b/Makefile
index f7e9eb9f08..2a926aaeb0 100644
--- a/Makefile
+++ b/Makefile
@@ -282,9 +282,17 @@ TAGS:
 
 .PHONY: cscope
 cscope:
-   rm -f "$(SRC_PATH)"/cscope.*
-   $(find-src-path) -print | sed -e 's,^\./,,' > "$(SRC_PATH)/cscope.files"
-   cscope -b -i"$(SRC_PATH)/cscope.files" -f"$(SRC_PATH)"/cscope.out
+   $(call quiet-command,   \
+   rm -f "$(SRC_PATH)/"cscope.* ,  \
+   "cscope", "Remove old $@ files")
+   $(call quiet-command,   \
+   ($(find-src-path) -print | sed -e 's,^\./,,'\
+   > "$(SRC_PATH)/cscope.files"),  \
+   "cscope", "Create file list")
+   $(call quiet-command,   \
+   cscope -b -i"$(SRC_PATH)/cscope.files"  \
+   -f"$(SRC_PATH)"/cscope.out, \
+   "cscope", "Re-index $(SRC_PATH)")
 
 # Needed by "meson install"
 export DESTDIR
-- 
2.20.1

[PULL 01/30] sdlaudio: remove leftover SDL1.2 code

From: Volker Rümelin 

Signed-off-by: Volker Rümelin 
Reviewed-by: Thomas Huth 
Message-id: 9315afe5-5958-c0b4-ea1e-14769511a...@t-online.de
Message-Id: <20210110100239.27588-1-vr_q...@t-online.de>
Signed-off-by: Gerd Hoffmann 
---
 audio/sdlaudio.c | 30 +-
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 21b7a0484bec..bf3cfb845616 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -240,28 +240,24 @@ static void sdl_callback (void *opaque, Uint8 *buf, int 
len)
 }
 }
 
-#define SDL_WRAPPER_FUNC(name, ret_type, args_decl, args, fail, unlock) \
-static ret_type glue(sdl_, name)args_decl   \
-{   \
-ret_type ret;   \
-\
-SDL_LockAudio();\
-\
-ret = glue(audio_generic_, name)args;   \
-\
-SDL_UnlockAudio();  \
-return ret; \
+#define SDL_WRAPPER_FUNC(name, ret_type, args_decl, args)  \
+static ret_type glue(sdl_, name)args_decl  \
+{  \
+ret_type ret;  \
+   \
+SDL_LockAudio();   \
+ret = glue(audio_generic_, name)args;  \
+SDL_UnlockAudio(); \
+   \
+return ret;\
 }
 
 SDL_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
- (hw, size), *size = 0, sdl_unlock)
+ (hw, size))
 SDL_WRAPPER_FUNC(put_buffer_out, size_t,
- (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size),
- /*nothing*/, sdl_unlock_and_post)
+ (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size))
 SDL_WRAPPER_FUNC(write, size_t,
- (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size),
- /*nothing*/, sdl_unlock_and_post)
-
+ (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size))
 #undef SDL_WRAPPER_FUNC
 
 static void sdl_fini_out (HWVoiceOut *hw)
-- 
2.29.2

[PULL 09/30] sdlaudio: add recording functions