Re: [PATCH 2/5] target/ppc: remove ppc_cpu_dump_statistics

[David Gibson]

On Thu, May 27, 2021 at 04:01:52PM +0200, Greg Kurz wrote:
> On Thu, 27 May 2021 10:22:50 -0300
> Bruno Piazera Larsen  wrote:
> 
> > 
> > On 27/05/2021 01:35, David Gibson wrote:
> > > On Thu, May 27, 2021 at 11:20:01AM +1000, David Gibson wrote:
> > >> On Wed, May 26, 2021 at 05:21:01PM -0300, Bruno Larsen (billionai) wrote:
> > >>> This function requires surce code modification to be useful, which means
> > >>> it probably is not used often, and the move to using decodetree means
> > >>> the statistics won't even be collected anymore.
> > >>>
> > >>> Also removed setting dump_statistics in ppc_cpu_realize, since it was
> > >>> only useful when in conjunction with ppc_cpu_dump_statistics.
> > >>>
> > >>> Suggested-by: Richard Henderson
> > >>> Signed-off-by: Bruno Larsen (billionai) 
> > >>> ---
> > >>>   target/ppc/cpu.h   |  1 -
> > >>>   target/ppc/cpu_init.c  |  3 ---
> > >>>   target/ppc/translate.c | 51 --
> > >>>   3 files changed, 55 deletions(-)
> > >>>
> > >>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > >>> index 203f07e48e..c3d1b492e4 100644
> > >>> --- a/target/ppc/cpu.h
> > >>> +++ b/target/ppc/cpu.h
> > >>> @@ -1256,7 +1256,6 @@ DECLARE_OBJ_CHECKERS(PPCVirtualHypervisor, 
> > >>> PPCVirtualHypervisorClass,
> > >>>   void ppc_cpu_do_interrupt(CPUState *cpu);
> > >>>   bool ppc_cpu_exec_interrupt(CPUState *cpu, int int_req);
> > >>>   void ppc_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
> > >>> -void ppc_cpu_dump_statistics(CPUState *cpu, int flags);
> > >>>   hwaddr ppc_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> > >>>   int ppc_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int 
> > >>> reg);
> > >>>   int ppc_cpu_gdb_read_register_apple(CPUState *cpu, GByteArray *buf, 
> > >>> int reg);
> > >>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> > >>> index f5ae2f150d..bd05f53fa4 100644
> > >>> --- a/target/ppc/cpu_init.c
> > >>> +++ b/target/ppc/cpu_init.c
> > >>> @@ -9250,9 +9250,6 @@ static void ppc_cpu_class_init(ObjectClass *oc, 
> > >>> void *data)
> > >>>   cc->class_by_name = ppc_cpu_class_by_name;
> > >>>   cc->has_work = ppc_cpu_has_work;
> > >>>   cc->dump_state = ppc_cpu_dump_state;
> > >>> -#ifdef CONFIG_TCG
> > >>> -cc->dump_statistics = ppc_cpu_dump_statistics;
> > >>> -#endif
> > >> This confuses me.  The ifdefs you're removing aren't present in my
> > >> tree, and AFAICT they never existed since your own patch created
> > >> cpu_init.c.
> > >>
> > >> So.. please rebase and check that.
> > > Duh, sorry, I looked at this set out of order with your latest !tcg
> > > patches.  Now that I've applied those, I've applied those one as well.
> > Let me just check, where do you keep your most updated tree? I'm 
> > rebasing on your github tree, but ppc-for-6.1 there seems quite outdated 
> > (still the same as main)
> 
> Try here:
> 
> https://gitlab.com/dgibson/qemu/-/commits/ppc-for-6.1/

Right, I moved to gitlab a while back.  I sometimes push to the old
github tree as well, but I don't already remember.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH 1/1] ppc/pef.c: initialize cgs->ready in kvmppc_svm_init()

[David Gibson]

On Fri, May 28, 2021 at 05:16:19PM -0300, Daniel Henrique Barboza wrote:
> QEMU is failing to launch a CGS pSeries guest in a host that has PEF
> support:
> 
> qemu-system-ppc64: ../softmmu/vl.c:2585: qemu_machine_creation_done: 
> Assertion `machine->cgs->ready' failed.
> Aborted
> 
> This is happening because we're not setting the cgs->ready flag that is
> asserted in qemu_machine_creation_done() during machine start.
> 
> cgs->ready is set in s390_pv_kvm_init() and sev_kvm_init(). Let's set it
> in kvmppc_svm_init() as well.
> 
> Reported-by: Ram Pai 
> Signed-off-by: Daniel Henrique Barboza 

Oops, that's an embarrasing omission.  Applied to ppc-for-6.1.

> ---
>  hw/ppc/pef.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/pef.c b/hw/ppc/pef.c
> index 573be3ed79..cc44d5e339 100644
> --- a/hw/ppc/pef.c
> +++ b/hw/ppc/pef.c
> @@ -41,7 +41,7 @@ struct PefGuest {
>  ConfidentialGuestSupport parent_obj;
>  };
>  
> -static int kvmppc_svm_init(Error **errp)
> +static int kvmppc_svm_init(ConfidentialGuestSupport *cgs, Error **errp)
>  {
>  #ifdef CONFIG_KVM
>  static Error *pef_mig_blocker;
> @@ -65,6 +65,8 @@ static int kvmppc_svm_init(Error **errp)
>  /* NB: This can fail if --only-migratable is used */
>  migrate_add_blocker(pef_mig_blocker, &error_fatal);
>  
> +cgs->ready = true;
> +
>  return 0;
>  #else
>  g_assert_not_reached();
> @@ -102,7 +104,7 @@ int pef_kvm_init(ConfidentialGuestSupport *cgs, Error 
> **errp)
>  return -1;
>  }
>  
> -return kvmppc_svm_init(errp);
> +return kvmppc_svm_init(cgs, errp);
>  }
>  
>  int pef_kvm_reset(ConfidentialGuestSupport *cgs, Error **errp)

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH 2/5] target/ppc: remove ppc_cpu_dump_statistics

[David Gibson]

On Thu, May 27, 2021 at 08:01:56AM +0200, Greg Kurz wrote:
> On Wed, 26 May 2021 17:21:01 -0300
> "Bruno Larsen (billionai)"  wrote:
> 
> > This function requires surce code modification to be useful, which means
> 
> s/surce/source
> 
> > it probably is not used often, and the move to using decodetree means
> > the statistics won't even be collected anymore.
> > 
> > Also removed setting dump_statistics in ppc_cpu_realize, since it was
> 
> s/ppc_cpu_realize/ppc_cpu_class_init

A rebase on main has triggered a conflict with this patch, so I've
removed it from my tree again.

> 
> > only useful when in conjunction with ppc_cpu_dump_statistics.
> > 
> > Suggested-by: Richard Henderson
> > Signed-off-by: Bruno Larsen (billionai) 
> > ---
> >  target/ppc/cpu.h   |  1 -
> >  target/ppc/cpu_init.c  |  3 ---
> >  target/ppc/translate.c | 51 --
> >  3 files changed, 55 deletions(-)
> > 
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index 203f07e48e..c3d1b492e4 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -1256,7 +1256,6 @@ DECLARE_OBJ_CHECKERS(PPCVirtualHypervisor, 
> > PPCVirtualHypervisorClass,
> >  void ppc_cpu_do_interrupt(CPUState *cpu);
> >  bool ppc_cpu_exec_interrupt(CPUState *cpu, int int_req);
> >  void ppc_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
> > -void ppc_cpu_dump_statistics(CPUState *cpu, int flags);
> >  hwaddr ppc_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> >  int ppc_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
> >  int ppc_cpu_gdb_read_register_apple(CPUState *cpu, GByteArray *buf, int 
> > reg);
> > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> > index f5ae2f150d..bd05f53fa4 100644
> > --- a/target/ppc/cpu_init.c
> > +++ b/target/ppc/cpu_init.c
> > @@ -9250,9 +9250,6 @@ static void ppc_cpu_class_init(ObjectClass *oc, void 
> > *data)
> >  cc->class_by_name = ppc_cpu_class_by_name;
> >  cc->has_work = ppc_cpu_has_work;
> >  cc->dump_state = ppc_cpu_dump_state;
> > -#ifdef CONFIG_TCG
> > -cc->dump_statistics = ppc_cpu_dump_statistics;
> > -#endif
> >  cc->set_pc = ppc_cpu_set_pc;
> >  cc->gdb_read_register = ppc_cpu_gdb_read_register;
> >  cc->gdb_write_register = ppc_cpu_gdb_write_register;
> > diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> > index 6c0f424d81..fc9fd790ca 100644
> > --- a/target/ppc/translate.c
> > +++ b/target/ppc/translate.c
> > @@ -8881,57 +8881,6 @@ int ppc_fixup_cpu(PowerPCCPU *cpu)
> >  return 0;
> >  }
> >  
> > -
> > -void ppc_cpu_dump_statistics(CPUState *cs, int flags)
> > -{
> > -#if defined(DO_PPC_STATISTICS)
> > -PowerPCCPU *cpu = POWERPC_CPU(cs);
> > -opc_handler_t **t1, **t2, **t3, *handler;
> > -int op1, op2, op3;
> > -
> > -t1 = cpu->env.opcodes;
> > -for (op1 = 0; op1 < 64; op1++) {
> > -handler = t1[op1];
> > -if (is_indirect_opcode(handler)) {
> > -t2 = ind_table(handler);
> > -for (op2 = 0; op2 < 32; op2++) {
> > -handler = t2[op2];
> > -if (is_indirect_opcode(handler)) {
> > -t3 = ind_table(handler);
> > -for (op3 = 0; op3 < 32; op3++) {
> > -handler = t3[op3];
> > -if (handler->count == 0) {
> > -continue;
> > -}
> > -qemu_printf("%02x %02x %02x (%02x %04d) %16s: "
> > -"%016" PRIx64 " %" PRId64 "\n",
> > -op1, op2, op3, op1, (op3 << 5) | op2,
> > -handler->oname,
> > -handler->count, handler->count);
> > -}
> > -} else {
> > -if (handler->count == 0) {
> > -continue;
> > -}
> > -qemu_printf("%02x %02x(%02x %04d) %16s: "
> > -"%016" PRIx64 " %" PRId64 "\n",
> > -op1, op2, op1, op2, handler->oname,
> > -handler->count, handler->count);
> > -}
> > -}
> > -} else {
> > -if (handler->count == 0) {
> > -continue;
> > -}
> > -qemu_printf("%02x   (%02x ) %16s: %016" PRIx64
> > -" %" PRId64 "\n",
> > -op1, op1, handler->oname,
> > -handler->count, handler->count);
> > -}
> > -}
> > -#endif
> > -}
> > -
> >  static bool decode_legacy(PowerPCCPU *cpu, DisasContext *ctx, uint32_t 
> > insn)
> >  {
> >  opc_handler_t **table, *handler;
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
  

[PATCH v7 4/4] net: Extend host forwarding to support IPv6

[Doug Evans]

Net option "-hostfwd" now supports IPv6 addresses.
Commands hostfwd_add, hostfwd_remove now support IPv6 addresses.

Tested:
avocado run tests/acceptance/hostfwd.py

Signed-off-by: Doug Evans 
---

Changes from v6:

No changes.

Changes from v5:

Recognize ipv4=,ipv6= options.

 hmp-commands.hx | 18 ++-
 net/slirp.c | 54 +
 tests/acceptance/hostfwd.py | 94 +
 3 files changed, 155 insertions(+), 11 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 435c591a1c..05f88e893b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1322,7 +1322,8 @@ ERST
 {
 .name   = "hostfwd_add",
 .args_type  = "arg1:s,arg2:s?",
-.params = "[netdev_id] 
[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport",
+.params = "[netdev_id] [tcp|udp]:[hostaddr]:hostport\n"
+  "[,ipv4=on|off][,ipv6=on|off]-[guestaddr]:guestport",
 .help   = "redirect TCP or UDP connections from host to guest 
(requires -net user)",
 .cmd= hmp_hostfwd_add,
 },
@@ -1330,13 +1331,20 @@ ERST
 SRST
 ``hostfwd_add``
   Redirect TCP or UDP connections from host to guest (requires -net user).
+  IPV6 addresses are wrapped in square brackets, IPV4 addresses are not.
+
+  Examples:
+  hostfwd_add net0 tcp:127.0.0.1:10022-:22
+  hostfwd_add net0 tcp:[::1]:10022-[fe80::1:2:3:4]:22
+  hostfwd_add net0 ::10022,ipv6-:22
 ERST
 
 #ifdef CONFIG_SLIRP
 {
 .name   = "hostfwd_remove",
 .args_type  = "arg1:s,arg2:s?",
-.params = "[netdev_id] [tcp|udp]:[hostaddr]:hostport",
+.params = "[netdev_id] [tcp|udp]:[hostaddr]:hostport\n"
+  "[,ipv4=on|off][,ipv6=on|off]",
 .help   = "remove host-to-guest TCP or UDP redirection",
 .cmd= hmp_hostfwd_remove,
 },
@@ -1345,6 +1353,12 @@ ERST
 SRST
 ``hostfwd_remove``
   Remove host-to-guest TCP or UDP redirection.
+  IPV6 addresses are wrapped in square brackets, IPV4 addresses are not.
+
+  Examples:
+  hostfwd_remove net0 tcp:127.0.0.1:10022
+  hostfwd_remove net0 tcp:[::1]:10022
+  hostfwd_remove net0 ::10022,ipv6
 ERST
 
 {
diff --git a/net/slirp.c b/net/slirp.c
index 2349eb2c23..075a283d35 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -664,25 +664,55 @@ static const char *parse_protocol(const char *str, bool 
*is_udp,
 return p;
 }
 
-static int parse_hostfwd_sockaddr(const char *str, int socktype,
+static int parse_hostfwd_sockaddr(const char *str, int family, int socktype,
   struct sockaddr_storage *saddr,
-  Error **errp)
+  bool *v6_only, Error **errp)
 {
 struct addrinfo hints, *res = NULL, *e;
 InetSocketAddress *addr = g_new(InetSocketAddress, 1);
 int gai_rc;
 int rc = -1;
+Error *err = NULL;
 
 const char *optstr = inet_parse_host_port(addr, str, errp);
 if (optstr == NULL) {
 goto fail_return;
 }
 
+if (inet_parse_ipv46(addr, optstr, errp) < 0) {
+goto fail_return;
+}
+
+if (v6_only) {
+bool v4 = addr->has_ipv4 && addr->ipv4;
+bool v6 = addr->has_ipv6 && addr->ipv6;
+*v6_only = v6 && !v4;
+}
+
 memset(&hints, 0, sizeof(hints));
 hints.ai_flags = AI_PASSIVE; /* ignored if host is not ""(->NULL) */
 hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV;
 hints.ai_socktype = socktype;
-hints.ai_family = PF_INET;
+hints.ai_family = inet_ai_family_from_address(addr, &err);
+if (err) {
+error_propagate(errp, err);
+goto fail_return;
+}
+if (family != PF_UNSPEC) {
+/* Guest must use same family as host (for now). */
+if (hints.ai_family != PF_UNSPEC && hints.ai_family != family) {
+error_setg(errp,
+   "unexpected address family for %s: expecting %s",
+   str, family == PF_INET ? "ipv4" : "ipv6");
+goto fail_return;
+}
+hints.ai_family = family;
+}
+
+/* For backward compatibility, treat an empty host spec as IPv4. */
+if (*addr->host == '\0' && hints.ai_family == PF_UNSPEC) {
+hints.ai_family = PF_INET;
+}
 
 /*
  * Calling getaddrinfo for guest addresses is dubious, but addresses are
@@ -764,8 +794,8 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 goto fail_syntax;
 }
 
-if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM,
-   &host_addr, &error) < 0) {
+if (parse_hostfwd_sockaddr(p, PF_UNSPEC, is_udp ? SOCK_DGRAM : SOCK_STREAM,
+   &host_addr, /*v6_only=*/NULL, &error) < 0) {
 goto fail_syntax;
 }
 
@@ -807,6 +837,7 @@ static int slirp_hostfwd(SlirpState *s, const char 
*redir_str, Error **errp)
 bool is_udp;
 Error *error = NULL;
 int port;
+   

[PATCH v7 1/4] slirp: Advance libslirp submodule to 4.5 release

[Doug Evans]

5eraph (2):
  disable_dns option
  limit vnameserver_addr to port 53

Akihiro Suda (1):
  libslirp.h: fix SlirpConfig v3 documentation

Doug Evans (11):
  Add ipv6 host forward support
  tcpx_listen: Pass sizeof(addr) to memset
  Reject host forwarding to ipv6 "addr-any"
  Add /build/ to .gitignore
  New utility slirp_ether_ntoa
  m_cleanup_list: make static
  New API routine slirp_neighbor_info
  Move DEBUG_CALL("if_start") to DEBUG_VERBOSE_CALL
  tcpx_listen: tcp_newtcpcb doesn't fail
  slirp_add_host*fwd: Ensure all error paths set errno
  Perform lazy guest address resolution for IPv6

Dr. David Alan Gilbert (1):
  ip_stripoptions use memmove

Giuseppe Scrivano (1):
  socket: consume empty packets

Hafiz Abid Qadeer (1):
  Fix a typo that can cause slow socket response on Windows.

Jindrich Novy (4):
  Fix possible infinite loops and use-after-free
  Use secure string copy to avoid overflow
  Be sure to initialize sockaddr structure
  Check lseek() for failure

Marc-André Lureau (28):
  Merge branch 'master' into 'master'
  Merge branch 'fix-slirpconfig-3-doc' into 'master'
  Fix use-afte-free in ip_reass() (CVE-2020-1983)
  Update CHANGELOG
  Merge branch 'cve-2020-1983' into 'master'
  Release v4.3.0
  Merge branch 'release-v4.3.0' into 'master'
  changelog: post-release
  util: do not silently truncate
  Merge branch 'slirp-fmt-truncate' into 'master'
  Release v4.3.1
  Merge branch 'release-v4.3.1' into 'master'
  changelog: post-release
  .gitlab-ci: add a Coverity stage
  Merge branch 'coverity' into 'master'
  Merge branch 'ios-support' into 'master'
  Merge branch 'master' into 'master'
  Remove the QEMU-special make build-system
  Merge branch 'qemu' into 'master'
  Release v4.4.0
  Merge branch '4.4.0-release' into 'master'
  changelog: post-release
  Remove some needless (void)casts
  Fix unused variables
  Merge branch 'gitignore-build' into 'master'
  Merge branch 'macos-deployment-target' into 'master'
  Merge branch 'philmd' into 'master'
  Release v4.5.0

Nathaniel Wesley Filardo (1):
  fork_exec_child_setup: improve signal handling

Paolo Bonzini (2):
  meson: remove meson-dist script
  meson: support compiling as subproject

Philippe Mathieu-Daudé (4):
  Fix win32 builds by using the SLIRP_PACKED definition
  Fix constness warnings
  Remove unnecessary break
  Remove alloca() call in get_dns_addr_resolv_conf()

Prasad J Pandit (1):
  slirp: check pkt_len before reading protocol header

Ralf Haferkamp (2):
  Drop bogus IPv6 messages
  Fix MTU check

Samuel Thibault (48):
  Merge branch 'ip6_payload_len' into 'master'
  Merge branch 'lp1878043' into 'master'
  udp, udp6, icmp: handle TTL value
  icmp, icmp6: Add icmp_forward_error and icmp6_forward_error
  udp, udp6, icmp, icmp6: Enable forwarding errors on Linux
  TCPIPHDR_DELTA: Fix potential negative value
  sosendoob: better document what urgc is used for
  Merge branch 'G_GNUC_PRINTF' into 'master'
  Merge branch 'CVE-2020-29129' into 'master'
  Merge branch 'ttl' into 'master'
  Merge branch 'errors' into 'master'
  Merge branch 'consume-empty-packet' into 'master'
  Merge branch 'void' into 'master'
  Merge branch 'master' into 'master'
  Merge branch 'unused' into 'master'
  Merge branch 'socket_delay' into 'master'
  tcp_subr: simplify code
  Merge branch 'ipv6-host-fwd-9-patch' into 'master'
  Document the slirp API
  Complete timeout documentation
  Merge branch 'memset-sizeof' into 'master'
  Merge branch 'reject-ipv6-addr-any' into 'master'
  ip6_output: fix memory leak on fast-send
  Merge branch 'ndp-leak' into 'master'
  Merge branch 'memory_leaks' into 'master'
  TODO for generalizing the hostfwd calls
  socket.h: add missing sbuf.h inclusion
  Expose udpx_listen and tcpx_listen as taking sockaddr
  Disable polling for PRI on MacOS
  Merge branch 'macos-pri' into 'master'
  Merge branch 'x_listen' into 'master'
  udpx/tcpx_listen: Add missing const qualifier
  sockaddr_*: add missing const qualifiers
  Merge branch 'm-cleanup-list-prototype' into 'master'
  Merge branch 'neighbor-info' into 'master'
  udpx/tcpx_listen: Use struct sockaddr * types
  Add ipv4/ipv6-agnostic host forwarding functions
  hostfwd: Add SLIRP_HOSTFWD_V6ONLY flag
  Merge branch 'hostxfwd' into 'master'
  Merge branch 'verbose-if-start' into 'master'
  Remove slirp_add/remove_ipv6_hostfwd
  Merge branch 'listen-errno' into 'master'
  Merge branch 'newtcpcb-no-fail' into 'master'
  Merge branch 'listen_v6only' into 'master'
  Merge branch 'lazy-ipv6-resolution' into 'master'
  ndp_table: For unspecified address, return broadcast etherne

[PATCH v7 3/4] net/slirp.c: Refactor address parsing

[Doug Evans]

... in preparation for adding ipv6 host forwarding support.

Tested:
avocado run tests/acceptance/hostfwd.py

Signed-off-by: Doug Evans 
---

Changes from v6:

Add support for --enable-slirp=system
Tested with system libslirp 4.4.0.

Changes from v5:

Use InetSocketAddress and getaddrinfo().
Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd.

 include/qemu/sockets.h  |   2 +
 net/slirp.c | 236 ++--
 tests/acceptance/hostfwd.py |  91 ++
 util/qemu-sockets.c |  17 ++-
 4 files changed, 278 insertions(+), 68 deletions(-)
 create mode 100644 tests/acceptance/hostfwd.py

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 94f4e8de83..6fd71775ce 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -29,6 +29,8 @@ int socket_set_fast_reuse(int fd);
 #define SHUT_RDWR 2
 #endif
 
+int sockaddr_getport(const struct sockaddr *addr);
+
 int inet_ai_family_from_address(InetSocketAddress *addr,
 Error **errp);
 const char *inet_parse_host_port(InetSocketAddress *addr,
diff --git a/net/slirp.c b/net/slirp.c
index ad3a838e0b..2349eb2c23 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -641,14 +641,106 @@ static SlirpState *slirp_lookup(Monitor *mon, const char 
*id)
 }
 }
 
+static const char *parse_protocol(const char *str, bool *is_udp,
+  Error **errp)
+{
+char buf[10];
+const char *p = str;
+
+if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
+error_setg(errp, "missing protocol name separator");
+return NULL;
+}
+
+if (!strcmp(buf, "tcp") || buf[0] == '\0') {
+*is_udp = false;
+} else if (!strcmp(buf, "udp")) {
+*is_udp = true;
+} else {
+error_setg(errp, "bad protocol name '%s'", buf);
+return NULL;
+}
+
+return p;
+}
+
+static int parse_hostfwd_sockaddr(const char *str, int socktype,
+  struct sockaddr_storage *saddr,
+  Error **errp)
+{
+struct addrinfo hints, *res = NULL, *e;
+InetSocketAddress *addr = g_new(InetSocketAddress, 1);
+int gai_rc;
+int rc = -1;
+
+const char *optstr = inet_parse_host_port(addr, str, errp);
+if (optstr == NULL) {
+goto fail_return;
+}
+
+memset(&hints, 0, sizeof(hints));
+hints.ai_flags = AI_PASSIVE; /* ignored if host is not ""(->NULL) */
+hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV;
+hints.ai_socktype = socktype;
+hints.ai_family = PF_INET;
+
+/*
+ * Calling getaddrinfo for guest addresses is dubious, but addresses are
+ * restricted to numeric only. Convert "" to NULL for getaddrinfo's
+ * benefit.
+ */
+gai_rc = getaddrinfo(*addr->host ? addr->host : NULL,
+ *addr->port ? addr->port : NULL, &hints, &res);
+if (gai_rc != 0) {
+error_setg(errp, "address resolution failed for '%s': %s",
+   str, gai_strerror(gai_rc));
+goto fail_return;
+}
+if (res->ai_next != NULL) {
+/*
+ * The caller only wants one address, and except for "any" for both
+ * ipv4 and ipv6 (which we've already precluded above), we shouldn't
+ * get more than one. To assist debugging print all we find.
+ */
+GString *s = g_string_new(NULL);
+for (e = res; e != NULL; e = e->ai_next) {
+char host[NI_MAXHOST];
+char serv[NI_MAXSERV];
+int ret = getnameinfo((struct sockaddr *)e->ai_addr, e->ai_addrlen,
+  host, sizeof(host),
+  serv, sizeof(serv),
+  NI_NUMERICHOST | NI_NUMERICSERV);
+if (ret == 0) {
+g_string_append_printf(s, "\n  %s:%s", host, serv);
+} else {
+g_string_append_printf(s, "\n  unknown, got: %s",
+   gai_strerror(ret));
+}
+}
+error_setg(errp, "multiple addresses resolved for '%s':%s",
+   str, s->str);
+g_string_free(s, TRUE);
+goto fail_return;
+}
+
+memcpy(saddr, res->ai_addr, res->ai_addrlen);
+rc = 0;
+
+ fail_return:
+qapi_free_InetSocketAddress(addr);
+if (res) {
+freeaddrinfo(res);
+}
+return rc;
+}
+
 void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 {
-struct in_addr host_addr = { .s_addr = INADDR_ANY };
-int host_port;
-char buf[256];
+struct sockaddr_storage host_addr;
 const char *src_str, *p;
 SlirpState *s;
-int is_udp = 0;
+bool is_udp;
+Error *error = NULL;
 int err;
 const char *arg1 = qdict_get_str(qdict, "arg1");
 const char *arg2 = qdict_get_try_str(qdict, "arg2");
@@ -664,110 +756,130 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict 
*qdict)
 return;
 }
 
+g_assert

[PATCH v7 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

[Doug Evans]

The parsing is moved into new function inet_parse_host_port.
Also split out is ipv4=flag, ipv6=flag processing into inet_parse_ipv46.
This is done in preparation for using these functions in net/slirp.c.

Signed-off-by: Doug Evans 
Reviewed-by: Marc-André Lureau 
---

Changes from v6:

No changes.

Changes from v5:

Also split out parsing of ipv4=on|off, ipv6=on|off

 include/qemu/sockets.h |  3 ++
 util/qemu-sockets.c| 65 +-
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 7d1f813576..94f4e8de83 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -31,6 +31,9 @@ int socket_set_fast_reuse(int fd);
 
 int inet_ai_family_from_address(InetSocketAddress *addr,
 Error **errp);
+const char *inet_parse_host_port(InetSocketAddress *addr,
+ const char *str, Error **errp);
+int inet_parse_ipv46(InetSocketAddress *addr, const char *optstr, Error 
**errp);
 int inet_parse(InetSocketAddress *addr, const char *str, Error **errp);
 int inet_connect(const char *str, Error **errp);
 int inet_connect_saddr(InetSocketAddress *saddr, Error **errp);
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 2463c49773..aa883eb84f 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -615,14 +615,12 @@ static int inet_parse_flag(const char *flagname, const 
char *optstr, bool *val,
 return 0;
 }
 
-int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
+const char *inet_parse_host_port(InetSocketAddress *addr, const char *str,
+ Error **errp)
 {
-const char *optstr, *h;
 char host[65];
 char port[33];
-int to;
 int pos;
-char *begin;
 
 memset(addr, 0, sizeof(*addr));
 
@@ -632,38 +630,32 @@ int inet_parse(InetSocketAddress *addr, const char *str, 
Error **errp)
 host[0] = '\0';
 if (sscanf(str, ":%32[^,]%n", port, &pos) != 1) {
 error_setg(errp, "error parsing port in address '%s'", str);
-return -1;
+return NULL;
 }
 } else if (str[0] == '[') {
 /* IPv6 addr */
 if (sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, &pos) != 2) {
 error_setg(errp, "error parsing IPv6 address '%s'", str);
-return -1;
+return NULL;
 }
 } else {
 /* hostname or IPv4 addr */
 if (sscanf(str, "%64[^:]:%32[^,]%n", host, port, &pos) != 2) {
 error_setg(errp, "error parsing address '%s'", str);
-return -1;
+return NULL;
 }
 }
 
 addr->host = g_strdup(host);
 addr->port = g_strdup(port);
 
-/* parse options */
-optstr = str + pos;
-h = strstr(optstr, ",to=");
-if (h) {
-h += 4;
-if (sscanf(h, "%d%n", &to, &pos) != 1 ||
-(h[pos] != '\0' && h[pos] != ',')) {
-error_setg(errp, "error parsing to= argument");
-return -1;
-}
-addr->has_to = true;
-addr->to = to;
-}
+return str + pos;
+}
+
+int inet_parse_ipv46(InetSocketAddress *addr, const char *optstr, Error **errp)
+{
+char *begin;
+
 begin = strstr(optstr, ",ipv4");
 if (begin) {
 if (inet_parse_flag("ipv4", begin + 5, &addr->ipv4, errp) < 0) {
@@ -678,6 +670,39 @@ int inet_parse(InetSocketAddress *addr, const char *str, 
Error **errp)
 }
 addr->has_ipv6 = true;
 }
+
+return 0;
+}
+
+int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
+{
+const char *optstr, *h;
+int to;
+int pos;
+char *begin;
+
+optstr = inet_parse_host_port(addr, str, errp);
+if (optstr == NULL) {
+return -1;
+}
+
+/* parse options */
+
+if (inet_parse_ipv46(addr, optstr, errp) < 0) {
+return -1;
+}
+
+h = strstr(optstr, ",to=");
+if (h) {
+h += 4;
+if (sscanf(h, "%d%n", &to, &pos) != 1 ||
+(h[pos] != '\0' && h[pos] != ',')) {
+error_setg(errp, "error parsing to= argument");
+return -1;
+}
+addr->has_to = true;
+addr->to = to;
+}
 begin = strstr(optstr, ",keep-alive");
 if (begin) {
 if (inet_parse_flag("keep-alive", begin + strlen(",keep-alive"),
-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

[PATCH v7 0/4] Add support for ipv6 host forwarding

[Doug Evans]

This patchset takes the original patch from Maxim,
https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html
and updates it.

Option hostfwd is extended to support ipv6 addresses.
Commands hostfwd_add, hostfwd_remove are extended as well.

Changes from v6:

1/4: Update to use libslirp v4.5.0 tag

The libslirp parts of the patch have been committed to the libslirp repo,
and are now in QEMU's copy of the libslirp repo.
Advancing QEMU to use Libslirp v4.5.0 is being done separately.
Discussion of patch 1/4 is left to that thread:
https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06010.html

2/4: No change

3/4: Add support for --enable-slirp=system
Tested with system libslirp 4.4.0.

4/4: No change

Changes from v5:

1/4 slirp: Advance libslirp submodule to current master
NOTE TO REVIEWERS: It may be a better use of everyone's time if a
maintainer takes on advancing QEMU's libslirp to libslirp's master.
Beyond that, I really don't know what to do except submit this patch as
is currently provided.

2/4: util/qemu-sockets.c: Split host:port parsing out of inet_parse

Also split out parsing of ipv4=on|off, ipv6=on|off

3/4: net/slirp.c: Refactor address parsing

Use InetSocketAddress and getaddrinfo().
Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd.

4/4: net: Extend host forwarding to support IPv6

Recognize ipv4=,ipv6= options.

Note: v5's 3/5 "Recognize []:port (empty ipv6 address)" has been deleted:
the churn on this patch series needs to be reduced.
This change is not required, and can easily be done in a later patch.

Changes from v4:

1/5 slirp: Advance libslirp submodule to add ipv6 host-forward support
NOTE TO REVIEWERS: I need some hand-holding to know what The Right
way to submit this particular patch is.

- no change

2/5 util/qemu-sockets.c: Split host:port parsing out of inet_parse

- move recognition of "[]:port" to separate patch
- allow passing NULL for ip_v6
- fix some formatting issues

3/5 inet_parse_host_and_addr: Recognize []:port (empty ipv6 address)

- new in this patchset revision

4/5 net/slirp.c: Refactor address parsing

- was 3/4 in v4
- fix some formatting issues

5/5 net: Extend host forwarding to support IPv6

- was 4/4 in v4
- fix some formatting issues

Changes from v3:

1/4 slirp: Advance libslirp submodule to add ipv6 host-forward support

- pick up latest libslirp patch to reject ipv6 addr-any for guest address
  - libslirp currently only provides a stateless DHCPv6 server, which means
it can't know in advance what the guest's IP address is, and thus
cannot do the "addr-any -> guest ip address" translation that is done
for ipv4

2/4 util/qemu-sockets.c: Split host:port parsing out of inet_parse

- this patch is new in v4
  - provides new utility: inet_parse_host_and_port, updates inet_parse
to use it

3/4 net/slirp.c: Refactor address parsing

- this patch renamed from 2/3 to 3/4
- call inet_parse_host_and_port from util/qemu-sockets.c
- added tests/acceptance/hostfwd.py

4/4 net: Extend host forwarding to support IPv6

- this patch renamed from 3/3 to 4/4
- ipv6 support added to existing hostfwd option, commands
  - instead of creating new ipv6 option, commands
- added tests to tests/acceptance/hostfwd.py

Changes from v2:
- split out libslirp commit
- clarify spelling of ipv6 addresses in docs
- tighten parsing of ipv6 addresses

Change from v1:
- libslirp part is now upstream
- net/slirp.c changes split into two pieces (refactor, add ipv6)
- added docs

Doug Evans (4):
  slirp: Advance libslirp submodule to 4.5 release
  util/qemu-sockets.c: Split host:port parsing out of inet_parse
  net/slirp.c: Refactor address parsing
  net: Extend host forwarding to support IPv6

 hmp-commands.hx |  18 ++-
 include/qemu/sockets.h  |   5 +
 net/slirp.c | 272 
 slirp   |   2 +-
 tests/acceptance/hostfwd.py | 185 
 util/qemu-sockets.c |  82 +++
 6 files changed, 473 insertions(+), 91 deletions(-)
 create mode 100644 tests/acceptance/hostfwd.py

-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

Re: [PATCH 1/1] ppc/pef.c: initialize cgs->ready in kvmppc_svm_init()

[Ram Pai]

On Fri, May 28, 2021 at 05:16:19PM -0300, Daniel Henrique Barboza wrote:
> QEMU is failing to launch a CGS pSeries guest in a host that has PEF
> support:
> 
> qemu-system-ppc64: ../softmmu/vl.c:2585: qemu_machine_creation_done: 
> Assertion `machine->cgs->ready' failed.
> Aborted
> 
> This is happening because we're not setting the cgs->ready flag that is
> asserted in qemu_machine_creation_done() during machine start.
> 
> cgs->ready is set in s390_pv_kvm_init() and sev_kvm_init(). Let's set it
> in kvmppc_svm_init() as well.
> 
> Reported-by: Ram Pai 
> Signed-off-by: Daniel Henrique Barboza 


Acked-by: Ram Pai 

> ---
>  hw/ppc/pef.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/pef.c b/hw/ppc/pef.c
> index 573be3ed79..cc44d5e339 100644
..snip..

Re: [PATCH v4 14/15] qemu-iotests: add option to show qemu binary logs on stdout

[Emanuele Giuseppe Esposito]

+
  imgfmt = os.environ.get('IMGFMT', 'raw')
  imgproto = os.environ.get('IMGPROTO', 'file')
  output_dir = os.environ.get('OUTPUT_DIR', '.')
@@ -614,6 +616,13 @@ def _post_shutdown(self) -> None:
  super()._post_shutdown()
  self.subprocess_check_valgrind(qemu_valgrind)
+    def _pre_launch(self) -> None:
+    super()._pre_launch()
+    if qemu_print and self._qemu_log_file is not None:
+    # set QEMU binary output to stdout
+    self._qemu_log_file.close()
+    self._qemu_log_file = None
+


So, many use of _private members actually show that proper way of doing 
this is adding an option to __init__ instead


And then add yet another bool variable in __init__ just to mark when use 
the log file or not? At this point, if we really don't want this here we 
can just create a public function in machine.py and call that...

This can also be shared with machine.py's _post_shutdown().



Interesting will pylint complain on using _private members outside of 
the home class?


No, test 297 tests it and prints no warning or error.


Thank you,
Emanuele

Re: [PATCH v4 08/15] docs/devel/testing: add -gdb option to the debugging section of QEMU iotests

[Emanuele Giuseppe Esposito]

On 28/05/2021 19:16, Vladimir Sementsov-Ogievskiy wrote:

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

Signed-off-by: Emanuele Giuseppe Esposito 


Reviewed-by: Vladimir Sementsov-Ogievskiy 


---
  docs/devel/testing.rst | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 8144e316a4..a746cab745 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -229,6 +229,17 @@ Debugging a test case
  The following options to the ``check`` script can be useful when 
debugging

  a failing test:
+* ``-gdb`` wraps every QEMU invocation in a ``gdbserver``, which 
waits for a
+  connection from a gdb client.  The options given to ``gdbserver`` 
(e.g. the
+  address on which to listen for connections) are taken from the 
``$GDB_OPTIONS``
+  environment variable.  By default (if ``$GDB_OPTIONS`` is empty), 
it listens on

+  ``localhost:12345``.
+  It is possible to connect to it for example with
+  ``gdb -iex "target remote $addr"``, where ``$addr`` is the address
+  ``gdbserver`` listens on.
+  If the ``-gdb`` option is not used, ``$GDB_OPTIONS`` is ignored,
+  regardless on whether it is set or not.
+
  * ``-d`` (debug) just increases the logging verbosity, showing
    for example the QMP commands and answers.



Didn't you think of an interface as simple as just wrap qemu binary by 
"gdb --args" and redirect stdin and stdout directly to the terminal 
where test is running? Wouldn't it more convenient? So, you just start 
./check -gdb , and you are in gdb. Or you need exactly gdb server, 
to attach from another machine?


Keeping this possibility in mind, it's probably better to call you 
option "-gdbserver"... But we can rename later if needed, finally, it's 
only a test framework.



See 
https://patchew.org/QEMU/20210414170352.29927-1-eespo...@redhat.com/20210414170352.29927-5-eespo...@redhat.com/ 


(penultimate email), there was a similar question:



Out of interest: Why gdbserver and not “just” gdb?  On Fedora, those are 
separate packages, so I don’t have gdbserver installed, that’s why I’m 
asking.


As far as I have tried, using only gdb with ./check is very hard to use, 
because the stdout is filtered out by the script.
So invoking gdb attaches it to QEMU, but it is not possible to start 
execution (run command) or interact with it, because of the python 
script filtering. This leaves the test hanging.


gdbserver is just something that a gdb client can attach to (for 
example, in another console or even in another host) for example with 
the command
# gdb -iex "target remote localhost:12345" . This provides a nice and 
separate gdb monitor to the client.


At this point I should put this somewhere, either in commit message or 
in the actual documentation...


I don't know about the name. "gdb" should also be short for "gdbserver" 
in a way.


Thank you,
Emanuele

Re: [PATCH v4 09/15] qemu-iotests: extend the check script to support valgrind for python tests

[Emanuele Giuseppe Esposito]

  sys.exit(os.EX_USAGE)
+qemu_valgrind = []
+if os.environ.get('VALGRIND_QEMU') == "y" and \
+    os.environ.get('NO_VALGRIND') != "y":
+    valgrind_logfile = "--log-file=" + test_dir.strip()





a bit strange that you need to strip test_dir here.. Why?

Yep, it's unnecessary



+VALGRIND_QEMU -- {VALGRIND_QEMU}
  """
  args = collections.defaultdict(str, self.get_env())



commit subject sais "support valgrind", but actually we only add a 
variable. valgrind is not actually used yet.. I'd reflect it in commit 
subject somehow



will replace with "prepare supporting"

Emanuele

[PATCH 1/1] ppc/pef.c: initialize cgs->ready in kvmppc_svm_init()

[Daniel Henrique Barboza]

QEMU is failing to launch a CGS pSeries guest in a host that has PEF
support:

qemu-system-ppc64: ../softmmu/vl.c:2585: qemu_machine_creation_done: Assertion 
`machine->cgs->ready' failed.
Aborted

This is happening because we're not setting the cgs->ready flag that is
asserted in qemu_machine_creation_done() during machine start.

cgs->ready is set in s390_pv_kvm_init() and sev_kvm_init(). Let's set it
in kvmppc_svm_init() as well.

Reported-by: Ram Pai 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/pef.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pef.c b/hw/ppc/pef.c
index 573be3ed79..cc44d5e339 100644
--- a/hw/ppc/pef.c
+++ b/hw/ppc/pef.c
@@ -41,7 +41,7 @@ struct PefGuest {
 ConfidentialGuestSupport parent_obj;
 };
 
-static int kvmppc_svm_init(Error **errp)
+static int kvmppc_svm_init(ConfidentialGuestSupport *cgs, Error **errp)
 {
 #ifdef CONFIG_KVM
 static Error *pef_mig_blocker;
@@ -65,6 +65,8 @@ static int kvmppc_svm_init(Error **errp)
 /* NB: This can fail if --only-migratable is used */
 migrate_add_blocker(pef_mig_blocker, &error_fatal);
 
+cgs->ready = true;
+
 return 0;
 #else
 g_assert_not_reached();
@@ -102,7 +104,7 @@ int pef_kvm_init(ConfidentialGuestSupport *cgs, Error 
**errp)
 return -1;
 }
 
-return kvmppc_svm_init(errp);
+return kvmppc_svm_init(cgs, errp);
 }
 
 int pef_kvm_reset(ConfidentialGuestSupport *cgs, Error **errp)
-- 
2.31.1

RE: [PATCH] docs/devel: Explain in more detail the TB chaining mechanisms

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> More completely, update the CPU state with any information that has been
> assumed constant.  For most guests, this is just the PC.  But e.g. for hppa
> this is both iaoq.f (cip) and iaoq.b (nip).
> 
> It is very much up to the guest to determine the set of data that is present 
> in
> cpu_get_tb_cpu_state, and what can be assumed across the break.

I’m not sure I understand what “assumed constant” means in this context. Would
it be fair to say that step 2 should update any CPU state information that is
required by the main loop to correctly locate and execute the next TB, but not
anything that would be needed if we were to jump directly from step 1 to the 
first
instruction in the next TB without going through the main loop?

Thanks,

--
Luis Pires
Instituto de Pesquisas ELDORADO 
Aviso Legal - Disclaimer 

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Swetha Joshi]

I apologize for the delay, here are the repro steps:
1. Remove CONFIG_ARM_VIRT=y from arm-softmmu.mak
2. In .gitlab-ci.yml, crossbuild.yml and in tests/vm/Makefile.include, in
all the places where we disable kvm using -disable-kvm, replace this
with -enable-kvm
3. Build

You should see errors pointing to these routines: virt_is_acpi_enabled,
acpi_ghes_record_errors

Thanks,
Swetha.

On Fri, May 28, 2021 at 12:08 AM Dongjiu Geng 
wrote:

> Peter Maydell  于2021年5月27日周四 上午2:19写道：
> >
> > On Wed, 26 May 2021 at 18:32, Swetha Joshi 
> wrote:
> > >
> > > Hello,
> > >
> > > One of the qemu machines we use has KVM enabled, but we don't want the
> CONFIG_ARM_VIRT enabled as it pulls in emulation of a variety of physical
> hardware that we don't need. The compilation errors I mentioned are not in
> the qemu mainline per say but we see them in one of the qemu derived
> machines we use.
> >
> > Sure, but unless you can give me a recipe for reproducing the
> > build failure in mainline I can't really help...
>
> Hi Swetha，
>  Yes,  Can you give a method that how to reproduce the build
> failure issues? Thanks
>
> >
> > thanks
> > -- PMM
>


-- 
Regards

Swetha Joshi.

[Bug 1929710] Related fix merged to nova (master)

[OpenStack Infra]

Reviewed:  https://review.opendev.org/c/openstack/nova/+/793219
Committed: 
https://opendev.org/openstack/nova/commit/d5ed968826895d362f4f2aa21decfdebb9b1fd84
Submitter: "Zuul (22348)"
Branch:master

commit d5ed968826895d362f4f2aa21decfdebb9b1fd84
Author: Lee Yarwood 
Date:   Wed May 26 19:27:45 2021 +0100

zuul: Skip swap_volume tests as part of nova-next

The volume update or swap_volume API has long been a source of gate
failures within Nova. Most recently we've seen increased instability
when running the temptest.api.compute.admin.test_volume_swap tests as
part of the nova-next job as documented in bug #1929710.

This change temporarily removes the failing test from the nova-next job
while the underlying issue is identified lower in the virt stack.

Change-Id: Ib56a034fb08e309981d0b4553b8cee8d16b10152
Related-Bug: #1929710

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

Title:
  virDomainGetBlockJobInfo fails during swap_volume as disk '$disk' not
  found in domain

Status in OpenStack Compute (nova):
  New
Status in QEMU:
  New

Bug description:
  Description
  ===

  The error handling around swap_volume is missing the following failure
  when calling virDomainGetBlockJobInfo() after the entire device is
  detached by QEMU (?) after it encounters a failure during the block
  copy job that at first pauses and then somehow resumes:

  https://8a5fc27780098c5ee1bc-
  3ac81d180a9c011938b2cbb0293272f3.ssl.cf5.rackcdn.com/790660/5/gate
  /nova-next/e915ed4/controller/logs/screen-n-cpu.txt

  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver [None 
req-7cfcd661-29d4-4cc3-bc54-db0e7fed1a6e tempest-TestVolumeSwap-1841575704 
tempest-TestVolumeSwap-1841575704-project-admin] Failure rebasing volume 
/dev/sdb on vdb.: libvirt.libvirtError: invalid argument: disk 'vdb' not found 
in domain
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver Traceback (most recent 
call last):
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver   File 
"/opt/stack/nova/nova/virt/libvirt/driver.py", line 2107, in _swap_volume
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver while not 
dev.is_job_complete():
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver   File 
"/opt/stack/nova/nova/virt/libvirt/guest.py", line 800, in is_job_complete
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver status = 
self.get_job_info()
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver   File 
"/opt/stack/nova/nova/virt/libvirt/guest.py", line 707, in get_job_info
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver status = 
self._guest._domain.blockJobInfo(self._disk, flags=0)
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver   File 
"/usr/local/lib/python3.8/dist-packages/eventlet/tpool.py", line 190, in doit
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver result = 
proxy_call(self._autowrap, f, *args, **kwargs)
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver   File 
"/usr/local/lib/python3.8/dist-packages/eventlet/tpool.py", line 148, in 
proxy_call
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver rv = execute(f, *args, 
**kwargs)
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver   File 
"/usr/local/lib/python3.8/dist-packages/eventlet/tpool.py", line 129, in execute
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver six.reraise(c, e, tb)
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver   File 
"/usr/local/lib/python3.8/dist-packages/six.py", line 719, in reraise
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver raise value
  May 26 09:49:47.314813 ubuntu-focal-vexxhost-ca-ymq-1-0024823853 
nova-compute[114649]: ERROR nova.virt.libvirt.driver   File 
"/usr/local/lib/python3.8/dist-packages/eventlet/tpool.py", line 83, in tworker
  May 26 09:49:47.

Re: [PATCH] vfio: Fix unregister SaveVMHandler in vfio_migration_finalize

[Kirti Wankhede]

On 5/28/2021 7:34 AM, Kunkun Jiang wrote:

Hi Philippe,

On 2021/5/27 21:44, Philippe Mathieu-Daudé wrote:

On 5/27/21 2:31 PM, Kunkun Jiang wrote:

In the vfio_migration_init(), the SaveVMHandler is registered for
VFIO device. But it lacks the operation of 'unregister'. It will
lead to 'Segmentation fault (core dumped)' in
qemu_savevm_state_setup(), if performing live migration after a
VFIO device is hot deleted.

Fixes: 7c2f5f75f94 (vfio: Register SaveVMHandlers for VFIO device)
Reported-by: Qixin Gan 
Signed-off-by: Kunkun Jiang 

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


---
  hw/vfio/migration.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 201642d75e..ef397ebe6c 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -892,6 +892,7 @@ void vfio_migration_finalize(VFIODevice *vbasedev)
  
remove_migration_state_change_notifier(&migration->migration_state);

  qemu_del_vm_change_state_handler(migration->vm_state);
+    unregister_savevm(VMSTATE_IF(vbasedev->dev), "vfio", vbasedev);

Hmm what about devices using "%s/vfio" id?

The unregister_savevm() needs 'VMSTATEIf *obj'. If we pass a non-null 'obj'
to unregister_svevm(), it will handle the devices using "%s/vfio" id with
the following code:

    if (obj) {
    char *oid = vmstate_if_get_id(obj);
    if (oid) {
    pstrcpy(id, sizeof(id), oid);
    pstrcat(id, sizeof(id), "/");
    g_free(oid);
    }
    }
    pstrcat(id, sizeof(id), idstr);


This fix seems fine to me.



By the way, I'm puzzled that register_savevm_live() and unregister_savevm()
handle devices using "%s/vfio" id differently. So I learned the commit
history of register_savevm_live() and unregister_savevm().

In the beginning, both them need 'DeviceState *dev', which are replaced
with VMStateIf in 3cad405babb. Later in ce62df5378b, the 'dev' was removed,
because no caller of register_savevm_live() need to pass a non-null 'dev'
at that time.

So now the vfio devices need to handle the 'id' first and then call
register_savevm_live(). I am wondering whether we need to add
'VMSTATEIf *obj' in register_savevm_live(). What do you think of this?



I think proposed change above is independent of this fix. I'll defer to 
other experts.


Reviewed by: Kirti Wankhede 

[PATCH v3 6/7] ACPI ERST: qtest for ERST

[Eric DeVolder]

This change provides a qtest that locates and then does a simple
interrogation of the ERST feature within the guest.

Signed-off-by: Eric DeVolder 
---
 tests/qtest/erst-test.c | 106 
 tests/qtest/meson.build |   2 +
 2 files changed, 108 insertions(+)
 create mode 100644 tests/qtest/erst-test.c

diff --git a/tests/qtest/erst-test.c b/tests/qtest/erst-test.c
new file mode 100644
index 000..ce7fc70
--- /dev/null
+++ b/tests/qtest/erst-test.c
@@ -0,0 +1,106 @@
+/*
+ * QTest testcase for ACPI ERST
+ *
+ * Copyright (c) 2021 Oracle
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bitmap.h"
+#include "qemu/uuid.h"
+#include "hw/acpi/acpi-defs.h"
+#include "boot-sector.h"
+#include "acpi-utils.h"
+#include "libqos/libqtest.h"
+#include "qapi/qmp/qdict.h"
+
+#define RSDP_ADDR_INVALID 0x10 /* RSDP must be below this address */
+
+static uint64_t acpi_find_erst(QTestState *qts)
+{
+uint32_t rsdp_offset;
+uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
+uint32_t rsdt_len, table_length;
+uint8_t *rsdt, *ent;
+uint64_t base = 0;
+
+/* Wait for guest firmware to finish and start the payload. */
+boot_sector_test(qts);
+
+/* Tables should be initialized now. */
+rsdp_offset = acpi_find_rsdp_address(qts);
+
+g_assert_cmphex(rsdp_offset, <, RSDP_ADDR_INVALID);
+
+acpi_fetch_rsdp_table(qts, rsdp_offset, rsdp_table);
+acpi_fetch_table(qts, &rsdt, &rsdt_len, &rsdp_table[16 /* RsdtAddress */],
+ 4, "RSDT", true);
+
+ACPI_FOREACH_RSDT_ENTRY(rsdt, rsdt_len, ent, 4 /* Entry size */) {
+uint8_t *table_aml;
+acpi_fetch_table(qts, &table_aml, &table_length, ent, 4, NULL, true);
+if (!memcmp(table_aml + 0 /* Header Signature */, "ERST", 4)) {
+/*
+ * Picking up ERST base address from the Register Region
+ * specified as part of the first Serialization Instruction
+ * Action (which is a Begin Write Operation).
+ */
+memcpy(&base, &table_aml[56], sizeof(base));
+g_free(table_aml);
+break;
+}
+g_free(table_aml);
+}
+g_free(rsdt);
+return base;
+}
+
+static char disk[] = "tests/erst-test-disk-XX";
+
+#define ERST_CMD()  \
+"-accel kvm -accel tcg "\
+"-drive id=hd0,if=none,file=%s,format=raw " \
+"-device ide-hd,drive=hd0 ", disk
+
+static void erst_get_error_log_address_range(void)
+{
+QTestState *qts;
+uint64_t log_address_range = 0;
+
+qts = qtest_initf(ERST_CMD());
+
+uint64_t base = acpi_find_erst(qts);
+g_assert(base != 0);
+
+/* Issue GET_ERROR_LOG_ADDRESS_RANGE command */
+qtest_writel(qts, base + 0, 0xD);
+/* Read GET_ERROR_LOG_ADDRESS_RANGE result */
+log_address_range = qtest_readq(qts, base + 8);\
+
+/* Check addr_range is offset of base */
+g_assert((base + 16) == log_address_range);
+
+qtest_quit(qts);
+}
+
+int main(int argc, char **argv)
+{
+int ret;
+
+ret = boot_sector_init(disk);
+if (ret) {
+return ret;
+}
+
+g_test_init(&argc, &argv, NULL);
+
+qtest_add_func("/erst/get-error-log-address-range",
+   erst_get_error_log_address_range);
+
+ret = g_test_run();
+boot_sector_cleanup(disk);
+
+return ret;
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 0c76738..deae443 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -66,6 +66,7 @@ qtests_i386 = \
   (config_all_devices.has_key('CONFIG_RTL8139_PCI') ? ['rtl8139-test'] : []) + 
 \
   (config_all_devices.has_key('CONFIG_E1000E_PCI_EXPRESS') ? 
['fuzz-e1000e-test'] : []) +   \
   (config_all_devices.has_key('CONFIG_ESP_PCI') ? ['am53c974-test'] : []) +
 \
+  (config_all_devices.has_key('CONFIG_ACPI') ? ['erst-test'] : []) +   
  \
   qtests_pci + 
 \
   ['fdc-test',
'ide-test',
@@ -237,6 +238,7 @@ qtests = {
   'bios-tables-test': [io, 'boot-sector.c', 'acpi-utils.c', 'tpm-emu.c'],
   'cdrom-test': files('boot-sector.c'),
   'dbus-vmstate-test': files('migration-helpers.c') + dbus_vmstate1,
+  'erst-test': files('erst-test.c', 'boot-sector.c', 'acpi-utils.c'),
   'ivshmem-test': [rt, '../../contrib/ivshmem-server/ivshmem-server.c'],
   'migration-test': files('migration-helpers.c'),
   'pxe-test': files('boot-sector.c'),
-- 
1.8.3.1

Re: [PULL 00/31] tcg patch queue

[Peter Maydell]

On Thu, 27 May 2021 at 00:47, Richard Henderson
 wrote:
>
> The following changes since commit 0319ad22bd5789e1eaa8a2dd5773db2d2c372f20:
>
>   Merge remote-tracking branch 
> 'remotes/stsquad/tags/pull-testing-and-misc-updates-250521-2' into staging 
> (2021-05-25 17:31:04 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210526
>
> for you to fetch changes up to 119065574d02deffc28fe5b6a864db9b467c6ffd:
>
>   hw/core: Constify TCGCPUOps (2021-05-26 15:33:59 -0700)
>
> 
> Adjust types for some memory access functions.
> Reduce inclusion of tcg headers.
> Fix watchpoints vs replay.
> Fix tcg/aarch64 roli expansion.
> Introduce SysemuCPUOps structure.


Applied, thanks.

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

-- PMM

[PATCH v3 7/7] ACPI ERST: step 6 of bios-tables-test.c

[Eric DeVolder]

Signed-off-by: Eric DeVolder 
---
 tests/data/acpi/pc/ERST | Bin 0 -> 976 bytes
 tests/data/acpi/q35/ERST| Bin 0 -> 976 bytes
 tests/qtest/bios-tables-test-allowed-diff.h |   4 
 3 files changed, 4 deletions(-)

diff --git a/tests/data/acpi/pc/ERST b/tests/data/acpi/pc/ERST
index 
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..7236018951f9d111d8cacaa93ee07a8dc3294f18
 100644
GIT binary patch
literal 976
zcmaKqSq_3Q6h#Y^dE9^rOK=GWV&b1h{BUvZ#VzQD#NN_})srw9ZqJfYH@yl5T!0*@ExA}PsmTmxA~y7^f&wF
z`=C;Mzb#Jlr!;>?JY$ah@BPi%Ksot29-5NhSucQ
c)srw9ZqJfYH@yl5T!0*@ExA}PsmTmxA~y7^f&wF
z`=C;Mzb#Jlr!;>?JY$ah@BPi%Ksot29-5NhSucQ
c

[PATCH v3 5/7] ACPI ERST: create ERST device for pc/x86 machines.

[Eric DeVolder]

This change enables ERST support for x86 guests.

ERST can be disabled at run-time, for example, with:

 -machine q35,erst=off

Signed-off-by: Eric DeVolder 
---
 hw/i386/acpi-build.c |  7 +++
 hw/i386/pc.c | 31 +++
 include/hw/i386/pc.h |  1 +
 3 files changed, 39 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index de98750..6ba79db 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -43,6 +43,7 @@
 #include "sysemu/tpm.h"
 #include "hw/acpi/tpm.h"
 #include "hw/acpi/vmgenid.h"
+#include "hw/acpi/erst.h"
 #include "hw/boards.h"
 #include "sysemu/tpm_backend.h"
 #include "hw/rtc/mc146818rtc_regs.h"
@@ -2388,6 +2389,12 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
*machine)
 ACPI_DEVICE_IF(x86ms->acpi_dev), x86ms->oem_id,
 x86ms->oem_table_id);
 
+if (pcms->erst_enabled) {
+acpi_add_table(table_offsets, tables_blob);
+build_erst(tables_blob, tables->linker,
+   x86ms->oem_id, x86ms->oem_table_id);
+}
+
 vmgenid_dev = find_vmgenid_dev();
 if (vmgenid_dev) {
 acpi_add_table(table_offsets, tables_blob);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8a84b25..b7b4cc4 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -74,6 +74,7 @@
 #include "qemu/cutils.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/cpu_hotplug.h"
+#include "hw/acpi/erst.h"
 #include "hw/boards.h"
 #include "acpi-build.h"
 #include "hw/mem/pc-dimm.h"
@@ -,6 +1112,7 @@ void pc_basic_device_init(struct PCMachineState *pcms,
 ISADevice *pit = NULL;
 MemoryRegion *ioport80_io = g_new(MemoryRegion, 1);
 MemoryRegion *ioportF0_io = g_new(MemoryRegion, 1);
+const X86MachineState *x86ms = X86_MACHINE(pcms);
 
 memory_region_init_io(ioport80_io, NULL, &ioport80_io_ops, NULL, 
"ioport80", 1);
 memory_region_add_subregion(isa_bus->address_space_io, 0x80, ioport80_io);
@@ -1153,6 +1155,11 @@ void pc_basic_device_init(struct PCMachineState *pcms,
 }
 *rtc_state = mc146818_rtc_init(isa_bus, 2000, rtc_irq);
 
+if (pcms->erst_enabled && x86_machine_is_acpi_enabled(x86ms)) {
+hwaddr base = HPET_BASE + 0x1UL;
+setup_erst_dev(base, error_fatal);
+}
+
 qemu_register_boot_set(pc_boot_set, *rtc_state);
 
 if (!xen_enabled() && pcms->pit_enabled) {
@@ -1529,6 +1536,22 @@ static void pc_machine_set_hpet(Object *obj, bool value, 
Error **errp)
 pcms->hpet_enabled = value;
 }
 
+#ifdef CONFIG_ACPI
+static bool pc_machine_get_erst(Object *obj, Error **errp)
+{
+PCMachineState *pcms = PC_MACHINE(obj);
+
+return pcms->erst_enabled;
+}
+
+static void pc_machine_set_erst(Object *obj, bool value, Error **errp)
+{
+PCMachineState *pcms = PC_MACHINE(obj);
+
+pcms->erst_enabled = value;
+}
+#endif
+
 static void pc_machine_get_max_ram_below_4g(Object *obj, Visitor *v,
 const char *name, void *opaque,
 Error **errp)
@@ -1628,6 +1651,9 @@ static void pc_machine_initfn(Object *obj)
 #ifdef CONFIG_HPET
 pcms->hpet_enabled = true;
 #endif
+#ifdef CONFIG_ACPI
+pcms->erst_enabled = true;
+#endif
 
 pc_system_flash_create(pcms);
 pcms->pcspk = isa_new(TYPE_PC_SPEAKER);
@@ -1752,6 +1778,11 @@ static void pc_machine_class_init(ObjectClass *oc, void 
*data)
 object_class_property_add_bool(oc, "hpet",
 pc_machine_get_hpet, pc_machine_set_hpet);
 
+#ifdef CONFIG_ACPI
+object_class_property_add_bool(oc, "erst",
+pc_machine_get_erst, pc_machine_set_erst);
+#endif
+
 object_class_property_add(oc, PC_MACHINE_MAX_FW_SIZE, "size",
 pc_machine_get_max_fw_size, pc_machine_set_max_fw_size,
 NULL, NULL);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index dcf060b..4458c8f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -45,6 +45,7 @@ typedef struct PCMachineState {
 bool sata_enabled;
 bool pit_enabled;
 bool hpet_enabled;
+bool erst_enabled;
 uint64_t max_fw_size;
 
 /* NUMA information: */
-- 
1.8.3.1

[PATCH v3 2/7] ACPI ERST: header file for ERST

[Eric DeVolder]

This change introduces the defintions for ACPI ERST.

Signed-off-by: Eric DeVolder 
---
 include/hw/acpi/erst.h | 82 ++
 1 file changed, 82 insertions(+)
 create mode 100644 include/hw/acpi/erst.h

diff --git a/include/hw/acpi/erst.h b/include/hw/acpi/erst.h
new file mode 100644
index 000..8a8ac5c
--- /dev/null
+++ b/include/hw/acpi/erst.h
@@ -0,0 +1,82 @@
+/*
+ * ACPI Error Record Serialization Table, ERST, Implementation
+ *
+ * Copyright (c) 2020 Oracle and/or its affiliates.
+ *
+ * See ACPI specification, "ACPI Platform Error Interfaces"
+ *  "Error Serialization"
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2 of the License.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see 
+ */
+#ifndef HW_ACPI_ERST_H
+#define HW_ACPI_ERST_H
+
+void build_erst(GArray *table_data, BIOSLinker *linker,
+const char *oem_id, const char *oem_table_id);
+void setup_erst_dev(hwaddr base, Error *error_fatal);
+
+#define TYPE_ACPI_ERST "acpi-erst"
+#define ACPI_ERST_BASE_PROP "base"
+#define ACPI_ERST_SIZE_PROP "size"
+#define ACPI_ERST_FILE_PROP "file"
+
+#define ACPI_ERST_ACTION_BEGIN_WRITE_OPERATION 0x0
+#define ACPI_ERST_ACTION_BEGIN_READ_OPERATION  0x1
+#define ACPI_ERST_ACTION_BEGIN_CLEAR_OPERATION 0x2
+#define ACPI_ERST_ACTION_END_OPERATION 0x3
+#define ACPI_ERST_ACTION_SET_RECORD_OFFSET 0x4
+#define ACPI_ERST_ACTION_EXECUTE_OPERATION 0x5
+#define ACPI_ERST_ACTION_CHECK_BUSY_STATUS 0x6
+#define ACPI_ERST_ACTION_GET_COMMAND_STATUS0x7
+#define ACPI_ERST_ACTION_GET_RECORD_IDENTIFIER 0x8
+#define ACPI_ERST_ACTION_SET_RECORD_IDENTIFIER 0x9
+#define ACPI_ERST_ACTION_GET_RECORD_COUNT  0xA
+#define ACPI_ERST_ACTION_BEGIN_DUMMY_WRITE_OPERATION   0xB
+#define ACPI_ERST_ACTION_RESERVED  0xC
+#define ACPI_ERST_ACTION_GET_ERROR_LOG_ADDRESS_RANGE   0xD
+#define ACPI_ERST_ACTION_GET_ERROR_LOG_ADDRESS_LENGTH  0xE
+#define ACPI_ERST_ACTION_GET_ERROR_LOG_ADDRESS_RANGE_ATTRIBUTES 0xF
+#define ACPI_ERST_ACTION_GET_EXECUTE_OPERATION_TIMINGS 0x10
+#define ACPI_ERST_MAX_ACTIONS \
+(ACPI_ERST_ACTION_GET_EXECUTE_OPERATION_TIMINGS + 1)
+
+#define ACPI_ERST_STATUS_SUCCESS0x00
+#define ACPI_ERST_STATUS_NOT_ENOUGH_SPACE   0x01
+#define ACPI_ERST_STATUS_HARDWARE_NOT_AVAILABLE 0x02
+#define ACPI_ERST_STATUS_FAILED 0x03
+#define ACPI_ERST_STATUS_RECORD_STORE_EMPTY 0x04
+#define ACPI_ERST_STATUS_RECORD_NOT_FOUND   0x05
+
+#define ACPI_ERST_INST_READ_REGISTER 0x00
+#define ACPI_ERST_INST_READ_REGISTER_VALUE   0x01
+#define ACPI_ERST_INST_WRITE_REGISTER0x02
+#define ACPI_ERST_INST_WRITE_REGISTER_VALUE  0x03
+#define ACPI_ERST_INST_NOOP  0x04
+#define ACPI_ERST_INST_LOAD_VAR1 0x05
+#define ACPI_ERST_INST_LOAD_VAR2 0x06
+#define ACPI_ERST_INST_STORE_VAR10x07
+#define ACPI_ERST_INST_ADD   0x08
+#define ACPI_ERST_INST_SUBTRACT  0x09
+#define ACPI_ERST_INST_ADD_VALUE 0x0A
+#define ACPI_ERST_INST_SUBTRACT_VALUE0x0B
+#define ACPI_ERST_INST_STALL 0x0C
+#define ACPI_ERST_INST_STALL_WHILE_TRUE  0x0D
+#define ACPI_ERST_INST_SKIP_NEXT_INSTRUCTION_IF_TRUE 0x0E
+#define ACPI_ERST_INST_GOTO  0x0F
+#define ACPI_ERST_INST_SET_SRC_ADDRESS_BASE  0x10
+#define ACPI_ERST_INST_SET_DST_ADDRESS_BASE  0x11
+#define ACPI_ERST_INST_MOVE_DATA 0x12
+
+#endif
+
-- 
1.8.3.1

[PATCH v3 3/7] ACPI ERST: support for ACPI ERST feature

[Eric DeVolder]

This change implements the support for the ACPI ERST feature[1,2].

The size of the ACPI ERST storage is declared via the QEMU
global parameter acpi-erst.size. The size can range from 64KiB
to to 64MiB. The default is 64KiB.

The location of the ACPI ERST storage backing file is declared
via the QEMU global parameter acpi-erst.file. The default
is acpi-erst.backing.

[1] "Advanced Configuration and Power Interface Specification",
version 6.2, May 2017.
https://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf

[2] "Unified Extensible Firmware Interface Specification",
version 2.8, March 2019.
https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_final.pdf

Signed-off-by: Eric DeVolder 
---
 hw/acpi/erst.c | 902 +
 1 file changed, 902 insertions(+)
 create mode 100644 hw/acpi/erst.c

diff --git a/hw/acpi/erst.c b/hw/acpi/erst.c
new file mode 100644
index 000..645a904
--- /dev/null
+++ b/hw/acpi/erst.c
@@ -0,0 +1,902 @@
+/*
+ * ACPI Error Record Serialization Table, ERST, Implementation
+ *
+ * Copyright (c) 2021 Oracle and/or its affiliates.
+ *
+ * See ACPI specification,
+ * "ACPI Platform Error Interfaces" : "Error Serialization"
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2 of the License.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see 
+ */
+
+#include 
+#include 
+#include 
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "qom/object_interfaces.h"
+#include "qemu/error-report.h"
+#include "migration/vmstate.h"
+#include "hw/qdev-properties.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/acpi-defs.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/bios-linker-loader.h"
+#include "exec/address-spaces.h"
+#include "sysemu/hostmem.h"
+#include "hw/acpi/erst.h"
+
+#ifdef _ERST_DEBUG
+#define erst_debug(fmt, ...) \
+do { fprintf(stderr, fmt, ## __VA_ARGS__); fflush(stderr); } while (0)
+#else
+#define erst_debug(fmt, ...) do { } while (0)
+#endif
+
+/* See UEFI spec, Appendix N Common Platform Error Record */
+/* UEFI CPER allows for an OSPM book keeping area in the record */
+#define UEFI_CPER_RECORD_MIN_SIZE 128U
+#define UEFI_CPER_SIZE_OFFSET 20U
+#define UEFI_CPER_RECORD_ID_OFFSET 96U
+#define IS_UEFI_CPER_RECORD(ptr) \
+(((ptr)[0] == 'C') && \
+ ((ptr)[1] == 'P') && \
+ ((ptr)[2] == 'E') && \
+ ((ptr)[3] == 'R'))
+#define THE_UEFI_CPER_RECORD_ID(ptr) \
+(*(uint64_t *)(&(ptr)[UEFI_CPER_RECORD_ID_OFFSET]))
+
+#define ERST_INVALID_RECORD_ID (~0UL)
+#define ERST_EXECUTE_OPERATION_MAGIC 0x9CUL
+#define ERST_CSR_ACTION (0UL << 3) /* action (cmd) */
+#define ERST_CSR_VALUE  (1UL << 3) /* argument/value (data) */
+
+/*
+ * As ERST_IOMEM_SIZE is used to map the ERST into the guest,
+ * it should/must be an integer multiple of PAGE_SIZE.
+ * NOTE that any change to this value will make any pre-
+ * existing backing files, not of the same ERST_IOMEM_SIZE,
+ * unusable to the guest.
+ */
+#define ERST_IOMEM_SIZE (2UL * 4096)
+
+/*
+ * This implementation is an ACTION (cmd) and VALUE (data)
+ * interface consisting of just two 64-bit registers.
+ */
+#define ERST_REG_LEN (2UL * sizeof(uint64_t))
+
+/*
+ * The space not utilized by the register interface is the
+ * buffer for exchanging ERST record contents.
+ */
+#define ERST_RECORD_SIZE (ERST_IOMEM_SIZE - ERST_REG_LEN)
+
+/*
+ * Mode to be used for backing file
+ */
+#define ACPIERST(obj) \
+OBJECT_CHECK(ERSTDeviceState, (obj), TYPE_ACPI_ERST)
+#define ACPIERST_CLASS(oc) \
+OBJECT_CLASS_CHECK(ERSTDeviceStateClass, (oc), TYPE_ACPI_ERST)
+#define ACPIERST_GET_CLASS(obj) \
+OBJECT_GET_CLASS(ERSTDeviceStateClass, (obj), TYPE_ACPI_ERST)
+
+typedef struct {
+SysBusDevice parent_obj;
+Object *hostmem_obj;
+HostMemoryBackend *hostmem;
+
+MemoryRegion iomem;
+uint32_t prop_size;
+hwaddr prop_base;
+char *prop_path;
+
+uint8_t operation;
+uint8_t busy_status;
+uint8_t command_status;
+uint32_t record_offset;
+uint32_t record_count;
+uint64_t reg_action;
+uint64_t reg_value;
+uint64_t record_identifier;
+
+unsigned next_record_index;
+uint8_t record[ERST_RECORD_SIZE]; /* read/written directly by guest */
+uint8_t tmp_record[ERST_RECORD_SIZE]; /* intermediate manipulation buffer 
*/
+
+} ERSTDeviceState;
+
+static unsigned copy_from_nvram_by_index(ERSTDeviceState *s, unsigned index)
+{
+/* Read an nvram entry into tmp_recor

[PATCH v3 0/7] acpi: Error Record Serialization Table, ERST, support for QEMU

[Eric DeVolder]

NOTE: Also, I wanted to push this v3 for review while alerting
that I will be on holiday through June 8 (possibly a few days
later).

NOTE: The patches are arranged such that each can be applied
in order and not break the build (except the 0001 patch). Igor
has hinted at changing this, but I'm unsure how else to
re-arrange these patches accordingly.

NOTE: With the conversion to TYPE_MEMORY_BACKEND_FILE, live
migration to a completely different host does not behave 
properly (it loses the ERST contents because the file is not
present on the new host). This still needs to be worked out.
Other than live migration, this patchset fully works.

This patchset introduces support for the ACPI Error Record
Serialization Table, ERST.

Linux uses the persistent storage filesystem, pstore, to record
information (eg. dmesg tail) upon panics and shutdowns.  Pstore is
independent of, and runs before, kdump.  In certain scenarios (ie.
hosts/guests with root filesystems on NFS/iSCSI where networking
software and/or hardware fails), pstore may contain the only
information available for post-mortem debugging.

Two common storage backends for the pstore filesystem are ACPI ERST
and UEFI. Most BIOS implement ACPI ERST; however, ACPI ERST is not
currently supported in QEMU, and UEFI is not utilized in all guests.
By implementing ACPI ERST within QEMU, then the ACPI ERST becomes a
viable pstore storage backend for virtual machines (as it is now for
bare metal machines).

Enabling support for ACPI ERST facilitates a consistent method to
capture kernel panic information in a wide range of guests: from
resource-constrained microvms to very large guests, and in
particular, in direct-boot environments (which would lack UEFI
run-time services).

Note that Microsoft Windows also utilizes the ACPI ERST for certain
crash information, if available.

The ACPI ERST persistent storage is contained within a single backing
file, with a default size of 64KiB. The size and filename of the
backing file can be obtained from QEMU parameters.

The ACPI specification[1], in Chapter "ACPI Platform Error Interfaces
(APEI)", and specifically subsection "Error Serialization", outlines
a method for storing error records into persistent storage.

[1] "Advanced Configuration and Power Interface Specification",
version 6.2, May 2017.
https://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf

[2] "Unified Extensible Firmware Interface Specification",
version 2.8, March 2019.
https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_final.pdf

Suggested-by: Konrad Wilk 
Signed-off-by: Eric DeVolder 

---
v3: 28may2021
 - Converted to using a TYPE_MEMORY_BACKEND_FILE object rather than
   internal array with explicit file operations, per Igor.
 - Changed the way the qdev and base address are handled, allowing
   ERST to be disabled at run-time. Also aligns better with other
   existing code.

v2: 8feb2021
 - Added qtest/smoke test per Paolo Bonzini
 - Split patch into smaller chunks, per Igo Mammedov
 - Did away with use of ACPI packed structures, per Igo Mammedov

v1: 26oct2020
 - initial post

---
 hw/acpi/erst.c | 909 +
 hw/acpi/meson.build|   1 +
 hw/i386/acpi-build.c   |   4 +
 include/hw/acpi/erst.h |  97 ++
 4 files changed, 1011 insertions(+)
 create mode 100644 hw/acpi/erst.c
 create mode 100644 include/hw/acpi/erst.h


Eric DeVolder (7):
  ACPI ERST: bios-tables-test.c steps 1 and 2
  ACPI ERST: header file for ERST
  ACPI ERST: support for ACPI ERST feature
  ACPI ERST: include ERST feature in build of ACPI support
  ACPI ERST: create ERST device for pc/x86 machines.
  ACPI ERST: qtest for ERST
  ACPI ERST: step 6 of bios-tables-test.c

 hw/acpi/erst.c   | 902 +++
 hw/acpi/meson.build  |   1 +
 hw/i386/acpi-build.c |   7 +
 hw/i386/pc.c |  31 ++
 include/hw/acpi/erst.h   |  82 
 include/hw/i386/pc.h |   1 +
 tests/data/acpi/microvm/ERST |   0
 tests/data/acpi/pc/ERST  | Bin 0 -> 976 bytes
 tests/data/acpi/q35/ERST | Bin 0 -> 976 bytes
 tests/qtest/erst-test.c  | 106 +
 tests/qtest/meson.build  |   2 +
 11 files changed, 1132 insertions(+)
 create mode 100644 hw/acpi/erst.c
 create mode 100644 include/hw/acpi/erst.h
 create mode 100644 tests/data/acpi/microvm/ERST
 create mode 100644 tests/data/acpi/pc/ERST
 create mode 100644 tests/data/acpi/q35/ERST
 create mode 100644 tests/qtest/erst-test.c

-- 
1.8.3.1

[PATCH v3 1/7] ACPI ERST: bios-tables-test.c steps 1 and 2

[Eric DeVolder]

Following the guidelines in tests/qtest/bios-tables-test.c, this
change adds empty placeholder files per step 1 for the new ERST
table, and excludes resulting changed files in bios-tables-test-allowed-diff.h
per step 2.

Signed-off-by: Eric DeVolder 
---
 tests/data/acpi/microvm/ERST| 0
 tests/data/acpi/pc/ERST | 0
 tests/data/acpi/q35/ERST| 0
 tests/qtest/bios-tables-test-allowed-diff.h | 4 
 4 files changed, 4 insertions(+)
 create mode 100644 tests/data/acpi/microvm/ERST
 create mode 100644 tests/data/acpi/pc/ERST
 create mode 100644 tests/data/acpi/q35/ERST

diff --git a/tests/data/acpi/microvm/ERST b/tests/data/acpi/microvm/ERST
new file mode 100644
index 000..e69de29
diff --git a/tests/data/acpi/pc/ERST b/tests/data/acpi/pc/ERST
new file mode 100644
index 000..e69de29
diff --git a/tests/data/acpi/q35/ERST b/tests/data/acpi/q35/ERST
new file mode 100644
index 000..e69de29
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523..e004c71 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,5 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/pc/ERST",
+"tests/data/acpi/q35/ERST",
+"tests/data/acpi/microvm/ERST",
+
-- 
1.8.3.1

[PATCH v3 4/7] ACPI ERST: include ERST feature in build of ACPI support

[Eric DeVolder]

This change includes ERST in the build of ACPI.

Signed-off-by: Eric DeVolder 
---
 hw/acpi/meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
index dd69577..262a8ee 100644
--- a/hw/acpi/meson.build
+++ b/hw/acpi/meson.build
@@ -4,6 +4,7 @@ acpi_ss.add(files(
   'aml-build.c',
   'bios-linker-loader.c',
   'utils.c',
+  'erst.c',
 ))
 acpi_ss.add(when: 'CONFIG_ACPI_CPU_HOTPLUG', if_true: files('cpu.c'))
 acpi_ss.add(when: 'CONFIG_ACPI_CPU_HOTPLUG', if_true: files('cpu_hotplug.c'))
-- 
1.8.3.1

Re: [PATCH v4 15/15] docs/devel/testing: add -p option to the debug section of QEMU iotests

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

Signed-off-by: Emanuele Giuseppe Esposito 


I'd merge it to previous patch. anyway:

Reviewed-by: Vladimir Sementsov-Ogievskiy 


---
  docs/devel/testing.rst | 4 
  1 file changed, 4 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index d743e88746..1192d6489e 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -250,6 +250,10 @@ a failing test:
  * ``-d`` (debug) just increases the logging verbosity, showing
for example the QMP commands and answers.
  
+* ``-p`` (print) redirect QEMU’s stdout and stderr to the test output,

+  instead of saving it into a log file in
+  ``$TEST_DIR/qemu-machine-``.
+
  Test case groups
  
  




--
Best regards,
Vladimir

Re: [PATCH v4 14/15] qemu-iotests: add option to show qemu binary logs on stdout

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

Using the flag -p, allow the qemu binary to print to stdout.

Reviewed-by: Max Reitz 
Signed-off-by: Emanuele Giuseppe Esposito 
---
  tests/qemu-iotests/check  | 4 +++-
  tests/qemu-iotests/iotests.py | 9 +
  tests/qemu-iotests/testenv.py | 9 +++--
  3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 2101cedfe3..51b90681ab 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -33,6 +33,8 @@ def make_argparser() -> argparse.ArgumentParser:
 help='pretty print output for make check')
  
  p.add_argument('-d', dest='debug', action='store_true', help='debug')

+p.add_argument('-p', dest='print', action='store_true',
+help='redirects qemu\'s stdout and stderr to the test output')
  p.add_argument('-gdb', action='store_true',
 help="start gdbserver with $GDB_OPTIONS options \
  ('localhost:12345' if $GDB_OPTIONS is empty)")
@@ -117,7 +119,7 @@ if __name__ == '__main__':
aiomode=args.aiomode, cachemode=args.cachemode,
imgopts=args.imgopts, misalign=args.misalign,
debug=args.debug, valgrind=args.valgrind,
-  gdb=args.gdb)
+  gdb=args.gdb, qprint=args.print)
  
  testfinder = TestFinder(test_dir=env.source_iotests)
  
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index 75f1e1711c..53a3916a91 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -80,6 +80,8 @@
  if gdb_qemu_env:
  qemu_gdb = ['gdbserver'] + gdb_qemu_env.strip().split(' ')
  
+qemu_print = os.environ.get('PRINT_QEMU', False)

+
  imgfmt = os.environ.get('IMGFMT', 'raw')
  imgproto = os.environ.get('IMGPROTO', 'file')
  output_dir = os.environ.get('OUTPUT_DIR', '.')
@@ -614,6 +616,13 @@ def _post_shutdown(self) -> None:
  super()._post_shutdown()
  self.subprocess_check_valgrind(qemu_valgrind)
  
+def _pre_launch(self) -> None:

+super()._pre_launch()
+if qemu_print and self._qemu_log_file is not None:
+# set QEMU binary output to stdout
+self._qemu_log_file.close()
+self._qemu_log_file = None
+


So, many use of _private members actually show that proper way of doing this is 
adding an option to __init__ instead

Interesting will pylint complain on using _private members outside of the home 
class?


  def add_object(self, opts):
  self._args.append('-object')
  self._args.append(opts)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 319d29cb0c..b79ce22fe9 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -74,7 +74,7 @@ class TestEnv(ContextManager['TestEnv']):
   'AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU',
   'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 
'IMGOPTSSYNTAX',
   'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 
'MALLOC_PERTURB_',
- 'GDB_OPTIONS']
+ 'GDB_OPTIONS', 'PRINT_QEMU']
  
  def get_env(self) -> Dict[str, str]:

  env = {}
@@ -166,7 +166,8 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
   misalign: bool = False,
   debug: bool = False,
   valgrind: bool = False,
- gdb: bool = False) -> None:
+ gdb: bool = False,
+ qprint: bool = False) -> None:
  self.imgfmt = imgfmt
  self.imgproto = imgproto
  self.aiomode = aiomode
@@ -174,6 +175,9 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
  self.misalign = misalign
  self.debug = debug
  
+if qprint:

+self.print_qemu = 'y'
+
  if gdb:
  self.gdb_options = os.environ.get('GDB_OPTIONS', DEF_GDB_OPTIONS)
  if not self.gdb_options:
@@ -283,6 +287,7 @@ def print_env(self) -> None:
  SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
  GDB_OPTIONS   -- {GDB_OPTIONS}
  VALGRIND_QEMU -- {VALGRIND_QEMU}
+PRINT_QEMU_OUTPUT -- {PRINT_QEMU}
  """
  
  args = collections.defaultdict(str, self.get_env())




5 places we need to modify to add a new option. That's not very good :( (not 
about your patch).

And I still doubt, that we want to add any new variable to print_env. If we do, 
than it's simpler to print all variables here, than, one place less to modify 
by hand.


Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH v4 13/15] docs/devel/testing: add -valgrind option to the debug section of QEMU iotests

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

Reviewed-by: Max Reitz
Signed-off-by: Emanuele Giuseppe Esposito



Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH v4 12/15] qemu-iotests: insert valgrind command line as wrapper for qemu binary

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

The priority will be given to gdb command line, meaning if the -gdb
parameter and -valgrind are given, gdb will be wrapped around
the qemu binary.


I'd prefer just return an error immediately if user specify both -gdb and 
-valgrind



Signed-off-by: Emanuele Giuseppe Esposito 
---
  tests/qemu-iotests/iotests.py | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index a06284acad..75f1e1711c 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -590,7 +590,8 @@ class VM(qtest.QEMUQtestMachine):
  def __init__(self, path_suffix=''):
  name = "qemu%s-%d" % (path_suffix, os.getpid())
  timer = 15.0 if not (qemu_gdb or qemu_valgrind) else None
-super().__init__(qemu_prog, qemu_opts, wrapper=qemu_gdb,
+wrapper = qemu_gdb if qemu_gdb else qemu_valgrind
+super().__init__(qemu_prog, qemu_opts, wrapper=wrapper,
   name=name,
   test_dir=test_dir,
   socket_scm_helper=socket_scm_helper,




Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

RE: [PATCH 03/11] softfloat: Introduce float_flag_inorm_denormal

[Michael Morrell]

I'm probably missing something, but why do we need both 
"float_flag_inorm_denormal" and "float_flag_iflush_denormal"?

Couldn't the code that sets these flags set just a single flag for all denormal 
inputs and the code that checks these flags check that single flag
combined with the "flush_inputs_to_zero" flag to accomplish what the two 
separate "input denormal" flags do?

   Michael

-Original Message-
From: Richard Henderson  
Sent: Wednesday, May 26, 2021 9:14 PM
To: qemu-devel@nongnu.org
Cc: Michael Morrell ; alex.ben...@linaro.org
Subject: [PATCH 03/11] softfloat: Introduce float_flag_inorm_denormal

Create a new exception flag for reporting input denormals that are not flushed 
to zero, they are normalized and treated as normal numbers.

Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat-types.h | 15 ---
 fpu/softfloat.c   | 84 +++
 fpu/softfloat-parts.c.inc |  1 +
 3 files changed, 36 insertions(+), 64 deletions(-)

diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h 
index e2d70ff556..174100e50e 100644
--- a/include/fpu/softfloat-types.h
+++ b/include/fpu/softfloat-types.h
@@ -143,13 +143,14 @@ typedef enum __attribute__((__packed__)) {
  */
 
 enum {
-float_flag_invalid   =  1,
-float_flag_divbyzero =  4,
-float_flag_overflow  =  8,
-float_flag_underflow = 16,
-float_flag_inexact   = 32,
-float_flag_iflush_denormal = 64,
-float_flag_oflush_denormal = 128
+float_flag_invalid = 0x0001,
+float_flag_divbyzero   = 0x0002,
+float_flag_overflow= 0x0004,
+float_flag_underflow   = 0x0008,
+float_flag_inexact = 0x0010,
+float_flag_inorm_denormal  = 0x0020,  /* denormal input, normalized */
+float_flag_iflush_denormal = 0x0040,  /* denormal input, flushed to zero */
+float_flag_oflush_denormal = 0x0080,  /* denormal result, flushed 
+ to zero */
 };
 
 /*
diff --git a/fpu/softfloat.c b/fpu/softfloat.c index cb077cf111..e54cdb274d 
100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -126,61 +126,23 @@ this code that are retained.
  * denormal/inf/NaN; (2) when operands are not guaranteed to lead to a 0 result
  * and the result is < the minimum normal.
  */
-#define GEN_INPUT_FLUSH__NOCHECK(name, soft_t)  \
+
+#define GEN_INPUT_FLUSH(name, soft_t)   \
 static inline void name(soft_t *a, float_status *s) \
 {   \
 if (unlikely(soft_t ## _is_denormal(*a))) { \
-*a = soft_t ## _set_sign(soft_t ## _zero,   \
- soft_t ## _is_neg(*a));\
-float_raise(float_flag_iflush_denormal, s);  \
+if (s->flush_inputs_to_zero) {  \
+*a = soft_t ## _set_sign(0, soft_t ## _is_neg(*a)); \
+float_raise(float_flag_iflush_denormal, s); \
+} else {\
+float_raise(float_flag_inorm_denormal, s);  \
+}   \
 }   \
 }
 
-GEN_INPUT_FLUSH__NOCHECK(float32_input_flush__nocheck, float32) 
-GEN_INPUT_FLUSH__NOCHECK(float64_input_flush__nocheck, float64) -#undef 
GEN_INPUT_FLUSH__NOCHECK
-
-#define GEN_INPUT_FLUSH1(name, soft_t)  \
-static inline void name(soft_t *a, float_status *s) \
-{   \
-if (likely(!s->flush_inputs_to_zero)) { \
-return; \
-}   \
-soft_t ## _input_flush__nocheck(a, s);  \
-}
-
-GEN_INPUT_FLUSH1(float32_input_flush1, float32) 
-GEN_INPUT_FLUSH1(float64_input_flush1, float64) -#undef GEN_INPUT_FLUSH1
-
-#define GEN_INPUT_FLUSH2(name, soft_t)  \
-static inline void name(soft_t *a, soft_t *b, float_status *s)  \
-{   \
-if (likely(!s->flush_inputs_to_zero)) { \
-return; \
-}   \
-soft_t ## _input_flush__nocheck(a, s);  \
-soft_t ## _input_flush__nocheck(b, s);  \
-}
-
-GEN_INPUT_FLUSH2(float32_input_flush2, float32) 
-GEN_INPUT_FLUSH2(float64_input_flush2, float64) -#undef GEN_INPUT_FLUSH2
-
-#define GEN_INPUT_FLUSH3(name, soft_t)  \
-static inline void name(soft_t *a, soft_t *b, soft_

Re: [PATCH v4 11/15] qemu-iotests: allow valgrind to read/delete the generated log file

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

When using -valgrind on the script tests, it generates a log file
in $TEST_DIR that is either read (if valgrind finds problems) or
otherwise deleted. Provide the same exact behavior when using
-valgrind on the python tests.

Signed-off-by: Emanuele Giuseppe Esposito 
---
  tests/qemu-iotests/iotests.py | 16 
  1 file changed, 16 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 5d75094ba6..a06284acad 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -597,6 +597,22 @@ def __init__(self, path_suffix=''):
   sock_dir=sock_dir, qmp_timer=timer)
  self._num_drives = 0
  
+def subprocess_check_valgrind(self, valgrind: List[str]) -> None:


For me just "check_valgrind" would be more intuitive.

I think, you also can use qemu_valgrind global variable directly.. What is the 
reason for passing it as parameter?


+if not valgrind or not self._popen:
+return
+
+valgrind_filename =  f"{test_dir}/{self._popen.pid}.valgrind"


should we use os.path.join instead? I don't know.. And don't care, as I don't 
use windows anyway. Still os.path.join is used everywhere in the file except 
for has_working_luks() function.. So, you are going add another exception.


+
+if self.exitcode() == 99:
+with open(valgrind_filename) as f:
+print(f.read())
+else:
+os.remove(valgrind_filename)
+
+def _post_shutdown(self) -> None:
+super()._post_shutdown()
+self.subprocess_check_valgrind(qemu_valgrind)
+
  def add_object(self, opts):
  self._args.append('-object')
  self._args.append(opts)



Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH v4 10/15] qemu-iotests: extent QMP socket timeout when using valgrind

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

As with gdbserver, valgrind delays the test execution, so
the default QMP socket timeout timeout too soon.


First, "Timeout" class is a generic class for timeouts, not relying to sockets. 
So,  commit message lacks information about that we modify generic context-provider class 
and why we do it.



Signed-off-by: Emanuele Giuseppe Esposito 
---
  tests/qemu-iotests/iotests.py | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 41462a80fc..5d75094ba6 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -489,12 +489,12 @@ def __init__(self, seconds, errmsg="Timeout"):
  self.seconds = seconds
  self.errmsg = errmsg
  def __enter__(self):
-if not qemu_gdb:
+if not (qemu_gdb or qemu_valgrind):
  signal.signal(signal.SIGALRM, self.timeout)
  signal.setitimer(signal.ITIMER_REAL, self.seconds)
  return self
  def __exit__(self, exc_type, value, traceback):
-if not qemu_gdb:
+if not (qemu_gdb or qemu_valgrind):


If you follow my suggestion on 05, you'll have to modify only one line instead 
of two.


  signal.setitimer(signal.ITIMER_REAL, 0)
  return False
  def timeout(self, signum, frame):
@@ -589,7 +589,7 @@ class VM(qtest.QEMUQtestMachine):
  
  def __init__(self, path_suffix=''):

  name = "qemu%s-%d" % (path_suffix, os.getpid())
-timer = 15.0 if not qemu_gdb else None
+timer = 15.0 if not (qemu_gdb or qemu_valgrind) else None
  super().__init__(qemu_prog, qemu_opts, wrapper=qemu_gdb,
   name=name,
   test_dir=test_dir,



still it should work as intended:
Reviewed-by: Vladimir Sementsov-Ogievskiy 


--
Best regards,
Vladimir

Re: [PATCH v4 09/15] qemu-iotests: extend the check script to support valgrind for python tests

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

Currently, the check script only parses the option and sets the
VALGRIND_QEMU environmental variable to "y".
Add another local python variable that prepares the command line,
identical to the one provided in the test scripts.

Because the python script does not know in advance the valgring
PID to assign to the log file name, use the "%p" flag in valgrind
log file name that automatically puts the process PID at runtime.

Reviewed-by: Max Reitz 
Signed-off-by: Emanuele Giuseppe Esposito 
---
  tests/qemu-iotests/check  |  7 ---
  tests/qemu-iotests/iotests.py | 11 +++
  tests/qemu-iotests/testenv.py |  1 +
  3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index b9820fdaaf..2101cedfe3 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -36,6 +36,10 @@ def make_argparser() -> argparse.ArgumentParser:
  p.add_argument('-gdb', action='store_true',
 help="start gdbserver with $GDB_OPTIONS options \
  ('localhost:12345' if $GDB_OPTIONS is empty)")
+p.add_argument('-valgrind', action='store_true',
+help='use valgrind, sets VALGRIND_QEMU environment '
+'variable')
+
  p.add_argument('-misalign', action='store_true',
 help='misalign memory allocations')
  p.add_argument('--color', choices=['on', 'off', 'auto'],
@@ -86,9 +90,6 @@ def make_argparser() -> argparse.ArgumentParser:
  g_bash.add_argument('-o', dest='imgopts',
  help='options to pass to qemu-img create/convert, '
  'sets IMGOPTS environment variable')
-g_bash.add_argument('-valgrind', action='store_true',
-help='use valgrind, sets VALGRIND_QEMU environment '
-'variable')
  
  g_sel = p.add_argument_group('test selecting options',

   'The following options specify test set '
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index c9628e6828..41462a80fc 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -97,6 +97,17 @@
  sys.stderr.write('Please run this test via the "check" script\n')
  sys.exit(os.EX_USAGE)
  
+qemu_valgrind = []

+if os.environ.get('VALGRIND_QEMU') == "y" and \
+os.environ.get('NO_VALGRIND') != "y":
+valgrind_logfile = "--log-file=" + test_dir.strip()


a bit strange that you need to strip test_dir here.. Why?


+# %p allows to put the valgrind process PID, since
+# we don't know it a priori (subprocess.Popen is
+# not yet invoked)
+valgrind_logfile += "/%p.valgrind"
+
+qemu_valgrind = ['valgrind', valgrind_logfile, '--error-exitcode=99']
+
  socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
  
  luks_default_secret_object = 'secret,id=keysec0,data=' + \

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 49ddd586ef..319d29cb0c 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -282,6 +282,7 @@ def print_env(self) -> None:
  SOCK_DIR  -- {SOCK_DIR}
  SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
  GDB_OPTIONS   -- {GDB_OPTIONS}
+VALGRIND_QEMU -- {VALGRIND_QEMU}
  """
  
  args = collections.defaultdict(str, self.get_env())




commit subject sais "support valgrind", but actually we only add a variable. 
valgrind is not actually used yet.. I'd reflect it in commit subject somehow

--
Best regards,
Vladimir

Re: [PATCH v4 08/15] docs/devel/testing: add -gdb option to the debugging section of QEMU iotests

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

Signed-off-by: Emanuele Giuseppe Esposito 


Reviewed-by: Vladimir Sementsov-Ogievskiy 


---
  docs/devel/testing.rst | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 8144e316a4..a746cab745 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -229,6 +229,17 @@ Debugging a test case
  The following options to the ``check`` script can be useful when debugging
  a failing test:
  
+* ``-gdb`` wraps every QEMU invocation in a ``gdbserver``, which waits for a

+  connection from a gdb client.  The options given to ``gdbserver`` (e.g. the
+  address on which to listen for connections) are taken from the 
``$GDB_OPTIONS``
+  environment variable.  By default (if ``$GDB_OPTIONS`` is empty), it listens 
on
+  ``localhost:12345``.
+  It is possible to connect to it for example with
+  ``gdb -iex "target remote $addr"``, where ``$addr`` is the address
+  ``gdbserver`` listens on.
+  If the ``-gdb`` option is not used, ``$GDB_OPTIONS`` is ignored,
+  regardless on whether it is set or not.
+
  * ``-d`` (debug) just increases the logging verbosity, showing
for example the QMP commands and answers.
  



Didn't you think of an interface as simple as just wrap qemu binary by "gdb --args" 
and redirect stdin and stdout directly to the terminal where test is running? Wouldn't it more 
convenient? So, you just start ./check -gdb , and you are in gdb. Or you need 
exactly gdb server, to attach from another machine?

Keeping this possibility in mind, it's probably better to call you option 
"-gdbserver"... But we can rename later if needed, finally, it's only a test 
framework.


--
Best regards,
Vladimir

Re: [PATCH v4 07/15] qemu-iotests: add gdbserver option to script tests too

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

The only limitation here is that running a script with gdbserver
will make the test output mismatch with the expected
results, making the test fail.

Signed-off-by: Emanuele Giuseppe Esposito


Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH v4 06/15] qemu_iotests: insert gdbserver command line as wrapper for qemu binary

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

Signed-off-by: Emanuele Giuseppe Esposito


Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH v4 05/15] qemu-iotests: delay QMP socket timers

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

Attaching gdbserver implies that the qmp socket
should wait indefinitely for an answer from QEMU.

Signed-off-by: Emanuele Giuseppe Esposito 
---
  tests/qemu-iotests/iotests.py | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index d667fde6f8..cf1ca60376 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -478,11 +478,13 @@ def __init__(self, seconds, errmsg="Timeout"):
  self.seconds = seconds
  self.errmsg = errmsg
  def __enter__(self):
-signal.signal(signal.SIGALRM, self.timeout)
-signal.setitimer(signal.ITIMER_REAL, self.seconds)
+if not qemu_gdb:
+signal.signal(signal.SIGALRM, self.timeout)
+signal.setitimer(signal.ITIMER_REAL, self.seconds)
  return self
  def __exit__(self, exc_type, value, traceback):
-signal.setitimer(signal.ITIMER_REAL, 0)
+if not qemu_gdb:
+signal.setitimer(signal.ITIMER_REAL, 0)
  return False
  def timeout(self, signum, frame):
  raise Exception(self.errmsg)


So, you just make the class do nothing.. I'd prefer something like this:

@contextmanager
def NoTimeout:
   yield

if qemu_gdb:
  Timeout = NoTimeout


anyway:

Reviewed-by: Vladimir Sementsov-Ogievskiy 



@@ -576,7 +578,7 @@ class VM(qtest.QEMUQtestMachine):
  
  def __init__(self, path_suffix=''):

  name = "qemu%s-%d" % (path_suffix, os.getpid())
-timer = 15.0
+timer = 15.0 if not qemu_gdb else None
  super().__init__(qemu_prog, qemu_opts, name=name,
   test_dir=test_dir,
   socket_scm_helper=socket_scm_helper,




Still, it's not simple to avoid any kind of timeouts. The most annoying would 
be timeouts in event_wait() / events_wait()

--
Best regards,
Vladimir

[RFC PATCH] configure: allow the overriding of default-config in the build

[Alex Bennée]

While the default config works well enough it does end up enabling a
lot of stuff. For more minimal builds we can pass a slimmed down list
of devices and let Kconfig work out what we want. For example:

  ../../configure --without-default-features \
--target-list=arm-softmmu,aarch64-softmmu \

--with-devices-aarch64=(pwd)/../../configs/aarch64-softmmu/aarch64-softmmu-64bit-only.mak

will override the aarch64-softmmu default devices to one of our own
choosing.

Signed-off-by: Alex Bennée 
Cc: Philippe Mathieu-Daudé 
Cc: Paolo Bonzini 
---
 configure| 16 
 .../aarch64-softmmu-64bit-only.mak   | 10 ++
 meson.build  |  3 ++-
 3 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 configs/aarch64-softmmu/aarch64-softmmu-64bit-only.mak

diff --git a/configure b/configure
index 90c0807347..5acb8b2ed5 100755
--- a/configure
+++ b/configure
@@ -921,6 +921,12 @@ for opt do
   ;;
   --without-default-devices) default_devices="false"
   ;;
+  --with-devices-*[!a-zA-Z0-9_-]*=*) error_exit "Passed bad 
--with-devices-cc-FOO option"
+  ;;
+  --with-devices-*)  device_arch=${opt#--with-devices-}; 
device_arch=${device_arch%%=*}
+ device_archs="$device_archs $device_arch"
+ eval "devices_${device_arch}=\$optarg"
+  ;;
   --without-default-features) # processed above
   ;;
   --enable-gprof) gprof="yes"
@@ -1767,6 +1773,7 @@ Advanced options (experts only):
   --without-default-devices  do not include any device that is not needed to
start the emulator (only use if you are including
desired devices in default-configs/devices/)
+  --with-devices-ARCH=PATH override default-configs/devices with your own file
   --enable-debug   enable common debug build options
   --enable-sanitizers  enable default sanitizers
   --enable-tsanenable thread sanitizer
@@ -6378,6 +6385,15 @@ if test "$skip_meson" = no; then
 
   echo "# Automatically generated by configure - do not modify" > $cross
   echo "[properties]" >> $cross
+
+  # unroll any custom device configs
+  if test -n "$device_archs"; then
+  for a in $device_archs; do
+  eval "c=\$devices_${a}"
+  echo "${a}-softmmu = [ '$c' ]" >> $cross
+  done
+  fi
+
   test -z "$cxx" && echo "link_language = 'c'" >> $cross
   echo "[built-in options]" >> $cross
   echo "c_args = [${CFLAGS:+$(meson_quote $CFLAGS)}]" >> $cross
diff --git a/configs/aarch64-softmmu/aarch64-softmmu-64bit-only.mak 
b/configs/aarch64-softmmu/aarch64-softmmu-64bit-only.mak
new file mode 100644
index 00..3626de9e3c
--- /dev/null
+++ b/configs/aarch64-softmmu/aarch64-softmmu-64bit-only.mak
@@ -0,0 +1,10 @@
+#
+# A version of the config that only supports 64bits and their devices.
+# This doesn't quite eliminate all 32 bit devices as some boards like
+# "virt" support both.
+#
+
+CONFIG_ARM_VIRT=y
+CONFIG_XLNX_ZYNQMP_ARM=y
+CONFIG_XLNX_VERSAL=y
+CONFIG_SBSA_REF=y
diff --git a/meson.build b/meson.build
index 3f065f53f2..656ebde513 100644
--- a/meson.build
+++ b/meson.build
@@ -1350,9 +1350,10 @@ foreach target : target_dirs
configuration: 
config_target_data)}
 
   if target.endswith('-softmmu')
+config_input = meson.get_external_property(target, 
'default-configs/devices' / target + '.mak')
 config_devices_mak = target + '-config-devices.mak'
 config_devices_mak = configure_file(
-  input: ['default-configs/devices' / target + '.mak', 'Kconfig'],
+  input: [config_input, 'Kconfig'],
   output: config_devices_mak,
   depfile: config_devices_mak + '.d',
   capture: true,
-- 
2.20.1

Re: [PATCH v4 04/15] qemu-iotests: add option to attach gdbserver

[Vladimir Sementsov-Ogievskiy]

20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:

Define -gdb flag and GDB_OPTIONS environment variable
to python tests to attach a gdbserver to each qemu instance.
This patch only adds and parses this flag, it does not yet add
the implementation for it.

if -gdb is not provided but $GDB_OPTIONS is set, ignore the
environment variable.

Signed-off-by: Emanuele Giuseppe Esposito 
---
  tests/qemu-iotests/check  |  6 +-
  tests/qemu-iotests/iotests.py |  5 +
  tests/qemu-iotests/testenv.py | 19 ---
  3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index d1c87ceaf1..b9820fdaaf 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -33,6 +33,9 @@ def make_argparser() -> argparse.ArgumentParser:
 help='pretty print output for make check')
  
  p.add_argument('-d', dest='debug', action='store_true', help='debug')

+p.add_argument('-gdb', action='store_true',
+   help="start gdbserver with $GDB_OPTIONS options \
+('localhost:12345' if $GDB_OPTIONS is empty)")
  p.add_argument('-misalign', action='store_true',
 help='misalign memory allocations')
  p.add_argument('--color', choices=['on', 'off', 'auto'],
@@ -112,7 +115,8 @@ if __name__ == '__main__':
  env = TestEnv(imgfmt=args.imgfmt, imgproto=args.imgproto,
aiomode=args.aiomode, cachemode=args.cachemode,
imgopts=args.imgopts, misalign=args.misalign,
-  debug=args.debug, valgrind=args.valgrind)
+  debug=args.debug, valgrind=args.valgrind,
+  gdb=args.gdb)
  
  testfinder = TestFinder(test_dir=env.source_iotests)
  
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index 5d78de0f0b..d667fde6f8 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -75,6 +75,11 @@
  qemu_prog = os.environ.get('QEMU_PROG', 'qemu')
  qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ')
  
+gdb_qemu_env = os.environ.get('GDB_OPTIONS')

+qemu_gdb = []
+if gdb_qemu_env:
+qemu_gdb = ['gdbserver'] + gdb_qemu_env.strip().split(' ')
+
  imgfmt = os.environ.get('IMGFMT', 'raw')
  imgproto = os.environ.get('IMGPROTO', 'file')
  output_dir = os.environ.get('OUTPUT_DIR', '.')
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 6d27712617..49ddd586ef 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -27,6 +27,7 @@
  import glob
  from typing import Dict, Any, Optional, ContextManager
  
+DEF_GDB_OPTIONS = 'localhost:12345'
  
  def isxfile(path: str) -> bool:

  return os.path.isfile(path) and os.access(path, os.X_OK)
@@ -72,7 +73,8 @@ class TestEnv(ContextManager['TestEnv']):
   'QEMU_NBD_OPTIONS', 'IMGOPTS', 'IMGFMT', 'IMGPROTO',
   'AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU',
   'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 
'IMGOPTSSYNTAX',
- 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_']
+ 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_',
+ 'GDB_OPTIONS']
  
  def get_env(self) -> Dict[str, str]:

  env = {}
@@ -163,7 +165,8 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
   imgopts: Optional[str] = None,
   misalign: bool = False,
   debug: bool = False,
- valgrind: bool = False) -> None:
+ valgrind: bool = False,
+ gdb: bool = False) -> None:
  self.imgfmt = imgfmt
  self.imgproto = imgproto
  self.aiomode = aiomode
@@ -171,6 +174,14 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: 
str,
  self.misalign = misalign
  self.debug = debug
  
+if gdb:

+self.gdb_options = os.environ.get('GDB_OPTIONS', DEF_GDB_OPTIONS)
+if not self.gdb_options:
+# cover the case 'export GDB_OPTIONS='
+self.gdb_options = DEF_GDB_OPTIONS
+elif 'GDB_OPTIONS' in os.environ:


please add a comment:

 # to not propagate it in prepare_subprocess()


+del os.environ['GDB_OPTIONS']
+
  if valgrind:
  self.valgrind_qemu = 'y'
  
@@ -269,7 +280,9 @@ def print_env(self) -> None:

  PLATFORM  -- {platform}
  TEST_DIR  -- {TEST_DIR}
  SOCK_DIR  -- {SOCK_DIR}
-SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}"""
+SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
+GDB_OPTIONS   -- {GDB_OPTIONS}
+"""
  
  args = collections.defaultdict(str, self.get_env())
  



Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH v2 2/2] target/ppc: removed all mentions to PPC_DUMP_CPU

[Richard Henderson]

On 5/27/21 11:08 AM, Bruno Larsen (billionai) wrote:

This feature will no longer be useful as ppc moves to using decotree for


decodetree


TCG. And building with it enabled is no longer possible, due to changes
in opc_handler_t. Since the last commit that mentions it happened in
2014, I think it is safe to remove it.

Signed-off-by: Bruno Larsen (billionai)
---
  target/ppc/cpu_init.c  | 205 -
  target/ppc/internal.h  |   2 -
  target/ppc/translate.c | 105 -
  3 files changed, 312 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 1/2] target/ppc: removed GEN_OPCODE decision tree

[Richard Henderson]

On 5/27/21 11:08 AM, Bruno Larsen (billionai) wrote:

since both, PPC_DO_STATISTICS and PPC_DUMP_CPU, are obsoleted as
target/ppc moves to decodetree, we can remove this ifdef based decision
tree, and only have what is now the standard option for the macro.

Suggested-by: Luis Pires
Signed-off-by: Bruno Larsen (billionai)
---
  target/ppc/translate.c | 79 --
  1 file changed, 79 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PULL 07/31] accel/tcg: Reduce 'exec/tb-context.h' inclusion

[Philippe Mathieu-Daudé]

On 5/27/21 1:46 AM, Richard Henderson wrote:
> From: Philippe Mathieu-Daudé 
> 
> Only 2 headers require "exec/tb-context.h". Instead of having
> all files including "exec/exec-all.h" also including it, directly
> include it where it is required:
> - accel/tcg/cpu-exec.c
> - accel/tcg/translate-all.c
> 
> For plugins/plugin.h, we were implicitly relying on
>   exec/exec-all.h -> exec/tb-context.h -> qemu/qht.h
> which is now included directly.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> Message-Id: <20210524170453.3791436-2-f4...@amsat.org>
> [rth: Fix plugins/plugin.h compilation]

Ah, I see you squashed:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg810344.html
I was hoping it goes via Alex's testing-next tree or trivial first,
so this wouldn't be a problem when you apply this patch.
Thanks anyway :)
Phil.

> Signed-off-by: Richard Henderson 
> ---
>  include/exec/exec-all.h   | 1 -
>  include/tcg/tcg.h | 1 -
>  plugins/plugin.h  | 1 +
>  accel/tcg/cpu-exec.c  | 1 +
>  accel/tcg/translate-all.c | 1 +
>  5 files changed, 3 insertions(+), 2 deletions(-)

> diff --git a/plugins/plugin.h b/plugins/plugin.h
> index 1aa29dcadd..55017e3581 100644
> --- a/plugins/plugin.h
> +++ b/plugins/plugin.h
> @@ -13,6 +13,7 @@
>  #define _PLUGIN_INTERNAL_H_
>  
>  #include 
> +#include "qemu/qht.h"
>  
>  #define QEMU_PLUGIN_MIN_VERSION 0

Re: [PATCH 07/11] target/rx: Use FloatRoundMode in helper_set_fpsw

[Yoshinori Sato]

On Thu, 27 May 2021 13:14:01 +0900,
Richard Henderson wrote:
> 
> Use the proper type for the roundmode array.
> 
> Cc: Philippe Mathieu-Daudé 
> Cc: Yoshinori Sato 
> Signed-off-by: Richard Henderson 

Reviewd-by: Yoshinori Sato 

> ---
>  target/rx/op_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/rx/op_helper.c b/target/rx/op_helper.c
> index 2139def3b2..b1772e9538 100644
> --- a/target/rx/op_helper.c
> +++ b/target/rx/op_helper.c
> @@ -120,7 +120,7 @@ static void update_fpsw(CPURXState *env, float32 ret, 
> uintptr_t retaddr)
>  
>  void helper_set_fpsw(CPURXState *env, uint32_t val)
>  {
> -static const int roundmode[] = {
> +static const FloatRoundMode roundmode[] = {
>  float_round_nearest_even,
>  float_round_to_zero,
>  float_round_up,
> -- 
> 2.25.1
> 
> 

Re: [PATCH 08/11] target/rx: Fix setting of FPSW.CE

[Yoshinori Sato]

On Thu, 27 May 2021 13:14:02 +0900,
Richard Henderson wrote:
> 
> The existing check was completely wrong, confused about the
> definition of the (previous) float_flag_{input,output}_denormal
> flags, then making sure that DN, the flush-to-zero bit, was off.
> 
> Update for the introduction of float_flag_inorm_denormal and
> float_flag_result_denormal, taking into account that DN now sets
> the softfloat flush-to-zero bits.
> 
> Cc: Philippe Mathieu-Daudé 
> Cc: Yoshinori Sato 
> Signed-off-by: Richard Henderson 

Reviewd-by: Yoshinori Sato 

> ---
>  target/rx/op_helper.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/target/rx/op_helper.c b/target/rx/op_helper.c
> index b1772e9538..c2e4f9a5e3 100644
> --- a/target/rx/op_helper.c
> +++ b/target/rx/op_helper.c
> @@ -97,9 +97,11 @@ static void update_fpsw(CPURXState *env, float32 ret, 
> uintptr_t retaddr)
>  if (xcpt & float_flag_inexact) {
>  SET_FPSW(X);
>  }
> -if ((xcpt & (float_flag_iflush_denormal
> - | float_flag_oflush_denormal))
> -&& !FIELD_EX32(env->fpsw, FPSW, DN)) {
> +/*
> + * If any input or output denormals, not flushed to zero, raise CE:
> + * unimplemented processing has been encountered.
> + */
> +if (xcpt & (float_flag_inorm_denormal | float_flag_result_denormal)) 
> {
>  env->fpsw = FIELD_DP32(env->fpsw, FPSW, CE, 1);
>  }
>  
> -- 
> 2.25.1
> 
> 

Re: [PATCH 06/11] target/rx: Handle the FPSW.DN bit in helper_set_fpsw

[Yoshinori Sato]

On Thu, 27 May 2021 13:14:00 +0900,
Richard Henderson wrote:
> 
> Both input and output denormals flush to zero when DN is set.
> 
> Cc: Philippe Mathieu-Daudé 
> Cc: Yoshinori Sato 
> Signed-off-by: Richard Henderson 

Reviewd-by: Yoshinori Sato 

> ---
>  target/rx/op_helper.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/target/rx/op_helper.c b/target/rx/op_helper.c
> index ef904eb5f9..2139def3b2 100644
> --- a/target/rx/op_helper.c
> +++ b/target/rx/op_helper.c
> @@ -127,13 +127,20 @@ void helper_set_fpsw(CPURXState *env, uint32_t val)
>  float_round_down,
>  };
>  uint32_t fpsw = env->fpsw;
> +bool dn;
> +
>  fpsw |= 0x7f03;
>  val &= ~0x8000;
>  fpsw &= val;
>  FIELD_DP32(fpsw, FPSW, FS, FIELD_EX32(fpsw, FPSW, FLAGS) != 0);
>  env->fpsw = fpsw;
> -set_float_rounding_mode(roundmode[FIELD_EX32(env->fpsw, FPSW, RM)],
> +
> +set_float_rounding_mode(roundmode[FIELD_EX32(fpsw, FPSW, RM)],
>  &env->fp_status);
> +
> +dn = FIELD_EX32(env->fpsw, FPSW, DN);
> +set_flush_to_zero(dn, &env->fp_status);
> +set_flush_inputs_to_zero(dn, &env->fp_status);
>  }
>  
>  #define FLOATOP(op, func)   \
> -- 
> 2.25.1
> 
> 

Re: [PATCH] HMP: added cpustats to removed_features.rst

[Alex Bennée]

Greg Kurz  writes:

> On Thu, 27 May 2021 13:50:34 -0300
> "Bruno Larsen (billionai)"  wrote:
>
>> Documented the removal of the HMP command cpustats
>> 
>
> It is the 'info cpustats' command.
>
>> Signed-off-by: Bruno Larsen (billionai) 
>> ---
>>  docs/system/removed-features.rst | 6 ++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/docs/system/removed-features.rst 
>> b/docs/system/removed-features.rst
>> index 5a462ac568..a88ff7aff4 100644
>> --- a/docs/system/removed-features.rst
>> +++ b/docs/system/removed-features.rst
>> @@ -249,6 +249,12 @@ Use ``migrate-set-parameters`` and ``info 
>> migrate-parameters`` instead.
>>  
>>  Use ``migrate-set-parameters`` instead.
>>  
>> +``cpustats`` (removed in 6.1)
>> +'
>
> ditto
>
>> +
>> +This command didn't produce any output already. Removed to avoid 
>> expectations
>> +of maintaining/fixing it.
>> +
>
> s/to avoid... it/with no replacement/ because that's what users
> are interested in.

We could point to alternatives:

  This command hasn't produced any output for some time and the build
  was broken anyway. If users are interested in gathering similar
  information in the future they should investigate collecting it with
  TCG plugins.

?

>
>>  Guest Emulator ISAs
>>  ---
>>  


-- 
Alex Bennée

Re: [PATCH v2 08/33] block/backup: drop support for copy_range

[Vladimir Sementsov-Ogievskiy]

20.05.2021 17:21, Vladimir Sementsov-Ogievskiy wrote:

copy_range is not a default behavior since 6a30f663d4c0b3c, and it's
now available only though x-perf experimantal argument, so it's OK to
drop it.

Even when backup is used to copy disk to same filesystem, and
filesystem supports zero-copy copy_range, copy_range is probably not
what we want for backup: backup has good property of making a copy of
active disk, with no impact to active disk itself (unlike creating a
snapshot). And if copy_range instead of copying data adds fs-level
references, and on next guest write COW operation occurs, it's seems
most possible, that new block will be allocated for original vm disk,
not for backup disk. Thus, fragmentation of original disk will
increase.

We can simply add support back on demand. Now we want to publish
copy-before-write filter, and instead of thinking how to pass
use-copy-range argument to block-copy (create x-block-copy parameter
for new public filter driver, or may be set it by hand after filter
node creation?), instead of this let's just drop copy-range support in
backup for now.

After this patch copy-range support in block-copy becomes unused. Let's
keep it for a while, it won't hurt:

1. If there would be request for supporting copy_range in backup
(and/or in a new public copy-before-write filter), it will be easy
to satisfy it.

2. Probably, qemu-img convert will reuse block-copy, and qemu-img has
option to enable copy-range. qemu-img convert is not a backup, and
copy_range may be more reasonable for some cases in context of
qemu-img convert.



Actually, I know one case, where copy_range for backup job may be reasonable:

Using backup in push-backup with fleecing scheme in

   [PATCH 0/6] push backup with fleecing

Of-course, no real sense in using push-backup-with-fleecing scheme with both 
temp image and final backup target being on the same file system (no benefit of 
fleecing, we can use simple backup without temporary image).

But we absolutely don't care about fragmentation of temp disk.

Still, it doesn't make sense, as temp-image and real-backup-target should not 
be on same file-system..

Could it be some distributed filesystem, where it still make sense to call 
copy_range? Theoretically could.


Another thought: I'm going also to implement RAM-cache driver, to optimize 
push-backup-with-fleecing scheme. I'll need a way to copy data from RAM-cache 
node to final-target. I can implement copy_range for RAM-cache, and this will 
allow to not create extra buffer, but use the buffer that is already allocated 
and own by RAM-cache.. Still, this behavior is obviously good, it should work 
automatically, no reason to make it optional..


Hmm, so what should be summarized:

- Actually, block-copy does copy_range. So, probably it's good to change the 
copy_range() function in qemu to fallback to read+write..

And about copy_range itself, what we want:

1. We want to control does it influence fragmentation of source disk. When 
copying from temporary image we don't care. But when source of block-copy is 
active disk in we do care to not influence how original disk lay in filesystem. 
Probably, we even want an option for copy_range() syscall to control this thing.

2. We want to be efficient with copy_size, ie size of chunks to copy. We even 
have existing issue in block-copy: write-zero is limited to 
BLOCK_COPY_MAX_BUFFER which is obviously inefficient.

For copy-size we should have some good defaults or automatic detection logic..

For copy_range fragmentation..

If we have some internal copy_range-like optimizations like zero-copy from 
RAM-cache node, or maybe copy compressed data from one qcow2 node to another 
without decompression, it should be done anyway, it shouldn't be set by user 
option. Still, for file-posix, we don't know, does underlying filesystem 
copy_range() implementation lead to fragmentation or not. And we don't know is 
user OK with it or not. So we need an option.. So, it's probably better to keep 
x-perm.copy-range for now, until we don't have a good idea on interface.

--
Best regards,
Vladimir

Re: [PATCH] arm: Consistently use "Cortex-Axx", not "Cortex Axx"

[Alex Bennée]

Peter Maydell  writes:

> The official punctuation for Arm CPU names uses a hyphen, like
> "Cortex-A9". We mostly follow this, but in a few places usage
> without the hyphen has crept in. Fix those so we consistently
> use the same way of writing the CPU name.
>
> This commit was created with:
>   git grep -z -l 'Cortex ' | xargs -0 sed -i 's/Cortex /Cortex-/'
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée

Re: [PULL 0/4] M68k for 6.1 patches

[Peter Maydell]

On Wed, 26 May 2021 at 21:15, Laurent Vivier  wrote:
>
> The following changes since commit 0319ad22bd5789e1eaa8a2dd5773db2d2c372f20:
>
>   Merge remote-tracking branch 
> 'remotes/stsquad/tags/pull-testing-and-misc-updates-250521-2' into staging 
> (2021-05-25 17:31:04 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu-m68k.git tags/m68k-for-6.1-pull-request
>
> for you to fetch changes up to 5e50c6c72bf8575f124ec9397411f4a2ff0d0206:
>
>   target/m68k: implement m68k "any instruction" trace mode (2021-05-26 
> 20:45:18 +0200)
>
> 
> m68k pull request 20210526
>
> implement m68k "any instruction" trace mode
>
> 



Applied, thanks.

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

-- PMM

Re: [PATCH] docs/devel: Explain in more detail the TB chaining mechanisms

[Richard Henderson]

On 5/28/21 5:30 AM, Luis Pires wrote:

+In its simplest, less optimized form, this is done by exiting from the
+current TB, going through the TB epilogue, and then back to the outer
+execution loop. That’s where QEMU looks for the next TB to execute,
+translating it from the guest architecture if it isn’t already available
+in memory. Then QEMU proceeds to execute this next TB, starting at the
+prologue and then moving on to the translated instructions.


It is important to mention that by exiting this way, we immediately re-evaluate 
cc->cpu_exec_interrupt().  It is mandatory to exit this way after any cpu state 
change that may unmask interrupts.


This is often referred to as "exit to the main loop" in the translators.  In my 
recent changes to the ppc translator, I introduced DISAS_EXIT* for the purpose.




+In order to accelerate the most common cases where the TB for the new
+simulated PC is already available, QEMU has mechanisms that allow
+multiple TBs to be chained directly, without having to go back to the
+outer execution loop as described above. These mechanisms are:
+
+``lookup_and_goto_ptr``
+^^^
+
+On platforms that support the ``lookup_and_goto_ptr`` mechanism, calling
+``tcg_gen_lookup_and_goto_ptr()`` will emit TCG instructions that call
+a helper function to look for the destination TB, based on
+the CPU state information. If the destination TB is available, a
+``goto_ptr`` TCG instruction is emitted to jump directly to its first
+instruction, skipping the epilogue - execution loop - prologue path.
+If the destination TB is not available, the ``goto_ptr`` instruction
+jumps to the epilogue, effectively exiting from the current TB and
+going back to the execution loop.


I'm one step shy of making TCG_TARGET_HAS_goto_ptr mandatory, and I don't think 
it's useful to focus on what the fallback mechanisms are.  In particular, 
lookup_and_goto_ptr will exit to the main loop with '-d nochain'.


The timeline is off here as well.  The goto_ptr tcg opcode is not conditionally 
emitted -- it is always emitted.  Better phrasing:


  ... will emit a call to ``helper_lookup_tb_ptr``.  This helper
  will look for an existing TB that matches the current CPU state.
  If the destination TB is available its code address is returned,
  otherwise the address of the JIT epilogue is returned.  The call
  to the helper is always followed by the tcg ``goto_ptr`` opcode,
  which branches to the returned address.  In this way, we either
  branch to the next TB or return to the main loop.



+On platforms that do not support this mechanism, the
+``tcg_gen_lookup_and_goto_ptr()`` function will just use
+``tcg_gen_exit_tb()`` to exit from the current TB.


Just drop this bit.


+``goto_tb + exit_tb``
+^
+
+On platforms that support this mechanism, the translation code usually
+implements branching by performing the following steps:


Again drop "on platforms that support", because they all do -- it's mandatory.

It's also very important to note when this cannot be used: the change in cpu 
state must be constant.  E.g. a direct branch not an indirect branch.  A 
surprising edge case here in the past has been a direct branch with a 
conditional delay slot nullification.


Moreover, even the direct branch cannot cross a page boundary, because memory 
mappings may change causing the code at the destination address to change.



+
+1. Call ``tcg_gen_goto_tb()`` passing a jump slot index (either 0 or 1)
+   as a parameter
+
+2. Emit TCG instructions to update the CPU state information with the
+   address of the next instruction to execute


More completely, update the CPU state with any information that has been 
assumed constant.  For most guests, this is just the PC.  But e.g. for hppa 
this is both iaoq.f (cip) and iaoq.b (nip).


It is very much up to the guest to determine the set of data that is present in 
cpu_get_tb_cpu_state, and what can be assumed across the break.



+The first time this whole sequence is translated to target instructions
+and executed, step 1 doesn’t do anything really useful, as it just jumps
+to step 2.


Timing problem.  When the whole sequence is translated is immaterial.  You mean 
the first time this sequence is executed.  Drop the "doesn't do anything 
useful" phrase.




Then the CPU state information gets updated and we exit from
+the current TB. As a result, the behavior is very similar to the less
+optimized form described earlier in this section.
+
+Next, the execution loop looks for the next TB to execute using the
+current CPU state information (creating the TB if it wasn’t already
+available) and, before starting to execute the new TB’s instructions,
+tries to patch the previously executed TB by associating one of its jump


s/tries to patch/patches/.  There's no failure possible.


+The most portable code patches TBs using indirect jumps. An indirect
+jump makes it easier to make the jump target modification atomic. On some
+host arc

Re: [PATCH RESEND 1/1] docs/devel: Add a single top-level header to MTTCG's doc

[Alex Bennée]

Luis Pires  writes:

> Without a single top-level header in the .rst file, the index ended
> up linking to all of the top-level headers separately. Now the index
> links to the top-level header at the beginning of the document and
> any inner headers are correctly linked as sub-items in the index.
>
> Signed-off-by: Luis Pires 

Queued to testing/next for want of a better tree. thanks.

> ---
>  docs/devel/multi-thread-tcg.rst | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/docs/devel/multi-thread-tcg.rst b/docs/devel/multi-thread-tcg.rst
> index 92a9eba13c..5b446ee08b 100644
> --- a/docs/devel/multi-thread-tcg.rst
> +++ b/docs/devel/multi-thread-tcg.rst
> @@ -4,8 +4,9 @@
>This work is licensed under the terms of the GNU GPL, version 2 or
>later. See the COPYING file in the top-level directory.
>  
> -Introduction
> -
> +==
> +Multi-threaded TCG
> +==
>  
>  This document outlines the design for multi-threaded TCG (a.k.a MTTCG)
>  system-mode emulation. user-mode emulation has always mirrored the


-- 
Alex Bennée

[PULL 3/3] hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X

[Gerd Hoffmann]

From: Ruimei Yan 

Per xHCI spec v1.2 chapter 4.17.5 page 296:

  If MSI or MSI-X interrupts are enabled, Interrupt Pending (IP)
  shall be cleared automatically when the PCI dword write generated
  by the interrupt assertion is complete.

Currently QEMU does not clear the IP flag in the MSI / MSI-X mode.
This causes subsequent spurious interrupt to be delivered to guests.
To solve this, we change the xhci intr_raise() hook routine to have
a bool return value that is passed to its caller (the xhci core),
with true indicating that IP should be self-cleared.

Fixes: 62c6ae04cf43 ("xhci: Initial xHCI implementation")
Fixes: 4c47f800631a ("xhci: add msix support")
Signed-off-by: Ruimei Yan 
[bmeng: move IP clear codes from xhci pci to xhci core]
Signed-off-by: Bin Meng 
Message-Id: <20210521024224.2277634-2-bmeng...@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-xhci.h| 2 +-
 hw/usb/hcd-xhci-pci.c| 8 +---
 hw/usb/hcd-xhci-sysbus.c | 4 +++-
 hw/usb/hcd-xhci.c| 8 ++--
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
index 7bba361f3bbd..98f598382adc 100644
--- a/hw/usb/hcd-xhci.h
+++ b/hw/usb/hcd-xhci.h
@@ -194,7 +194,7 @@ typedef struct XHCIState {
 uint32_t flags;
 uint32_t max_pstreams_mask;
 void (*intr_update)(XHCIState *s, int n, bool enable);
-void (*intr_raise)(XHCIState *s, int n, bool level);
+bool (*intr_raise)(XHCIState *s, int n, bool level);
 DeviceState *hostOpaque;
 
 /* Operational Registers */
diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
index b6acd1790c1a..e934b1a5b1fb 100644
--- a/hw/usb/hcd-xhci-pci.c
+++ b/hw/usb/hcd-xhci-pci.c
@@ -57,7 +57,7 @@ static void xhci_pci_intr_update(XHCIState *xhci, int n, bool 
enable)
 }
 }
 
-static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
+static bool xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
 {
 XHCIPciState *s = container_of(xhci, XHCIPciState, xhci);
 PCIDevice *pci_dev = PCI_DEVICE(s);
@@ -70,13 +70,15 @@ static void xhci_pci_intr_raise(XHCIState *xhci, int n, 
bool level)
 
 if (msix_enabled(pci_dev) && level) {
 msix_notify(pci_dev, n);
-return;
+return true;
 }
 
 if (msi_enabled(pci_dev) && level) {
 msi_notify(pci_dev, n);
-return;
+return true;
 }
+
+return false;
 }
 
 static void xhci_pci_reset(DeviceState *dev)
diff --git a/hw/usb/hcd-xhci-sysbus.c b/hw/usb/hcd-xhci-sysbus.c
index 42e2574c8298..a14e4381960e 100644
--- a/hw/usb/hcd-xhci-sysbus.c
+++ b/hw/usb/hcd-xhci-sysbus.c
@@ -16,11 +16,13 @@
 #include "hw/acpi/aml-build.h"
 #include "hw/irq.h"
 
-static void xhci_sysbus_intr_raise(XHCIState *xhci, int n, bool level)
+static bool xhci_sysbus_intr_raise(XHCIState *xhci, int n, bool level)
 {
 XHCISysbusState *s = container_of(xhci, XHCISysbusState, xhci);
 
 qemu_set_irq(s->irq[n], level);
+
+return false;
 }
 
 void xhci_sysbus_reset(DeviceState *dev)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 46212b1e695a..e01700039b13 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -551,7 +551,9 @@ static void xhci_intr_update(XHCIState *xhci, int v)
 level = 1;
 }
 if (xhci->intr_raise) {
-xhci->intr_raise(xhci, 0, level);
+if (xhci->intr_raise(xhci, 0, level)) {
+xhci->intr[0].iman &= ~IMAN_IP;
+}
 }
 }
 if (xhci->intr_update) {
@@ -579,7 +581,9 @@ static void xhci_intr_raise(XHCIState *xhci, int v)
 return;
 }
 if (xhci->intr_raise) {
-xhci->intr_raise(xhci, v, true);
+if (xhci->intr_raise(xhci, v, true)) {
+xhci->intr[v].iman &= ~IMAN_IP;
+}
 }
 }
 
-- 
2.31.1

[PULL 2/3] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to

[Gerd Hoffmann]

From: Ruimei Yan 

At present MSI / MSI-X interrupts are triggered regardless of the
irq level. We should have checked the level to determine whether
the interrupt needs to be delivered.

The level check logic was present in early versions of the xhci
model, but got dropped later by a rework of interrupt handling
under commit 4c4abe7cc903 ("xhci: rework interrupt handling").

Fixes: 4c4abe7cc903 ("xhci: rework interrupt handling")
Signed-off-by: Ruimei Yan 
Signed-off-by: Bin Meng 
Message-Id: <20210521024224.2277634-1-bmeng...@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-xhci-pci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
index 9421734d0fe2..b6acd1790c1a 100644
--- a/hw/usb/hcd-xhci-pci.c
+++ b/hw/usb/hcd-xhci-pci.c
@@ -67,12 +67,13 @@ static void xhci_pci_intr_raise(XHCIState *xhci, int n, 
bool level)
  msi_enabled(pci_dev))) {
 pci_set_irq(pci_dev, level);
 }
-if (msix_enabled(pci_dev)) {
+
+if (msix_enabled(pci_dev) && level) {
 msix_notify(pci_dev, n);
 return;
 }
 
-if (msi_enabled(pci_dev)) {
+if (msi_enabled(pci_dev) && level) {
 msi_notify(pci_dev, n);
 return;
 }
-- 
2.31.1

[PULL 0/3] Usb 20210528 patches

[Gerd Hoffmann]

The following changes since commit c8616fc7670b884de5f74d2767aade224c1c5c3a:

  Merge remote-tracking branch 'remotes/philmd/tags/gitlab-ci-20210527' into 
staging (2021-05-27 16:32:57 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/usb-20210528-pull-request

for you to fetch changes up to fc967aad408eb4777b099d17ada1f39be5f6fd2e:

  hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X (2021-05-28 
09:10:20 +0200)


usb: bugfixes for hid and xhci.



Katsuhiro Ueno (1):
  hw/input/hid: Add support for keys of jp106 keyboard.

Ruimei Yan (2):
  hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to
  hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X

 hw/usb/hcd-xhci.h|  2 +-
 hw/input/hid.c   |  4 ++--
 hw/usb/hcd-xhci-pci.c| 13 -
 hw/usb/hcd-xhci-sysbus.c |  4 +++-
 hw/usb/hcd-xhci.c|  8 ++--
 5 files changed, 20 insertions(+), 11 deletions(-)

-- 
2.31.1

[PULL 1/3] hw/input/hid: Add support for keys of jp106 keyboard.

[Gerd Hoffmann]

From: Katsuhiro Ueno 

Add support for the following keys: KATAKANAHIRAGANA, HENKAN, MUHENKAN,
RO, and YEN.  Before this commit, these keys did not work as expected
when a jp106 keyboard was connected to the guest as a usb-kbd device.

Signed-off-by: Katsuhiro Ueno 
Message-Id: 
Signed-off-by: Gerd Hoffmann 
---
 hw/input/hid.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/input/hid.c b/hw/input/hid.c
index e1d2e4608374..8aab0521f405 100644
--- a/hw/input/hid.c
+++ b/hw/input/hid.c
@@ -51,8 +51,8 @@ static const uint8_t hid_usage_keys[0x100] = {
 0x45, 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e,
 0xe8, 0xe9, 0x71, 0x72, 0x73, 0x00, 0x00, 0x00,
 0x00, 0x00, 0x00, 0x85, 0x00, 0x00, 0x00, 0x00,
-0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-0x00, 0x00, 0x00, 0x00, 0x00, 0xe3, 0xe7, 0x65,
+0x88, 0x00, 0x00, 0x87, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x8a, 0x00, 0x8b, 0x00, 0x89, 0xe7, 0x65,
 
 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-- 
2.31.1

[PATCH 1/2] block-copy: fix block_copy_task_entry() progress update

[Vladimir Sementsov-Ogievskiy]

Don't report successful progress on failure, when call_state->ret is
set.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/block-copy.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index c2e5090412..f9e871b64f 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -439,9 +439,11 @@ static coroutine_fn int block_copy_task_entry(AioTask 
*task)
 
 ret = block_copy_do_copy(t->s, t->offset, t->bytes, t->zeroes,
  &error_is_read);
-if (ret < 0 && !t->call_state->ret) {
-t->call_state->ret = ret;
-t->call_state->error_is_read = error_is_read;
+if (ret < 0) {
+if (!t->call_state->ret) {
+t->call_state->ret = ret;
+t->call_state->error_is_read = error_is_read;
+}
 } else {
 progress_work_done(t->s->progress, t->bytes);
 }
-- 
2.29.2

[PATCH 2/2] block-copy: refactor copy_range handling

[Vladimir Sementsov-Ogievskiy]

Currently we update s->use_copy_range and s->copy_size in
block_copy_do_copy().

It's not very good:

1. block_copy_do_copy() is intended to be a simple function, that wraps
bdrv_co_ functions for need of block copy. That's why we don't pass
BlockCopyTask into it. So, block_copy_do_copy() is bad place for
manipulation with generic state of block-copy process

2. We are going to make block-copy thread-safe. So, it's good to move
manipulation with state of block-copy to the places where we'll need
critical sections anyway, to not introduce extra synchronization
primitives in block_copy_do_copy().

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/block-copy.c | 71 +++---
 1 file changed, 48 insertions(+), 23 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index f9e871b64f..c96fe31054 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -65,6 +65,7 @@ typedef struct BlockCopyTask {
 int64_t offset;
 int64_t bytes;
 bool zeroes;
+bool copy_range;
 QLIST_ENTRY(BlockCopyTask) list;
 CoQueue wait_queue; /* coroutines blocked on this task */
 } BlockCopyTask;
@@ -183,6 +184,7 @@ static BlockCopyTask *block_copy_task_create(BlockCopyState 
*s,
 .call_state = call_state,
 .offset = offset,
 .bytes = bytes,
+.copy_range = s->use_copy_range,
 };
 qemu_co_queue_init(&task->wait_queue);
 QLIST_INSERT_HEAD(&s->tasks, task, list);
@@ -342,11 +344,17 @@ static coroutine_fn int block_copy_task_run(AioTaskPool 
*pool,
  *
  * No sync here: nor bitmap neighter intersecting requests handling, only copy.
  *
+ * @copy_range is in-out argument: if *copy_range is false, copy_range is not
+ * done. If *copy_range is true, copy_range attempt is done. If copy_range
+ * attempt failed, the function fallback to usual read+write and *copy_range is
+ * set to false. *copy_range and zeroes must not be true simultaneously.
+ *
  * Returns 0 on success.
  */
 static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
int64_t offset, int64_t bytes,
-   bool zeroes, bool *error_is_read)
+   bool zeroes, bool *copy_range,
+   bool *error_is_read)
 {
 int ret;
 int64_t nbytes = MIN(offset + bytes, s->len) - offset;
@@ -359,6 +367,7 @@ static int coroutine_fn block_copy_do_copy(BlockCopyState 
*s,
 assert(offset + bytes <= s->len ||
offset + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size));
 assert(nbytes < INT_MAX);
+assert(!(*copy_range && zeroes));
 
 if (zeroes) {
 ret = bdrv_co_pwrite_zeroes(s->target, offset, nbytes, s->write_flags &
@@ -370,32 +379,15 @@ static int coroutine_fn block_copy_do_copy(BlockCopyState 
*s,
 return ret;
 }
 
-if (s->use_copy_range) {
+if (*copy_range) {
 ret = bdrv_co_copy_range(s->source, offset, s->target, offset, nbytes,
  0, s->write_flags);
 if (ret < 0) {
 trace_block_copy_copy_range_fail(s, offset, ret);
-s->use_copy_range = false;
-s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER);
+*copy_range = false;
 /* Fallback to read+write with allocated buffer */
 } else {
-if (s->use_copy_range) {
-/*
- * Successful copy-range. Now increase copy_size.  copy_range
- * does not respect max_transfer (it's a TODO), so we factor
- * that in here.
- *
- * Note: we double-check s->use_copy_range for the case when
- * parallel block-copy request unsets it during previous
- * bdrv_co_copy_range call.
- */
-s->copy_size =
-MIN(MAX(s->cluster_size, BLOCK_COPY_MAX_COPY_RANGE),
-QEMU_ALIGN_DOWN(block_copy_max_transfer(s->source,
-s->target),
-s->cluster_size));
-}
-goto out;
+return 0;
 }
 }
 
@@ -431,14 +423,44 @@ out:
 return ret;
 }
 
+static void block_copy_handle_copy_range_result(BlockCopyState *s,
+bool is_success)
+{
+if (!s->use_copy_range) {
+/* already disabled */
+return;
+}
+
+if (is_success) {
+/*
+ * Successful copy-range. Now increase copy_size.  copy_range
+ * does not respect max_transfer (it's a TODO), so we factor
+ * that in here.
+ */
+s->copy_size =
+MIN(MAX(s->cluster_size, BLOCK_COPY_MAX_COPY_RANGE),
+QEMU_ALIGN_DOWN(block_copy_max_transfer(s->source,
+

[PATCH 0/2] block-copy: small fix and refactor

[Vladimir Sementsov-Ogievskiy]

Hi all!

This is my suggestion how to refactor block-copy to avoid extra atomic
operations in 
"[PATCH v2 0/7] block-copy: protect block-copy internal structures"

Vladimir Sementsov-Ogievskiy (2):
  block-copy: fix block_copy_task_entry() progress update
  block-copy: refactor copy_range handling

 block/block-copy.c | 79 +++---
 1 file changed, 53 insertions(+), 26 deletions(-)

-- 
2.29.2

[PATCH RESEND 1/1] docs/devel: Add a single top-level header to MTTCG's doc

[Luis Pires]

Without a single top-level header in the .rst file, the index ended
up linking to all of the top-level headers separately. Now the index
links to the top-level header at the beginning of the document and
any inner headers are correctly linked as sub-items in the index.

Signed-off-by: Luis Pires 
---
 docs/devel/multi-thread-tcg.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/devel/multi-thread-tcg.rst b/docs/devel/multi-thread-tcg.rst
index 92a9eba13c..5b446ee08b 100644
--- a/docs/devel/multi-thread-tcg.rst
+++ b/docs/devel/multi-thread-tcg.rst
@@ -4,8 +4,9 @@
   This work is licensed under the terms of the GNU GPL, version 2 or
   later. See the COPYING file in the top-level directory.
 
-Introduction
-
+==
+Multi-threaded TCG
+==
 
 This document outlines the design for multi-threaded TCG (a.k.a MTTCG)
 system-mode emulation. user-mode emulation has always mirrored the
-- 
2.25.1

Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields

[Vladimir Sementsov-Ogievskiy]

28.05.2021 14:01, Paolo Bonzini wrote:

On 28/05/21 12:24, Paolo Bonzini wrote:


It's still more complicated, because you need to add some kind of

 method = s->method;


This would even have to be a separate, one-line critical section...



Hm, so, we should set .use_copy_range in task, when it is initialized.




 ret = block_copy_do_copy(..., method);
 if (ret < 0 && method <= COPY_RANGE_SMALL) {
 method = COPY_RANGE_READ_WRITE;
 ret = block_copy_do_copy(..., method);
 }
 lock();
 if (method == s->method) {
 /* compute new method */
 }

which makes it more complicated than this patch IMO.  But yeah at least it's a 
viable alternative to the atomics.





OK, I'm OK with patch as is. Finally I can refactor it later on top if needed.. 
I'll try now do some refactoring, you'll probably want to base on it, or 
vise-versa, I'll rebase it later on top of these patches.

--
Best regards,
Vladimir

[PATCH 1/2] docs/devel: Add a single top-level header to MTTCG's doc

[Luis Pires]

Without a single top-level header in the .rst file, the index ended
up linking to all of the top-level headers separately. Now the index
links to the top-level header at the beginning of the document and
any inner headers are correctly linked as sub-items in the index.

Signed-off-by: Luis Pires 
---
 docs/devel/multi-thread-tcg.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/devel/multi-thread-tcg.rst b/docs/devel/multi-thread-tcg.rst
index 92a9eba13c..5b446ee08b 100644
--- a/docs/devel/multi-thread-tcg.rst
+++ b/docs/devel/multi-thread-tcg.rst
@@ -4,8 +4,9 @@
   This work is licensed under the terms of the GNU GPL, version 2 or
   later. See the COPYING file in the top-level directory.
 
-Introduction
-
+==
+Multi-threaded TCG
+==
 
 This document outlines the design for multi-threaded TCG (a.k.a MTTCG)
 system-mode emulation. user-mode emulation has always mirrored the
-- 
2.25.1

[PATCH] docs/devel: Explain in more detail the TB chaining mechanisms

[Luis Pires]

Signed-off-by: Luis Pires 
---

Being new to QEMU, I went through the code to try to understand how
lookup_and_goto_ptr, goto_tb and exit_tb work, and when each should be used.
Thought I'd share what I learned by documenting it, as it might be
useful to other people starting to work on TCG, and will also allow
others to comment on any parts I misunderstood.

 docs/devel/tcg.rst | 96 --
 1 file changed, 85 insertions(+), 11 deletions(-)

diff --git a/docs/devel/tcg.rst b/docs/devel/tcg.rst
index 4ebde44b9d..d3354b8dcb 100644
--- a/docs/devel/tcg.rst
+++ b/docs/devel/tcg.rst
@@ -11,13 +11,14 @@ performances.
 QEMU's dynamic translation backend is called TCG, for "Tiny Code
 Generator". For more information, please take a look at ``tcg/README``.
 
-Some notable features of QEMU's dynamic translator are:
+The following sections outline some notable features and implementation
+details of QEMU's dynamic translator.
 
 CPU state optimisations
 ---
 
-The target CPUs have many internal states which change the way it
-evaluates instructions. In order to achieve a good speed, the
+The target CPUs have many internal states which change the way they
+evaluate instructions. In order to achieve a good speed, the
 translation phase considers that some state information of the virtual
 CPU cannot change in it. The state is recorded in the Translation
 Block (TB). If the state changes (e.g. privilege level), a new TB will
@@ -31,18 +32,91 @@ Direct block chaining
 -
 
 After each translated basic block is executed, QEMU uses the simulated
-Program Counter (PC) and other cpu state information (such as the CS
+Program Counter (PC) and other CPU state information (such as the CS
 segment base value) to find the next basic block.
 
-In order to accelerate the most common cases where the new simulated PC
-is known, QEMU can patch a basic block so that it jumps directly to the
-next one.
-
-The most portable code uses an indirect jump. An indirect jump makes
-it easier to make the jump target modification atomic. On some host
-architectures (such as x86 or PowerPC), the ``JUMP`` opcode is
+In its simplest, less optimized form, this is done by exiting from the
+current TB, going through the TB epilogue, and then back to the outer
+execution loop. That’s where QEMU looks for the next TB to execute,
+translating it from the guest architecture if it isn’t already available
+in memory. Then QEMU proceeds to execute this next TB, starting at the
+prologue and then moving on to the translated instructions.
+
+In order to accelerate the most common cases where the TB for the new
+simulated PC is already available, QEMU has mechanisms that allow
+multiple TBs to be chained directly, without having to go back to the
+outer execution loop as described above. These mechanisms are:
+
+``lookup_and_goto_ptr``
+^^^
+
+On platforms that support the ``lookup_and_goto_ptr`` mechanism, calling
+``tcg_gen_lookup_and_goto_ptr()`` will emit TCG instructions that call
+a helper function to look for the destination TB, based on
+the CPU state information. If the destination TB is available, a
+``goto_ptr`` TCG instruction is emitted to jump directly to its first
+instruction, skipping the epilogue - execution loop - prologue path.
+If the destination TB is not available, the ``goto_ptr`` instruction
+jumps to the epilogue, effectively exiting from the current TB and
+going back to the execution loop.
+
+On platforms that do not support this mechanism, the
+``tcg_gen_lookup_and_goto_ptr()`` function will just use
+``tcg_gen_exit_tb()`` to exit from the current TB.
+
+``goto_tb + exit_tb``
+^
+
+On platforms that support this mechanism, the translation code usually
+implements branching by performing the following steps:
+
+1. Call ``tcg_gen_goto_tb()`` passing a jump slot index (either 0 or 1)
+   as a parameter
+
+2. Emit TCG instructions to update the CPU state information with the
+   address of the next instruction to execute
+
+3. Call ``tcg_gen_exit_tb()`` passing the address of the current TB and
+   the jump slot index again
+
+Step 1, ``tcg_gen_goto_tb()``, will emit a ``goto_tb`` TCG
+instruction that later on gets translated to a jump to an address
+associated with the specified jump slot. Initially, this is the address
+of step 2's instructions, which update the CPU state information. Step 3,
+``tcg_gen_exit_tb()``, exits from the current TB returning a tagged
+pointer composed of the last executed TB’s address and the jump slot
+index.
+
+The first time this whole sequence is translated to target instructions
+and executed, step 1 doesn’t do anything really useful, as it just jumps
+to step 2. Then the CPU state information gets updated and we exit from
+the current TB. As a result, the behavior is very similar to the less
+optimized form described earlier in this section.
+
+Next, the execution loop looks for the nex

Re: [PULL 0/2] Block patches for 5.1.0-rc4

[Vladimir Sementsov-Ogievskiy]

11.08.2020 12:54, Max Reitz wrote:

On 11.08.20 11:39, Peter Maydell wrote:

On Tue, 11 Aug 2020 at 10:35, Max Reitz  wrote:


Hi,

There is a bug in the backup job that breaks backups from images whose
size is not aligned to the job's cluster size (i.e., qemu crashes
because of a failed assertion).  If this bug makes it into the release,
it would be a regression from 5.0.

On one hand, this is probably a rare configuration that should not
happen in practice.  On the other, it is a regression, and the fix
(patch 1) is simple.  So I think it would be good to have this in 5.1.


I'm really reluctant to have to roll an rc4...


Well, that’s the information there is on this: Regression, simple fix,
but little relevance in practice, and late to the party.
If, given this, you don’t want to roll an rc4, then that’s how it is.



Recently bug was reproduced by accidentally starting backup with source = cdrom 
(image was not 64k-cluster aligned).

Fedora 33, Rhel/Centos 8 are affected.

Could this go to stable branch?

--
Best regards,
Vladimir

Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields

[Vladimir Sementsov-Ogievskiy]

28.05.2021 14:01, Paolo Bonzini wrote:

On 28/05/21 12:24, Paolo Bonzini wrote:


It's still more complicated, because you need to add some kind of

 method = s->method;


This would even have to be a separate, one-line critical section...



Or atomic operation.. What I don't like that all troubles are for unused code. 
Many things may change to the moment when we actually reuse this for qemu-img 
convert.

And, qemu-img convert probably don't need this complicated logic with different 
methods. It may be better just return error if copy_range failed. What's a good 
reason for fall-back to normal write if copy-range is explicitly requested by 
user?




 ret = block_copy_do_copy(..., method);
 if (ret < 0 && method <= COPY_RANGE_SMALL) {
 method = COPY_RANGE_READ_WRITE;
 ret = block_copy_do_copy(..., method);
 }
 lock();
 if (method == s->method) {
 /* compute new method */
 }

which makes it more complicated than this patch IMO.  But yeah at least it's a 
viable alternative to the atomics.






--
Best regards,
Vladimir

Re: [PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed

[Klaus Jensen]

On May 28 11:05, Niklas Cassel wrote:

From: Niklas Cassel 

In the Zoned Namespace Command Set Specification, chapter
2.5.1 Managing resources

"The controller may transition zones in the ZSIO:Implicitly Opened state
to the ZSC:Closed state for resource management purposes."

The word may in this sentence means that automatically transitioning
an implicitly opened zone to closed is completely optional.

Add a new parameter so that the user can control if this automatic
transitioning should be performed or not.

Being able to control this can help with verifying that e.g. a user-space
program behaves properly even without this optional ZNS feature.

The default value is set to true, in order to not change the existing
behavior.

Signed-off-by: Niklas Cassel 
---
hw/nvme/ctrl.c | 9 -
hw/nvme/ns.c   | 2 ++
hw/nvme/nvme.h | 1 +
3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 40a7efcea9..d00f0297a5 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -141,6 +141,11 @@
 *
 * zoned.cross_read=
 * Setting this property to true enables Read Across Zone Boundaries.
+ *
+ * zoned.auto_transition=
+ * Indicates if zones in zone state implicitly opened can be
+ * automatically transitioned to zone state closed for resource
+ * management purposes.
 */

#include "qemu/osdep.h"
@@ -1699,7 +1704,9 @@ static uint16_t nvme_zrm_open_flags(NvmeNamespace *ns, 
NvmeZone *zone,
/* fallthrough */

case NVME_ZONE_STATE_CLOSED:
-nvme_zrm_auto_transition_zone(ns);
+if (ns->params.auto_transition_zones) {
+nvme_zrm_auto_transition_zone(ns);
+}
status = nvme_aor_check(ns, act, 1);
if (status) {
return status;
diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 3fec9c6273..31dee43d30 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -531,6 +531,8 @@ static Property nvme_ns_props[] = {
   params.max_open_zones, 0),
DEFINE_PROP_UINT32("zoned.descr_ext_size", NvmeNamespace,
   params.zd_extension_size, 0),
+DEFINE_PROP_BOOL("zoned.auto_transition", NvmeNamespace,
+ params.auto_transition_zones, true),
DEFINE_PROP_END_OF_LIST(),
};

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 81a35cda14..bd86054db2 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -100,6 +100,7 @@ typedef struct NvmeNamespaceParams {
uint32_t max_active_zones;
uint32_t max_open_zones;
uint32_t zd_extension_size;
+bool auto_transition_zones;
} NvmeNamespaceParams;

typedef struct NvmeNamespace {
--
2.31.1



Looks good Niklas!

Reviewed-by: Klaus Jensen 


signature.asc
Description: PGP signature

Re: [PATCH 1/3] qga-win: Increase VSS freeze timeout to 60 secs instead of 10

[Konstantin Kostiuk]

ping

On Mon, Apr 5, 2021 at 4:14 PM Basil Salman  wrote:

> Currently Requester freeze times out after 10 seconds, while
> the default timeout for Writer Freeze is 60 seconds. according to
> VSS Documentation [1].
> [1]:
> https://docs.microsoft.com/en-us/windows/win32/vss/overview-of-processing-a-backup-under-vss
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1909073
>
> Signed-off-by: Basil Salman 
> Signed-off-by: Basil Salman 
> ---
>  qga/vss-win32/requester.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
> index 5378c55d23..940a2c8f55 100644
> --- a/qga/vss-win32/requester.cpp
> +++ b/qga/vss-win32/requester.cpp
> @@ -18,7 +18,7 @@
>  #include 
>
>  /* Max wait time for frozen event (VSS can only hold writes for 10
> seconds) */
> -#define VSS_TIMEOUT_FREEZE_MSEC 1
> +#define VSS_TIMEOUT_FREEZE_MSEC 6
>
>  /* Call QueryStatus every 10 ms while waiting for frozen event */
>  #define VSS_TIMEOUT_EVENT_MSEC 10
> --
> 2.17.2
>
>

[PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed

[Niklas Cassel]

From: Niklas Cassel 

In the Zoned Namespace Command Set Specification, chapter
2.5.1 Managing resources

"The controller may transition zones in the ZSIO:Implicitly Opened state
to the ZSC:Closed state for resource management purposes."

The word may in this sentence means that automatically transitioning
an implicitly opened zone to closed is completely optional.

Add a new parameter so that the user can control if this automatic
transitioning should be performed or not.

Being able to control this can help with verifying that e.g. a user-space
program behaves properly even without this optional ZNS feature.

The default value is set to true, in order to not change the existing
behavior.

Signed-off-by: Niklas Cassel 
---
 hw/nvme/ctrl.c | 9 -
 hw/nvme/ns.c   | 2 ++
 hw/nvme/nvme.h | 1 +
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 40a7efcea9..d00f0297a5 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -141,6 +141,11 @@
  *
  * zoned.cross_read=
  * Setting this property to true enables Read Across Zone Boundaries.
+ *
+ * zoned.auto_transition=
+ * Indicates if zones in zone state implicitly opened can be
+ * automatically transitioned to zone state closed for resource
+ * management purposes.
  */
 
 #include "qemu/osdep.h"
@@ -1699,7 +1704,9 @@ static uint16_t nvme_zrm_open_flags(NvmeNamespace *ns, 
NvmeZone *zone,
 /* fallthrough */
 
 case NVME_ZONE_STATE_CLOSED:
-nvme_zrm_auto_transition_zone(ns);
+if (ns->params.auto_transition_zones) {
+nvme_zrm_auto_transition_zone(ns);
+}
 status = nvme_aor_check(ns, act, 1);
 if (status) {
 return status;
diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 3fec9c6273..31dee43d30 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -531,6 +531,8 @@ static Property nvme_ns_props[] = {
params.max_open_zones, 0),
 DEFINE_PROP_UINT32("zoned.descr_ext_size", NvmeNamespace,
params.zd_extension_size, 0),
+DEFINE_PROP_BOOL("zoned.auto_transition", NvmeNamespace,
+ params.auto_transition_zones, true),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 81a35cda14..bd86054db2 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -100,6 +100,7 @@ typedef struct NvmeNamespaceParams {
 uint32_t max_active_zones;
 uint32_t max_open_zones;
 uint32_t zd_extension_size;
+bool auto_transition_zones;
 } NvmeNamespaceParams;
 
 typedef struct NvmeNamespace {
-- 
2.31.1

Re: [PATCH v1 2/6] meson.build: fix cosmetics of compiler display

[Philippe Mathieu-Daudé]

On 5/27/21 6:03 PM, Alex Bennée wrote:
> If you specify something like --cc="ccache gcc" on your configure line
> the summary output misses the rest of the cmd_array. Do some string
> joining to make it complete.
> 
> Signed-off-by: Alex Bennée 
> Tested-by: Thomas Huth 
> ---
>  meson.build | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields

[Paolo Bonzini]

On 28/05/21 12:24, Paolo Bonzini wrote:


It's still more complicated, because you need to add some kind of

 method = s->method;


This would even have to be a separate, one-line critical section...

Paolo


 ret = block_copy_do_copy(..., method);
 if (ret < 0 && method <= COPY_RANGE_SMALL) {
     method = COPY_RANGE_READ_WRITE;
     ret = block_copy_do_copy(..., method);
     }
 lock();
     if (method == s->method) {
     /* compute new method */
     }

which makes it more complicated than this patch IMO.  But yeah at least 
it's a viable alternative to the atomics.

Re: [PATCH v2 5/7] block-copy: add QemuMutex lock for BlockCopyCallState list

[Paolo Bonzini]

On 18/05/21 12:07, Emanuele Giuseppe Esposito wrote:

+qemu_mutex_lock(&call_state->s->calls_lock);
  QLIST_INSERT_HEAD(&call_state->s->calls, call_state, list);
+qemu_mutex_unlock(&call_state->s->calls_lock);


Let's just use tasks_lock here (maybe even rename it to just "lock").

Paolo

Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields

[Paolo Bonzini]

On 26/05/21 19:18, Vladimir Sementsov-Ogievskiy wrote:


It's actually the original idea of block_copy_do_copy() function: do 
only simple copy, don't interact with the state. It's a kind of wrapper 
on top of bdrv_co.


So, actually updating s->use_copy_range here was a bad idea.


It's still more complicated, because you need to add some kind of

method = s->method;
ret = block_copy_do_copy(..., method);
if (ret < 0 && method <= COPY_RANGE_SMALL) {
method = COPY_RANGE_READ_WRITE;
ret = block_copy_do_copy(..., method);
}
lock();
if (method == s->method) {
/* compute new method */
}

which makes it more complicated than this patch IMO.  But yeah at least 
it's a viable alternative to the atomics.


Paolo

Re: [PATCH v2] HMP: added info cpustats to removed_features.rst

[Greg Kurz]

On Thu, 27 May 2021 14:56:02 -0300
"Bruno Larsen (billionai)"  wrote:

> Documented the removal of the HMP command info cpustats
> 
> Signed-off-by: Bruno Larsen (billionai) 
> ---

Reviewed-by: Greg Kurz 

>  docs/system/removed-features.rst | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/docs/system/removed-features.rst 
> b/docs/system/removed-features.rst
> index 5a462ac568..2feae41089 100644
> --- a/docs/system/removed-features.rst
> +++ b/docs/system/removed-features.rst
> @@ -249,6 +249,11 @@ Use ``migrate-set-parameters`` and ``info 
> migrate-parameters`` instead.
>  
>  Use ``migrate-set-parameters`` instead.
>  
> +``info cpustats`` (removed in 6.1)
> +'
> +
> +This command didn't produce any output already. Removed with no replacement.
> +
>  Guest Emulator ISAs
>  ---
>  

Re: [PATCH 0/5] esp: fixes for MacOS toolbox ROM

[Mark Cave-Ayland]

On 19/05/2021 11:07, Mark Cave-Ayland wrote:


This patchset contains more ESP fixes from my attempts to boot MacOS under
the QEMU q800 machine (along with a related NetBSD fix).

With these patches it is possible for the MacOS toolbox ROM and MacOS drivers
to detect and access SCSI drives and CDROMs during the MacOS boot process.

This patchset has been tested on top of the ESP fix series posted yesterday
(see https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg05763.html) with
the extended set of ESP test images without noticing any regressions.

Signed-off-by: Mark Cave-Ayland 

[q800-macos-upstream patchset series: 1]


Mark Cave-Ayland (5):
   esp: allow non-DMA callback in esp_transfer_data() initial transfer
   esp: handle non-DMA transfers from the target one byte at a time
   esp: ensure PDMA write transfers are flushed from the FIFO to the
 target immediately
   esp: revert 75ef849696 "esp: correctly fill bus id with requested lun"
   esp: correctly accumulate extended messages for PDMA

  hw/scsi/esp.c | 137 ++
  1 file changed, 83 insertions(+), 54 deletions(-)


Ping? I'd be particularly interested if anyone could clarify the history around the 
code removed by patch 4...



ATB,

Mark.

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Dongjiu Geng]

Peter Maydell  于2021年5月27日周四 上午2:19写道：
>
> On Wed, 26 May 2021 at 18:32, Swetha Joshi  wrote:
> >
> > Hello,
> >
> > One of the qemu machines we use has KVM enabled, but we don't want the 
> > CONFIG_ARM_VIRT enabled as it pulls in emulation of a variety of physical 
> > hardware that we don't need. The compilation errors I mentioned are not in 
> > the qemu mainline per say but we see them in one of the qemu derived 
> > machines we use.
>
> Sure, but unless you can give me a recipe for reproducing the
> build failure in mainline I can't really help...

Hi Swetha，
 Yes,  Can you give a method that how to reproduce the build
failure issues? Thanks

>
> thanks
> -- PMM

Re: [PATCH 0/2] esp: minor fixes for older Linux versions

[Mark Cave-Ayland]

On 18/05/2021 22:25, Mark Cave-Ayland wrote:


Following on from the ESP changes in QEMU 6.0 someone pointed out that the old
Linux 2.6 ESP driver as used in Aurelian's SPARC image at
https://people.debian.org/~aurel32/qemu/sparc/ emits a constant stream of
"esp0: STEP_ASEL for tgt 0" messages to the console during boot.

These patches solve the issue so that the older image is able to boot cleanly
once again. The first patch is a genuine bug fix for the QEMU 6.0 changes whilst
the second works around the delayed bus phase change when deferring IO transfers
from the end of the command phase to the start of the information transfer
phase.

I've tested this using the extended suite of images used for the QEMU 6.0 
changes
and confirmed that there are no other regressions.

Signed-off-by: Mark Cave-Ayland 


Mark Cave-Ayland (2):
   esp: only assert INTR_DC interrupt flag if selection fails
   esp: only set ESP_RSEQ at the start of the select sequence

  hw/scsi/esp.c | 13 ++---
  1 file changed, 10 insertions(+), 3 deletions(-)


Ping?


ATB,

Mark.

Re: [PATCH 3/3] ui/vdagent: fix clipboard info memory leak in error path

[Gerd Hoffmann]

On Wed, May 26, 2021 at 10:12:48AM +0100, Stefan Hajnoczi wrote:
> If the size of a VD_AGENT_CLIPBOARD_GRAB message is invalid we leak info
> when returning early.
> 
> Thanks to Coverity for spotting this:
> 
> *** CID 1453266:  Resource leaks  (RESOURCE_LEAK)
> /qemu/ui/vdagent.c: 465 in vdagent_chr_recv_clipboard()
> 459 info = qemu_clipboard_info_new(&vd->cbpeer, s);
> 460 if (size > sizeof(uint32_t) * 10) {
> 461 /*
> 462  * spice has 6 types as of 2021. Limiting to 10 entries
> 463  * so we we have some wiggle room.
> 464  */
> >>> CID 1453266:  Resource leaks  (RESOURCE_LEAK)
> >>> Variable "info" going out of scope leaks the storage it points to.
> 465 return;
> 466 }
> 467 while (size >= sizeof(uint32_t)) {
> 468 trace_vdagent_cb_grab_type(GET_NAME(type_name, *(uint32_t 
> *)data));
> 469 switch (*(uint32_t *)data) {
> 470 case VD_AGENT_CLIPBOARD_UTF8_TEXT:
> 
> Fixes: f0349f4d8947ad32d0fa4678cbf5dbb356fcbda1 ("ui/vdagent: add clipboard 
> support")
> Cc: Gerd Hoffmann 
> Signed-off-by: Stefan Hajnoczi 

Reviewed-by: Gerd Hoffmann