[Bug 1914870] Re: libvixl compilation failure on Debian unstable

2021-04-29 Thread Thomas Huth
I think we had some c++ related fixes merged in the last weeks ... is
this still reproducible with the current 6.0-rc5 version of QEMU?

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

Title:
  libvixl compilation failure on Debian unstable

Status in QEMU:
  New

Bug description:
  As of commit 0e324626306:

  $ lsb_release -d
  Description:Debian GNU/Linux bullseye/sid

  Project version: 5.2.50
  C compiler for the host machine: cc (gcc 10.2.1 "cc (Debian 10.2.1-6) 10.2.1 
20210110")
  C linker for the host machine: cc ld.bfd 2.35.1
  C++ compiler for the host machine: c++ (gcc 10.2.1 "c++ (Debian 10.2.1-6) 
10.2.1 20210110")
  C++ linker for the host machine: c++ ld.bfd 2.35.1

  [6/79] Compiling C++ object libcommon.fa.p/disas_libvixl_vixl_utils.cc.o
  FAILED: libcommon.fa.p/disas_libvixl_vixl_utils.cc.o 
  c++ -Ilibcommon.fa.p -I. -I.. -Iqapi -Itrace -Iui/shader 
-I/usr/include/capstone -I/usr/include/glib-2.0 
-I/usr/lib/hppa-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -pipe -Wall 
-Winvalid-pch -Wnon-virtual-dtor -Werror -std=gnu++11 -O2 -g -isystem 
/home/philmd/qemu/linux-headers -isystem linux-headers -iquote . -iquote 
/home/philmd/qemu -iquote /home/philmd/qemu/include -iquote 
/home/philmd/qemu/disas/libvixl -iquote /home/philmd/qemu/tcg/hppa -iquote 
/home/philmd/qemu/accel/tcg -pthread -D__STDC_LIMIT_MACROS 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -U_FORTIFY_SOURCE 
-D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
-Wundef -Wwrite-strings -fno-strict-aliasing -fno-common -fwrapv -Wtype-limits 
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body 
-Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fPIE -MD -MQ 
libcommon.fa.p/disas_libvixl_vixl_utils.cc.o -MF 
libcommon.fa.p/disas_libvixl_vixl_utils.cc.o.d -o 
libcommon.fa.p/disas_libvixl_vixl_utils.cc.o -c ../disas/libvixl/vixl/utils.cc
  In file included from /home/philmd/qemu/disas/libvixl/vixl/utils.h:30,
   from ../disas/libvixl/vixl/utils.cc:27:
  /usr/include/string.h:36:43: error: missing binary operator before token "("
 36 | #if defined __cplusplus && (__GNUC_PREREQ (4, 4) \
|   ^
  /usr/include/string.h:53:62: error: missing binary operator before token "("
 53 | #if defined __USE_MISC || defined __USE_XOPEN || __GLIBC_USE (ISOC2X)
|  ^
  /usr/include/string.h:165:21: error: missing binary operator before token "("
165 |  || __GLIBC_USE (LIB_EXT2) || __GLIBC_USE (ISOC2X))
| ^
  /usr/include/string.h:174:43: error: missing binary operator before token "("
174 | #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2) || __GLIBC_USE 
(ISOC2X)
|   ^
  /usr/include/string.h:492:19: error: missing binary operator before token "("
492 | #if __GNUC_PREREQ (3,4)
|   ^
  In file included from /home/philmd/qemu/disas/libvixl/vixl/utils.h:30,
   from ../disas/libvixl/vixl/utils.cc:27:
  /usr/include/string.h:28:1: error: ‘__BEGIN_DECLS’ does not name a type
 28 | __BEGIN_DECLS
| ^
  In file included from /home/philmd/qemu/disas/libvixl/vixl/utils.h:30,
   from ../disas/libvixl/vixl/utils.cc:27:
  /usr/include/string.h:44:8: error: ‘size_t’ has not been declared
 44 |size_t __n) __THROW __nonnull ((1, 2));
|^~
  /usr/include/string.h:44:20: error: expected initializer before ‘__THROW’
 44 |size_t __n) __THROW __nonnull ((1, 2));
|^~~
  /usr/include/string.h:47:56: error: ‘size_t’ has not been declared
 47 | extern void *memmove (void *__dest, const void *__src, size_t __n)
|^~
  /usr/include/string.h:48:6: error: expected initializer before ‘__THROW’
 48 |  __THROW __nonnull ((1, 2));
|  ^~~
  /usr/include/string.h:61:42: error: ‘size_t’ has not been declared
 61 | extern void *memset (void *__s, int __c, size_t __n) __THROW 
__nonnull ((1));
|  ^~

  Is there a package dependency missing?

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



[Bug 1874678] Re: [Feature request] python-qemu package

2021-04-29 Thread Thomas Huth
This is an automated cleanup. This bug report has been moved
to QEMU's new bug tracker on gitlab.com and thus gets marked
as 'expired' now. Please continue with the discussion here:

 https://gitlab.com/qemu-project/qemu/-/issues/50


** Changed in: qemu
   Status: In Progress => Expired

** Changed in: qemu
 Assignee: John Snow (jnsnow) => (unassigned)

** Bug watch added: gitlab.com/qemu-project/qemu/-/issues #50
   https://gitlab.com/qemu-project/qemu/-/issues/50

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

Title:
  [Feature request] python-qemu package

Status in QEMU:
  Expired

Bug description:
  It would be useful to have the python/qemu/ files published as a
  Python pip package, so users from distribution can also use the QEMU
  python methods (in particular for testing) without having to clone the
  full repository.

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



Re: [RUST] Add crate for generic vhost-user-i2c backend daemon

2021-04-29 Thread Viresh Kumar
On 28-04-21, 17:13, Trilok Soni wrote:
> Viresh,
> 
> For rust-vmm, you need to create the new issue in the right project.
> You can probably pick up vmm-reference project at rust-vmm and ask
> for the new crate.

Done.

https://github.com/rust-vmm/vmm-reference/issues/118

> There is also bi-weekly meetings which is attended by me, Vatsa and
> rust-vmm developers where it can be put up as agenda. 

That would be great.

> The minimal requirement for the new crate is to have less (or almost
> none) dependencies on other crates so that they can be independently
> tested in the rust-vmm CI.

It depends on a bunch of crates, specially vhost and
vhost-user-backend.

> Anyways, please file a new issue and I
> will ask Vatsa and others to comment there. 

Thanks Trilok.

-- 
viresh



Re: [PATCH v2 01/15] linux-user/s390x: Fix sigframe types

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:33, Richard Henderson wrote:

Noticed via gitlab clang-user job:

   TESTsignals on s390x
../linux-user/s390x/signal.c:258:9: runtime error: \
   1.84467e+19 is outside the range of representable values of \
   type 'unsigned long'

Which points to the fact that we were performing a double-to-uint64_t
conversion while storing the fp registers, instead of just copying
the data across.

Turns out there are several errors:

target_ulong is the size of the target register, whereas abi_ulong
is the target 'unsigned long' type.  Not a big deal here, since we
only support 64-bit s390x, but not correct either.

In target_sigcontext and target ucontext, we used a host pointer
instead of a target pointer, aka abi_ulong.

Fixing this allows the removal of a cast to __put_user.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 26 +-
  1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index b68b44ae7e..707fb603d7 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -37,13 +37,14 @@
  
  typedef struct {

  target_psw_t psw;
-target_ulong gprs[__NUM_GPRS];
-unsigned int acrs[__NUM_ACRS];
+abi_ulong gprs[__NUM_GPRS];
+abi_uint acrs[__NUM_ACRS];
  } target_s390_regs_common;
  
  typedef struct {

-unsigned int fpc;
-double   fprs[__NUM_FPRS];
+uint32_t fpc;
+uint32_t pad;
+uint64_t fprs[__NUM_FPRS];
  } target_s390_fp_regs;
  
  typedef struct {

@@ -51,22 +52,22 @@ typedef struct {
  target_s390_fp_regs fpregs;
  } target_sigregs;
  
-struct target_sigcontext {

-target_ulong   oldmask[_SIGCONTEXT_NSIG_WORDS];
-target_sigregs *sregs;
-};
+typedef struct {
+abi_ulong oldmask[_SIGCONTEXT_NSIG_WORDS];
+abi_ulong sregs;
+} target_sigcontext;
  
  typedef struct {

  uint8_t callee_used_stack[__SIGNAL_FRAMESIZE];
-struct target_sigcontext sc;
+target_sigcontext sc;
  target_sigregs sregs;
  int signo;
  uint8_t retcode[S390_SYSCALL_SIZE];
  } sigframe;
  
  struct target_ucontext {

-target_ulong tuc_flags;
-struct target_ucontext *tuc_link;
+abi_ulong tuc_flags;
+abi_ulong tuc_link;
  target_stack_t tuc_stack;
  target_sigregs tuc_mcontext;
  target_sigset_t tuc_sigmask;   /* mask last for extensibility */
@@ -143,8 +144,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
  
  save_sigregs(env, &frame->sregs);
  
-__put_user((abi_ulong)(unsigned long)&frame->sregs,

-   (abi_ulong *)&frame->sc.sregs);
+__put_user((abi_ulong)(unsigned long)&frame->sregs, &frame->sc.sregs);
  
  /* Set up to return from userspace.  If provided, use a stub

 already in userspace.  */



Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH v2 03/15] linux-user/s390x: Remove PSW_ADDR_AMODE

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:33, Richard Henderson wrote:

This is an unnecessary complication since we only
support 64-bit mode.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 17 ++---
  1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index fece8ab97b..1dfca71fa9 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -31,7 +31,6 @@
  #define _SIGCONTEXT_NSIG_BPW64 /* FIXME: 31-bit mode -> 32 */
  #define _SIGCONTEXT_NSIG_WORDS  (_SIGCONTEXT_NSIG / _SIGCONTEXT_NSIG_BPW)
  #define _SIGMASK_COPY_SIZE(sizeof(unsigned long)*_SIGCONTEXT_NSIG_WORDS)
-#define PSW_ADDR_AMODE0xUL /* 0x8000UL for 
31-bit */
  #define S390_SYSCALL_OPCODE ((uint16_t)0x0a00)
  
  typedef struct {

@@ -148,11 +147,9 @@ void setup_frame(int sig, struct target_sigaction *ka,
  /* Set up to return from userspace.  If provided, use a stub
 already in userspace.  */
  if (ka->sa_flags & TARGET_SA_RESTORER) {
-env->regs[14] = (unsigned long)
-ka->sa_restorer | PSW_ADDR_AMODE;
+env->regs[14] = ka->sa_restorer;
  } else {
-env->regs[14] = (frame_addr + offsetof(sigframe, retcode))
-| PSW_ADDR_AMODE;
+env->regs[14] = frame_addr + offsetof(sigframe, retcode);
  __put_user(S390_SYSCALL_OPCODE | TARGET_NR_sigreturn,
 &frame->retcode);
  }
@@ -162,7 +159,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
  
  /* Set up registers for signal handler */

  env->regs[15] = frame_addr;
-env->psw.addr = (target_ulong) ka->_sa_handler | PSW_ADDR_AMODE;
+env->psw.addr = ka->_sa_handler;
  
  env->regs[2] = sig; //map_signal(sig);

  env->regs[3] = frame_addr += offsetof(typeof(*frame), sc);
@@ -210,10 +207,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
  /* Set up to return from userspace.  If provided, use a stub
 already in userspace.  */
  if (ka->sa_flags & TARGET_SA_RESTORER) {
-env->regs[14] = ka->sa_restorer | PSW_ADDR_AMODE;
+env->regs[14] = ka->sa_restorer;
  } else {
-env->regs[14] = (frame_addr + offsetof(typeof(*frame), retcode))
-| PSW_ADDR_AMODE;
+env->regs[14] = frame_addr + offsetof(typeof(*frame), retcode);
  __put_user(S390_SYSCALL_OPCODE | TARGET_NR_rt_sigreturn,
 &frame->retcode);
  }
@@ -223,7 +219,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
  
  /* Set up registers for signal handler */

  env->regs[15] = frame_addr;
-env->psw.addr = (target_ulong) ka->_sa_handler | PSW_ADDR_AMODE;
+env->psw.addr = ka->_sa_handler;
  
  env->regs[2] = sig; //map_signal(sig);

  env->regs[3] = frame_addr + offsetof(typeof(*frame), info);
@@ -248,7 +244,6 @@ restore_sigregs(CPUS390XState *env, target_sigregs *sc)
  trace_user_s390x_restore_sigregs(env, (unsigned long 
long)sc->regs.psw.addr,
   (unsigned long long)env->psw.addr);
  __get_user(env->psw.addr, &sc->regs.psw.addr);
-/* FIXME: 31-bit -> | PSW_ADDR_AMODE */
  
  for (i = 0; i < 16; i++) {

  __get_user(env->aregs[i], &sc->regs.acrs[i]);



Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH v2 05/15] linux-user/s390x: Fix trace in restore_regs

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:33, Richard Henderson wrote:

Directly reading sc->regs.psw.addr misses the bswap
that may be performed by __get_user.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index e455a9818d..dcc6f7bc02 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -232,16 +232,17 @@ give_sigsegv:
  
  static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)

  {
+target_ulong prev_addr;
  int i;
  
  for (i = 0; i < 16; i++) {

  __get_user(env->regs[i], &sc->regs.gprs[i]);
  }
  
+prev_addr = env->psw.addr;

  __get_user(env->psw.mask, &sc->regs.psw.mask);
-trace_user_s390x_restore_sigregs(env, (unsigned long 
long)sc->regs.psw.addr,
- (unsigned long long)env->psw.addr);
  __get_user(env->psw.addr, &sc->regs.psw.addr);
+trace_user_s390x_restore_sigregs(env, env->psw.addr, prev_addr);
  
  for (i = 0; i < 16; i++) {

  __get_user(env->aregs[i], &sc->regs.acrs[i]);



Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH v2 02/15] linux-user/s390x: Use uint16_t for signal retcode

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:33, Richard Henderson wrote:

Using the right type simplifies the frame setup.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 9 -
  1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 707fb603d7..fece8ab97b 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -25,7 +25,6 @@
  #define __NUM_FPRS 16
  #define __NUM_ACRS 16
  
-#define S390_SYSCALL_SIZE   2

  #define __SIGNAL_FRAMESIZE  160 /* FIXME: 31-bit mode -> 96 */
  
  #define _SIGCONTEXT_NSIG64

@@ -62,7 +61,7 @@ typedef struct {
  target_sigcontext sc;
  target_sigregs sregs;
  int signo;
-uint8_t retcode[S390_SYSCALL_SIZE];
+uint16_t retcode;
  } sigframe;
  
  struct target_ucontext {

@@ -75,7 +74,7 @@ struct target_ucontext {
  
  typedef struct {

  uint8_t callee_used_stack[__SIGNAL_FRAMESIZE];
-uint8_t retcode[S390_SYSCALL_SIZE];
+uint16_t retcode;
  struct target_siginfo info;
  struct target_ucontext uc;
  } rt_sigframe;
@@ -155,7 +154,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
  env->regs[14] = (frame_addr + offsetof(sigframe, retcode))
  | PSW_ADDR_AMODE;
  __put_user(S390_SYSCALL_OPCODE | TARGET_NR_sigreturn,
-   (uint16_t *)(frame->retcode));
+   &frame->retcode);
  }
  
  /* Set up backchain. */

@@ -216,7 +215,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
  env->regs[14] = (frame_addr + offsetof(typeof(*frame), retcode))
  | PSW_ADDR_AMODE;
  __put_user(S390_SYSCALL_OPCODE | TARGET_NR_rt_sigreturn,
-   (uint16_t *)(frame->retcode));
+   &frame->retcode);
  }
  
  /* Set up backchain. */




Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH v2 06/15] linux-user/s390x: Fix sigcontext sregs value

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:33, Richard Henderson wrote:

Using the host address of &frame->sregs is incorrect.
We need the guest address.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index dcc6f7bc02..f8515dd332 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -142,7 +142,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
  
  save_sigregs(env, &frame->sregs);
  
-__put_user((abi_ulong)(unsigned long)&frame->sregs, &frame->sc.sregs);

+__put_user(frame_addr + offsetof(sigframe, sregs), &frame->sc.sregs);
  
  /* Set up to return from userspace.  If provided, use a stub

 already in userspace.  */



Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH v2 04/15] linux-user/s390x: Remove restore_sigregs return value

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:33, Richard Henderson wrote:

The function cannot fail.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 14 +++---
  1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 1dfca71fa9..e455a9818d 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -230,10 +230,8 @@ give_sigsegv:
  force_sigsegv(sig);
  }
  
-static int

-restore_sigregs(CPUS390XState *env, target_sigregs *sc)
+static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)
  {
-int err = 0;
  int i;
  
  for (i = 0; i < 16; i++) {

@@ -251,8 +249,6 @@ restore_sigregs(CPUS390XState *env, target_sigregs *sc)
  for (i = 0; i < 16; i++) {
  __get_user(*get_freg(env, i), &sc->fpregs.fprs[i]);
  }
-
-return err;
  }
  
  long do_sigreturn(CPUS390XState *env)

@@ -271,9 +267,7 @@ long do_sigreturn(CPUS390XState *env)
  target_to_host_sigset_internal(&set, &target_set);
  set_sigmask(&set); /* ~_BLOCKABLE? */
  
-if (restore_sigregs(env, &frame->sregs)) {

-goto badframe;
-}
+restore_sigregs(env, &frame->sregs);
  
  unlock_user_struct(frame, frame_addr, 0);

  return -TARGET_QEMU_ESIGRETURN;
@@ -297,9 +291,7 @@ long do_rt_sigreturn(CPUS390XState *env)
  
  set_sigmask(&set); /* ~_BLOCKABLE? */
  
-if (restore_sigregs(env, &frame->uc.tuc_mcontext)) {

-goto badframe;
-}
+restore_sigregs(env, &frame->uc.tuc_mcontext);
  
  target_restore_altstack(&frame->uc.tuc_stack, env);
  



Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH v2 07/15] linux-user/s390x: Use tswap_sigset in setup_rt_frame

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:34, Richard Henderson wrote:

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 6 +-
  1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index f8515dd332..4dde55d4d5 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -182,7 +182,6 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
  target_siginfo_t *info,
  target_sigset_t *set, CPUS390XState *env)
  {
-int i;
  rt_sigframe *frame;
  abi_ulong frame_addr;
  
@@ -199,10 +198,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,

  __put_user((abi_ulong)0, (abi_ulong *)&frame->uc.tuc_link);
  target_save_altstack(&frame->uc.tuc_stack, env);
  save_sigregs(env, &frame->uc.tuc_mcontext);
-for (i = 0; i < TARGET_NSIG_WORDS; i++) {
-__put_user((abi_ulong)set->sig[i],
-   (abi_ulong *)&frame->uc.tuc_sigmask.sig[i]);
-}
+tswap_sigset(&frame->uc.tuc_sigmask, set);
  
  /* Set up to return from userspace.  If provided, use a stub

 already in userspace.  */



Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [RFC PATCH v2 2/4] hw/arm/virt: Parse -smp cluster parameter in virt_smp_parse

2021-04-29 Thread Andrew Jones
On Thu, Apr 29, 2021 at 10:14:37AM +0800, wangyanan (Y) wrote:
> On 2021/4/28 18:31, Andrew Jones wrote:
> > On Tue, Apr 13, 2021 at 04:31:45PM +0800, Yanan Wang wrote:
> > >   } else if (sockets == 0) {
> > >   threads = threads > 0 ? threads : 1;
> > > -sockets = cpus / (cores * threads);
> > > +sockets = cpus / (clusters * cores * threads);
> > >   sockets = sockets > 0 ? sockets : 1;
> > If we initialize clusters to zero instead of one and add lines in
> > 'cpus == 0 || cores == 0' and 'sockets == 0' like
> > 'clusters = clusters > 0 ? clusters : 1' as needed, then I think we can
> > add
> > 
> >   } else if (clusters == 0) {
> >   threads = threads > 0 ? threads : 1;
> >   clusters = cpus / (sockets * cores * thread);
> >   clusters = clusters > 0 ? clusters : 1;
> >   }
> > 
> > here.
> I have thought about this kind of format before, but there is a little bit
> difference between these two ways. Let's chose the better and more
> reasonable one of the two.
> 
> Way A currently in this patch:
> If value of clusters is not explicitly specified in -smp command line, we
> assume
> that users don't want to support clusters, for compatibility we initialized
> the
> value to 1. So that with cmdline "-smp cpus=24, sockets=2, cores=6", we will
> parse out the topology description like below:
> cpus=24, sockets=2, clusters=1, cores=6, threads=2
> 
> Way B that you suggested for this patch:
> Whether value of clusters is explicitly specified in -smp command line or
> not,
> we assume that clusters are supported and calculate the value. So that with
> cmdline "-smp cpus=24, sockets=2, cores=6", we will parse out the topology
> description like below:
> cpus =24, sockets=2, clusters=2, cores=6, threads=1
> 
> But I think maybe we should not assume too much about what users think
> through the -smp command line. We should just assume that all levels of
> cpu topology are supported and calculate them, and users should be more
> careful if they want to get the expected results with not so complete
> cmdline.
> If I'm right, then Way B should be better. :)
>

Hi Yanan,

We're already assuming the user wants to describe clusters to the guest
because we require at least one per socket. If we want the user to have a
choice between using clusters or not, then I guess we need to change the
logic in the PPTT and the cpu-map to only generate the cluster level when
the number of clusters is not zero. And then change this parser to not
require clusters at all.

I'm not a big fan of these auto-calculated values either, but the
documentation says that it'll do that, and it's been done that way
forever, so I think we're stuck with it for the -smp option. Hmm, I was
just about to say that x86 computes all its values, but I see the most
recently added one, 'dies', is implemented the way you're proposing we
implement 'clusters', i.e. default to one and don't calculate it when it's
missing. I actually consider that either a documentation bug or an smp
parsing bug, though.

Another possible option, for Arm, because only the cpus and maxcpus
parameters of -smp have ever worked, is to document, for Arm, that if even
one parameter other than cpus or maxcpus is provided, then all parameters
must be provided. We can still decide if clusters=0 is valid, but we'll
enforce that everything is explicit and that the product (with or without
clusters) matches maxcpus.

Requiring every parameter might be stricter than necessary, though, I
think we're mostly concerned with cpus/maxcpus, sockets, and cores.
clusters can default to one or zero (whatever we choose and document),
threads can default to one, and cpus can default to maxcpus or maxcpus can
default to cpus, but at least one of those must be provided. And, if
sockets are provided, then cores must be provided and vice versa. If
neither sockets nor cores are provided, then nothing else besides cpus and
maxcpus may be provided, and that would mean to not generate any topology
descriptions for the guest.

Thanks,
drew




Re: [PATCH v2 08/15] linux-user/s390x: Tidy save_sigregs

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:34, Richard Henderson wrote:

The "save" routines copied from the kernel, which are currently
commented out, are unnecessary in qemu.  We can copy from env
where the kernel needs special instructions.  Fix comment style.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 4dde55d4d5..eabfe4293f 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -104,23 +104,25 @@ get_sigframe(struct target_sigaction *ka, CPUS390XState 
*env, size_t frame_size)
  static void save_sigregs(CPUS390XState *env, target_sigregs *sregs)
  {
  int i;
-//save_access_regs(current->thread.acrs); FIXME
  
-/* Copy a 'clean' PSW mask to the user to avoid leaking

-   information about whether PER is currently on.  */
+/*
+ * Copy a 'clean' PSW mask to the user to avoid leaking
+ * information about whether PER is currently on.
+ */
  __put_user(env->psw.mask, &sregs->regs.psw.mask);
  __put_user(env->psw.addr, &sregs->regs.psw.addr);
+
  for (i = 0; i < 16; i++) {
  __put_user(env->regs[i], &sregs->regs.gprs[i]);
  }
  for (i = 0; i < 16; i++) {
  __put_user(env->aregs[i], &sregs->regs.acrs[i]);
  }
+
  /*
   * We have to store the fp registers to current->thread.fp_regs
   * to merge them with the emulated registers.
   */
-//save_fp_regs(¤t->thread.fp_regs); FIXME
  for (i = 0; i < 16; i++) {
  __put_user(*get_freg(env, i), &sregs->fpregs.fprs[i]);
  }



Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH v2 09/15] linux-user/s390x: Clean up single-use gotos in signal.c

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:34, Richard Henderson wrote:

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 29 -
  1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index eabfe4293f..64a9eab097 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -137,7 +137,8 @@ void setup_frame(int sig, struct target_sigaction *ka,
  frame_addr = get_sigframe(ka, env, sizeof(*frame));
  trace_user_setup_frame(env, frame_addr);
  if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
-goto give_sigsegv;
+force_sigsegv(sig);
+return;
  }
  
  __put_user(set->sig[0], &frame->sc.oldmask[0]);

@@ -174,10 +175,6 @@ void setup_frame(int sig, struct target_sigaction *ka,
  /* Place signal number on stack to allow backtrace from handler.  */
  __put_user(env->regs[2], &frame->signo);
  unlock_user_struct(frame, frame_addr, 1);
-return;
-
-give_sigsegv:
-force_sigsegv(sig);
  }
  
  void setup_rt_frame(int sig, struct target_sigaction *ka,

@@ -190,7 +187,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
  frame_addr = get_sigframe(ka, env, sizeof *frame);
  trace_user_setup_rt_frame(env, frame_addr);
  if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
-goto give_sigsegv;
+force_sigsegv(sig);
+return;
  }
  
  tswap_siginfo(&frame->info, info);

@@ -222,10 +220,6 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
  env->regs[2] = sig; //map_signal(sig);
  env->regs[3] = frame_addr + offsetof(typeof(*frame), info);
  env->regs[4] = frame_addr + offsetof(typeof(*frame), uc);
-return;
-
-give_sigsegv:
-force_sigsegv(sig);
  }
  
  static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)

@@ -259,7 +253,8 @@ long do_sigreturn(CPUS390XState *env)
  
  trace_user_do_sigreturn(env, frame_addr);

  if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1)) {
-goto badframe;
+force_sig(TARGET_SIGSEGV);
+return -TARGET_QEMU_ESIGRETURN;
  }
  __get_user(target_set.sig[0], &frame->sc.oldmask[0]);
  
@@ -270,10 +265,6 @@ long do_sigreturn(CPUS390XState *env)
  
  unlock_user_struct(frame, frame_addr, 0);

  return -TARGET_QEMU_ESIGRETURN;
-
-badframe:
-force_sig(TARGET_SIGSEGV);
-return -TARGET_QEMU_ESIGRETURN;
  }
  
  long do_rt_sigreturn(CPUS390XState *env)

@@ -284,7 +275,8 @@ long do_rt_sigreturn(CPUS390XState *env)
  
  trace_user_do_rt_sigreturn(env, frame_addr);

  if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1)) {
-goto badframe;
+force_sig(TARGET_SIGSEGV);
+return -TARGET_QEMU_ESIGRETURN;
  }
  target_to_host_sigset(&set, &frame->uc.tuc_sigmask);
  
@@ -296,9 +288,4 @@ long do_rt_sigreturn(CPUS390XState *env)
  
  unlock_user_struct(frame, frame_addr, 0);

  return -TARGET_QEMU_ESIGRETURN;
-
-badframe:
-unlock_user_struct(frame, frame_addr, 0);
-force_sig(TARGET_SIGSEGV);
-return -TARGET_QEMU_ESIGRETURN;
  }



Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH v2 10/15] linux-user/s390x: Set psw.mask properly for the signal handler

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:34, Richard Henderson wrote:

Note that PSW_ADDR_{64,32} are called PSW_MASK_{EA,BA}
in the kernel source.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 64a9eab097..17f617c655 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -162,6 +162,9 @@ void setup_frame(int sig, struct target_sigaction *ka,
  
  /* Set up registers for signal handler */

  env->regs[15] = frame_addr;
+/* Force default amode and default user address space control. */
+env->psw.mask = PSW_MASK_64 | PSW_MASK_32 | PSW_ASC_PRIMARY
+  | (env->psw.mask & ~PSW_MASK_ASC);
  env->psw.addr = ka->_sa_handler;
  
  env->regs[2] = sig; //map_signal(sig);

@@ -215,6 +218,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
  
  /* Set up registers for signal handler */

  env->regs[15] = frame_addr;
+/* Force default amode and default user address space control. */
+env->psw.mask = PSW_MASK_64 | PSW_MASK_32 | PSW_ASC_PRIMARY
+  | (env->psw.mask & ~PSW_MASK_ASC);
  env->psw.addr = ka->_sa_handler;
  
  env->regs[2] = sig; //map_signal(sig);




Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH v2 11/15] linux-user/s390x: Add stub sigframe argument for last_break

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:34, Richard Henderson wrote:

In order to properly present these arguments, we need to add
code to target/s390x to record LowCore parameters for user-only.

But in the meantime, at least zero the missing last_break
argument, and fixup the comment style in the vicinity.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 16 ++--
  1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 17f617c655..bc41b01c5d 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -167,13 +167,16 @@ void setup_frame(int sig, struct target_sigaction *ka,
| (env->psw.mask & ~PSW_MASK_ASC);
  env->psw.addr = ka->_sa_handler;
  
-env->regs[2] = sig; //map_signal(sig);

+env->regs[2] = sig;
  env->regs[3] = frame_addr += offsetof(typeof(*frame), sc);
  
-/* We forgot to include these in the sigcontext.

-   To avoid breaking binary compatibility, they are passed as args. */
-env->regs[4] = 0; // FIXME: no clue... current->thread.trap_no;
-env->regs[5] = 0; // FIXME: no clue... current->thread.prot_addr;
+/*
+ * We forgot to include these in the sigcontext.
+ * To avoid breaking binary compatibility, they are passed as args.
+ */
+env->regs[4] = 0; /* FIXME: regs->int_code & 127 */
+env->regs[5] = 0; /* FIXME: regs->int_parm_long */
+env->regs[6] = 0; /* FIXME: current->thread.last_break */
  
  /* Place signal number on stack to allow backtrace from handler.  */

  __put_user(env->regs[2], &frame->signo);
@@ -223,9 +226,10 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
| (env->psw.mask & ~PSW_MASK_ASC);
  env->psw.addr = ka->_sa_handler;
  
-env->regs[2] = sig; //map_signal(sig);

+env->regs[2] = sig;
  env->regs[3] = frame_addr + offsetof(typeof(*frame), info);
  env->regs[4] = frame_addr + offsetof(typeof(*frame), uc);
+env->regs[5] = 0; /* FIXME: current->thread.last_break */
  }
  
  static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)




Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH v2 12/15] linux-user/s390x: Fix frame_addr corruption in setup_frame

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:34, Richard Henderson wrote:

The original value of frame_addr is still required for
its use in the call to unlock_user_struct below.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index bc41b01c5d..81ba59b46a 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -168,7 +168,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
  env->psw.addr = ka->_sa_handler;
  
  env->regs[2] = sig;

-env->regs[3] = frame_addr += offsetof(typeof(*frame), sc);
+env->regs[3] = frame_addr + offsetof(typeof(*frame), sc);
  
  /*

   * We forgot to include these in the sigcontext.



Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH v2 13/15] linux-user/s390x: Add build asserts for sigset sizes

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:34, Richard Henderson wrote:

At point of usage, it's not immediately obvious that
we don't need a loop to copy these arrays.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 81ba59b46a..839a7ae4b3 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -141,6 +141,8 @@ void setup_frame(int sig, struct target_sigaction *ka,
  return;
  }
  
+/* Make sure that we're initializing all of oldmask. */

+QEMU_BUILD_BUG_ON(ARRAY_SIZE(frame->sc.oldmask) != 1);
  __put_user(set->sig[0], &frame->sc.oldmask[0]);
  
  save_sigregs(env, &frame->sregs);

@@ -266,6 +268,9 @@ long do_sigreturn(CPUS390XState *env)
  force_sig(TARGET_SIGSEGV);
  return -TARGET_QEMU_ESIGRETURN;
  }
+
+/* Make sure that we're initializing all of target_set. */
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(target_set.sig) != 1);
  __get_user(target_set.sig[0], &frame->sc.oldmask[0]);
  
  target_to_host_sigset_internal(&set, &target_set);




Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb




Re: [PATCH] virtio-net: failover: add missing remove_migration_state_change_notifier()

2021-04-29 Thread Jason Wang



在 2021/4/28 下午6:14, Michael S. Tsirkin 写道:

On Tue, Apr 27, 2021 at 03:02:34PM +0100, Dr. David Alan Gilbert wrote:

* Laurent Vivier (lviv...@redhat.com) wrote:

In the failover case configuration, virtio_net_device_realize() uses an
add_migration_state_change_notifier() to add a state notifier, but this
notifier is not removed by the unrealize function when the virtio-net
card is unplugged.

If the card is unplugged and a migration is started, the notifier is
called and as it is not valid anymore QEMU crashes.

This patch fixes the problem by adding the
remove_migration_state_change_notifier() in virtio_net_device_unrealize().

The problem can be reproduced with:

   $ qemu-system-x86_64 -enable-kvm -m 1g -M q35 \
 -device pcie-root-port,slot=4,id=root1 \
 -device pcie-root-port,slot=5,id=root2 \
 -device virtio-net-pci,id=net1,mac=52:54:00:6f:55:cc,failover=on,bus=root1 
\
 -monitor stdio disk.qcow2
   (qemu) device_del net1
   (qemu) migrate "exec:gzip -c > STATEFILE.gz"

   Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
   0x in ?? ()
   (gdb) bt
   #0  0x in  ()
   #1  0x55d726d7 in notifier_list_notify (...)
   at .../util/notify.c:39
   #2  0x55842c1a in migrate_fd_connect (...)
   at .../migration/migration.c:3975
   #3  0x55950f7d in migration_channel_connect (...)
   error@entry=0x0) at .../migration/channel.c:107
   #4  0x55910922 in exec_start_outgoing_migration (...)
   at .../migration/exec.c:42

Reported-by: Igor Mammedov 
Signed-off-by: Laurent Vivier 

Yep, I think that's OK.


Reviewed-by: Dr. David Alan Gilbert 

Reviewed-by: Michael S. Tsirkin 

net stuff so I expect Jason will merge this ...



Ok, I've queued this.

Thanks





---
  hw/net/virtio-net.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 66b9ff451185..914051feb75b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3373,6 +3373,7 @@ static void virtio_net_device_unrealize(DeviceState *dev)
  
  if (n->failover) {

  device_listener_unregister(&n->primary_listener);
+remove_migration_state_change_notifier(&n->migration_state);
  }
  
  max_queues = n->multiqueue ? n->max_queues : 1;

--
2.30.2


--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK







Re: [PATCH v2 14/15] linux-user/s390x: Clean up signal.c

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:34, Richard Henderson wrote:

Reorder the function bodies to correspond to the kernel source.

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 67 ---
  1 file changed, 41 insertions(+), 26 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 839a7ae4b3..9d470e4ca0 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -133,6 +133,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
  {
  sigframe *frame;
  abi_ulong frame_addr;
+abi_ulong restorer;
  
  frame_addr = get_sigframe(ka, env, sizeof(*frame));

  trace_user_setup_frame(env, frame_addr);
@@ -141,28 +142,39 @@ void setup_frame(int sig, struct target_sigaction *ka,
  return;
  }
  
+/* Set up backchain. */

+__put_user(env->regs[15], (abi_ulong *) frame);
+
+/* Create struct sigcontext on the signal stack. */
  /* Make sure that we're initializing all of oldmask. */
  QEMU_BUILD_BUG_ON(ARRAY_SIZE(frame->sc.oldmask) != 1);
  __put_user(set->sig[0], &frame->sc.oldmask[0]);
-
-save_sigregs(env, &frame->sregs);
-
  __put_user(frame_addr + offsetof(sigframe, sregs), &frame->sc.sregs);
  
-/* Set up to return from userspace.  If provided, use a stub

-   already in userspace.  */
+/* Create _sigregs on the signal stack */
+save_sigregs(env, &frame->sregs);
+
+/*
+ * ??? The kernel uses regs->gprs[2] here, which is not yet the signo.
+ * Moreover the comment talks about allowing backtrace, which is really
+ * done by the r15 copy above.
+ */
+__put_user(sig, &frame->signo);
+
+/*
+ * Set up to return from userspace.
+ * If provided, use a stub already in userspace.
+ */
  if (ka->sa_flags & TARGET_SA_RESTORER) {
-env->regs[14] = ka->sa_restorer;
+restorer = ka->sa_restorer;
  } else {
-env->regs[14] = frame_addr + offsetof(sigframe, retcode);
+restorer = frame_addr + offsetof(sigframe, retcode);
  __put_user(S390_SYSCALL_OPCODE | TARGET_NR_sigreturn,
 &frame->retcode);
  }
  
-/* Set up backchain. */

-__put_user(env->regs[15], (abi_ulong *) frame);
-
  /* Set up registers for signal handler */
+env->regs[14] = restorer;
  env->regs[15] = frame_addr;
  /* Force default amode and default user address space control. */
  env->psw.mask = PSW_MASK_64 | PSW_MASK_32 | PSW_ASC_PRIMARY
@@ -180,8 +192,6 @@ void setup_frame(int sig, struct target_sigaction *ka,
  env->regs[5] = 0; /* FIXME: regs->int_parm_long */
  env->regs[6] = 0; /* FIXME: current->thread.last_break */
  
-/* Place signal number on stack to allow backtrace from handler.  */

-__put_user(env->regs[2], &frame->signo);
  unlock_user_struct(frame, frame_addr, 1);
  }
  
@@ -191,6 +201,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,

  {
  rt_sigframe *frame;
  abi_ulong frame_addr;
+abi_ulong restorer;
  
  frame_addr = get_sigframe(ka, env, sizeof *frame);

  trace_user_setup_rt_frame(env, frame_addr);
@@ -199,29 +210,33 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
  return;
  }
  
-tswap_siginfo(&frame->info, info);

+/* Set up backchain. */
+__put_user(env->regs[15], (abi_ulong *) frame);
  
-/* Create the ucontext.  */

-__put_user(0, &frame->uc.tuc_flags);
-__put_user((abi_ulong)0, (abi_ulong *)&frame->uc.tuc_link);
-target_save_altstack(&frame->uc.tuc_stack, env);
-save_sigregs(env, &frame->uc.tuc_mcontext);
-tswap_sigset(&frame->uc.tuc_sigmask, set);
-
-/* Set up to return from userspace.  If provided, use a stub
-   already in userspace.  */
+/*
+ * Set up to return from userspace.
+ * If provided, use a stub already in userspace.
+ */
  if (ka->sa_flags & TARGET_SA_RESTORER) {
-env->regs[14] = ka->sa_restorer;
+restorer = ka->sa_restorer;
  } else {
-env->regs[14] = frame_addr + offsetof(typeof(*frame), retcode);
+restorer = frame_addr + offsetof(typeof(*frame), retcode);
  __put_user(S390_SYSCALL_OPCODE | TARGET_NR_rt_sigreturn,
 &frame->retcode);
  }
  
-/* Set up backchain. */

-__put_user(env->regs[15], (abi_ulong *) frame);
+/* Create siginfo on the signal stack. */
+tswap_siginfo(&frame->info, info);
+
+/* Create ucontext on the signal stack. */
+__put_user(0, &frame->uc.tuc_flags);
+__put_user(0, &frame->uc.tuc_link);
+target_save_altstack(&frame->uc.tuc_stack, env);
+save_sigregs(env, &frame->uc.tuc_mcontext);
+tswap_sigset(&frame->uc.tuc_sigmask, set);
  
  /* Set up registers for signal handler */

+env->regs[14] = restorer;
  env->regs[15] = frame_addr;
  /* Force default amode and default user address space control. */
  env->psw.mask = PSW_MASK_64 | PSW_MASK_32 |

Re: [RFC] AVR watchdog

2021-04-29 Thread Philippe Mathieu-Daudé
On 4/28/21 4:17 PM, Fred Konrad wrote:
> Hi,
> 
> I fall on a segfault while running the wdr instruction on AVR:
> 
> (gdb) bt
>  #0  0xadd0b23a in gdb_get_cpu_pid (cpu=0xaf5a4af0) at
>    ../gdbstub.c:718
>  #1  0xadd0b2dd in gdb_get_cpu_process (cpu=0xaf5a4af0) at
>    ../gdbstub.c:743
>  #2  0xadd0e477 in gdb_set_stop_cpu (cpu=0xaf5a4af0) at
>    ../gdbstub.c:2742
>  #3  0xadc99b96 in cpu_handle_guest_debug
> (cpu=0xaf5a4af0) at
>    ../softmmu/cpus.c:306
>  #4  0xadcc66ab in rr_cpu_thread_fn (arg=0xaf5a4af0) at
>    ../accel/tcg/tcg-accel-ops-rr.c:224
>  #5  0xadefaf12 in qemu_thread_start (args=0xaf5d9870) at
>    ../util/qemu-thread-posix.c:521
>  #6  0x7f692d940ea5 in start_thread () from /lib64/libpthread.so.0
>  #7  0x7f692d6699fd in clone () from /lib64/libc.so.6
> 
> Wondering if there are some plan/on-going work to implement this watchdog?
> 
> ---
> 
> Also meanwhile I though about a workaround like that:
> 
> diff --git a/target/avr/helper.c b/target/avr/helper.c
> index 35e1019594..7944ed21f4 100644
> --- a/target/avr/helper.c
> +++ b/target/avr/helper.c
> @@ -24,6 +24,7 @@
>  #include "exec/exec-all.h"
>  #include "exec/address-spaces.h"
>  #include "exec/helper-proto.h"
> +#include "sysemu/runstate.h"
> 
>  bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  {
> @@ -191,7 +192,7 @@ void helper_wdr(CPUAVRState *env)
>  CPUState *cs = env_cpu(env);
> 
>  /* WD is not implemented yet, placeholder */
> -    cs->exception_index = EXCP_DEBUG;
> +    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);

Eh, this is the opposite... This opcode kicks the watchdog,
it does not trigger it.

>  cpu_loop_exit(cs);
>  }
> 
> In the case the guest wants to reset the board through the watchdog,
> would that
> make sense to swap to that?

Why not simply log the opcode and keep going?

-- >8 --
diff --git a/target/avr/helper.c b/target/avr/helper.c
index 35e10195940..981c29da453 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -190,7 +190,3 @@ void helper_wdr(CPUAVRState *env)
 {
-CPUState *cs = env_cpu(env);
-
-/* WD is not implemented yet, placeholder */
-cs->exception_index = EXCP_DEBUG;
-cpu_loop_exit(cs);
+qemu_log_mask(LOG_UNIMP, "Watchdog Timer Reset\n");
 }
---

Regards,

Phil.



Re: [PATCH v2 15/15] linux-user/s390x: Handle vector regs in signal stack

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:34, Richard Henderson wrote:

Signed-off-by: Richard Henderson 
---
  linux-user/s390x/signal.c | 62 +--
  1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 9d470e4ca0..b537646e60 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -50,6 +50,12 @@ typedef struct {
  target_s390_fp_regs fpregs;
  } target_sigregs;
  
+typedef struct {

+uint64_t vxrs_low[16];
+uint64_t vxrs_high[16][2];
+uint8_t reserved[128];
+} target_sigregs_ext;
+
  typedef struct {
  abi_ulong oldmask[_SIGCONTEXT_NSIG_WORDS];
  abi_ulong sregs;
@@ -60,15 +66,20 @@ typedef struct {
  target_sigcontext sc;
  target_sigregs sregs;
  int signo;
+target_sigregs_ext sregs_ext;
  uint16_t retcode;
  } sigframe;
  
+#define TARGET_UC_VXRS 2

+
  struct target_ucontext {
  abi_ulong tuc_flags;
  abi_ulong tuc_link;
  target_stack_t tuc_stack;
  target_sigregs tuc_mcontext;
-target_sigset_t tuc_sigmask;   /* mask last for extensibility */
+target_sigset_t tuc_sigmask;
+uint8_t reserved[128 - sizeof(target_sigset_t)];


Guess I'd have used an unnamed union here

union {
target_sigset_t tuc_sigmask;
uint8_t reserved[128];
};


+target_sigregs_ext tuc_mcontext_ext;
  };
  
  typedef struct {

@@ -128,6 +139,24 @@ static void save_sigregs(CPUS390XState *env, 
target_sigregs *sregs)
  }
  }
  
+static void save_sigregs_ext(CPUS390XState *env, target_sigregs_ext *ext)

+{
+int i;
+
+/*
+ * if (MACHINE_HAS_VX) ...
+ * That said, we always allocate the stack storage and the
+ * space is always available in env.
+ */
+for (i = 0; i < 16; ++i) {
+   __put_user(env->vregs[i][1], &ext->vxrs_low[i]);
+}
+for (i = 0; i < 16; ++i) {
+   __put_user(env->vregs[i + 16][0], &ext->vxrs_high[i][0]);
+   __put_user(env->vregs[i + 16][1], &ext->vxrs_high[i][1]);
+}
+}
+
  void setup_frame(int sig, struct target_sigaction *ka,
   target_sigset_t *set, CPUS390XState *env)
  {
@@ -161,6 +190,9 @@ void setup_frame(int sig, struct target_sigaction *ka,
   */
  __put_user(sig, &frame->signo);
  
+/* Create sigregs_ext on the signal stack. */

+save_sigregs_ext(env, &frame->sregs_ext);
+
  /*
   * Set up to return from userspace.
   * If provided, use a stub already in userspace.
@@ -202,6 +234,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
  rt_sigframe *frame;
  abi_ulong frame_addr;
  abi_ulong restorer;
+abi_ulong uc_flags;
  
  frame_addr = get_sigframe(ka, env, sizeof *frame);

  trace_user_setup_rt_frame(env, frame_addr);
@@ -229,10 +262,15 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
  tswap_siginfo(&frame->info, info);
  
  /* Create ucontext on the signal stack. */

-__put_user(0, &frame->uc.tuc_flags);
+uc_flags = 0;
+if (s390_has_feat(S390_FEAT_VECTOR)) {
+uc_flags |= TARGET_UC_VXRS;
+}
+__put_user(uc_flags, &frame->uc.tuc_flags);
  __put_user(0, &frame->uc.tuc_link);
  target_save_altstack(&frame->uc.tuc_stack, env);
  save_sigregs(env, &frame->uc.tuc_mcontext);
+save_sigregs_ext(env, &frame->uc.tuc_mcontext_ext);
  tswap_sigset(&frame->uc.tuc_sigmask, set);
  
  /* Set up registers for signal handler */

@@ -271,6 +309,24 @@ static void restore_sigregs(CPUS390XState *env, 
target_sigregs *sc)
  }
  }
  
+static void restore_sigregs_ext(CPUS390XState *env, target_sigregs_ext *ext)

+{
+int i;
+
+/*
+ * if (MACHINE_HAS_VX) ...
+ * That said, we always allocate the stack storage and the
+ * space is always available in env.
+ */
+for (i = 0; i < 16; ++i) {
+   __get_user(env->vregs[i][1], &ext->vxrs_low[i]);
+}
+for (i = 0; i < 16; ++i) {
+   __get_user(env->vregs[i + 16][0], &ext->vxrs_high[i][0]);
+   __get_user(env->vregs[i + 16][1], &ext->vxrs_high[i][1]);
+}
+}
+
  long do_sigreturn(CPUS390XState *env)
  {
  sigframe *frame;
@@ -292,6 +348,7 @@ long do_sigreturn(CPUS390XState *env)
  set_sigmask(&set); /* ~_BLOCKABLE? */
  
  restore_sigregs(env, &frame->sregs);

+restore_sigregs_ext(env, &frame->sregs_ext);
  
  unlock_user_struct(frame, frame_addr, 0);

  return -TARGET_QEMU_ESIGRETURN;
@@ -313,6 +370,7 @@ long do_rt_sigreturn(CPUS390XState *env)
  set_sigmask(&set); /* ~_BLOCKABLE? */
  
  restore_sigregs(env, &frame->uc.tuc_mcontext);

+restore_sigregs_ext(env, &frame->uc.tuc_mcontext_ext);
  
  target_restore_altstack(&frame->uc.tuc_stack, env);
  



LGTM

Reviewed-by: David Hildenbrand 


--
Thanks,

David / dhildenb




Re: [PATCH] meson: change buildtype when debug_info=no

2021-04-29 Thread Joelle van Dyne
On Wed, Apr 28, 2021 at 10:07 PM Philippe Mathieu-Daudé
 wrote:
>
> On 4/28/21 9:55 PM, Joelle van Dyne wrote:
> > Meson defaults builds to 'debugoptimized' which adds '-g -O2'
> > to CFLAGS. If the user specifies '--disable-debug-info' we
> > should instead build with 'release' which does not emit any
> > debug info.
> >
> > Signed-off-by: Joelle van Dyne 
> > ---
> >  configure | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/configure b/configure
> > index 4f374b4889..5c3568cbc3 100755
> > --- a/configure
> > +++ b/configure
> > @@ -6398,6 +6398,7 @@ NINJA=$ninja $meson setup \
> >  --sysconfdir "$sysconfdir" \
> >  --localedir "$localedir" \
> >  --localstatedir "$local_statedir" \
> > +--buildtype $(if test "$debug_info" = yes; then echo 
> > "debugoptimized"; else echo "release"; fi) \
>
> NAck. You are changing the default (which is 'debug') to 'release'.

I thought 'debugoptimized' was the default? From my build logs,
there's always '-g -O2' which is why I needed to make this change. The
default for 'debug_info' is yes so this keeps it on 'debugoptimized'
and uses 'release' when explicitly disabling debug_info.

>
> This should be at least mentioned in the commit description, but
> I don't think this is what we want here. 'release' enables -O3,
> which is certainly not supported. The 'debug' profile is what we
> have been and are testing.
>
> I'd be OK if you had used "debugoptimized else debug".
>
> The mainstream project would rather use 'debug'/'debugoptimized', or
> 'minsize', which are already tested. We might consider allowing forks
> to use 'plain' profile eventually. But the 'release' type is an
> unsupported landmine IMHO.
>
> If you want to use something else, it should be an explicit argument
> to ./configure, then you are on your own IMO.

What do I need to avoid '-g'?

-j

>
> Regards,
>
> Phil.
>



Re: [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled

2021-04-29 Thread Philippe Mathieu-Daudé
On 4/28/21 8:00 PM, Philippe Mathieu-Daudé wrote:
> On 4/28/21 6:34 PM, Richard Henderson wrote:
>> On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
>>>   seccomp = not_found
>>> -if not get_option('seccomp').auto() or have_system or have_tools
>>> +if not get_option('seccomp').auto() or have_system or have_tools or
>>> not get_option('virtiofsd').auto()
>>>     seccomp = dependency('libseccomp', version: '>=2.3.0',
>>>  required: get_option('seccomp'),
>>>  method: 'pkg-config', kwargs: static_kwargs)
>>
>> This construct is wrong, both before and after, as I read it.
>>
>> not get_option(foo).auto() is true for both enabled and disabled.  If
>> disabled, why are we examining the dependency?  If auto, if we have all
>> of the dependencies we want to enable the feature -- if we don't probe
>> for the dependency, how can we enable it?
>>
>> This error seems to be offset by the OR have_* tests, for which the
>> logic also seems off.
>>
>> I think the test should have been
>>
>>   if (have_system or have_tools) and
> 
> Yes but virtiofsd is not a tool... It is a standalone binary.
> Maybe have_system is the culprit here:
> 
>   have_system = have_system or target.endswith('-softmmu')
> 
> We should somewhere add:
> 
>   have_system = have_system or something('virtiofsd')

So this hunk does fix the issue ...:

-- >8 --
--- a/meson.build
+++ b/meson.build
@@ -52,4 +52,5 @@
 endforeach
 have_tools = 'CONFIG_TOOLS' in config_host
+# virtiofsd depends on sysemu
+have_system = have_system or not get_option('virtiofsd').disabled()
 have_block = have_system or have_tools

---

> However I wonder if we aren't going to build many objects
> that are irrelevant for virtiofsd.

Based on top of
https://www.mail-archive.com/qemu-devel@nongnu.org/msg799069.html
to remove libsoftfloat, 216 objects are required to build virtiofsd:

[216/216] Linking target tools/virtiofsd/virtiofsd

This one-line fix seems good enough (to keep virtiofsd as a 'tool').




Re: [PATCH v2 00/15] linux-user/s390x: some signal fixes

2021-04-29 Thread David Hildenbrand

On 28.04.21 21:33, Richard Henderson wrote:

Version 2 splits lazy do-it-all patch.


Yap, that helped a lot :)


--
Thanks,

David / dhildenb




Re: [PATCH v1] vhost-vdpa: Set discarding of RAM broken when initializing the backend

2021-04-29 Thread David Hildenbrand

On 02.03.21 17:21, David Hildenbrand wrote:

Similar to VFIO, vDPA will go ahead an map+pin all guest memory. Memory
that used to be discarded will get re-populated and if we
discard+re-access memory after mapping+pinning, the pages mapped into the
vDPA IOMMU will go out of sync with the actual pages mapped into the user
space page tables.

Set discarding of RAM broken such that:
- virtio-mem and vhost-vdpa run mutually exclusive
- virtio-balloon is inhibited and no memory discards will get issued

In the future, we might be able to support coordinated discarding of RAM
as used by virtio-mem and as planned for VFIO.

Cc: Jason Wang 
Cc: Michael S. Tsirkin 
Cc: Cindy Lu 
Signed-off-by: David Hildenbrand 
---

Note: I was not actually able to reproduce/test as I fail to get the
vdpa_sim/vdpa_sim_net running on upstream Linux (whetever vdpa, vhost_vdpa,
vdpa_sim, vdpa_sim_net modules I probe, and in which order, no vdpa devices
appear under /sys/bus/vdpa/devices/ or /dev/).

---
  hw/virtio/vhost-vdpa.c | 13 +
  1 file changed, 13 insertions(+)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 01d2101d09..86058d4041 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -278,6 +278,17 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void 
*opaque)
  uint64_t features;
  assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA);
  trace_vhost_vdpa_init(dev, opaque);
+int ret;
+
+/*
+ * Similar to VFIO, we end up pinning all guest memory and have to
+ * disable discarding of RAM.
+ */
+ret = ram_block_discard_disable(true);
+if (ret) {
+error_report("Cannot set discarding of RAM broken");
+return ret;
+}
  
  v = opaque;

  v->dev = dev;
@@ -302,6 +313,8 @@ static int vhost_vdpa_cleanup(struct vhost_dev *dev)
  memory_listener_unregister(&v->listener);
  
  dev->opaque = NULL;

+ram_block_discard_disable(false);
+
  return 0;
  }
  



@MST, do you have this on your radar? thanks

--
Thanks,

David / dhildenb




Re: [PATCH] spapr: Modify ibm, get-config-addr-info2 to set DEVNUM in PE config address.

2021-04-29 Thread Alexey Kardashevskiy




On 4/28/21 22:33, Oliver O'Halloran wrote:

On Tue, Apr 27, 2021 at 9:56 PM Mahesh Salgaonkar  wrote:


With upstream kernel, especially after commit 98ba956f6a389
("powerpc/pseries/eeh: Rework device EEH PE determination") we see that KVM
guest isn't able to enable EEH option for PCI pass-through devices anymore.


How are you passing the devices through to the guest?


[root@atest-guest ~]# dmesg | grep EEH
[0.032337] EEH: pSeries platform initialized
[0.298207] EEH: No capable adapters found: recovery disabled.
[root@atest-guest ~]#

So far the linux kernel was assuming pe_config_addr equal to device's
config_addr and using it to enable EEH on the PE through ibm,set-eeh-option
RTAS call. Which wasn't the correct way as per PAPR. The linux kernel
commit 98ba956f6a389 fixed this flow. With that fixed, linux now gets the
PE config address first using the ibm,get-config-addr-info2 RTAS call, and
then uses the found address as argument to ibm,set-eeh-option RTAS call to
enable EEH on the PCI device.


That's not really correct. EEH being enabled or disabled is a per-PE
setting rather than a per-device setting. The fact there's not a 1-1
correspondence between devices and PEs is a large part of why the
get-config-addr-info2 RTAS call exists in the first place.
Unfortunately, the initial implementation of EEH support in linux
conflated the two because in the past there was typically a single
device within a PE. However, that assumption was never really correct
and it has long outlived its usefulness.


This works on PowerVM lpar but fails in qemu
KVM guests. This is because ibm,set-eeh-option RTAS call in qemu expects PE
config address bits 16:20 be populated with device number (DEVNUM).

The rtas call ibm,get-config-addr-info2 in qemu always returns the PE
config address in form of "00BB0001" (i.e.  <00>) where
"BB" represents the bus number of PE's primary bus and with device number
information always set to zero. However until commit 98ba956f6a389 this
return value wasn't used to enable EEH on the PCI device.

Now in qemu guest with recent kernel the ibm,set-eeh-option RTAS call fails
with -3 return value indicating that there is no PCI device exist for the
specified pe config address. The rtas_ibm_set_eeh_option call uses
pci_find_device() to get the PC device that matches specific bus and devfn
extracted from PE config address passed as argument. Since the DEVFN part
of PE config always contains zero, pci_find_device() fails to find the
specific PCI device and hence fails to enable the EEH capability.

hw/ppc/spapr_pci_vfio.c: spapr_phb_vfio_eeh_set_option()
case RTAS_EEH_ENABLE: {
 PCIHostState *phb;
 PCIDevice *pdev;

 /*
  * The EEH functionality is enabled on basis of PCI device,
  * instead of PE. We need check the validity of the PCI
  * device address.
  */
 phb = PCI_HOST_BRIDGE(sphb);
 pdev = pci_find_device(phb->bus,
(addr >> 16) & 0xFF, (addr >> 8) & 0xFF);
 if (!pdev || !object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
 return RTAS_OUT_PARAM_ERROR;
 }

This patch fixes this issue by populating DEVNUM field (bits 16:20) of PE
config address with device number.


I don't think this is a good idea and I'm fairly sure you're
introducing some subtle breakage here. As an example, say that on the
host side you have two devices on the same bus:

:00:00.0 - card 1
:00:01.0 - card 2

On PowerNV (i.e. the hypervisor) these would be placed into the same
hardware PE since they're on the same bus. Now, if I take both devices
and pass them through to the guest on the same PHB and bus (use
0005:ff) we'll have:

0005:ff:00.0 - card 1
0005:ff:01.0 - card 2

With this patch applied get-config-addr-info2 returns 00BBD001, so the
PE we get for each device will be:

card 1 - 00ff0001
card 2 - 00ff1001

Which implies the two are in different PEs. As a result, if the guest
requests a reset of card 1's PE then the guest will see an unexpected
reset of card 2 as well. From the hypervisor's point of view the two
are in the same PE so this is a legitimate thing to do, but due to
this patch the guest doesn't know that.



Agree. I guess we should only use vphbid:00:00.0 as a PE config address 
in QEMU as there is really just one per vphb which allows EEH.




As far as I can remember this is why you're supposed to pass each EEH
capable devices to the guest on a seperate spapr-phb (which matches
what PHYP does). Alexy can probably tell you more.



The primary reason was that the EEH subdriver in VFIO did a poor job 
synchronizing states from different PEs so recovery was either tricky or 
broken:


https://git.qemu.org/?p=qemu.git;a=commitdiff;h=3153119;hp=f1a6cf3ef734aab142d5f7ce52e219474ababf6b


--
Alexey



[PATCH v2 1/2] meson: Select 'have_system' when virtiofsd is enabled

2021-04-29 Thread Philippe Mathieu-Daudé
When not explicitly select a sysemu target and building virtiofsd,
the seccomp/cap-ng libraries are not resolved, leading to this error:

  $ configure --target-list=i386-linux-user --disable-tools --enable-virtiofsd
  tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd requires 
libcap-ng-devel and seccomp-devel

Fix by enabling sysemu (have_system) when virtiofsd is built.

Reported-by: Mahmoud Mandour 
Signed-off-by: Philippe Mathieu-Daudé 
---
 meson.build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meson.build b/meson.build
index c6f4b0cf5e8..f858935ad95 100644
--- a/meson.build
+++ b/meson.build
@@ -51,6 +51,8 @@
   have_system = have_system or target.endswith('-softmmu')
 endforeach
 have_tools = 'CONFIG_TOOLS' in config_host
+# virtiofsd depends on sysemu
+have_system = have_system or not get_option('virtiofsd').disabled()
 have_block = have_system or have_tools
 
 python = import('python').find_installation()
-- 
2.26.3




[PATCH v2 0/2] virtiofsd: Meson build fix

2021-04-29 Thread Philippe Mathieu-Daudé
Meson fix to allow building virtiofsd without sysemu/tools.

Since v1:
- reworked meson (Richard)
- added CI job (Dave)

Regards,

Phil.

Supersedes: <20210428144813.417170-1-phi...@redhat.com>

Philippe Mathieu-Daudé (2):
  meson: Select 'have_system' when virtiofsd is enabled
  gitlab-ci: Add a job to build virtiofsd standalone

 meson.build|  2 ++
 .gitlab-ci.yml | 13 +
 2 files changed, 15 insertions(+)

-- 
2.26.3





[PATCH v2 2/2] gitlab-ci: Add a job to build virtiofsd standalone

2021-04-29 Thread Philippe Mathieu-Daudé
Add a job which builds virtiofsd without any emulation or tool.

Signed-off-by: Philippe Mathieu-Daudé 
---
https://gitlab.com/philmd/qemu/-/jobs/1222007991
Duration: 7 minutes 48 seconds
---
 .gitlab-ci.yml | 13 +
 1 file changed, 13 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 52d65d6c04f..ba3c7ade6ca 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -800,6 +800,19 @@ build-libvhost-user:
 - meson
 - ninja
 
+build-virtiofsd-fedora:
+  <<: *native_build_job_definition
+  needs:
+job: amd64-fedora-container
+  variables:
+IMAGE: fedora
+CONFIGURE_ARGS: --enable-virtiofsd
+--disable-system --disable-user --disable-tools --disable-docs
+  artifacts:
+expire_in: 2 days
+paths:
+  - build/tools/virtiofsd/virtiofsd
+
 # No targets are built here, just tools, docs, and unit tests. This
 # also feeds into the eventual documentation deployment steps later
 build-tools-and-docs-debian:
-- 
2.26.3




[Bug 1833661] Re: Linux kernel oops on Malta board while accessing pflash

2021-04-29 Thread Thomas Huth
This is an automated cleanup. This bug report has been moved
to QEMU's new bug tracker on gitlab.com and thus gets marked
as 'expired' now. Please continue with the discussion here:

 https://gitlab.com/qemu-project/qemu/-/issues/51


** Changed in: qemu
   Status: Confirmed => Expired

** Bug watch added: gitlab.com/qemu-project/qemu/-/issues #51
   https://gitlab.com/qemu-project/qemu/-/issues/51

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

Title:
  Linux kernel oops on Malta board while accessing pflash

Status in QEMU:
  Expired

Bug description:
  commit 33d609990621dea6c7d056c86f707b8811320ac1

  While running tests/acceptance/linux_ssh_mips_malta.py, the big-endian
  tests fail:

physmap-flash.0: Found 1 x32 devices at 0x0 in 32-bit bank. Manufacturer ID 
0x00 Chip ID 0x00
Intel/Sharp Extended Query Table at 0x0031
Using buffer write method
Searching for RedBoot partition table in physmap-flash.0 at offset 
0x1003f
Creating 3 MTD partitions on "physmap-flash.0":
0x-0x0010 : "YAMON"
0x0010-0x003e : "User FS"
0x003e-0x0040 : "Board Config"
CPU 0 Unable to handle kernel paging request at virtual address 0014

  The 64-bit test fails with:

CPU 0 Unable to handle kernel paging request at virtual address
  0028

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



Re: [PATCH v4 10/12] qtest/qmp-cmd-test: Make test build-independent from accelerator

2021-04-29 Thread Philippe Mathieu-Daudé
On 4/29/21 7:43 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé  writes:
> 
>> Now than we can probe if the TCG accelerator is available
>> at runtime with a QMP command, do it once at the beginning
>> and only register the tests we can run.
>> We can then replace the #ifdef'ry by a runtime check.
>>
>> Suggested-by: Paolo Bonzini 
>> Signed-off-by: Philippe Mathieu-Daudé 
> 
> Please read the last remark first.  The other ones are detail; feel free
> to skip them until we're done with the last one.
> 
>> ---
>>  tests/qtest/qmp-cmd-test.c | 18 ++
>>  1 file changed, 14 insertions(+), 4 deletions(-)

>> +tcg_accel_available = qtest_has_accel("tcg");
>> +
> 
> When does tcg_accel_available differ from defined(CONFIG_TCG)?

qtest_has_accel("tcg") is a runtime check, while defined(CONFIG_TCG)
is build-time.

Having tests doing runtime checking allow to:

- have easier reviews, having less #ifdef'ry
- build them once for all targets
- build them once for all ./configure options
  (thinking about CI, the a single job could build the tests, then
  we run them using the QEMU binaries from other jobs)
- use the same binaries to test the built binary and the distribution
  installed one
- remove the dependencies between tests and binaries

> 
>>  g_test_init(&argc, &argv, NULL);
>>  
>>  qmp_schema_init(&schema);
> 




Re: [PATCH v2 2/2] gitlab-ci: Add a job to build virtiofsd standalone

2021-04-29 Thread Daniel P . Berrangé
On Thu, Apr 29, 2021 at 10:33:46AM +0200, Philippe Mathieu-Daudé wrote:
> Add a job which builds virtiofsd without any emulation or tool.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> https://gitlab.com/philmd/qemu/-/jobs/1222007991
> Duration: 7 minutes 48 seconds
> ---
>  .gitlab-ci.yml | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 52d65d6c04f..ba3c7ade6ca 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -800,6 +800,19 @@ build-libvhost-user:
>  - meson
>  - ninja
>  
> +build-virtiofsd-fedora:
> +  <<: *native_build_job_definition
> +  needs:
> +job: amd64-fedora-container
> +  variables:
> +IMAGE: fedora
> +CONFIGURE_ARGS: --enable-virtiofsd
> +--disable-system --disable-user --disable-tools --disable-docs
> +  artifacts:
> +expire_in: 2 days
> +paths:
> +  - build/tools/virtiofsd/virtiofsd

I'm not convinced that this job is justiable given our need to keep
the total CI pipeline size constrained. The precedent this sets is
that we need to test every configure args combination for each binary
we build. That is not scalable as a pattern. Neither this virtiofsd
arg scenario, nor others is going to be commonly used by downstream
consumers of QEMU, so the payoff from having this job is also small.


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




Re: [PATCH] make vfio and DAX cache work together

2021-04-29 Thread Dr. David Alan Gilbert
* Alex Williamson (alex.william...@redhat.com) wrote:
> On Wed, 28 Apr 2021 20:17:23 +0100
> "Dr. David Alan Gilbert"  wrote:
> 
> > * Dev Audsin (dev.devaq...@gmail.com) wrote:
> > > Thanks Dave for your explanation.
> > > Any suggestions on how to make VFIO not attempt to map into the
> > > unaccessible and unallocated RAM.  
> > 
> > I'm not sure;:
> > 
> > static bool vfio_listener_skipped_section(MemoryRegionSection *section)
> > {
> > return (!memory_region_is_ram(section->mr) &&
> > !memory_region_is_iommu(section->mr)) ||
> >section->offset_within_address_space & (1ULL << 63);
> > }
> > 
> > I'm declaring that region with memory_region_init_ram_ptr;  should I be?
> > it's not quite like RAM.
> > But then I *do* want a kvm slot for it, and I do want it to be accessed
> > by mapping rather htan calling IO functions; that makes me think mr->ram
> > has to be true.
> > But then do we need to add another flag to memory-regions; if we do,
> > what is it;
> >a) We don't want an 'is_virtio_fs' - it needs to be more generic
> >b) 'no_vfio' also feels wrong
> > 
> > Is perhaps 'not_lockable' the right thing to call it?
> 
> This reasoning just seems to lead back to "it doesn't work, therefore
> don't do it" rather than identifying the property of the region that
> makes it safe not to map it for device DMA (assuming that's actually
> the case). 

Yes, I'm struggling to get to what that generic form of that property
is, possibly because I've not got an example of another case to compare
it with.

> It's clearly "RAM" as far as QEMU is concerned given how
> it's created, but does it actually appear in the VM as generic physical
> RAM that the guest OS could program to the device as a DMA target?  If
> not, what property makes that so, create a flag for that.  Thanks,

The guest sees it as a PCI-bar; so it knows it's not 'generic physical
RAM' - but can a guest set other BARs (like frame buffers or pmem) as
DMA targets?  If so, how do I distinguish our bar?

Dave

> Alex
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK




Re: [PATCH v2 1/2] meson: Select 'have_system' when virtiofsd is enabled

2021-04-29 Thread Peter Maydell
On Thu, 29 Apr 2021 at 09:33, Philippe Mathieu-Daudé  wrote:
>
> When not explicitly select a sysemu target and building virtiofsd,
> the seccomp/cap-ng libraries are not resolved, leading to this error:
>
>   $ configure --target-list=i386-linux-user --disable-tools --enable-virtiofsd
>   tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd requires 
> libcap-ng-devel and seccomp-devel
>
> Fix by enabling sysemu (have_system) when virtiofsd is built.
>
> Reported-by: Mahmoud Mandour 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  meson.build | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index c6f4b0cf5e8..f858935ad95 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -51,6 +51,8 @@
>have_system = have_system or target.endswith('-softmmu')
>  endforeach
>  have_tools = 'CONFIG_TOOLS' in config_host
> +# virtiofsd depends on sysemu
> +have_system = have_system or not get_option('virtiofsd').disabled()

This looks odd. The natural assumption is that "have_system" ought to mean
"we are building a system emulator", not "we are building a system emulator
or virtiofsd".

thanks
-- PMM



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

2021-04-29 Thread Konstantin Kostiuk
ping

On Thu, Apr 22, 2021 at 10:43 AM Konstantin Kostiuk 
wrote:

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


Re: [RFC PATCH v2 2/4] hw/arm/virt: Parse -smp cluster parameter in virt_smp_parse

2021-04-29 Thread wangyanan (Y)



On 2021/4/29 15:16, Andrew Jones wrote:

On Thu, Apr 29, 2021 at 10:14:37AM +0800, wangyanan (Y) wrote:

On 2021/4/28 18:31, Andrew Jones wrote:

On Tue, Apr 13, 2021 at 04:31:45PM +0800, Yanan Wang wrote:

   } else if (sockets == 0) {
   threads = threads > 0 ? threads : 1;
-sockets = cpus / (cores * threads);
+sockets = cpus / (clusters * cores * threads);
   sockets = sockets > 0 ? sockets : 1;

If we initialize clusters to zero instead of one and add lines in
'cpus == 0 || cores == 0' and 'sockets == 0' like
'clusters = clusters > 0 ? clusters : 1' as needed, then I think we can
add

   } else if (clusters == 0) {
   threads = threads > 0 ? threads : 1;
   clusters = cpus / (sockets * cores * thread);
   clusters = clusters > 0 ? clusters : 1;
   }

here.

I have thought about this kind of format before, but there is a little bit
difference between these two ways. Let's chose the better and more
reasonable one of the two.

Way A currently in this patch:
If value of clusters is not explicitly specified in -smp command line, we
assume
that users don't want to support clusters, for compatibility we initialized
the
value to 1. So that with cmdline "-smp cpus=24, sockets=2, cores=6", we will
parse out the topology description like below:
cpus=24, sockets=2, clusters=1, cores=6, threads=2

Way B that you suggested for this patch:
Whether value of clusters is explicitly specified in -smp command line or
not,
we assume that clusters are supported and calculate the value. So that with
cmdline "-smp cpus=24, sockets=2, cores=6", we will parse out the topology
description like below:
cpus =24, sockets=2, clusters=2, cores=6, threads=1

But I think maybe we should not assume too much about what users think
through the -smp command line. We should just assume that all levels of
cpu topology are supported and calculate them, and users should be more
careful if they want to get the expected results with not so complete
cmdline.
If I'm right, then Way B should be better. :)


Hi Yanan,

We're already assuming the user wants to describe clusters to the guest
because we require at least one per socket. If we want the user to have a
choice between using clusters or not, then I guess we need to change the
logic in the PPTT and the cpu-map to only generate the cluster level when
the number of clusters is not zero. And then change this parser to not
require clusters at all.

Hi Drew,

I think this kind of change will introduce more complexity and actually 
is not necessary.
Not generating cluster level at all and generating cluster level (one 
per socket) are same
to kernel. Without cluster description provided, kernel will initialize 
all cores in the same

cluster which also means one cluster per socket.

So we should only ensure value of clusters per socket is one if we don't 
want to use clusters,
and don't need to care about whether or not to generate description in 
PPTT and cpu-map.

Is this right?

I'm not a big fan of these auto-calculated values either, but the
documentation says that it'll do that, and it's been done that way
forever, so I think we're stuck with it for the -smp option. Hmm, I was
just about to say that x86 computes all its values, but I see the most
recently added one, 'dies', is implemented the way you're proposing we
implement 'clusters', i.e. default to one and don't calculate it when it's
missing. I actually consider that either a documentation bug or an smp
parsing bug, though.

My propose originally came from implementation of x86.

Another possible option, for Arm, because only the cpus and maxcpus
parameters of -smp have ever worked, is to document, for Arm, that if even
one parameter other than cpus or maxcpus is provided, then all parameters
must be provided. We can still decide if clusters=0 is valid, but we'll
enforce that everything is explicit and that the product (with or without
clusters) matches maxcpus.

Requiring every parameter explicitly will be most stable but indeed strict.

Currently all the parsers use way B to calculate value of thread if it 
is not provided explicitly.
So users should ensure the -smp cmdline they provided can result in that 
parsed threads will

be 1 if they don't want to support multiple threads in one core.

Very similar to thread, users should also ensure the provided cmdline 
can result in that parsed
clusters will be 1 if they don't want to support multiple clusters in 
one socket.


So I'm wondering if we can just add some commit in the documentation to 
tell users that they
should ensure this if they don't want support it. And as for calculation 
of clusters, we follow

the logic of other parameters as you suggested in way B.

Thanks,
Yanan


Requiring every parameter might be stricter than necessary, though, I
think we're mostly concerned with cpus/maxcpus, sockets, and cores.
clusters can default to one or zero (whatever we choose and document),
threads can default to one, 

Re: [PATCH] spapr: Modify ibm, get-config-addr-info2 to set DEVNUM in PE config address.

2021-04-29 Thread Mahesh J Salgaonkar
On 2021-04-28 22:33:45 Wed, Oliver O'Halloran wrote:
> On Tue, Apr 27, 2021 at 9:56 PM Mahesh Salgaonkar  
> wrote:
> >
> > With upstream kernel, especially after commit 98ba956f6a389
> > ("powerpc/pseries/eeh: Rework device EEH PE determination") we see that KVM
> > guest isn't able to enable EEH option for PCI pass-through devices anymore.
> 
> How are you passing the devices through to the guest?

I am using libvirt with below xml section to add pass-through:


  
  

  
  


  
  

  
  


Looks like libvirt does not allow pass through device in slot zero, and
throws following error.

error: XML error: Invalid PCI address :01:00.0. slot must be >= 1
Failed. Try again? [y,n,i,f,?]:

> 
> > [root@atest-guest ~]# dmesg | grep EEH
> > [0.032337] EEH: pSeries platform initialized
> > [0.298207] EEH: No capable adapters found: recovery disabled.
> > [root@atest-guest ~]#
> >
> > So far the linux kernel was assuming pe_config_addr equal to device's
> > config_addr and using it to enable EEH on the PE through ibm,set-eeh-option
> > RTAS call. Which wasn't the correct way as per PAPR. The linux kernel
> > commit 98ba956f6a389 fixed this flow. With that fixed, linux now gets the
> > PE config address first using the ibm,get-config-addr-info2 RTAS call, and
> > then uses the found address as argument to ibm,set-eeh-option RTAS call to
> > enable EEH on the PCI device.
> 
> That's not really correct. EEH being enabled or disabled is a per-PE
> setting rather than a per-device setting. The fact there's not a 1-1
> correspondence between devices and PEs is a large part of why the
> get-config-addr-info2 RTAS call exists in the first place.
> Unfortunately, the initial implementation of EEH support in linux
> conflated the two because in the past there was typically a single
> device within a PE. However, that assumption was never really correct
> and it has long outlived its usefulness.
> 
> > This works on PowerVM lpar but fails in qemu
> > KVM guests. This is because ibm,set-eeh-option RTAS call in qemu expects PE
> > config address bits 16:20 be populated with device number (DEVNUM).
> >
> > The rtas call ibm,get-config-addr-info2 in qemu always returns the PE
> > config address in form of "00BB0001" (i.e.  <00>) where
> > "BB" represents the bus number of PE's primary bus and with device number
> > information always set to zero. However until commit 98ba956f6a389 this
> > return value wasn't used to enable EEH on the PCI device.
> >
> > Now in qemu guest with recent kernel the ibm,set-eeh-option RTAS call fails
> > with -3 return value indicating that there is no PCI device exist for the
> > specified pe config address. The rtas_ibm_set_eeh_option call uses
> > pci_find_device() to get the PC device that matches specific bus and devfn
> > extracted from PE config address passed as argument. Since the DEVFN part
> > of PE config always contains zero, pci_find_device() fails to find the
> > specific PCI device and hence fails to enable the EEH capability.
> >
> > hw/ppc/spapr_pci_vfio.c: spapr_phb_vfio_eeh_set_option()
> >case RTAS_EEH_ENABLE: {
> > PCIHostState *phb;
> > PCIDevice *pdev;
> >
> > /*
> >  * The EEH functionality is enabled on basis of PCI device,
> >  * instead of PE. We need check the validity of the PCI
> >  * device address.
> >  */
> > phb = PCI_HOST_BRIDGE(sphb);
> > pdev = pci_find_device(phb->bus,
> >(addr >> 16) & 0xFF, (addr >> 8) & 0xFF);
> > if (!pdev || !object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
> > return RTAS_OUT_PARAM_ERROR;
> > }
> >
> > This patch fixes this issue by populating DEVNUM field (bits 16:20) of PE
> > config address with device number.
> 
> I don't think this is a good idea and I'm fairly sure you're
> introducing some subtle breakage here. As an example, say that on the
> host side you have two devices on the same bus:
> 
> :00:00.0 - card 1
> :00:01.0 - card 2
> 
> On PowerNV (i.e. the hypervisor) these would be placed into the same
> hardware PE since they're on the same bus. Now, if I take both devices
> and pass them through to the guest on the same PHB and bus (use
> 0005:ff) we'll have:
> 
> 0005:ff:00.0 - card 1
> 0005:ff:01.0 - card 2

It looks like libvirt does not support pass through device in slot zero.
Hence these appears as below in guest:

 0005:ff:01.0 - card 1
 0005:ff:02.0 - card 2

And with get-config-addr-info2 returning 00BB0001, ibm,set-eeh-option
RTAS call tries to check if device is present on the bus of spapr_phb at
bus->devices[devfn] where devfn is 0. And since qemu does not support
pass through device on slot zero bus->devices[0] is always NULL. And
hence it fails to enable EEH.

> 
> With this patch applied get-config-addr-info2 returns 00BBD001, so the
> PE we get for each device will be:
> 
> card 1 - 00ff0001
> ca

Re: [PATCH 2/5] vhost-user-blk: Use Error more consistently

2021-04-29 Thread Kevin Wolf
Am 28.04.2021 um 20:08 hat Raphael Norwitz geschrieben:
> Code looks ok - question about the commit message.
> 
> Acked-by: Raphael Norwitz 
> 
> On Thu, Apr 22, 2021 at 07:02:18PM +0200, Kevin Wolf wrote:
> > Now that vhost_user_blk_connect() is not called from an event handler
> > any more, but directly from vhost_user_blk_device_realize(), we don't
> > have to resort to error_report() any more, but can use Error again.
> 
> vhost_user_blk_connect() is still called by vhost_user_blk_event() which
> is registered as an event handler. I don't understand your point around
> us having had to use error_report() before, but not anymore. Can you
> clarify?

What I meant is that vhost_user_blk_event() can't really make use of an
Error object other than passing it to error_report_err(), which has the
same result as directly using error_report().

With the new code where vhost_user_blk_device_realize() calls the
function directly, we can actually return the error to its caller so
that it ends up in the QMP result or the command line error message.

The result is still not great because vhost_user_blk_connect() doesn't
know the original error message. We'd have to add Error to
vhost_dev_init() and the functions that it calls to get the real error
messages, but at least it's a first step in the right direction.

We already figured that we need to change error reporting so we can know
whether we should retry, so I guess this can be solved at the same time.

Kevin




Re: [PATCH 4/5] virtio: Fail if iommu_platform is requested, but unsupported

2021-04-29 Thread Kevin Wolf
Am 28.04.2021 um 21:24 hat Raphael Norwitz geschrieben:
> On Thu, Apr 22, 2021 at 07:02:20PM +0200, Kevin Wolf wrote:
> > Commit 2943b53f6 (' virtio: force VIRTIO_F_IOMMU_PLATFORM') made sure
> > that vhost can't just reject VIRTIO_F_IOMMU_PLATFORM when it was
> > requested. However, just adding it back to the negotiated flags isn't
> > right either because it promises support to the guest that the device
> > actually doesn't support. One example of a vhost-user device that
> > doesn't have support for the flag is the vhost-user-blk export of QEMU.
> > 
> > Instead of successfully creating a device that doesn't work, just fail
> > to plug the device when it doesn't support the feature, but it was
> > requested. This results in much clearer error messages.
> > 
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935031
> > Signed-off-by: Kevin Wolf 
> > ---
> >  hw/virtio/virtio-bus.c | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> > index d6332d45c3..859978d248 100644
> > --- a/hw/virtio/virtio-bus.c
> > +++ b/hw/virtio/virtio-bus.c
> > @@ -69,6 +69,11 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error 
> > **errp)
> >  return;
> >  }
> >
> 
> Can you explain this check a little more?
> 
> Above we have:
> bool has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);

If I underdstand the code correctly, at this point this is still the
unchanged value of the iommu_platform=on|off qdev property as given by
the user.

> and then we get the host features from the bckend:
> vdev->host_features = vdc->get_features(vdev, vdev->host_features

Yes, and now a flag is only set if the user had requested it and the
backend also supports it.

> So as is this is catching the case where vdev->host_features had
> VIRTIO_F_IOMMU_PLATFORM set before (by default?), but doesn't now that
> the features have been retrieved? 
> 
> Why not just:
> if (!virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) {

We don't want to fail if the user hadn't even requested the feature, but
just if it was requested, but could not be provided.

> > +if (has_iommu && !virtio_host_has_feature(vdev, 
> > VIRTIO_F_IOMMU_PLATFORM)) {
> > +error_setg(errp, "iommu_platform=true is not supported by the 
> > device");
> > +return;
> > +}
> > +
> >  if (klass->device_plugged != NULL) {
> >  klass->device_plugged(qbus->parent, &local_err);
> >  }

Kevin




Re: [PATCH v2 2/2] gitlab-ci: Add a job to build virtiofsd standalone

2021-04-29 Thread Philippe Mathieu-Daudé
On 4/29/21 10:43 AM, Daniel P. Berrangé wrote:
> On Thu, Apr 29, 2021 at 10:33:46AM +0200, Philippe Mathieu-Daudé wrote:
>> Add a job which builds virtiofsd without any emulation or tool.
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>> https://gitlab.com/philmd/qemu/-/jobs/1222007991
>> Duration: 7 minutes 48 seconds
>> ---
>>  .gitlab-ci.yml | 13 +
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index 52d65d6c04f..ba3c7ade6ca 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -800,6 +800,19 @@ build-libvhost-user:
>>  - meson
>>  - ninja
>>  
>> +build-virtiofsd-fedora:
>> +  <<: *native_build_job_definition
>> +  needs:
>> +job: amd64-fedora-container
>> +  variables:
>> +IMAGE: fedora
>> +CONFIGURE_ARGS: --enable-virtiofsd
>> +--disable-system --disable-user --disable-tools --disable-docs
>> +  artifacts:
>> +expire_in: 2 days
>> +paths:
>> +  - build/tools/virtiofsd/virtiofsd
> 
> I'm not convinced that this job is justiable given our need to keep
> the total CI pipeline size constrained. The precedent this sets is
> that we need to test every configure args combination for each binary
> we build. That is not scalable as a pattern. Neither this virtiofsd
> arg scenario, nor others is going to be commonly used by downstream
> consumers of QEMU, so the payoff from having this job is also small.

I'm not sure "our current pipelines is too busy because we don't have
a clear idea what is tested and what is duplicated" justifies no more
tests can be added, but it is a effective way to have the current set
cleaned.

Anyhow, if mainstream isn't interested by this configuration, it could
be added to the virtio-fs/qemu fork. Alternatively mainstream with:

  only:
variables:
  - $CI_PROJECT_NAMESPACE == 'virtio-fs'




Re: [PATCH] meson: change buildtype when debug_info=no

2021-04-29 Thread Philippe Mathieu-Daudé
On 4/29/21 9:33 AM, Joelle van Dyne wrote:
> On Wed, Apr 28, 2021 at 10:07 PM Philippe Mathieu-Daudé
>  wrote:
>>
>> On 4/28/21 9:55 PM, Joelle van Dyne wrote:
>>> Meson defaults builds to 'debugoptimized' which adds '-g -O2'
>>> to CFLAGS. If the user specifies '--disable-debug-info' we
>>> should instead build with 'release' which does not emit any
>>> debug info.
>>>
>>> Signed-off-by: Joelle van Dyne 
>>> ---
>>>  configure | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/configure b/configure
>>> index 4f374b4889..5c3568cbc3 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -6398,6 +6398,7 @@ NINJA=$ninja $meson setup \
>>>  --sysconfdir "$sysconfdir" \
>>>  --localedir "$localedir" \
>>>  --localstatedir "$local_statedir" \
>>> +--buildtype $(if test "$debug_info" = yes; then echo 
>>> "debugoptimized"; else echo "release"; fi) \
>>
>> NAck. You are changing the default (which is 'debug') to 'release'.
> 
> I thought 'debugoptimized' was the default? From my build logs,
> there's always '-g -O2' which is why I needed to make this change. The
> default for 'debug_info' is yes so this keeps it on 'debugoptimized'
> and uses 'release' when explicitly disabling debug_info.

Again, I'm not concerned between 'debugoptimized' VS 'debug',
I'm worried to use 'release', because of the b_ndebug=if-release
option which disable assertions (unsupported QEMU build mode).

Also, 'release' sets optimization=3 which isn't supported neither.

Per https://mesonbuild.com/Builtin-options.html#build-type-options

  All other combinations of debug and optimization set buildtype
  to 'custom'.

So maybe this is what you want, with debug=false and optimization=2?

> 
>>
>> This should be at least mentioned in the commit description, but
>> I don't think this is what we want here. 'release' enables -O3,
>> which is certainly not supported. The 'debug' profile is what we
>> have been and are testing.
>>
>> I'd be OK if you had used "debugoptimized else debug".
>>
>> The mainstream project would rather use 'debug'/'debugoptimized', or
>> 'minsize', which are already tested. We might consider allowing forks
>> to use 'plain' profile eventually. But the 'release' type is an
>> unsupported landmine IMHO.
>>
>> If you want to use something else, it should be an explicit argument
>> to ./configure, then you are on your own IMO.
> 
> What do I need to avoid '-g'?

Why don't you simply use ./configure --extra-cflags='-g0 -O2'?

Regards,

Phil.




[Bug 1916344] Re: User mode networking not working properly on QEMU on Mac OS X host

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  User mode networking not working properly on QEMU on Mac OS X host

Status in QEMU:
  New

Bug description:
  Steps to reproduce:

  1. Install QEMU using homebrew on Mac OS X (I tried on Catalina and Big Sur)
  2. Spin up a guest VM (say) Cent OS 8 using user mode networking.
  3. Install podman inside the guest
  4. Run podman pull alpine

  The result is:

  [root@localhost ~]# podman pull alpine
  Resolved "alpine" as an alias 
(/etc/containers/registries.conf.d/shortnames.conf)
  Trying to pull docker.io/library/alpine:latest...
  Getting image source signatures
  Copying blob ba3557a56b15 [==] 2.7MiB / 
2.7MiB
    unexpected EOF
  Error: Error writing blob: error storing blob to file 
"/var/tmp/storage851171596/1": error happened during read: unexpected EOF

  This is happening because QEMU is telling the guest that the TCP
  connection is closed even before reading all the data from the host
  socket and forwarding it to the guest.

  This issue doesn't happen on a Linux host. So, that tells me that this
  has something to do with QEMU installation on Mac OS X.

  This could be a slirp related issue. So, QEMU/slirp may need to work
  together on fixing this. Here's the link to the libslirp issue:

  https://gitlab.freedesktop.org/slirp/libslirp/-/issues/35

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



Let's remove some deprecated stuff

2021-04-29 Thread Markus Armbruster
If you're cc'ed, you added a section to docs/system/deprecated.rst that
is old enough to permit removal.  This is *not* a demand to remove, it's
a polite request to consider whether the time for removal has come.
Extra points for telling us in a reply.  "We should remove, but I can't
do it myself right now" is a valid answer.  Let's review the file:

System emulator command line arguments
--

Kővágó, Zoltán:

``QEMU_AUDIO_`` environment variables and ``-audio-help`` (since 4.0)
'

The ``-audiodev`` argument is now the preferred way to specify audio
backend settings instead of environment variables.  To ease migration to
the new format, the ``-audiodev-help`` option can be used to convert
the current values of the environment variables to ``-audiodev`` options.

Kővágó, Zoltán:

Creating sound card devices and vnc without ``audiodev=`` property (since 
4.2)

''

When not using the deprecated legacy audio config, each sound card
should specify an ``audiodev=`` property.  Additionally, when using
vnc, you should specify an ``audiodev=`` property if you plan to
transmit audio through the VNC protocol.

Gerd Hoffmann:

Creating sound card devices using ``-soundhw`` (since 5.1)
''

Sound card devices should be created using ``-device`` instead.  The
names are the same for most devices.  The exceptions are ``hda`` which
needs two devices (``-device intel-hda -device hda-duplex``) and
``pcspk`` which can be activated using ``-machine
pcspk-audiodev=``.

[...]

Alistair Francis:

RISC-V ``-bios`` (since 5.1)


QEMU 4.1 introduced support for the -bios option in QEMU for RISC-V for the
RISC-V virt machine and sifive_u machine. QEMU 4.1 had no changes to the
default behaviour to avoid breakages.

QEMU 5.1 changes the default behaviour from ``-bios none`` to ``-bios 
default``.

QEMU 5.1 has three options:
 1. ``-bios default`` - This is the current default behavior if no -bios 
option
  is included. This option will load the default OpenSBI firmware 
automatically.
  The firmware is included with the QEMU release and no user 
interaction is
  required. All a user needs to do is specify the kernel they want to 
boot
  with the -kernel option
 2. ``-bios none`` - QEMU will not automatically load any firmware. It is up
  to the user to load all the images they need.
 3. ``-bios `` - Tells QEMU to load the specified file as the 
firmwrae.

[...]

QEMU Machine Protocol (QMP) commands


Myself, but I only documented it; it's actually Kevin Wolf:

``blockdev-open-tray``, ``blockdev-close-tray`` argument ``device`` (since 
2.8.0)

'

Use argument ``id`` instead.

``eject`` argument ``device`` (since 2.8.0)
'''

Use argument ``id`` instead.

``blockdev-change-medium`` argument ``device`` (since 2.8.0)


Use argument ``id`` instead.

``block_set_io_throttle`` argument ``device`` (since 2.8.0)
'''

Use argument ``id`` instead.

Myself:

``blockdev-add`` empty string argument ``backing`` (since 2.10.0)
'

Use argument value ``null`` instead.

Myself, but I only documented it; it's actually Kevin Wolf:

``block-commit`` arguments ``base`` and ``top`` (since 3.1.0)
'

Use arguments ``base-node`` and ``top-node`` instead.

Kevin Wolf:

``nbd-server-add`` and ``nbd-server-remove`` (since 5.2)


Use the more generic commands ``block-export-add`` and ``block-export-del``
instead.  As part of this deprecation, where ``nbd-server-add`` used a
single ``bitmap``, the new ``block-export-add`` uses a list of ``bitmaps``.


[...]

System emulator CPUS


Thomas Huth:

``moxie`` CPU (since 5.2.0)
'''

The ``moxie`` guest CPU support is deprecated and will be removed in
a future version of QEMU. It's unclear whether anybody is still using
CPU emulation in QEMU, and there are no test images available to make
sure that the code is still working.

``lm32`` CPUs (since 5.2.0)
'''

The ``lm32`` guest CPU support is deprecated and will be removed in
a future version of QEMU. 

[Bug 1890395] Re: qmp/hmp: crash if client closes socket too early

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  qmp/hmp: crash if client closes socket too early

Status in QEMU:
  New

Bug description:
  Qemu crashes on qmp/hmp command if client closes connection before
  reading the whole response from the socket.

  Reproducer:

  1. Start arbitrary vm via qemu
  2. Send e.g. hmp command 'info mem'
  3. Abort before whole response came back

  
  Stack Trace:

  Stack trace of thread 6493:
  #0  0x559902fd2d30 object_get_class (qemu-system-x86_64)
  #1  0x559903071020 qio_channel_create_watch (qemu-system-x86_64)
  #2  0x55990305f437 qemu_chr_fe_add_watch (qemu-system-x86_64)
  #3  0x559902f7340d monitor_flush_locked (qemu-system-x86_64)
  #4  0x559902f7360e monitor_flush_locked (qemu-system-x86_64)
  #5  0x559902f74342 qmp_send_response (qemu-system-x86_64)
  #6  0x559902f74409 monitor_qmp_respond (qemu-system-x86_64)
  #7  0x559902f74bc0 monitor_qmp_bh_dispatcher (qemu-system-x86_64)
  #8  0x5599030c37be aio_bh_call (qemu-system-x86_64)
  #9  0x5599030c6dd0 aio_dispatch (qemu-system-x86_64)
  #10 0x5599030c369e aio_ctx_dispatch (qemu-system-x86_64)
  #11 0x7f5b6d37f417 g_main_context_dispatch (libglib-2.0.so.0)
  #12 0x5599030c5e0a glib_pollfds_poll (qemu-system-x86_64)
  #13 0x559902dd75df main_loop (qemu-system-x86_64)
  #14 0x559902c59f49 main (qemu-system-x86_64)
  #15 0x7f5b6bfeab97 __libc_start_main (libc.so.6)
  #16 0x559902c5d38a _start (qemu-system-x86_64)

  #0  0x559902fd2d30 in object_get_class (obj=obj@entry=0x0) at 
./qom/object.c:909
  #1  0x559903071020 in qio_channel_create_watch (ioc=0x0, 
condition=(G_IO_OUT | G_IO_HUP)) at ./io/channel.c:281
  klass = 
  __func__ = "qio_channel_create_watch"
  ret = 
  #2  0x55990305f437 in qemu_chr_fe_add_watch (be=be@entry=0x559905a7f460, 
cond=cond@entry=(G_IO_OUT | G_IO_HUP), func=func@entry=0x559902f734b0 
, user_data=user_data@entry=0x559905a7f460) at 
./chardev/char-fe.c:367
  s = 0x5599055782c0
  src = 
  tag = 
  __func__ = "qemu_chr_fe_add_watch"
  #3  0x559902f7340d in monitor_flush_locked (mon=mon@entry=0x559905a7f460) 
at ./monitor/monitor.c:140
  rc = 219264
  len = 3865832
  buf = 0x7f5afc00e480 "{\"return\": 
\"9eb48000-9eb480099000 ", '0' , "99000 
-rw\\r\\n9eb480099000-9eb48009b000 ", '0' , "2000 
-r-\\r\\n9eb48009b000-9eb48680 06765000 
-rw\\r\\n9eb4868000"...
  #4  0x559902f7360e in monitor_flush_locked (mon=0x559905a7f460) at 
./monitor/monitor.c:160
  i = 3865830
  c = 
  #5  0x559902f7360e in monitor_puts (mon=mon@entry=0x559905a7f460, 
str=0x7f5aa1eda010 "{\"return\": \"9eb48000-9eb480099000 ", '0' 
, "99000 -rw\\r\\n9eb480099000-9eb48009b000 ", '0' 
, "2000 -r-\\r\\n9eb48009b000-9eb48680 
06765000 -rw\\r\\n9eb4868000"...) at ./monitor/monitor.c:167
  i = 3865830
  c = 
  #6  0x559902f74342 in qmp_send_response (mon=0x559905a7f460, 
rsp=) at ./monitor/qmp.c:119
  data = 
  json = 0x559906c88380
  #7  0x559902f74409 in monitor_qmp_respond (rsp=0x559905bbf740, 
mon=0x559905a7f460) at ./monitor/qmp.c:132
  old_mon = 
  rsp = 0x559905bbf740
  error = 
  #8  0x559902f74409 in monitor_qmp_dispatch (mon=0x559905a7f460, 
req=) at ./monitor/qmp.c:161
  old_mon = 
  rsp = 0x559905bbf740
  error = 
  #9  0x559902f74bc0 in monitor_qmp_bh_dispatcher (data=) at 
./monitor/qmp.c:234
  id = 
  rsp = 
  need_resume = true
  mon = 0x559905a7f460
  __PRETTY_FUNCTION__ = "monitor_qmp_bh_dispatcher"
  #10 0x5599030c37be in aio_bh_call (bh=0x559905571b40) at ./util/async.c:89
  bh = 0x559905571b40
  bhp = 
  next = 0x5599055718f0
  ret = 1
  deleted = false
  #11 0x5599030c37be in aio_bh_poll (ctx=ctx@entry=0x5599055706f0) at 
./util/async.c:117
  bh = 0x559905571b40
  bhp = 
  next = 0x5599055718f0
  ret = 1
  deleted = false
  #12 0x5599030c6dd0 in aio_dispatch (ctx=0x5599055706f0) at 
./util/aio-posix.c:459
  #13 0x5599030c369e in aio_ctx_dispatch (source=, 
callback=, user_data=) at ./util/async.c:260
  ctx = 
  #14 0x7f5b6d37f417 in g_main_context_dispatch () at 
/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
  #15 0x5599030c5e0a in glib_pollfds_poll () at ./util/main-loop.c:219
  context = 0x559905652420
  pfds = 
  context = 0x559905652420
  ret = 1
  mlpoll = {state = 0, timeout = 4294967295, pollfds = 0x559905651800}
  #16 0x5599030c5e0a in os_host_main_loop_wait (timeout=14451267) at 
./util/main-loop.c:242
 

[Bug 1833053] Re: qemu guest crashes on spice client USB redirected device removal

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  qemu guest crashes on spice client USB redirected device removal

Status in QEMU:
  New

Bug description:
  Hello,

  I am experiencing guest crashes, which cannot be reproduced at all
  times, but are pretty frequent (4 out of 5 tries it would crash). The
  guest crashes when a previously attached USB redirected device through
  SPICE has been removed by the client.

  Steps to reproduce:
  1.) Start windows 10 guest with display driver Spice
  2.) Connect to the console with remote-viewer spice://IP:PORT or via 
virt-viewer (tunnelled through SSH)
  3.) Attach a client USB device, for example storage device, iPhone or Android 
phone
  4.) Observe the guest OS detects it and sets it up
  5.) Go back to 'USB device selection' and untick the USB device
  6.) Observe the guest VM crashed and the below assertion was printed in the 
qemu log for this virtual machine:

  qemu-system-x86_64: 
/var/tmp/portage/app-emulation/qemu-4.0.0-r3/work/qemu-4.0.0/hw/usb/core.c:720: 
usb_ep_get: Assertion `dev != NULL' failed.
  2019-06-17 09:25:09.160+: shutting down, reason=crashed

  
  Versions of related packages on the host:
  app-emulation/qemu-4.0.0-r3
  app-emulation/spice-0.14.0-r2:0
  app-emulation/spice-protocol-0.12.14:0
  net-misc/spice-gtk-0.35:0
  Kernel: 5.1.7-gentoo on Intel x86_64 CPU

  Version of the spice-tools on the guest:
  virtio-win 0.1-126
  QXL 0.1-21
  mingw-vdagent-win 0.8.0

  QEMU command line (generated by libvirt):

  /usr/bin/qemu-system-x86_64 -name guest=W10VM,debug-threads=on -S
  -object
  secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-41-W10VM
  /master-key.aes -machine pc-i440fx-2.12,accel=kvm,usb=off,vmport=off
  ,dump-guest-core=off -cpu
  qemu64,hv_time,hv_relaxed,hv_vapic,hv_spinlocks=0x1fff,hv_synic,hv_stimer
  -m 4500 -realtime mlock=off -smp
  2,maxcpus=4,sockets=4,cores=1,threads=1 -uuid b39afae2-5085-4659-891c-
  b3c65e65af2e -no-user-config -nodefaults -chardev
  socket,id=charmonitor,fd=26,server,nowait -mon
  chardev=charmonitor,id=monitor,mode=control -rtc
  base=localtime,driftfix=slew -no-hpet -global kvm-
  pit.lost_tick_policy=delay -no-shutdown -global PIIX4_PM.disable_s3=1
  -global PIIX4_PM.disable_s4=1 -boot menu=off,strict=on -device ich9
  -usb-ehci1,id=usb,bus=pci.0,addr=0x5.0x7 -device ich9-usb-
  uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5
  -device ich9-usb-
  uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x5.0x1 -device ich9
  -usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2 -device
  virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x8 -device virtio-serial-
  pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive
  file=/libvirt/images/W10VM.qcow2,format=qcow2,if=none,id=drive-
  scsi0-0-0-1,cache=unsafe,discard=unmap,detect-zeroes=unmap -device
  scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=1,device_id=drive-
  scsi0-0-0-1,drive=drive-scsi0-0-0-1,id=scsi0-0-0-1,bootindex=1,write-
  cache=on -netdev tap,fd=28,id=hostnet0,vhost=on,vhostfd=29 -device
  virtio-net-
  pci,netdev=hostnet0,id=net0,mac=52:54:00:44:f6:21,bus=pci.0,addr=0x3
  -chardev spicevmc,id=charchannel0,name=vdagent -device
  virtserialport,bus=virtio-
  serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
  -chardev socket,id=charchannel1,fd=30,server,nowait -device
  virtserialport,bus=virtio-
  serial0.0,nr=3,chardev=charchannel1,id=channel1,name=org.qemu.guest_agent.0
  -chardev spiceport,id=charchannel2,name=org.spice-space.webdav.0
  -device virtserialport,bus=virtio-
  serial0.0,nr=2,chardev=charchannel2,id=channel2,name=org.spice-
  space.webdav.0 -spice port=5901,addr=0.0.0.0,seamless-migration=on
  -device qxl-
  
vga,id=video0,ram_size=134217728,vram_size=134217728,vram64_size_mb=0,vgamem_mb=64,max_outputs=1,bus=pci.0,addr=0x2
  -device intel-hda,id=sound0,bus=pci.0,addr=0x4 -device hda-
  duplex,id=sound0-codec0,bus=sound0.0,cad=0 -chardev
  spicevmc,id=charredir0,name=usbredir -device usb-
  redir,chardev=charredir0,id=redir0,bus=usb.0,port=1 -chardev
  spicevmc,id=charredir1,name=usbredir -device usb-
  redir,chardev=charredir1,id=redir1,bus=usb.0,port=2 -device virtio-
  balloon-pci,id=balloon0,bus=pci.0,addr=0x7 -sandbox
  on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny
  -msg timestamp=on

  
  I have attempted to collect a backtrace, but will need direction as I am not 
sure on which thread to listen and where to set the breakpoint, 'thread apply 
all backtrace' does not seem to work well with the qemu process...

  Thank you

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



[Bug 1883083] Re: QEMU: block/vvfat driver issues

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  QEMU: block/vvfat driver issues

Status in QEMU:
  New

Bug description:
  Nathan Huckleberry  has reported following issues
  in the block/vvfat driver for the virtual VFAT file system image, used
  to share a host system directory with a guest VM.

  Please note:
-> 
https://www.qemu.org/docs/master/system/images.html#virtual-fat-disk-images

  Virtual VFAT read/write support is available only for (beta) testing
  purposes.

  Following issues are reproducible with:

 host)$ ./bin/qemu-system-x86_64 -nographic -enable-kvm \
-drive file=fat:rw:/tmp/var/run/,index=2  -m 2048 
/var/lib/libvirt/images/f27vm.qcow2

guest)# mount -t vfat /dev/sdb1 /mnt/

  The attached reproducers (run inside a guest) include:

  1. dir.sh: - directory traversal on the host
 - It creates a file under /mnt/
 - Then edits the VFAT directory entry to make it -> /mnt/../y
 - The handle_renames_and_mkdirs() routine does not check this new file name
   and creates a file outside of the shared directory on the host

  2. dos.sh: hits an assertion failure in vvfat driver
 - Creates a deep directory tree like - /mnt/0/1/2/3/4/5/6/../29/30/
 - While updating vvfat commits, driver hits an assertion in
   handle_renames_and_mkdirs
 ...
 } else if (commit->action == ACTION_MKDIR) {
 ...
 assert(j < s->mapping.next);<== it fails

  3. read.sh: reads past vvfat directory entries
 - Creates a file with: echo "x" > /mnt/a
 - Reads past VVFAT directory entry structure with

 # head -c 100 $MNTDEV | xxd | grep x -A 512

 - It may disclose some heap addresses.

  4. write.sh: heap buffer overflow
 - Creates large number of files as /mnt/file[1..35]
 - while syncing directory tree with the host, driver hits an overflow
   while doing memmove(3) in array_roll() routine

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



[Bug 1793904] Re: files are randomly overwritten by Zero Bytes

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  files are randomly overwritten by Zero Bytes

Status in QEMU:
  New

Bug description:
  Hello together,

  I am currently tracking down a "Hard to reproduce" bug on my systems
  that I first discovered during gitlab installation:

  
  Here is the Text from the Gitlab Bug 
https://gitlab.com/gitlab-org/gitlab-ce/issues/51023
  
--

  Steps to reproduce

  I still do not have all the steps together to reproduce, so far it is:
  apt install gitlab-ce and
  gitlab-rake backup:recovery
  Then it works for some time before it fails.

  What is the current bug behavior?

  I have a 12 hour old Installation of gitlab ce 11.2.3-ce.0 for debian
  stretch on a fresh debian stretch system together with our imported
  data. However it turns out that some gitlab related files contain Zero
  bytes instead of actual data.

  root@gitlab:~# xxd -l 16 /opt/gitlab/bin/gitlab-ctl
  :          

  This behaviour is somewhat strange because it was working for a few
  minutes/hours. I did write a shell script to find out which files are
  affected of this memory loss. It turns out that only files located
  under /opt/gitlab are affected, if I rule out files like
  /var/log/faillog and some postgresql table files.

  What I find even stranger is that it does not seem to affect
  Logfiles/databases/git_repositorys but application files, like .rb
  scripts. and not all of them. No non gitlab package is affected.

  What is the expected correct behavior?
  Binarys and .rb files should stay as they are.

  Possible fixes

  I am still investigating, I hope that it is not an infrastructure problem 
(libvirt/qemu/glusterfs) it can still be one but the point that files of 
/opt/gitlab are affected and not any logfile and that we to not have similar 
problems with any other system leads me to the application for now.
  If I would have used docker the same problem might have caused a reboot of 
the container.
  But for the Debian package it is a bit of work to recover. That is all a 
workaround, however.
  
-

  I do have found 2 more systems having the same problem with different
  software:

  root@erp:~# xxd -l 16 /usr/share/perl/5.26.2/constant.pm
  :          

  The Filesize itself is, compared with another machine 1660 Bytes
  for both the corrupted and the intact file. It looks to me from the
  outside that if some data in the qcow2 file is written too many bytes
  get written so it sometimes overwites data of existing files located
  right after the position in memory where the write goes to.

  I would like to rule out Linux+Ext4 filesystems because I find it
  highly unlikely that such an error keeps undiscovered in that part of
  the environment for long. I think the same might go for qemu.

  Which leaves qemu, gemu+gluster:// mount, qcow2 volumes, glusterfs,
  network. So I am now going to check if I can find any system which
  gets its volumes via fusermount instead of gluster:// path if the
  error is gone there. This may take a while.

  
  - some software versions---

  QEMU emulator version 2.12.0 (Debian 1:2.12+dfsg-3)
  Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

  libvirt-daemon-driver-storage-gluster/testing,unstable,now 4.6.0-2
  amd64 [installed]

  ii  glusterfs-client   4.1.3-1amd64

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



[Bug 1779955] Re: qemu linux-user requires read permissions on memory passed to syscalls that should only need write access

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  qemu linux-user requires read permissions on memory passed to syscalls
  that should only need write access

Status in QEMU:
  Confirmed

Bug description:
  When read() function takes an mmap'ed address as output buffer, it
  returns EFAULT. The expected behavior is it should just work.

  The following code works for qemu-system-arm, but not for qemu-arm-
  static.

  QEMU version affected: latest release 2.12.0.

  Steps to reproduce (please substitute /path/to/qemu-arm-static with
  the path of the binary, and /tmp/a.cpp with the example source code
  attached):

  # First register binfmt_misc
  [hidden]$ docker run --rm --privileged multiarch/qemu-user-static:register 
--reset

  # Compile the code and run
  [hidden]$ docker run --rm -it -v /tmp/a.cpp:/tmp/a.cpp -v 
/path/to/qemu-arm-static:/usr/bin/qemu-arm-static arm32v7/ubuntu:18.04 bash -c 
'{ apt update -y && apt install -y g++; } >& /dev/null && g++ -std=c++14 
/tmp/a.cpp -o /tmp/a.out && echo hehe > /tmp/haha.txt && /tmp/a.out'
  ofd=3
  ftruncate=0
  mmap=0xff3f5000
  fd=4
  0xff3f5023 -1 14

  The expected result in qemu-system-arm as well as natively on x86_64 host:
  hidden$ ./a.out
  ofd=3
  ftruncate=0
  mmap=0xb6fb7000
  fd=4
  0xb6fb7023 5 0

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



[Bug 1371915] Re: Make Uninstall Rule Requested

2021-04-29 Thread Thomas Huth
** Tags removed: qemu ubuntu uninstall

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

Title:
  Make Uninstall Rule Requested

Status in QEMU:
  In Progress

Bug description:
  Environment: Ubuntu 14.04 - Qemu 2.1.1
  --
  I've configured qemu with some --prefix, compiled the sources and installed 
the binaries; now, for some reason, I need to uninstall qemu to configure it 
with the default prefix, recompile the sources and reinstall the binaries.
  However, there's no rule to uninstall qemu.

  All other packages which I have compiled and installed on my system
  offer the possibility to uninstall it: why not Qemu?

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



[Bug 1745316] Re: SDL1.x>SDL2 regressions: non-usbtablet mouse position reporting is broken, and VGA/compatmonitor/serial/etc view switching is unusable

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  SDL1.x>SDL2 regressions: non-usbtablet mouse position reporting is
  broken, and VGA/compatmonitor/serial/etc view switching is unusable

Status in QEMU:
  Incomplete

Bug description:
  Hi,

  I almost exclusively use -sdl when I use QEMU. The GTK UI (I'm on
  Linux) distinctly takes a few extra seconds to start on every boot,
  and I don't really ever use the extra controls it provides. I hope the
  SDL-based UI never goes away :)

  The SDL 1.2 > SDL 2.0 update (committed between June 8-20 2017)
  introduced the following two regressions:

  - PS/2 and serial mouse position reporting became completely broken
  (only usbtablet works)

  - The compatmonitor/serial/parallel "views" try to open new windows,
  which does not play well on my system at all

  First of all, here are the bisection details:

  https://github.com/qemu/qemu/commit/269c20b2bbd2aa8531e0cdc741fb166f290d7a2b
(June 8 2017): the last version that works

  https://github.com/qemu/qemu/commit/7e56accdaf35234b69c33c85e4a44a5d56325e53
(June 20 2017): the first version that fails

  Here are the changelists between these two revisions:

  https://github.com/qemu/qemu/compare/269c20b...7e56acc
  (compare direction: OLD to NEW) (Commits: 60   Files changed: 85)

  https://github.com/qemu/qemu/compare/7e56acc...269c20b
  (compare direction: NEW to OLD) (Commits: 41   Files changed: 108)

  (Someone else more familiar with Git might know why GitHub returns
  results for both compare directions. I'm including both links just in
  case.)

  I've found that configuring commit 7e56acc using --with-sdlapi=1.2
  completely remedies all issues. Thus, I think the issue is with SDL 2.

  == #1: Broken mouse position reporting =

  This surfaces immediately with older operating systems. I first
  experienced it when trying to install OS/2 for the first time, and
  thought there was something wrong with OS/2. Then I experienced the
  same issues in Windows 3.1 and MS-DOS applications and I knew
  something was up with QEMU.

  In a nice coincidence, I've recently been playing around with
  prehistorically ancient Linux systems, and while looking around a
  Linux 0.99-based SLS system from 1992 I discovered a low-level
  (console) mouse-testing utility buried in /usr/X386. This utility only
  works when I configure QEMU to use a serial mouse, but it reveals some
  very interesing data: the dx and dy values ("d" = "delta", right?)
  received by the kernel do not contain relative values such as -1, -10,
  2, 5, etc, but instead absolute values such as 0, 12, 37, 112, 329,
  etc.

  Similarly, if I configure Windows 3.1 to use a serial mouse, the mouse
  position jumps exponentially around the screen relative to my mouse
  movements (it's very hard to control), consistent with delta values
  being reported as absolute instead of relative.

  I found that the DOS CuteMouse driver comes with a mouse-testing
  program. CuteMouse absolutely refuses to detect QEMU's serial mouse
  for some reason (?!), but when QEMU is running in PS/2 mode, the mouse
  tester that comes with CuteMouse reports that the mouse at 632,192 no
  matter how much I move the mouse around the window. If I look
  carefully I can see the DOS cursor flickering back and forth as I move
  the mouse and the tester rewrites the line showing the position info,
  so I don't think the test program is broken.

  I got curious and wondered if this was actually an internal SDL bug.
  However, the following test program reports perfect values for me:

  --8<

  #include 
  #include "SDL2/SDL.h"
  int main(void) {
SDL_Init(SDL_INIT_VIDEO);
SDL_Window *window = SDL_CreateWindow("Mouse test", 
SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED,
640, 480, SDL_WINDOW_RESIZABLE
);
if (window == NULL) {
perror(SDL_GetError());
exit(1);
}
for (;;) {
SDL_Event event;
while (SDL_PollEvent(&event)) {
if (event.type == SDL_MOUSEMOTION) {
printf(
"x=%d y=%d xrel=%d yrel=%d\n",
event.motion.x, event.motion.y,
event.motion.xrel, event.motion.yrel
);
}
if (
event.type == SDL_KEYDOWN ||
event.type == SDL_QUIT
) {
SDL_DestroyWindow(window);
SDL_Quit();
exit(0);
}
  

[Bug 1734474] Re: Maemo does not boot on emulated N800

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  Maemo does not boot on emulated N800

Status in QEMU:
  In Progress

Bug description:
  I start QEMU with qemu-system-arm-m 130 -M n800 -kernel zImage.1 -mtdblock 
maemo.img -append "root=/dev/mtdblock3 rootfstype=jffs2"
  On QEMU 1.2.0 see "NOKIA" logo and then desktop appears, but on 1.5.0 and 
newer (including latest versions) I see only white screen and no signs of life. 
Was this caused by regression or any syntax change?

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



[Bug 1745312] Re: Regression report: Disk subsystem I/O failures/issues surfacing in DOS/early Windows [two separate issues: one bisected, one root-caused]

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  Regression report: Disk subsystem I/O failures/issues surfacing in
  DOS/early Windows [two separate issues: one bisected, one root-caused]

Status in QEMU:
  Incomplete

Bug description:
  [Headsup: This report is long-ish due to the amount of detail I've
  stumbled on along the way that I think is relevant to include. I can't
  speak as to the complexity of the actual bugs, but the size of this
  report should not suggest that the reproduction process is
  particularly headache-inducing.]

  Hi!

  I recently needed to fire up some ancient software for research
  purposes and got very distracted discovering and playing with old
  versions of Windows :). In the process I've discovered some glitches
  with disk I/O.

  I believe I've stumbled on two completely separate issues that
  coincidentally surfaced at the same time. It's possible that
  components of this report will be re-filed as more specific new bugs,
  but I'm not an authority on QEMU internals or how to narrow
  down/categorize what I've found.

  - The first bug only surfaces when the "isapc" machine type is used.
  It intermittently produces "General failure {read,writ}ing drive _"
  under MS-DOS 6.22, and also somehow interferes with early bootstrap of
  Windows NT 4 (in NTLDR). Enabling or disabling KVM (I'm on Linux)
  appears to make no difference whatsoever, which may help with
  debugging.

  - The second issue involves
- a WinNT4 disk image
- created by running through a bog-standard NT4 install inside QEMU 2.9.0
- which will now fail to boot in any version of QEMU - even version 1.0
  - but which VirtualBox will boot fine
- but only if I point VirtualBox at QEMU's raw disk image via a
  hacked-together VMDK file
- if the raw image is converted to VHD(X), VirtualBox will also fail
  to boot the image with exactly the same error as QEMU
- this state of affairs is not affected by image sparseness (which makes
  sense)

  I'm confident I've bisected the first issue.

  I wasn't able to bisect the second issue (as all tested versions of
  QEMU behaved identically), but I've figured out a working repro
  testcase and I believe I've managed to pin down a solid root cause.


  == #1: Intermittent I/O issues when `-M isapc` is used =

  These symptoms sometimes take a small amount of time and fiddling to
  trigger, but I AM able to consistently surface them on my machine
  after a short while. (I am very very interested to hear if others
  cannot reproduce them.)

  So, first of all:

  https://github.com/qemu/qemu/commit/306ec6c3cece7004429c79c1ac93d49919f1f1cc
(Jul 30 2013): the last version that works

  https://github.com/qemu/qemu/commit/e689f7c668cbd9d08f330e17c3dd3a059c9553d3
(Oct 30 2013): the first version that intermittently fails

  Maybe lift out and build these branches while reading. *shrug*
  (How to do this can be found at the end of this report - along with a 
time-saving ./configure line, FWIW)

  Here are the changelists between these two revisions:

  https://github.com/qemu/qemu/compare/306ec6c...e689f7c
  (Compare direction: OLD to NEW) (Commits: 166  Files changed: 192)

  https://github.com/qemu/qemu/compare/e689f7c...306ec6c
  (Compare direction: NEW to OLD) (Commits: 30   Files changed: 22)

  (Someone else more familiar with Git might know why GitHub returns
  results for both compare directions, and/or if the 2nd link is useful
  information. The first link returns a lot more results than the 2nd
  one, at least. Does comparing new>old return deletions?)

  ---

  Now on to the symptoms. In a moment I'll describe reproduction.

  # MS-DOS 6.22

  The first symptom I discovered was that trivial read and write
  operations under MS-DOS would sometimes fail:

C:\>echo test > hi

General failure writing drive C
Abort, Retry, Fail?

  Anything else that exercises the disk behaves similarly:

C:\>dir /s > nul

General failure reading drive C
Abort, Retry, Fail?

  (Note that the above demonstrates both write and read failures)

  (Also, FWIW, `dir /s` == `ls -R`)

  The behavior of the I/O errors is not possible to characterise as it
  fluctuates so much. For example something as simple as DIR can produce
  wildly differing results: in one run, poking around with DIR ended
  with DOS deciding C:\ was empty at one point; at another point in a
  different run C:\ mysteriously dropped 50% of its contents only to
  magically gain it all back moments later after some poking around in
  one of the subdirectories that was still visible.

  The time it takes to trigger these errors is also highly variable.
  QEMU may fall over as early as hanging forever at "Starting MS-
  DOS...", or I might get all the way into Windows 3.1 before it
  triggers (i

[Bug 1696180] Re: Issues with qemu-img, libgfapi, and encryption at rest

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  Issues with qemu-img, libgfapi, and encryption at rest

Status in QEMU:
  Triaged

Bug description:
  Hi,

  Encryption-at-rest has been supported for some time now.  The client
  is responsible for encrypting the files with a help of a master key
  file.  I have a properly setup environment and everything appears to
  be working fine but when I use qemu-img to move a file to gluster I
  get the following:

  
  # qemu-img convert -f raw -O raw linux.iso gluster://gluster01/virt0/linux.raw
  [2017-06-06 16:52:25.489720] E [mem-pool.c:579:mem_put] 
(-->/lib64/libglusterfs.so.0(syncop_lookup+0x4e5) [0x7f30f7a36d35] 
-->/lib64/libglusterfs.so.0(+0x59f02) [0x7f30f7a32f02] 
-->/lib64/libglusterfs.so.0(mem_put+0x190) [0x7f30f7a24a60] ) 0-mem-pool: 
mem-pool ptr is NULL
  [2017-06-06 16:52:25.490778] E [mem-pool.c:579:mem_put] 
(-->/lib64/libglusterfs.so.0(syncop_lookup+0x4e5) [0x7f30f7a36d35] 
-->/lib64/libglusterfs.so.0(+0x59f02) [0x7f30f7a32f02] 
-->/lib64/libglusterfs.so.0(mem_put+0x190) [0x7f30f7a24a60] ) 0-mem-pool: 
mem-pool ptr is NULL
  [2017-06-06 16:52:25.492263] E [mem-pool.c:579:mem_put] 
(-->/lib64/libglusterfs.so.0(syncop_lookup+0x4e5) [0x7f30f7a36d35] 
-->/lib64/libglusterfs.so.0(+0x59f02) [0x7f30f7a32f02] 
-->/lib64/libglusterfs.so.0(mem_put+0x190) [0x7f30f7a24a60] ) 0-mem-pool: 
mem-pool ptr is NULL
  [2017-06-06 16:52:25.497226] E [mem-pool.c:579:mem_put] 
(-->/lib64/libglusterfs.so.0(syncop_create+0x44d) [0x7f30f7a3cf5d] 
-->/lib64/libglusterfs.so.0(+0x59f02) [0x7f30f7a32f02] 
-->/lib64/libglusterfs.so.0(mem_put+0x190) [0x7f30f7a24a60] ) 0-mem-pool: 
mem-pool ptr is NULL

  On and on until I get this message:

  [2017-06-06 17:00:03.467361] E [MSGID: 108006] [afr-common.c:4409:afr_notify] 
0-virt0-replicate-0: All subvolumes are down. Going offline until atleast one 
of them comes back up.
  [2017-06-06 17:00:03.467442] E [MSGID: 108006] [afr-common.c:4409:afr_notify] 
0-virt0-replicate-1: All subvolumes are down. Going offline until atleast one 
of them comes back up.

  I asked for help assuming it's a problem with glusterfs and was told
  it appears qemu-img's implementation of libgfapi doesn't call the
  xlator function correctly.

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



[Bug 1910723] Re: NULL pointer dereference issues in am53c974 SCSI host bus adapter

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  NULL pointer dereference issues in am53c974 SCSI host bus adapter

Status in QEMU:
  Fix Committed

Bug description:
  Two NULL pointer dereference issues were found in the am53c974 SCSI
  host bus adapter emulation of QEMU. They could occur while handling
  the 'Information Transfer' command (CMD_TI) in function handle_ti() in
  hw/scsi/esp.c, and could be abused by a malicious guest to crash the
  QEMU process on the host resulting in a denial of service.

  Both issues were reported by Cheolwoo Myung (Seoul National
  University). To reproduce them, configure and run QEMU as follows.
  Please find attached the required disk images.

  $ ./configure --target-list=x86_64-softmmu --enable-kvm --enable-sanitizers
  $ make
  $ ./qemu-system-x86_64 -m 512 -drive 
file=./hyfuzz.img,index=0,media=disk,format=raw \
  -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \
  -drive id=SysDisk,if=none,file=./disk.img

  Additional info:
  RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909766
  RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909769

  ASAN logs:
  ==672133== 
  hw/scsi/scsi-bus.c:1385:12: runtime error: member access within null pointer 
of type 'struct SCSIRequest'
  AddressSanitizer:DEADLYSIGNAL 
   
  = 
  ==672133==ERROR: AddressSanitizer: SEGV on unknown address 0x0171 (pc 
0x55bd63e20b85 bp 0x7f4b6fffdfa0 sp 0x7f4b6fffdf70 T7)
  ==672133==The signal is caused by a READ memory access. 
  ==672133==Hint: address points to the zero page.  
   
  #0 0x55bd63e20b85 in scsi_req_continue hw/scsi/scsi-bus.c:1385
  #1 0x55bd63ab34fb in esp_do_dma hw/scsi/esp.c:453   
  #2 0x55bd63ab4b3c in handle_ti hw/scsi/esp.c:549  
  #3 0x55bd63ab72a9 in esp_reg_write hw/scsi/esp.c:691 
  #4 0x55bd63d7b5dd in esp_pci_io_write hw/scsi/esp-pci.c:206
  #5 0x55bd645d55a3 in memory_region_write_accessor softmmu/memory.c:491
  #6 0x55bd645d5a24 in access_with_adjusted_size softmmu/memory.c:552
  #7 0x55bd645e2baa in memory_region_dispatch_write softmmu/memory.c:1501
  #8 0x55bd646b75ff in flatview_write_continue softmmu/physmem.c:2759
  #9 0x55bd646b79d1 in flatview_write softmmu/physmem.c:2799
  #10 0x55bd646b8341 in address_space_write softmmu/physmem.c:2891   
  #11 0x55bd646b83f9 in address_space_rw softmmu/physmem.c:2901
  #12 0x55bd648c4736 in kvm_handle_io accel/kvm/kvm-all.c:2285
  #13 0x55bd648c69c8 in kvm_cpu_exec accel/kvm/kvm-all.c:2531
  #14 0x55bd647b2413 in kvm_vcpu_thread_fn accel/kvm/kvm-cpus.c:49
  #15 0x55bd64f560de in qemu_thread_start util/qemu-thread-posix.c:521
  #16 0x7f4b981763f8 in start_thread (/lib64/libpthread.so.0+0x93f8)
  #17 0x7f4b980a3902 in __GI___clone (/lib64/libc.so.6+0x101902)

  ---

  ==672020==
  hw/scsi/esp.c:196:62: runtime error: member access within null pointer of 
type 'struct SCSIDevice'
  AddressSanitizer:DEADLYSIGNAL 
   
  = 
  ==672020==ERROR: AddressSanitizer: SEGV on unknown address 0x0098 (pc 
0x559bc99946fd bp 0x7f08bd737fb0 sp 0x7f08bd737f70 T7)
  ==672020==The signal is caused by a READ memory access. 
  ==672020==Hint: address points to the zero page.  
   
  #0 0x559bc99946fd in do_busid_cmd hw/scsi/esp.c:196
  #1 0x559bc9994e71 in do_cmd hw/scsi/esp.c:220   
  #2 0x559bc999ae81 in handle_ti hw/scsi/esp.c:555  
  #3 0x559bc999d2a9 in esp_reg_write hw/scsi/esp.c:691 
  #4 0x559bc9c615dd in esp_pci_io_write hw/scsi/esp-pci.c:206
  #5 0x559bca4bb5a3 in memory_region_write_accessor softmmu/memory.c:491
  #6 0x559bca4bba24 in access_with_adjusted_size softmmu/memory.c:552
  #7 0x559bca4c8baa in memory_region_dispatch_write softmmu/memory.c:1501
  #8 0x559bca59d5ff in flatview_write_continue softmmu/physmem.c:2759
  #9 0x559bca59d9d1 in flatview_write softmmu/physmem.c:2799
  #10 0x559bca59e341 in address_space_write softmmu/physmem.c:2891   
  #11 0x559bca59e3f9 in address_space_rw softmmu/physmem.c:2901
  #12 0x559bca7aa736 in kvm_handle_io accel/kvm/kvm-all.c:2285
  #13 0x559bca7ac9c8 in kvm_cpu_exec accel/kvm/kvm-all.c:2531
  #14 0x559bca698413 in kvm_vcpu_thread_fn accel/kvm/kvm-cpus.c:49
  #15 0x559bcae3c0de in qemu_thread_start util/qemu-thread-posix.c:521
  #16 0x7f08e57ba3f8 in start_thread (/lib64/libpthread.so.0+0x93f8)
  

[Bug 1910696] Re: Qemu fails to start with error " There is no option group 'spice'"

2021-04-29 Thread Thomas Huth
** Tags removed: qemu

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

Title:
  Qemu fails to start with error " There is no option group 'spice'"

Status in QEMU:
  New

Bug description:
  After upgrade from 5.1.0 to 5.2.0, qemu fails on start with error:
  `
  /usr/bin/qemu-system-x86_64 -S -name trinti -uuid 
f8ad2ff6-8808-4f42-8f0b-9e23acd20f84 -daemonize -cpu host -nographic -serial 
chardev:console -nodefaults -no-reboot -no-user-config -sandbox 
on,obsolete=deny,elevateprivileges=allow,spawn=deny,resourcecontrol=deny 
-readconfig /var/log/lxd/trinti/qemu.conf -pidfile /var/log/lxd/trinti/qemu.pid 
-D /var/log/lxd/trinti/qemu.log -chroot /var/lib/lxd/virtual-machines/trinti 
-smbios type=2,manufacturer=Canonical Ltd.,product=LXD -runas nobody: 
  qemu-system-x86_64:/var/log/lxd/trinti/qemu.conf:27: There is no option group 
'spice'
  qemu-system-x86_64: -readconfig /var/log/lxd/trinti/qemu.conf: read config 
/var/log/lxd/trinti/qemu.conf: Invalid argument
  `
  Bisected to first bad commit: 
https://github.com/qemu/qemu/commit/cbe5fa11789035c43fd2108ac6f45848954954b5

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



[Bug 1926521] Re: QEMU-user ignores MADV_DONTNEED

2021-04-29 Thread Laurent Vivier
** Tags added: linux-user

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

Title:
  QEMU-user ignores MADV_DONTNEED

Status in QEMU:
  New

Bug description:
  There is comment int the code "This is a hint, so ignoring and returning 
success is ok"
  
https://github.com/qemu/qemu/blob/b1cffefa1b163bce9aebc3416f562c1d3886eeaa/linux-user/syscall.c#L11941

  But it seems incorrect with the current state of Linux

  "man madvise" or https://man7.org/linux/man-pages/man2/madvise.2.html
  says the following:
  >>  These advice values do not influence the semantics
  >>   of the application (except in the case of MADV_DONTNEED)

  >> After a successful MADV_DONTNEED operation, the semantics
  >> of memory access in the specified region are changed:
  >> subsequent accesses of pages in the range will succeed,
  >> but will result in either repopulating the memory contents
  >> from the up-to-date contents of the underlying mapped file
  >> (for shared file mappings, shared anonymous mappings, and
  >> shmem-based techniques such as System V shared memory
  >> segments) or zero-fill-on-demand pages for anonymous
  >> private mappings.

  Some applications use this behavior clear memory and it
  would be nice to be able to run them on QEMU without
  workarounds.

  Reproducer on "Debian 5.10.24 x86_64 GNU/Linux" as a host.

  
  ```
  #include "assert.h"
  #include "stdio.h"
  #include 
  #include 

  int main() {
char *P = (char *)mmap(0, 4096, PROT_READ | PROT_WRITE,
   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
assert(P);
*P = 'A';
while (madvise(P, 4096, MADV_DONTNEED) == -1 && errno == EAGAIN) {
}
assert(*P == 0);

printf("OK\n");
  }

  /*
  gcc /tmp/madvice.c -o /tmp/madvice

  qemu-x86_64 /tmp/madvice
  madvice: /tmp/madvice.c:13: main: Assertion `*P == 0' failed.
  qemu: uncaught target signal 6 (Aborted) - core dumped
  Aborted

  /tmp/madvice
  OK

  
  */

  ```

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



Re: Let's remove some deprecated stuff

2021-04-29 Thread Gerd Hoffmann
  Hi,

> ``QEMU_AUDIO_`` environment variables and ``-audio-help`` (since 4.0)
> Creating sound card devices and vnc without ``audiodev=`` property (since 
> 4.2)
> Creating sound card devices using ``-soundhw`` (since 5.1)

I think these three should be dropped together, to minimize disruption.

Where do we strand in terms of libvirt support?  IIRC audiodev= support
in libvirt is rather recent (merged this year).  I'd tend to wait a bit
longer because of that.

Daniel?

take care,
  Gerd




[Bug 1734474] Re: Maemo does not boot on emulated N800

2021-04-29 Thread Philippe Mathieu-Daudé
Fixed in v5.2.0?
ab135622cf4 ("tmp105: Correct handling of temperature limit checks")
e1919889ef7 ("hw/misc/tmp105: reset the T_low and T_High registers")

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

Title:
  Maemo does not boot on emulated N800

Status in QEMU:
  In Progress

Bug description:
  I start QEMU with qemu-system-arm-m 130 -M n800 -kernel zImage.1 -mtdblock 
maemo.img -append "root=/dev/mtdblock3 rootfstype=jffs2"
  On QEMU 1.2.0 see "NOKIA" logo and then desktop appears, but on 1.5.0 and 
newer (including latest versions) I see only white screen and no signs of life. 
Was this caused by regression or any syntax change?

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



Re: Let's remove some deprecated stuff

2021-04-29 Thread Daniel P . Berrangé
On Thu, Apr 29, 2021 at 12:18:42PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > ``QEMU_AUDIO_`` environment variables and ``-audio-help`` (since 4.0)
> > Creating sound card devices and vnc without ``audiodev=`` property 
> > (since 4.2)
> > Creating sound card devices using ``-soundhw`` (since 5.1)
> 
> I think these three should be dropped together, to minimize disruption.
> 
> Where do we strand in terms of libvirt support?  IIRC audiodev= support
> in libvirt is rather recent (merged this year).  I'd tend to wait a bit
> longer because of that.
> 
> Daniel?

Libvirt added supoort for -audio in 7.2.0, release April 4th, so only
one month ago.

If we drop the features in QEMU in this dev cycle though, this won't
impact most users until QEMU 6.1 releases in mid August. I'm perfectly
ok with people who use unreleased QEMU git master needing to update
their libvirt. The final release date is far enough away that distros
will have had new enough libvirt for a good while.


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




[Bug 1926521] Re: QEMU-user ignores MADV_DONTNEED

2021-04-29 Thread Laurent Vivier
We had a tentative patch in the past:

[PATCH v3] linux-user: add support for MADV_DONTNEED
https://patchew.org/QEMU/20180827084037.25316-1-simon.hausm...@qt.io/

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

Title:
  QEMU-user ignores MADV_DONTNEED

Status in QEMU:
  New

Bug description:
  There is comment int the code "This is a hint, so ignoring and returning 
success is ok"
  
https://github.com/qemu/qemu/blob/b1cffefa1b163bce9aebc3416f562c1d3886eeaa/linux-user/syscall.c#L11941

  But it seems incorrect with the current state of Linux

  "man madvise" or https://man7.org/linux/man-pages/man2/madvise.2.html
  says the following:
  >>  These advice values do not influence the semantics
  >>   of the application (except in the case of MADV_DONTNEED)

  >> After a successful MADV_DONTNEED operation, the semantics
  >> of memory access in the specified region are changed:
  >> subsequent accesses of pages in the range will succeed,
  >> but will result in either repopulating the memory contents
  >> from the up-to-date contents of the underlying mapped file
  >> (for shared file mappings, shared anonymous mappings, and
  >> shmem-based techniques such as System V shared memory
  >> segments) or zero-fill-on-demand pages for anonymous
  >> private mappings.

  Some applications use this behavior clear memory and it
  would be nice to be able to run them on QEMU without
  workarounds.

  Reproducer on "Debian 5.10.24 x86_64 GNU/Linux" as a host.

  
  ```
  #include "assert.h"
  #include "stdio.h"
  #include 
  #include 

  int main() {
char *P = (char *)mmap(0, 4096, PROT_READ | PROT_WRITE,
   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
assert(P);
*P = 'A';
while (madvise(P, 4096, MADV_DONTNEED) == -1 && errno == EAGAIN) {
}
assert(*P == 0);

printf("OK\n");
  }

  /*
  gcc /tmp/madvice.c -o /tmp/madvice

  qemu-x86_64 /tmp/madvice
  madvice: /tmp/madvice.c:13: main: Assertion `*P == 0' failed.
  qemu: uncaught target signal 6 (Aborted) - core dumped
  Aborted

  /tmp/madvice
  OK

  
  */

  ```

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



Re: Let's remove some deprecated stuff

2021-04-29 Thread Peter Maydell
On Thu, 29 Apr 2021 at 11:02, Markus Armbruster  wrote:
>
> If you're cc'ed, you added a section to docs/system/deprecated.rst that
> is old enough to permit removal.  This is *not* a demand to remove, it's
> a polite request to consider whether the time for removal has come.
> Extra points for telling us in a reply.  "We should remove, but I can't
> do it myself right now" is a valid answer.  Let's review the file:

> I'm not sure there's anything to remove here, but anyway, Peter Maydell:

This isn't one of mine -- I just show up in git blame because this
section predates the conversion from texi to rst. It was originally
added by Eduardo (cc'd) in commit aa5b9692871.

> Runnability guarantee of CPU models (since 4.1.0)
> '
>
> Previous versions of QEMU never changed existing CPU models in
> ways that introduced additional host software or hardware
> requirements to the VM.  This allowed management software to
> safely change the machine type of an existing VM without
> introducing new requirements ("runnability guarantee").  This
> prevented CPU models from being updated to include CPU
> vulnerability mitigations, leaving guests vulnerable in the
> default configuration.
>
> The CPU model runnability guarantee won't apply anymore to
> existing CPU models.  Management software that needs runnability
> guarantees must resolve the CPU model aliases using the
> ``alias-of`` field returned by the ``query-cpu-definitions`` QMP
> command.
>
> While those guarantees are kept, the return value of
> ``query-cpu-definitions`` will have existing CPU model aliases
> point to a version that doesn't break runnability guarantees
> (specifically, version 1 of those CPU models).  In future QEMU
> versions, aliases will point to newer CPU model versions
> depending on the machine type, so management software must
> resolve CPU model aliases before starting a virtual machine.

thanks
-- PMM



Re: [PATCH v4 01/12] MAINTAINERS: Add qtest/arm-cpu-features.c to ARM TCG CPUs section

2021-04-29 Thread Alex Bennée


Philippe Mathieu-Daudé  writes:

> We want the ARM maintainers and the qemu-arm@ list to be
> notified when this file is modified. Add an entry to the
> 'ARM TCG CPUs' section in the MAINTAINERS file.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Alex Bennée 

> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 36055f14c59..d5df4ba7891 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -156,6 +156,7 @@ S: Maintained
>  F: target/arm/
>  F: tests/tcg/arm/
>  F: tests/tcg/aarch64/
> +F: tests/qtest/arm-cpu-features.c
>  F: hw/arm/
>  F: hw/cpu/a*mpcore.c
>  F: include/hw/cpu/a*mpcore.h


-- 
Alex Bennée



Re: Let's remove some deprecated stuff

2021-04-29 Thread Peter Maydell
On Thu, 29 Apr 2021 at 11:28, Daniel P. Berrangé  wrote:
>
> On Thu, Apr 29, 2021 at 12:18:42PM +0200, Gerd Hoffmann wrote:
> >   Hi,
> >
> > > ``QEMU_AUDIO_`` environment variables and ``-audio-help`` (since 4.0)
> > > Creating sound card devices and vnc without ``audiodev=`` property 
> > > (since 4.2)
> > > Creating sound card devices using ``-soundhw`` (since 5.1)
> >
> > I think these three should be dropped together, to minimize disruption.
> >
> > Where do we strand in terms of libvirt support?  IIRC audiodev= support
> > in libvirt is rather recent (merged this year).  I'd tend to wait a bit
> > longer because of that.
> >
> > Daniel?
>
> Libvirt added supoort for -audio in 7.2.0, release April 4th, so only
> one month ago.
>
> If we drop the features in QEMU in this dev cycle though, this won't
> impact most users until QEMU 6.1 releases in mid August. I'm perfectly
> ok with people who use unreleased QEMU git master needing to update
> their libvirt. The final release date is far enough away that distros
> will have had new enough libvirt for a good while.

It does feel to me that dropping the old options now would be being
a bit over-eager, though. The deprecation cycle time is a minimum, not
a target :-)

-- PMM



[Bug 1862887] Re: qemu does not load pulseaudio modules properly

2021-04-29 Thread Thomas Huth
** Tags removed: builds
** Tags added: audio

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

Title:
  qemu does not load pulseaudio modules properly

Status in QEMU:
  New

Bug description:
  Hello,

  This is on Arch-linux(latest) and the qemu 4.2.0 version made from git clone 
https://github.com/spheenik/qemu.git
  with:
   ./configure --prefix=/opt/qemu-test --python=/usr/bin/python2 
--target-list=x86_64-softmmu 
  --audio-drv-list=pa --disable-werror
  added to the build.

  I've been workin on a passthrough Windows 10 vm this month and have been 
steadily seeing some promising progress. My block/issue at the moment is 
integrating the audio now that the GPU has been succesfully passed through. 
  I've been going back and forth between the audio options for Pulseaudio and 
cannot change the following issue:
  pulseaudio: pa_context_connect() failed
  pulseaudio: Reason: Connection refused
  pulseaudio: Failed to initialize PA contextlibusb: error [udev_hotplug_event] 
ignoring udev action bind
  I leave my current operable build followed by some of the options that I have 
tried using to correct this despite the following errors not changing

  This is my current operable build:

  #!/bin/bash

  vmname="windows10vm"

  if ps -ef | grep /opt/qemu-test/bin/qemu-system-x86_64 | grep -q 
multifunction=on; then
  echo "A passthrough VM is already running." &
  exit 1

  else

  /opt/qemu-test/bin/qemu-system-x86_64 \
  -m 12G \
  -drive id=disk0,if=virtio,cache=none,format=raw,file=.../win2.img \
  -drive file=.../Win10_1909_English_x64.iso,index=1,media=cdrom \
  -drive file=.../virtio-win-0.1.171.iso,index=2,media=cdrom \
  -boot order=dc \
  -bios /usr/share/ovmf/x64/OVMF_CODE.fd \
  -name $vmname,process=$vmname \
  -machine type=q35,accel=kvm,vmport=off \
  -cpu host,kvm=off \
  -smp 3,sockets=1,cores=3,threads=1 \
  -device virtio-balloon \
  -rtc clock=host,base=localtime \
  -vga none \
  -nographic \
  -serial none \
  -parallel none \
  -soundhw hda \
  -usb \
  -device usb-host,vendorid=...,productid=... \
  -device usb-host,vendorid=...,productid=... \
  -device usb-host,vendorid=...,productid=... \
  -device vfio-pci,host=...,multifunction=on \
  -device vfio-pci,host=... \
  -device e1000,netdev=net0 \
  -netdev user,id=net0,hostfwd=tcp::...-:22 \

  Here's a list of setting combinations I had tried to resolve this:

  #export QEMU_AUDIO_DRV=pa
  #QEMU_ALSA_DAC_BUFFER_SIZE=512 QEMU_ALSA_DAC_PERIOD_SIZE=170
  #export QEMU_PA_SAMPLES=8192 
  #export QEMU_AUDIO_TIMER_PERIOD=99
  #export QEMU_PA_SERVER=/run/user/1000/pulse/native
  #export 
QEMU_PA_SINK=alsa_output.usb-C-Media_Electronics_Inc._USB_Audio_Device-00.analog-stereo
  #export QEMU_PA_SOURCE=input

  -audiodev pa,id=pa1,server=server=/run/user/1000/pulse/native

  At best I have removed an XDG_RUNTIME_DIR error but other than that
  this build has no audio compatability.

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



Re: Let's remove some deprecated stuff

2021-04-29 Thread Daniel P . Berrangé
On Thu, Apr 29, 2021 at 11:59:41AM +0200, Markus Armbruster wrote:
> Myself, but I only documented it; it's actually Kevin Wolf:
> 
> ``blockdev-open-tray``, ``blockdev-close-tray`` argument ``device`` 
> (since 2.8.0)
> 
> '
> 
> Use argument ``id`` instead.
> 
> ``eject`` argument ``device`` (since 2.8.0)
> '''
> 
> Use argument ``id`` instead.
> 
> ``blockdev-change-medium`` argument ``device`` (since 2.8.0)
> 
> 
> Use argument ``id`` instead.
> 
> ``block_set_io_throttle`` argument ``device`` (since 2.8.0)
> '''
> 
> Use argument ``id`` instead.

FYI, I did prepare patches for these already, but they broke the iotests.

I found it difficult to figure out the right fix for the iotests, becuase
IIUC "device" and "id" values are different, and I didn't see what "id"
to use when args are still using -drive, not -blockdev.


> Myself:
> 
> ``blockdev-add`` empty string argument ``backing`` (since 2.10.0)
> '
> 
> Use argument value ``null`` instead.
> 
> Myself, but I only documented it; it's actually Kevin Wolf:
> 
> ``block-commit`` arguments ``base`` and ``top`` (since 3.1.0)
> '
> 
> Use arguments ``base-node`` and ``top-node`` instead.

I've also got patches that remove these two, but didn't submit them
as they were behind the patches for the "device" removal.



> Block device options
> 
> 
> Myself:
> 
> ``"backing": ""`` (since 2.12.0)
> 
> 
> In order to prevent QEMU from automatically opening an image's backing
> chain, use ``"backing": null`` instead.

Unless I'm mistaken,  this appeared to end up being a dupe of the
blockdev-add with empty string deprecation above.

> Myself:
> 
> ``rbd`` keyvalue pair encoded filenames: ``""`` (since 3.1.0)
> ^
> 
> Options for ``rbd`` should be specified according to its runtime options,
> like other block drivers.  Legacy parsing of keyvalue pair encoded
> filenames is useful to open images with the old format for backing files;
> These image files should be updated to use the current format.
> 
> Example of legacy encoding::
> 
>   json:{"file.driver":"rbd", "file.filename":"rbd:rbd/name"}
> 
> The above, converted to the current supported format::
> 
>   json:{"file.driver":"rbd", "file.pool":"rbd", "file.image":"name"}

Got patch for this too

All my unsubmitted patches related to block are here:

  https://gitlab.com/berrange/qemu/-/commits/dep-block

NB I've not compile tested them recently since rebasing to git master.


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




[Bug 1883560] Re: mips linux-user builds occasionly crash randomly only to be fixed by a full clean re-build

2021-04-29 Thread Thomas Huth
Does this problem still persist after we've switched the build system to
meson?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  mips linux-user builds occasionly crash randomly only to be fixed by a
  full clean re-build

Status in QEMU:
  Incomplete

Bug description:
  From time to time I find check-tcg crashes with a one of the MIPS
  binaries. The last time it crashed was running the test:

./mips64el-linux-user/qemu-mips64el ./tests/tcg/mips64el-linux-
  user/threadcount

  Inevitably after some time noodling around wondering what could be
  causing this weird behaviour I wonder if it is a build issue. I wipe
  all the mips* build directories, re-run configure and re-build and
  voila problem goes away.

  It seems there must be some sort of build artefact which isn't being
  properly re-generated on a build update which causes weird problems.
  Additional data point if I:

rm -rf mips64el-linux-user
../../configure
make

  then I see failures in mip32 builds - eg:

  GEN mipsn32el-linux-user/config-target.h
In file included from /home/alex/lsrc/qemu.git/linux-user/syscall_defs.h:10,
 from /home/alex/lsrc/qemu.git/linux-user/qemu.h:16,
 from /home/alex/lsrc/qemu.git/linux-user/linuxload.c:5:
/home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h:1: error: 
unterminated #ifndef
 #ifndef LINUX_USER_MIPS64_SYSCALL_NR_H

make[1]: *** [/home/alex/lsrc/qemu.git/rules.mak:69: 
linux-user/linuxload.o] Error 1
make[1]: *** Waiting for unfinished jobs

  which implies there is a cross dependency between different targets
  somewhere. If I executed:

rm -rf mips*

  before re-configuring and re-building then everything works again.

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



Re: Let's remove some deprecated stuff

2021-04-29 Thread Daniel P . Berrangé
On Thu, Apr 29, 2021 at 11:29:42AM +0100, Peter Maydell wrote:
> On Thu, 29 Apr 2021 at 11:28, Daniel P. Berrangé  wrote:
> >
> > On Thu, Apr 29, 2021 at 12:18:42PM +0200, Gerd Hoffmann wrote:
> > >   Hi,
> > >
> > > > ``QEMU_AUDIO_`` environment variables and ``-audio-help`` (since 
> > > > 4.0)
> > > > Creating sound card devices and vnc without ``audiodev=`` property 
> > > > (since 4.2)
> > > > Creating sound card devices using ``-soundhw`` (since 5.1)
> > >
> > > I think these three should be dropped together, to minimize disruption.
> > >
> > > Where do we strand in terms of libvirt support?  IIRC audiodev= support
> > > in libvirt is rather recent (merged this year).  I'd tend to wait a bit
> > > longer because of that.
> > >
> > > Daniel?
> >
> > Libvirt added supoort for -audio in 7.2.0, release April 4th, so only
> > one month ago.
> >
> > If we drop the features in QEMU in this dev cycle though, this won't
> > impact most users until QEMU 6.1 releases in mid August. I'm perfectly
> > ok with people who use unreleased QEMU git master needing to update
> > their libvirt. The final release date is far enough away that distros
> > will have had new enough libvirt for a good while.
> 
> It does feel to me that dropping the old options now would be being
> a bit over-eager, though. The deprecation cycle time is a minimum, not
> a target :-)

Note the QEMU since has been ready since 4.0, in April 2019 so 2 years.
We dropped the ball on getting this implemented in libvirt, since we
had almost no config options for sound at all in libvirt. We had just
hardcoded 3 sound backends based on the graphics frontend.

So in terms of historic libvirt compatibility, we've only ever relied
on the QEMU_AUDIODRIVER env, none of the other million audio env vars.

IOW, if QEMU was to be conservative, you can drop all env vars except
the main QEMU_AUDIODRIVER.


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




[Bug 1863678] Re: qemu and virtio-vga black screen in Android

2021-04-29 Thread Thomas Huth
slirp is a separate project now ... if the problem persists, could you
please report this in the
https://gitlab.freedesktop.org/slirp/libslirp/-/issues bug tracker?
Thanks!

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

Title:
  qemu and virtio-vga black screen in Android

Status in QEMU:
  New

Bug description:
  QEMU emulator version 4.2.50

  kernel 5.3.0-29-generic
  host Ubuntu 19.10
  guest: Android 8.1

  While trying to compile I get the following error

  qemu/slirp/src/misc.c:146: undefined reference to
  `g_spawn_async_with_fds'

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



[Bug 1904954] Re: lan9118 bug peeked received message size not equal to actual received message size

2021-04-29 Thread Thomas Huth
** Tags removed: netwroking
** Tags added: networking

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

Title:
  lan9118 bug peeked received message size not equal to actual received
  message size

Status in QEMU:
  Fix Committed

Bug description:
  peeked message size is not equal to read message size

  Bug in the code at line:
  https://github.com/qemu/qemu/blob/master/hw/net/lan9118.c#L1209

  s->tx_status_fifo_head should be s->rx_status_fifo_head

  Could also be a security bug, as the user could allocate a buffer of
  size peeked data smaller than the actual packet received, which could
  cause a buffer overflow.

  Thanks,

  Alfred

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



Re: [PATCH v4 09/12] target/hexagon: import lexer for idef-parser

2021-04-29 Thread Paolo Montesel
On Wed, Apr 28, 2021 at 5:55 PM Taylor Simpson  wrote:
>
>
>
> >From: Paolo Montesel 
> >Sent: Wednesday, April 28, 2021 5:25 AM
> >To: Taylor Simpson 
> >Cc: Alessandro Di Federico ; qemu-devel@nongnu.org; Brian 
> >Cain ; ni...@rev.ng; >phi...@redhat.com; 
> >richard.hender...@linaro.org; Alessandro Di Federico 
> >Subject: Re: [PATCH v4 09/12] target/hexagon: import lexer for idef-parser
> >
> >Thanks for spotting this. It's actually a bug in the lexer. The token 
> >`{IMM_ID}"iV"` didn't initialize `bit_width`. Now it does. This is the 
> >>result:
> >
> >void emit_J2_jump(DisasContext *ctx, Insn *insn, Packet *pkt, int riV)
> >/* fIMMEXT(riV); (riV = riV & ~3); (PC = fREAD_PC()+riV);} */
> >{
> >int64_t qemu_tmp_0 = ~((int64_t)3ULL);
> >int32_t qemu_tmp_1 = riV & qemu_tmp_0;
> >riV = qemu_tmp_1;
> >TCGv_i32 tmp_0 = tcg_temp_local_new_i32();
> >tcg_gen_movi_i32(tmp_0, ctx->base.pc_next);
> >TCGv_i32 tmp_1 = tcg_temp_local_new_i32();
> >tcg_gen_addi_i32(tmp_1, tmp_0, (int64_t)riV);
> >tcg_temp_free_i32(tmp_0);
> >gen_write_new_pc(tmp_1);
> >tcg_temp_free_i32(tmp_1);
> >}
> >
> >The `(int64_t)riV` cast is actually useless so I simply dropped it, thanks 
> >for pointing it out.
> >
> >This is all gonna be in the next patchset ofc.
> >
> >~Paolo
>
> This could be further simplified by doing the add in the parser and generating
> TCGv tmp_1 = tcg_const_tl(ctx->base.pc_next + riV);
> Have you looked at the host code that is generated?  I would expect it to do 
> the constant folding, so the executed code is OK.  However, there's extra 
> time spent building up TCG that could be avoided.

I agree. Since relative jumps happen somewhat often, I went ahead and
added two immediate types (one for the current PC and one for next
PC).
I also noticed that we were using temps for `rvalue_materialize` when,
in fact, we can simply use `tcg_const`.
Overall it should give a nice improvement.

Anyway, this is how the code looks like now:

void emit_J2_jump(DisasContext *ctx, Insn *insn, Packet *pkt, int riV)
/* fIMMEXT(riV); (riV = riV & ~3); (PC = fREAD_PC()+riV);} */
{
int64_t qemu_tmp_0 = ~((int64_t)3ULL);
int32_t qemu_tmp_1 = riV & qemu_tmp_0;
riV = qemu_tmp_1;
int32_t qemu_tmp_2 = ctx->base.pc_next + riV;
TCGv_i32 tmp_0 = tcg_const_i32(qemu_tmp_2);
gen_write_new_pc(tmp_0);
tcg_temp_free_i32(tmp_0);
}

That doesn't look too bad (:



[Bug 1269628] Re: Feature Request: Please add TCG OPAL 2 emulation support to the virtio disk emulation

2021-04-29 Thread Thomas Huth
** Tags removed: feature request
** Tags added: feature-request

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

Title:
  Feature Request:  Please add TCG OPAL 2 emulation support to the
  virtio disk emulation

Status in QEMU:
  New

Bug description:
  In order to allow windows guests (and soon, linux guests) which are
  TCG OPAL 2 aware to perform disk encryption in a native fashion with
  hardware acceleration, please add TCG OPAL 2 emulation to the VIRTIO
  driver.

  Encryption should occur at the host level using any cryptographic
  facilities available to the host, for example AES-NI, Cryptography
  Hardware, underlying block device cryptography support where available
  or any other cryptography facility that may be developed and
  implemented in the future.

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



Re: Let's remove some deprecated stuff

2021-04-29 Thread Gerd Hoffmann
  Hi,

> IOW, if QEMU was to be conservative, you can drop all env vars except
> the main QEMU_AUDIODRIVER.

As already mentioned above I want drop all legacy audio bits at once.

Leaving in the compatibility bits in for one or two more releases is
IMHO better than removing it partly now and the remaining bits in a
year.

take care,
  Gerd




[Bug 1926596] [NEW] qemu-monitor-event command stukcs randomly

2021-04-29 Thread Ganesh Gosavi
Public bug reported:

We are using kvm virtualization on our servers, We use 
"qemu-monitor-command"(drive-backup) to take qcow2 backups and to monitor them 
we use "qemu-monitor-event" command 
For eg:-
/usr/bin/virsh qemu-monitor-event VPSNAME --event 
"BLOCK_JOB_COMPLETED\|BLOCK_JOB_ERROR" --regex

the above command stucks randomly (backup completes but still it is
waiting) and because of which other vms backup are stucked until we kill
that process.

Can you suggest how can we debug this further to find the actual issue.


/usr/bin/virsh version

Compiled against library: libvirt 4.5.0
Using library: libvirt 4.5.0
Using API: QEMU 4.5.0
Running hypervisor: QEMU 2.0.0

cat /etc/os-release
NAME="CentOS Linux"
VERSION="7 (Core)"
ID="centos"
ID_LIKE="rhel fedora"
VERSION_ID="7"
PRETTY_NAME="CentOS Linux 7 (Core)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:centos:centos:7"
HOME_URL="https://www.centos.org/";
BUG_REPORT_URL="https://bugs.centos.org/";

CENTOS_MANTISBT_PROJECT="CentOS-7"
CENTOS_MANTISBT_PROJECT_VERSION="7"
REDHAT_SUPPORT_PRODUCT="centos"
REDHAT_SUPPORT_PRODUCT_VERSION="7"

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  qemu-monitor-event command stukcs randomly

Status in QEMU:
  New

Bug description:
  We are using kvm virtualization on our servers, We use 
"qemu-monitor-command"(drive-backup) to take qcow2 backups and to monitor them 
we use "qemu-monitor-event" command 
  For eg:-
  /usr/bin/virsh qemu-monitor-event VPSNAME --event 
"BLOCK_JOB_COMPLETED\|BLOCK_JOB_ERROR" --regex

  the above command stucks randomly (backup completes but still it is
  waiting) and because of which other vms backup are stucked until we
  kill that process.

  Can you suggest how can we debug this further to find the actual
  issue.

  
  /usr/bin/virsh version

  Compiled against library: libvirt 4.5.0
  Using library: libvirt 4.5.0
  Using API: QEMU 4.5.0
  Running hypervisor: QEMU 2.0.0

  cat /etc/os-release
  NAME="CentOS Linux"
  VERSION="7 (Core)"
  ID="centos"
  ID_LIKE="rhel fedora"
  VERSION_ID="7"
  PRETTY_NAME="CentOS Linux 7 (Core)"
  ANSI_COLOR="0;31"
  CPE_NAME="cpe:/o:centos:centos:7"
  HOME_URL="https://www.centos.org/";
  BUG_REPORT_URL="https://bugs.centos.org/";

  CENTOS_MANTISBT_PROJECT="CentOS-7"
  CENTOS_MANTISBT_PROJECT_VERSION="7"
  REDHAT_SUPPORT_PRODUCT="centos"
  REDHAT_SUPPORT_PRODUCT_VERSION="7"

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



[Bug 1926044] Re: QEMU-user doesn't report HWCAP2_MTE

2021-04-29 Thread Thomas Huth
** Tags removed: user
** Tags added: linux-user

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

Title:
  QEMU-user doesn't report HWCAP2_MTE

Status in QEMU:
  In Progress

Bug description:
  Reproducible on ffa090bc56e73e287a63261e70ac02c0970be61a

  Host Debian 5.10.24 x86_64 GNU

  Configured with "configure --disable-system --enable-linux-user
  --static"

  This one works and prints "OK" as expected:
  clang tests/tcg/aarch64/mte-3.c -target aarch64-linux-gnu  -fsanitize=memtag 
-march=armv8+memtag
  qemu-aarch64 --cpu max -L /usr/aarch64-linux-gnu ./a.out && echo OK

  
  This one fails and print "0":
  cat mytest.c
  #include 
  #include 

  #ifndef HWCAP2_MTE
  #define HWCAP2_MTE (1 << 18)
  #endif

  int main(int ac, char **av)
  {
  printf("%d\n", (int)(getauxval(AT_HWCAP2) & HWCAP2_MTE));
  }

  
  clang mytest.c -target aarch64-linux-gnu  -fsanitize=memtag 
-march=armv8+memtag
  qemu-aarch64 --cpu max -L /usr/aarch64-linux-gnu ./a.out

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



[Bug 1785485] Re: Mouse moves erratically when using scroll wheel on Windows NT 4, Windows 95, and Windows 3.1 guests

2021-04-29 Thread Thomas Huth
** Tags removed: qemu-system-i386
** Tags added: i386

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

Title:
  Mouse moves erratically when using scroll wheel on Windows NT 4,
  Windows 95, and Windows 3.1 guests

Status in QEMU:
  New

Bug description:
  QEMU version: 3.0.0 RC3
  Guests: Windows NT 4.0, Windows 95, Windows 3.1

  Program: When the user uses the scroll wheel, the mouse's movement
  becomes erratic.

  This is noticed immediately when the scroll wheel is used. Sometimes
  the problem can be fixed by moving the scroll wheel some more.

  My theory is this problem is because of the lack of support for the
  Microsoft Intellimouse in these guest operating systems.

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



Re: [RFC PATCH v2 2/4] hw/arm/virt: Parse -smp cluster parameter in virt_smp_parse

2021-04-29 Thread Andrew Jones
On Thu, Apr 29, 2021 at 04:56:06PM +0800, wangyanan (Y) wrote:
> 
> On 2021/4/29 15:16, Andrew Jones wrote:
> > On Thu, Apr 29, 2021 at 10:14:37AM +0800, wangyanan (Y) wrote:
> > > On 2021/4/28 18:31, Andrew Jones wrote:
> > > > On Tue, Apr 13, 2021 at 04:31:45PM +0800, Yanan Wang wrote:
> > > > >} else if (sockets == 0) {
> > > > >threads = threads > 0 ? threads : 1;
> > > > > -sockets = cpus / (cores * threads);
> > > > > +sockets = cpus / (clusters * cores * threads);
> > > > >sockets = sockets > 0 ? sockets : 1;
> > > > If we initialize clusters to zero instead of one and add lines in
> > > > 'cpus == 0 || cores == 0' and 'sockets == 0' like
> > > > 'clusters = clusters > 0 ? clusters : 1' as needed, then I think we can
> > > > add
> > > > 
> > > >} else if (clusters == 0) {
> > > >threads = threads > 0 ? threads : 1;
> > > >clusters = cpus / (sockets * cores * thread);
> > > >clusters = clusters > 0 ? clusters : 1;
> > > >}
> > > > 
> > > > here.
> > > I have thought about this kind of format before, but there is a little bit
> > > difference between these two ways. Let's chose the better and more
> > > reasonable one of the two.
> > > 
> > > Way A currently in this patch:
> > > If value of clusters is not explicitly specified in -smp command line, we
> > > assume
> > > that users don't want to support clusters, for compatibility we 
> > > initialized
> > > the
> > > value to 1. So that with cmdline "-smp cpus=24, sockets=2, cores=6", we 
> > > will
> > > parse out the topology description like below:
> > > cpus=24, sockets=2, clusters=1, cores=6, threads=2
> > > 
> > > Way B that you suggested for this patch:
> > > Whether value of clusters is explicitly specified in -smp command line or
> > > not,
> > > we assume that clusters are supported and calculate the value. So that 
> > > with
> > > cmdline "-smp cpus=24, sockets=2, cores=6", we will parse out the topology
> > > description like below:
> > > cpus =24, sockets=2, clusters=2, cores=6, threads=1
> > > 
> > > But I think maybe we should not assume too much about what users think
> > > through the -smp command line. We should just assume that all levels of
> > > cpu topology are supported and calculate them, and users should be more
> > > careful if they want to get the expected results with not so complete
> > > cmdline.
> > > If I'm right, then Way B should be better. :)
> > > 
> > Hi Yanan,
> > 
> > We're already assuming the user wants to describe clusters to the guest
> > because we require at least one per socket. If we want the user to have a
> > choice between using clusters or not, then I guess we need to change the
> > logic in the PPTT and the cpu-map to only generate the cluster level when
> > the number of clusters is not zero. And then change this parser to not
> > require clusters at all.
> Hi Drew,
> 
> I think this kind of change will introduce more complexity and actually is
> not necessary.
> Not generating cluster level at all and generating cluster level (one per
> socket) are same
> to kernel. Without cluster description provided, kernel will initialize all
> cores in the same
> cluster which also means one cluster per socket.

Which kernel? All kernels? One without the cluster support patches [1]?

[1] 
https://lore.kernel.org/lkml/20210420001844.9116-1-song.bao@hisilicon.com/#t

> 
> So we should only ensure value of clusters per socket is one if we don't
> want to use clusters,
> and don't need to care about whether or not to generate description in PPTT
> and cpu-map.
> Is this right?

Depends on your answer to my 'which kernel' questions.

> > I'm not a big fan of these auto-calculated values either, but the
> > documentation says that it'll do that, and it's been done that way
> > forever, so I think we're stuck with it for the -smp option. Hmm, I was
> > just about to say that x86 computes all its values, but I see the most
> > recently added one, 'dies', is implemented the way you're proposing we
> > implement 'clusters', i.e. default to one and don't calculate it when it's
> > missing. I actually consider that either a documentation bug or an smp
> > parsing bug, though.
> My propose originally came from implementation of x86.
> > Another possible option, for Arm, because only the cpus and maxcpus
> > parameters of -smp have ever worked, is to document, for Arm, that if even
> > one parameter other than cpus or maxcpus is provided, then all parameters
> > must be provided. We can still decide if clusters=0 is valid, but we'll
> > enforce that everything is explicit and that the product (with or without
> > clusters) matches maxcpus.
> Requiring every parameter explicitly will be most stable but indeed strict.
> 
> Currently all the parsers use way B to calculate value of thread if it is
> not provided explicitly.
> So users should ensure the -smp cmdline they provided can result in that
> parsed threads will
> be 

[Bug 1884017] Re: Intermittently erratic mouse under Windows 95

2021-04-29 Thread Thomas Huth
** Tags removed: qemu-system-i386
** Tags added: i386

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

Title:
  Intermittently erratic mouse under Windows 95

Status in QEMU:
  New

Bug description:
  The mouse works fine maybe 75-80% of the time, but intermittently
  (every 20-30 seconds or so), moving the mouse will cause the pointer
  to fly around the screen at high speed, usually colliding with the
  edges, and much more problematically, click all the mouse buttons at
  random, even if you are not clicking. This causes random objects on
  the screen to be clicked and dragged around, rendering the system
  generally unusable.

  I don't know if this is related to #1785485 - it happens even if you
  never use the scroll wheel.

  qemu version: 5.0.0 (openSUSE Tumbleweed)

  Launch command line: qemu-system-i386 -hda win95.qcow2 -cpu pentium2
  -m 16 -vga cirrus -soundhw sb16 -nic user,model=pcnet -rtc
  base=localtime

  OS version: Windows 95 4.00.950 C

  I have made the disk image available here:
  https://home.gloveraoki.me/share/win95.qcow2.lz

  Setup notes: In order to make Windows 95 detect the system devices
  correctly, after first install you must change the driver for "Plug
  and Play BIOS" to "PCI bus". I have already done this in the above
  image.

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



Re: [PATCH] meson: change buildtype when debug_info=no

2021-04-29 Thread Paolo Bonzini

On 28/04/21 21:55, Joelle van Dyne wrote:

Meson defaults builds to 'debugoptimized' which adds '-g -O2'
to CFLAGS. If the user specifies '--disable-debug-info' we
should instead build with 'release' which does not emit any
debug info.

Signed-off-by: Joelle van Dyne 


This is not needed.  buildtype is a shortcut for the optimization and
debug options, we need not bother with it because configure sets the
individual options already:

-Doptimization=$(if test "$debug" = yes; then echo 0; else echo 2; fi) \
-Ddebug=$(if test "$debug_info" = yes; then echo true; else echo false; 
fi) \

Paolo


---
  configure | 1 +
  1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 4f374b4889..5c3568cbc3 100755
--- a/configure
+++ b/configure
@@ -6398,6 +6398,7 @@ NINJA=$ninja $meson setup \
  --sysconfdir "$sysconfdir" \
  --localedir "$localedir" \
  --localstatedir "$local_statedir" \
+--buildtype $(if test "$debug_info" = yes; then echo "debugoptimized"; else echo 
"release"; fi) \
  -Ddocdir="$docdir" \
  -Dqemu_firmwarepath="$firmwarepath" \
  -Dqemu_suffix="$qemu_suffix" \






Re: Let's remove some deprecated stuff

2021-04-29 Thread Paolo Bonzini

On 29/04/21 12:35, Daniel P. Berrangé wrote:

Note the QEMU since has been ready since 4.0, in April 2019 so 2 years.
We dropped the ball on getting this implemented in libvirt, since we
had almost no config options for sound at all in libvirt. We had just
hardcoded 3 sound backends based on the graphics frontend.

So in terms of historic libvirt compatibility, we've only ever relied
on the QEMU_AUDIODRIVER env, none of the other million audio env vars.

IOW, if QEMU was to be conservative, you can drop all env vars except
the main QEMU_AUDIODRIVER.


I like that, and keeping -soundhw as well until an audiodev-based 
replacement is there.


Paolo




Re: Let's remove some deprecated stuff

2021-04-29 Thread Paolo Bonzini

On 29/04/21 11:59, Markus Armbruster wrote:

Gerd Hoffmann:

 Creating sound card devices using ``-soundhw`` (since 5.1)
 ''

 Sound card devices should be created using ``-device`` instead.  The
 names are the same for most devices.  The exceptions are ``hda`` which
 needs two devices (``-device intel-hda -device hda-duplex``) and
 ``pcspk`` which can be activated using ``-machine
 pcspk-audiodev=``.


For this to go away, I'd rather have something like the -nic option that 
provides an easy way to set up the front end and back end.


In other words you would do something like -audiohw 
,model=xxx and it gets desugared automatically to either


   -audiodev ,id=foo -device devname,audiodev=xxx

or

   -audiodev ,id=foo -M propname=foo

Paolo




[Bug 1734474] Re: Maemo does not boot on emulated N800

2021-04-29 Thread Thomas Huth
Yes, I think we can close this now.

** Changed in: qemu
   Status: In Progress => Fix Released

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

Title:
  Maemo does not boot on emulated N800

Status in QEMU:
  Fix Released

Bug description:
  I start QEMU with qemu-system-arm-m 130 -M n800 -kernel zImage.1 -mtdblock 
maemo.img -append "root=/dev/mtdblock3 rootfstype=jffs2"
  On QEMU 1.2.0 see "NOKIA" logo and then desktop appears, but on 1.5.0 and 
newer (including latest versions) I see only white screen and no signs of life. 
Was this caused by regression or any syntax change?

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



Re: Let's remove some deprecated stuff

2021-04-29 Thread Thomas Huth

On 29/04/2021 11.59, Markus Armbruster wrote:

If you're cc'ed, you added a section to docs/system/deprecated.rst that
is old enough to permit removal.  This is *not* a demand to remove, it's
a polite request to consider whether the time for removal has come.
Extra points for telling us in a reply.  "We should remove, but I can't
do it myself right now" is a valid answer.  Let's review the file:

[...]

Thomas Huth:

 ``moxie`` CPU (since 5.2.0)
 '''

 The ``moxie`` guest CPU support is deprecated and will be removed in
 a future version of QEMU. It's unclear whether anybody is still using
 CPU emulation in QEMU, and there are no test images available to make
 sure that the code is still working.


I'm fine with dropping moxie now - I've never seen anybody using it and I've 
never spotted any binaries in the internet that could still be used for 
regression testing of this target. And I've also put Anthony Green on CC: 
when I suggested the deprecation and he never replied. So I think it's 
really completely unused.



 ``lm32`` CPUs (since 5.2.0)
 '''

 The ``lm32`` guest CPU support is deprecated and will be removed in
 a future version of QEMU. The only public user of this architecture
 was the milkymist project, which has been dead for years; there was
 never an upstream Linux port.

 ``unicore32`` CPUs (since 5.2.0)
 

 The ``unicore32`` guest CPU support is deprecated and will be removed in
 a future version of QEMU. Support for this CPU was removed from the
 upstream Linux kernel, and there is no available upstream toolchain
 to build binaries for it.


I didn't add these two entries to the deprecation list, I just moved them 
around since they were in the wrong section. Both have been added by Peter 
instead (commit d8498005122 and 8e4ff4a8d2b)


 Thomas




Re: Let's remove some deprecated stuff

2021-04-29 Thread Peter Maydell
On Thu, 29 Apr 2021 at 12:19, Thomas Huth  wrote:
>
> On 29/04/2021 11.59, Markus Armbruster wrote:
> > If you're cc'ed, you added a section to docs/system/deprecated.rst that
> > is old enough to permit removal.  This is *not* a demand to remove, it's
> > a polite request to consider whether the time for removal has come.
> > Extra points for telling us in a reply.  "We should remove, but I can't
> > do it myself right now" is a valid answer.  Let's review the file:
> [...]
> > Thomas Huth:
> >
> >  ``moxie`` CPU (since 5.2.0)
> >  '''
> >
> >  The ``moxie`` guest CPU support is deprecated and will be removed in
> >  a future version of QEMU. It's unclear whether anybody is still using
> >  CPU emulation in QEMU, and there are no test images available to make
> >  sure that the code is still working.
>
> I'm fine with dropping moxie now - I've never seen anybody using it and I've
> never spotted any binaries in the internet that could still be used for
> regression testing of this target. And I've also put Anthony Green on CC:
> when I suggested the deprecation and he never replied. So I think it's
> really completely unused.
>
> >  ``lm32`` CPUs (since 5.2.0)
> >  '''
> >
> >  The ``lm32`` guest CPU support is deprecated and will be removed in
> >  a future version of QEMU. The only public user of this architecture
> >  was the milkymist project, which has been dead for years; there was
> >  never an upstream Linux port.
> >
> >  ``unicore32`` CPUs (since 5.2.0)
> >  
> >
> >  The ``unicore32`` guest CPU support is deprecated and will be removed 
> > in
> >  a future version of QEMU. Support for this CPU was removed from the
> >  upstream Linux kernel, and there is no available upstream toolchain
> >  to build binaries for it.
>
> I didn't add these two entries to the deprecation list, I just moved them
> around since they were in the wrong section. Both have been added by Peter
> instead (commit d8498005122 and 8e4ff4a8d2b)

Yes, I think moxie, lm32 and unicore32 are all OK to drop now.

-- PMM



[Bug 1824778] Re: PowerPC64: tlbivax does not work for addresses above 4G

2021-04-29 Thread Thomas Huth
This is an automated cleanup. This bug report has been moved
to QEMU's new bug tracker on gitlab.com and thus gets marked
as 'expired' now. Please continue with the discussion here:

 https://gitlab.com/qemu-project/qemu/-/issues/52


** Changed in: qemu
   Status: New => Expired

** Bug watch added: gitlab.com/qemu-project/qemu/-/issues #52
   https://gitlab.com/qemu-project/qemu/-/issues/52

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

Title:
  PowerPC64: tlbivax does not work for addresses above 4G

Status in QEMU:
  Expired

Bug description:
  The tlbivax instruction in QEMU does not work for address above 4G. The 
reason behind this is a simple 32bit trunction of an address.
  Changing the argument ea from uint32_t to target_ulong for the function 
booke206_invalidate_ea_tlb() in target/ppc/mmu_helper.c solves the issue.

  I did not reproduce this using Linux so I have no public example for
  reproducing it. However it's a pretty straight forward change.

  Issue can be seen in all version of QEMU.

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



[PATCH v5 00/10] numa/exec/migration: Fix resizing RAM blocks while migrating

2021-04-29 Thread David Hildenbrand
v4 has been floating around for a while. Let's see if we can find someone
to merge this; or at least give some more feedback ... all patches have
at least one RB.


I realized that resizing RAM blocks while the guest is being migrated
(precopy: resize while still running on the source, postcopy: resize
 while already running on the target) is buggy. In case of precopy, we
can simply cancel migration. Postcopy handling is more involved. Resizing
can currently happen during a guest reboot, triggered by ACPI rebuilds.

Along with the fixes, some cleanups.

--

Example to highlight one part of the problem:

1. Start a paused VM (where a ramblock resize will trigger when booting):
  sudo build/qemu-system-x86_64 \
   --enable-kvm \
   -S \
   -machine q35,nvdimm=on \
   -smp 1 \
   -cpu host \
   -m size=20G,slots=8,maxmem=22G \
   -object memory-backend-file,id=mem0,mem-path=/tmp/nvdimm,size=256M \
   -device nvdimm,label-size=131072,memdev=mem0,id=nvdimm0,slot=1 \
   -nodefaults \
   -chardev stdio,nosignal,id=serial \
   -device isa-serial,chardev=serial \
   -chardev socket,id=monitor,path=/var/tmp/monitor,server,nowait \
   -mon chardev=monitor,mode=readline \
   -device vmgenid \
   -device intel-iommu \
   -nographic

2. Starting precopy and then starting the VM to trigger resizing during
   precopy:
  QEMU 5.2.95 monitor - type 'help' for more information
  (qemu) migrate -d "exec:gzip -c > STATEFILE.gz"
  QEMU 5.2.95 monitor - type 'help' for more information
  (qemu) cont

3a. Before this series, migration never completes:
  QEMU 5.2.95 monitor - type 'help' for more information
  (qemu) info migrate
  globals:
  store-global-state: on
  only-migratable: off
  send-configuration: on
  send-section-footer: on
  decompress-error-check: on
  clear-bitmap-shift: 18
  Migration status: active
  total time: 43826 ms
  expected downtime: 300 ms
  setup: 5 ms
  transferred ram: 65981 kbytes
  throughput: 8.27 mbps
  remaining ram: 18446744073709551612 kbytes
  total ram: 21234188 kbytes
  duplicate: 5308454 pages
  skipped: 0 pages
  normal: 93 pages
  normal bytes: 372 kbytes
  dirty sync count: 1
  page size: 4 kbytes
  multifd bytes: 0 kbytes
  pages-per-second: 0

4. With this change, migration is properly aborted:
  (qemu) info migrate
  globals:
  store-global-state: on
  only-migratable: off
  send-configuration: on
  send-section-footer: on
  decompress-error-check: on
  clear-bitmap-shift: 18
  Migration status: cancelled
  total time: 0 ms

--

Cc: Eduardo Habkost 
Cc: Marcel Apfelbaum 
Cc: "Michael S. Tsirkin" 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Juan Quintela 
Cc: "Dr. David Alan Gilbert" 
Cc: Peter Xu 
Cc: Alex Williamson 

v4 -> v5:
- Rephrased some patch descriptions
- Dropped some patches to reduce the footprint
-- "stubs/ram-block: Remove stubs that are no longer needed"
-- "migration/ram: Tolerate partially changed mappings in postcopy code"
- Removed as already upstream now
-- "migration/ram: Consolidate variable reset after placement in
ram_load_postcopy()"

v3 -> v4:
- Rebased and retested
- Added RBs

v2 -> v3:
- Rebased on current master
- Added RBs
- "migration/ram: Tolerate partially changed mappings in postcopy code"
-- Extended the comment for the uffdio unregister part.

v1 -> v2:
- "util: vfio-helpers: Factor out and fix processing of existing ram
   blocks"
-- Stringify error
- "migraton/ram: Handle RAM block resizes during precopy"
-- Simplified check if we're migrating on the source
- "exec: Relax range check in ram_block_discard_range()"
-- Added to make discard during resizes actually work
- "migration/ram: Discard new RAM when growing RAM blocks after
   ram_postcopy_incoming_init()"
-- Better checks if in the right postcopy mode.
-- Better patch subject/description/comments
- "migration/ram: Handle RAM block resizes during postcopy"
-- Better comments
-- Adapt to changed postcopy checks
- "migrate/ram: Get rid of "place_source" in ram_load_postcopy()"
-- Dropped, as broken
- "migration/ram: Tolerate partially changed mappings in postcopy code"
-- Better comment / description. Clarify that no implicit wakeup will
   happen
-- Warn on EINVAL (older kernels)
-- Wake up any waiter explicitly

David Hildenbrand (10):
  util: vfio-helpers: Factor out and fix processing of existing ram
blocks
  numa: Teach ram block notifiers about resizeable ram blocks
  numa: Make all callbacks of ram block notifiers optional
  migration/ram: Handle RAM block resizes during precopy
  exec: Relax range check in ram_block_discard_range()
  migration/ram: Discard RAM when growing RAM blocks after
ram_postcopy_incoming_init()
  migration/ram: Simplify host page handling in ram_load_postcopy()
  migration/ram: Handle RAM block resizes during postcopy
  migration/multifd: Pri

[PATCH v5 03/10] numa: Make all callbacks of ram block notifiers optional

2021-04-29 Thread David Hildenbrand
Let's make add/remove optional. We want to introduce a RAM block
notifier for RAM migration that is only interested in resize events.

Reviewed-by: Peter Xu 
Signed-off-by: David Hildenbrand 
---
 hw/core/numa.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/core/numa.c b/hw/core/numa.c
index 921bf86ab4..64b9334050 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -819,8 +819,11 @@ static int ram_block_notify_add_single(RAMBlock *rb, void 
*opaque)
 void ram_block_notifier_add(RAMBlockNotifier *n)
 {
 QLIST_INSERT_HEAD(&ram_list.ramblock_notifiers, n, next);
+
 /* Notify about all existing ram blocks. */
-qemu_ram_foreach_block(ram_block_notify_add_single, n);
+if (n->ram_block_added) {
+qemu_ram_foreach_block(ram_block_notify_add_single, n);
+}
 }
 
 void ram_block_notifier_remove(RAMBlockNotifier *n)
@@ -833,7 +836,9 @@ void ram_block_notify_add(void *host, size_t size, size_t 
max_size)
 RAMBlockNotifier *notifier;
 
 QLIST_FOREACH(notifier, &ram_list.ramblock_notifiers, next) {
-notifier->ram_block_added(notifier, host, size, max_size);
+if (notifier->ram_block_added) {
+notifier->ram_block_added(notifier, host, size, max_size);
+}
 }
 }
 
@@ -842,7 +847,9 @@ void ram_block_notify_remove(void *host, size_t size, 
size_t max_size)
 RAMBlockNotifier *notifier;
 
 QLIST_FOREACH(notifier, &ram_list.ramblock_notifiers, next) {
-notifier->ram_block_removed(notifier, host, size, max_size);
+if (notifier->ram_block_removed) {
+notifier->ram_block_removed(notifier, host, size, max_size);
+}
 }
 }
 
-- 
2.30.2




[PATCH v5 07/10] migration/ram: Simplify host page handling in ram_load_postcopy()

2021-04-29 Thread David Hildenbrand
Add two new helper functions. This will come in come handy once we want to
handle ram block resizes while postcopy is active.

Note that ram_block_from_stream() will already print proper errors.

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: David Hildenbrand 
---
 migration/ram.c | 55 -
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 693851d7a0..72df5228ee 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3121,6 +3121,20 @@ static inline void *host_from_ram_block_offset(RAMBlock 
*block,
 return block->host + offset;
 }
 
+static void *host_page_from_ram_block_offset(RAMBlock *block,
+ ram_addr_t offset)
+{
+/* Note: Explicitly no check against offset_in_ramblock(). */
+return (void *)QEMU_ALIGN_DOWN((uintptr_t)block->host + offset,
+   block->page_size);
+}
+
+static ram_addr_t host_page_offset_from_ram_block_offset(RAMBlock *block,
+ ram_addr_t offset)
+{
+return ((uintptr_t)block->host + offset) & (block->page_size - 1);
+}
+
 static inline void *colo_cache_from_block_offset(RAMBlock *block,
  ram_addr_t offset, bool record_bitmap)
 {
@@ -3524,13 +3538,12 @@ static int ram_load_postcopy(QEMUFile *f)
 MigrationIncomingState *mis = migration_incoming_get_current();
 /* Temporary page that is later 'placed' */
 void *postcopy_host_page = mis->postcopy_tmp_page;
-void *this_host = NULL;
+void *host_page = NULL;
 bool all_zero = true;
 int target_pages = 0;
 
 while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
 ram_addr_t addr;
-void *host = NULL;
 void *page_buffer = NULL;
 void *place_source = NULL;
 RAMBlock *block = NULL;
@@ -3555,9 +3568,12 @@ static int ram_load_postcopy(QEMUFile *f)
 if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
  RAM_SAVE_FLAG_COMPRESS_PAGE)) {
 block = ram_block_from_stream(f, flags);
+if (!block) {
+ret = -EINVAL;
+break;
+}
 
-host = host_from_ram_block_offset(block, addr);
-if (!host) {
+if (!offset_in_ramblock(block, addr)) {
 error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
 ret = -EINVAL;
 break;
@@ -3575,19 +3591,17 @@ static int ram_load_postcopy(QEMUFile *f)
  * of a host page in one chunk.
  */
 page_buffer = postcopy_host_page +
-  ((uintptr_t)host & (block->page_size - 1));
+  host_page_offset_from_ram_block_offset(block, addr);
+/* If all TP are zero then we can optimise the place */
 if (target_pages == 1) {
-this_host = (void *)QEMU_ALIGN_DOWN((uintptr_t)host,
-block->page_size);
-} else {
+host_page = host_page_from_ram_block_offset(block, addr);
+} else if (host_page != host_page_from_ram_block_offset(block,
+addr)) {
 /* not the 1st TP within the HP */
-if (QEMU_ALIGN_DOWN((uintptr_t)host, block->page_size) !=
-(uintptr_t)this_host) {
-error_report("Non-same host page %p/%p",
-  host, this_host);
-ret = -EINVAL;
-break;
-}
+error_report("Non-same host page %p/%p", host_page,
+ host_page_from_ram_block_offset(block, addr));
+ret = -EINVAL;
+break;
 }
 
 /*
@@ -3666,16 +3680,11 @@ static int ram_load_postcopy(QEMUFile *f)
 }
 
 if (!ret && place_needed) {
-/* This gets called at the last target page in the host page */
-void *place_dest = (void *)QEMU_ALIGN_DOWN((uintptr_t)host,
-   block->page_size);
-
 if (all_zero) {
-ret = postcopy_place_page_zero(mis, place_dest,
-   block);
+ret = postcopy_place_page_zero(mis, host_page, block);
 } else {
-ret = postcopy_place_page(mis, place_dest,
-  place_source, block);
+ret = postcopy_place_page(mis, host_page, place_source,
+  block);
 }
 place_needed = false;
 target_pages = 0;
-- 
2.30.2




[PATCH v5 01/10] util: vfio-helpers: Factor out and fix processing of existing ram blocks

2021-04-29 Thread David Hildenbrand
Factor it out into common code when a new notifier is registered, just
as done with the memory region notifier. This keeps logic about how to
process existing ram blocks at a central place.

Just like when adding a new ram block, we have to register the max_length.
Ram blocks are only "fake resized". All memory (max_length) is mapped.

Print the warning from inside qemu_vfio_ram_block_added().

Reviewed-by: Peter Xu 
Signed-off-by: David Hildenbrand 
---
 hw/core/numa.c| 14 ++
 include/exec/cpu-common.h |  1 +
 softmmu/physmem.c |  5 +
 util/vfio-helpers.c   | 29 -
 4 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/hw/core/numa.c b/hw/core/numa.c
index 68cee65f61..7f08c27a6d 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -803,9 +803,23 @@ void query_numa_node_mem(NumaNodeMem node_mem[], 
MachineState *ms)
 }
 }
 
+static int ram_block_notify_add_single(RAMBlock *rb, void *opaque)
+{
+const ram_addr_t max_size = qemu_ram_get_max_length(rb);
+void *host = qemu_ram_get_host_addr(rb);
+RAMBlockNotifier *notifier = opaque;
+
+if (host) {
+notifier->ram_block_added(notifier, host, max_size);
+}
+return 0;
+}
+
 void ram_block_notifier_add(RAMBlockNotifier *n)
 {
 QLIST_INSERT_HEAD(&ram_list.ramblock_notifiers, n, next);
+/* Notify about all existing ram blocks. */
+qemu_ram_foreach_block(ram_block_notify_add_single, n);
 }
 
 void ram_block_notifier_remove(RAMBlockNotifier *n)
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 5a0a2d93e0..ccabed4003 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -57,6 +57,7 @@ const char *qemu_ram_get_idstr(RAMBlock *rb);
 void *qemu_ram_get_host_addr(RAMBlock *rb);
 ram_addr_t qemu_ram_get_offset(RAMBlock *rb);
 ram_addr_t qemu_ram_get_used_length(RAMBlock *rb);
+ram_addr_t qemu_ram_get_max_length(RAMBlock *rb);
 bool qemu_ram_is_shared(RAMBlock *rb);
 bool qemu_ram_is_uf_zeroable(RAMBlock *rb);
 void qemu_ram_set_uf_zeroable(RAMBlock *rb);
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 85034d9c11..bd2c0dc4ec 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -1697,6 +1697,11 @@ ram_addr_t qemu_ram_get_used_length(RAMBlock *rb)
 return rb->used_length;
 }
 
+ram_addr_t qemu_ram_get_max_length(RAMBlock *rb)
+{
+return rb->max_length;
+}
+
 bool qemu_ram_is_shared(RAMBlock *rb)
 {
 return rb->flags & RAM_SHARED;
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 97dfa3fd57..92b9565797 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -463,8 +463,14 @@ static void qemu_vfio_ram_block_added(RAMBlockNotifier *n,
   void *host, size_t size)
 {
 QEMUVFIOState *s = container_of(n, QEMUVFIOState, ram_notifier);
+int ret;
+
 trace_qemu_vfio_ram_block_added(s, host, size);
-qemu_vfio_dma_map(s, host, size, false, NULL);
+ret = qemu_vfio_dma_map(s, host, size, false, NULL);
+if (ret) {
+error_report("qemu_vfio_dma_map(%p, %zu) failed: %s", host, size,
+ strerror(-ret));
+}
 }
 
 static void qemu_vfio_ram_block_removed(RAMBlockNotifier *n,
@@ -477,33 +483,14 @@ static void qemu_vfio_ram_block_removed(RAMBlockNotifier 
*n,
 }
 }
 
-static int qemu_vfio_init_ramblock(RAMBlock *rb, void *opaque)
-{
-void *host_addr = qemu_ram_get_host_addr(rb);
-ram_addr_t length = qemu_ram_get_used_length(rb);
-int ret;
-QEMUVFIOState *s = opaque;
-
-if (!host_addr) {
-return 0;
-}
-ret = qemu_vfio_dma_map(s, host_addr, length, false, NULL);
-if (ret) {
-fprintf(stderr, "qemu_vfio_init_ramblock: failed %p %" PRId64 "\n",
-host_addr, (uint64_t)length);
-}
-return 0;
-}
-
 static void qemu_vfio_open_common(QEMUVFIOState *s)
 {
 qemu_mutex_init(&s->lock);
 s->ram_notifier.ram_block_added = qemu_vfio_ram_block_added;
 s->ram_notifier.ram_block_removed = qemu_vfio_ram_block_removed;
-ram_block_notifier_add(&s->ram_notifier);
 s->low_water_mark = QEMU_VFIO_IOVA_MIN;
 s->high_water_mark = QEMU_VFIO_IOVA_MAX;
-qemu_ram_foreach_block(qemu_vfio_init_ramblock, s);
+ram_block_notifier_add(&s->ram_notifier);
 }
 
 /**
-- 
2.30.2




[PATCH v5 08/10] migration/ram: Handle RAM block resizes during postcopy

2021-04-29 Thread David Hildenbrand
Resizing while migrating is dangerous and does not work as expected.
The whole migration code works with the usable_length of a ram block and
does not expect this value to change at random points in time.

In the case of postcopy, relying on used_length is racy as soon as the
guest is running. Also, when used_length changes we might leave the
uffd handler registered for some memory regions, reject valid pages
when migrating and fail when sending the recv bitmap to the source.

Resizing can be trigger *after* (but not during) a reset in
ACPI code by the guest
- hw/arm/virt-acpi-build.c:acpi_ram_update()
- hw/i386/acpi-build.c:acpi_ram_update()

Let's remember the original used_length in a separate variable and
use it in relevant postcopy code. Make sure to update it when we resize
during precopy, when synchronizing the RAM block sizes with the source.

Reviewed-by: Peter Xu 
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: David Hildenbrand 
---
 include/exec/ramblock.h  | 10 ++
 migration/postcopy-ram.c | 15 ---
 migration/ram.c  | 11 +--
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
index 07d50864d8..664701b759 100644
--- a/include/exec/ramblock.h
+++ b/include/exec/ramblock.h
@@ -59,6 +59,16 @@ struct RAMBlock {
  */
 unsigned long *clear_bmap;
 uint8_t clear_bmap_shift;
+
+/*
+ * RAM block length that corresponds to the used_length on the migration
+ * source (after RAM block sizes were synchronized). Especially, after
+ * starting to run the guest, used_length and postcopy_length can differ.
+ * Used to register/unregister uffd handlers and as the size of the 
received
+ * bitmap. Receiving any page beyond this length will bail out, as it
+ * could not have been valid on the source.
+ */
+ram_addr_t postcopy_length;
 };
 #endif
 #endif
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index ab482adef1..2e9697bdd2 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -17,6 +17,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/rcu.h"
 #include "exec/target_page.h"
 #include "migration.h"
 #include "qemu-file.h"
@@ -30,6 +31,7 @@
 #include "qemu/error-report.h"
 #include "trace.h"
 #include "hw/boards.h"
+#include "exec/ramblock.h"
 
 /* Arbitrary limit on size of each discard command,
  * keeps them around ~200 bytes
@@ -452,6 +454,13 @@ static int init_range(RAMBlock *rb, void *opaque)
 ram_addr_t length = qemu_ram_get_used_length(rb);
 trace_postcopy_init_range(block_name, host_addr, offset, length);
 
+/*
+ * Save the used_length before running the guest. In case we have to
+ * resize RAM blocks when syncing RAM block sizes from the source during
+ * precopy, we'll update it manually via the ram block notifier.
+ */
+rb->postcopy_length = length;
+
 /*
  * We need the whole of RAM to be truly empty for postcopy, so things
  * like ROMs and any data tables built during init must be zero'd
@@ -474,7 +483,7 @@ static int cleanup_range(RAMBlock *rb, void *opaque)
 const char *block_name = qemu_ram_get_idstr(rb);
 void *host_addr = qemu_ram_get_host_addr(rb);
 ram_addr_t offset = qemu_ram_get_offset(rb);
-ram_addr_t length = qemu_ram_get_used_length(rb);
+ram_addr_t length = rb->postcopy_length;
 MigrationIncomingState *mis = opaque;
 struct uffdio_range range_struct;
 trace_postcopy_cleanup_range(block_name, host_addr, offset, length);
@@ -580,7 +589,7 @@ static int nhp_range(RAMBlock *rb, void *opaque)
 const char *block_name = qemu_ram_get_idstr(rb);
 void *host_addr = qemu_ram_get_host_addr(rb);
 ram_addr_t offset = qemu_ram_get_offset(rb);
-ram_addr_t length = qemu_ram_get_used_length(rb);
+ram_addr_t length = rb->postcopy_length;
 trace_postcopy_nhp_range(block_name, host_addr, offset, length);
 
 /*
@@ -624,7 +633,7 @@ static int ram_block_enable_notify(RAMBlock *rb, void 
*opaque)
 struct uffdio_register reg_struct;
 
 reg_struct.range.start = (uintptr_t)qemu_ram_get_host_addr(rb);
-reg_struct.range.len = qemu_ram_get_used_length(rb);
+reg_struct.range.len = rb->postcopy_length;
 reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING;
 
 /* Now tell our userfault_fd that it's responsible for this area */
diff --git a/migration/ram.c b/migration/ram.c
index 72df5228ee..6d4d13d345 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -243,7 +243,7 @@ int64_t ramblock_recv_bitmap_send(QEMUFile *file,
 return -1;
 }
 
-nbits = block->used_length >> TARGET_PAGE_BITS;
+nbits = block->postcopy_length >> TARGET_PAGE_BITS;
 
 /*
  * Make sure the tmp bitmap buffer is big enough, e.g., on 32bit
@@ -3573,7 +3573,13 @@ static int ram_load_postcopy(QEMUFile *f)
 break;
 }
 
-if (!offset_in_ramblock(block, addr)) {
+/*
+   

[PATCH v5 02/10] numa: Teach ram block notifiers about resizeable ram blocks

2021-04-29 Thread David Hildenbrand
Ram block notifiers are currently not aware of resizes. To properly
handle resizes during migration, we want to teach ram block notifiers about
resizeable ram.

Introduce the basic infrastructure but keep using max_size in the
existing notifiers. Supply the max_size when adding and removing ram
blocks. Also, notify on resizes.

Acked-by: Paul Durrant 
Reviewed-by: Peter Xu 
Cc: xen-de...@lists.xenproject.org
Cc: haxm-t...@intel.com
Cc: Paul Durrant 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Wenchao Wang 
Cc: Colin Xu 
Signed-off-by: David Hildenbrand 
---
 hw/core/numa.c | 22 +-
 hw/i386/xen/xen-mapcache.c |  7 ---
 include/exec/ramlist.h | 13 +
 softmmu/physmem.c  | 12 ++--
 target/i386/hax/hax-mem.c  |  5 +++--
 target/i386/sev.c  | 18 ++
 util/vfio-helpers.c| 16 
 7 files changed, 61 insertions(+), 32 deletions(-)

diff --git a/hw/core/numa.c b/hw/core/numa.c
index 7f08c27a6d..921bf86ab4 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -806,11 +806,12 @@ void query_numa_node_mem(NumaNodeMem node_mem[], 
MachineState *ms)
 static int ram_block_notify_add_single(RAMBlock *rb, void *opaque)
 {
 const ram_addr_t max_size = qemu_ram_get_max_length(rb);
+const ram_addr_t size = qemu_ram_get_used_length(rb);
 void *host = qemu_ram_get_host_addr(rb);
 RAMBlockNotifier *notifier = opaque;
 
 if (host) {
-notifier->ram_block_added(notifier, host, max_size);
+notifier->ram_block_added(notifier, host, size, max_size);
 }
 return 0;
 }
@@ -827,20 +828,31 @@ void ram_block_notifier_remove(RAMBlockNotifier *n)
 QLIST_REMOVE(n, next);
 }
 
-void ram_block_notify_add(void *host, size_t size)
+void ram_block_notify_add(void *host, size_t size, size_t max_size)
 {
 RAMBlockNotifier *notifier;
 
 QLIST_FOREACH(notifier, &ram_list.ramblock_notifiers, next) {
-notifier->ram_block_added(notifier, host, size);
+notifier->ram_block_added(notifier, host, size, max_size);
 }
 }
 
-void ram_block_notify_remove(void *host, size_t size)
+void ram_block_notify_remove(void *host, size_t size, size_t max_size)
 {
 RAMBlockNotifier *notifier;
 
 QLIST_FOREACH(notifier, &ram_list.ramblock_notifiers, next) {
-notifier->ram_block_removed(notifier, host, size);
+notifier->ram_block_removed(notifier, host, size, max_size);
+}
+}
+
+void ram_block_notify_resize(void *host, size_t old_size, size_t new_size)
+{
+RAMBlockNotifier *notifier;
+
+QLIST_FOREACH(notifier, &ram_list.ramblock_notifiers, next) {
+if (notifier->ram_block_resized) {
+notifier->ram_block_resized(notifier, host, old_size, new_size);
+}
 }
 }
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
index 5b120ed44b..d6dcea65d1 100644
--- a/hw/i386/xen/xen-mapcache.c
+++ b/hw/i386/xen/xen-mapcache.c
@@ -169,7 +169,8 @@ static void xen_remap_bucket(MapCacheEntry *entry,
 
 if (entry->vaddr_base != NULL) {
 if (!(entry->flags & XEN_MAPCACHE_ENTRY_DUMMY)) {
-ram_block_notify_remove(entry->vaddr_base, entry->size);
+ram_block_notify_remove(entry->vaddr_base, entry->size,
+entry->size);
 }
 if (munmap(entry->vaddr_base, entry->size) != 0) {
 perror("unmap fails");
@@ -211,7 +212,7 @@ static void xen_remap_bucket(MapCacheEntry *entry,
 }
 
 if (!(entry->flags & XEN_MAPCACHE_ENTRY_DUMMY)) {
-ram_block_notify_add(vaddr_base, size);
+ram_block_notify_add(vaddr_base, size, size);
 }
 
 entry->vaddr_base = vaddr_base;
@@ -452,7 +453,7 @@ static void xen_invalidate_map_cache_entry_unlocked(uint8_t 
*buffer)
 }
 
 pentry->next = entry->next;
-ram_block_notify_remove(entry->vaddr_base, entry->size);
+ram_block_notify_remove(entry->vaddr_base, entry->size, entry->size);
 if (munmap(entry->vaddr_base, entry->size) != 0) {
 perror("unmap fails");
 exit(-1);
diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
index 26704aa3b0..ece6497ee2 100644
--- a/include/exec/ramlist.h
+++ b/include/exec/ramlist.h
@@ -65,15 +65,20 @@ void qemu_mutex_lock_ramlist(void);
 void qemu_mutex_unlock_ramlist(void);
 
 struct RAMBlockNotifier {
-void (*ram_block_added)(RAMBlockNotifier *n, void *host, size_t size);
-void (*ram_block_removed)(RAMBlockNotifier *n, void *host, size_t size);
+void (*ram_block_added)(RAMBlockNotifier *n, void *host, size_t size,
+size_t max_size);
+void (*ram_block_removed)(RAMBlockNotifier *n, void *host, size_t size,
+  size_t max_size);
+void (*ram_block_resized)(RAMBlockNotifier *n, void *host, size_t old_size,
+  size_t new_size);
 QLIST_ENTRY(RAMBlockNotifier) next;
 };
 
 void ram_block_notifier_add(RAMBlockNotifier *n);
 vo

[PATCH v5 04/10] migration/ram: Handle RAM block resizes during precopy

2021-04-29 Thread David Hildenbrand
Resizing while migrating is dangerous and does not work as expected.
The whole migration code works on the usable_length of ram blocks and does
not expect this to change at random points in time.

In the case of precopy, the ram block size must not change on the source,
after syncing the RAM block list in ram_save_setup(), so as long as the
guest is still running on the source.

Resizing can be trigger *after* (but not during) a reset in
ACPI code by the guest
- hw/arm/virt-acpi-build.c:acpi_ram_update()
- hw/i386/acpi-build.c:acpi_ram_update()

Use the ram block notifier to get notified about resizes. Let's simply
cancel migration and indicate the reason. We'll continue running on the
source. No harm done.

Update the documentation. Postcopy will be handled separately.

Reviewed-by: Peter Xu 
Signed-off-by: David Hildenbrand 
---
 include/exec/memory.h | 10 ++
 migration/migration.c |  9 +++--
 migration/migration.h |  1 +
 migration/ram.c   | 31 +++
 softmmu/physmem.c |  5 +++--
 5 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 5728a681b2..c8b9088924 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -131,7 +131,7 @@ typedef struct IOMMUTLBEvent {
 #define RAM_SHARED (1 << 1)
 
 /* Only a portion of RAM (used_length) is actually used, and migrated.
- * This used_length size can change across reboots.
+ * Resizing RAM while migrating can result in the migration being canceled.
  */
 #define RAM_RESIZEABLE (1 << 2)
 
@@ -955,7 +955,9 @@ void memory_region_init_ram_shared_nomigrate(MemoryRegion 
*mr,
  * RAM.  Accesses into the region will
  * modify memory directly.  Only an initial
  * portion of this RAM is actually used.
- * The used size can change across reboots.
+ * Changing the size while migrating
+ * can result in the migration being
+ * canceled.
  *
  * @mr: the #MemoryRegion to be initialized.
  * @owner: the object that tracks the region's reference count
@@ -1586,8 +1588,8 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr);
 
 /* memory_region_ram_resize: Resize a RAM region.
  *
- * Only legal before guest might have detected the memory size: e.g. on
- * incoming migration, or right after reset.
+ * Resizing RAM while migrating can result in the migration being canceled.
+ * Care has to be taken if the guest might have already detected the memory.
  *
  * @mr: a memory region created with @memory_region_init_resizeable_ram.
  * @newsize: the new size the region
diff --git a/migration/migration.c b/migration/migration.c
index 8ca034136b..2dea8e2fc6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -223,13 +223,18 @@ void migration_object_init(void)
 dirty_bitmap_mig_init();
 }
 
+void migration_cancel(void)
+{
+migrate_fd_cancel(current_migration);
+}
+
 void migration_shutdown(void)
 {
 /*
  * Cancel the current migration - that will (eventually)
  * stop the migration using this structure
  */
-migrate_fd_cancel(current_migration);
+migration_cancel();
 object_unref(OBJECT(current_migration));
 
 /*
@@ -2310,7 +2315,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
 
 void qmp_migrate_cancel(Error **errp)
 {
-migrate_fd_cancel(migrate_get_current());
+migration_cancel();
 }
 
 void qmp_migrate_continue(MigrationStatus state, Error **errp)
diff --git a/migration/migration.h b/migration/migration.h
index db6708326b..f7b388d718 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -375,5 +375,6 @@ int foreach_not_ignored_block(RAMBlockIterFunc func, void 
*opaque);
 void migration_make_urgent_request(void);
 void migration_consume_urgent_request(void);
 bool migration_rate_limit(void);
+void migration_cancel(void);
 
 #endif
diff --git a/migration/ram.c b/migration/ram.c
index 4682f3625c..195fabbab0 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -53,6 +53,7 @@
 #include "block.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/cpu-throttle.h"
+#include "sysemu/runstate.h"
 #include "savevm.h"
 #include "qemu/iov.h"
 #include "multifd.h"
@@ -4138,8 +4139,38 @@ static SaveVMHandlers savevm_ram_handlers = {
 .resume_prepare = ram_resume_prepare,
 };
 
+static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host,
+  size_t old_size, size_t new_size)
+{
+ram_addr_t offset;
+RAMBlock *rb = qemu_ram_block_from_host(host, false, &offset);
+Error *err = NULL;
+
+if (ramblock_is_ignored(rb)) {
+return;
+}
+
+if (!migration_is_idle()) {
+/*
+ * Precopy code on the source cannot deal with the size of RAM blocks
+ * changing at r

[PATCH v5 05/10] exec: Relax range check in ram_block_discard_range()

2021-04-29 Thread David Hildenbrand
We want to make use of ram_block_discard_range() in the RAM block resize
callback when growing a RAM block, *before* used_length is changed.
Let's relax the check. As RAM blocks always mmap the whole max_length area,
we cannot corrupt unrelated data.

Reviewed-by: Peter Xu 
Signed-off-by: David Hildenbrand 
---
 softmmu/physmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 4834003d47..4e587de150 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -3503,7 +3503,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, 
size_t length)
 goto err;
 }
 
-if ((start + length) <= rb->used_length) {
+if ((start + length) <= rb->max_length) {
 bool need_madvise, need_fallocate;
 if (!QEMU_IS_ALIGNED(length, rb->page_size)) {
 error_report("ram_block_discard_range: Unaligned length: %zx",
@@ -3570,7 +3570,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, 
size_t length)
 } else {
 error_report("ram_block_discard_range: Overrun block '%s' (%" PRIu64
  "/%zx/" RAM_ADDR_FMT")",
- rb->idstr, start, length, rb->used_length);
+ rb->idstr, start, length, rb->max_length);
 }
 
 err:
-- 
2.30.2




[PATCH v5 06/10] migration/ram: Discard RAM when growing RAM blocks after ram_postcopy_incoming_init()

2021-04-29 Thread David Hildenbrand
In case we grow our RAM after ram_postcopy_incoming_init() (e.g., when
synchronizing the RAM block state with the migration source), the resized
part would not get discarded. Let's perform that when being notified
about a resize while postcopy has been advised, but is not listening
yet. With precopy, the process is as following:

1. VM created
- RAM blocks are created
2. Incomming migration started
- Postcopy is advised
- All pages in RAM blocks are discarded
3. Precopy starts
- RAM blocks are resized to match the size on the migration source.
- RAM pages from precopy stream are loaded
- Uffd handler is registered, postcopy starts listening
4. Guest started, postcopy running
- Pagefaults get resolved, pages get placed

Reviewed-by: Peter Xu 
Signed-off-by: David Hildenbrand 
---
 migration/ram.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index 195fabbab0..693851d7a0 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4142,6 +4142,7 @@ static SaveVMHandlers savevm_ram_handlers = {
 static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host,
   size_t old_size, size_t new_size)
 {
+PostcopyState ps = postcopy_state_get();
 ram_addr_t offset;
 RAMBlock *rb = qemu_ram_block_from_host(host, false, &offset);
 Error *err = NULL;
@@ -4162,6 +4163,35 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier 
*n, void *host,
 error_free(err);
 migration_cancel();
 }
+
+switch (ps) {
+case POSTCOPY_INCOMING_ADVISE:
+/*
+ * Update what ram_postcopy_incoming_init()->init_range() does at the
+ * time postcopy was advised. Syncing RAM blocks with the source will
+ * result in RAM resizes.
+ */
+if (old_size < new_size) {
+if (ram_discard_range(rb->idstr, old_size, new_size - old_size)) {
+error_report("RAM block '%s' discard of resized RAM failed",
+ rb->idstr);
+}
+}
+break;
+case POSTCOPY_INCOMING_NONE:
+case POSTCOPY_INCOMING_RUNNING:
+case POSTCOPY_INCOMING_END:
+/*
+ * Once our guest is running, postcopy does no longer care about
+ * resizes. When growing, the new memory was not available on the
+ * source, no handler needed.
+ */
+break;
+default:
+error_report("RAM block '%s' resized during postcopy state: %d",
+ rb->idstr, ps);
+exit(-1);
+}
 }
 
 static RAMBlockNotifier ram_mig_ram_notifier = {
-- 
2.30.2




[PATCH v5 09/10] migration/multifd: Print used_length of memory block

2021-04-29 Thread David Hildenbrand
We actually want to print the used_length, against which we check.

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

diff --git a/migration/multifd.c b/migration/multifd.c
index a6677c45c8..0a4803cfcc 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -361,7 +361,7 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, 
Error **errp)
 if (offset > (block->used_length - qemu_target_page_size())) {
 error_setg(errp, "multifd: offset too long %" PRIu64
" (max " RAM_ADDR_FMT ")",
-   offset, block->max_length);
+   offset, block->used_length);
 return -1;
 }
 p->pages->iov[i].iov_base = block->host + offset;
-- 
2.30.2




[PATCH v5 10/10] migration/ram: Use offset_in_ramblock() in range checks

2021-04-29 Thread David Hildenbrand
We never read or write beyond the used_length of memory blocks when
migrating. Make this clearer by using offset_in_ramblock() consistently.

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: David Hildenbrand 
---
 migration/ram.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 6d4d13d345..1814c6cf03 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1368,8 +1368,8 @@ static bool find_dirty_block(RAMState *rs, 
PageSearchStatus *pss, bool *again)
 *again = false;
 return false;
 }
-if ram_addr_t)pss->page) << TARGET_PAGE_BITS)
->= pss->block->used_length) {
+if (!offset_in_ramblock(pss->block,
+((ram_addr_t)pss->page) << TARGET_PAGE_BITS)) {
 /* Didn't find anything in this RAM Block */
 pss->page = 0;
 pss->block = QLIST_NEXT_RCU(pss->block, next);
@@ -1894,7 +1894,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t 
start, ram_addr_t len)
 rs->last_req_rb = ramblock;
 }
 trace_ram_save_queue_pages(ramblock->idstr, start, len);
-if (start + len > ramblock->used_length) {
+if (!offset_in_ramblock(ramblock, start + len - 1)) {
 error_report("%s request overrun start=" RAM_ADDR_FMT " len="
  RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
  __func__, start, len, ramblock->used_length);
@@ -3739,8 +3739,8 @@ void colo_flush_ram_cache(void)
 while (block) {
 offset = migration_bitmap_find_dirty(ram_state, block, offset);
 
-if (((ram_addr_t)offset) << TARGET_PAGE_BITS
->= block->used_length) {
+if (!offset_in_ramblock(block,
+((ram_addr_t)offset) << TARGET_PAGE_BITS)) 
{
 offset = 0;
 block = QLIST_NEXT_RCU(block, next);
 } else {
-- 
2.30.2




[RFC PATCH 00/27] Virtio sound card implementation

2021-04-29 Thread Shreyansh Chouhan
This patch series aims to implement the virtio sound card
as defined in the virtio specs (v8). The specs can be found
at the following github repo:
https://github.com/oasis-tcs/virtio-spec

This patch series is not complete yet, but here is what's
already been done:

- The device is initialized properly and is recognized
  by the guest as a sound card device.
- Output stream works but the output is very noisy. (Which
  is what I wanted coments on.)

What remains to be done:

- Input streams yet to be done.
- The jacks are initialized with a default config, but
  they are not mapped to any streams for now.
- Channel maps are yet to be implemented.

I'd like to request some comments on the following points:

- The output from the sound card is accompanied by periodic
  white noise. I do not know why this is happening. I tried
  debugging it by writing the buffers to a new wav file and
  sure enough the contents of the file were different at
  some places, but I couldn't find what must be causing it.
  (Relevant patches: #19, #20, #21 and #25.) What steps should
  I take for debugging this?

- If I try and output a wav file of a different size, that
  sets the period_bytes to 4004, I get an assert failure in
  the object_unref function defined in qom/object.c. (Function
  defined on line #681, assert on line #690.)
assert(obj->parent == NULL);
  I tried taking a look at the stack trace for when this failure
  happens. In the stacktrace I found out that this happened
  when QEMU was trying to unmap the out_sg from the VirtQueue
  element. This failure doesn't happen if I am using a different
  wav file, that sets the period_bytes to something else.
  (Relevant patches: #19, #20, #21 and #25.)
  What could be causing this problem?

- What is the suggested way of waiting? When the driver issues
  the VIRTIO_SND_PCM_STOP ctrl command I want to wait for the
  buffers existing in tx vq to be consumed before closing the
  stream.

Shreyansh Chouhan (27):
  virtio-snd: Add virtio sound header file
  virtio-snd: Add jack control structures
  virtio-snd: Add PCM control structures
  virtio-snd: Add chmap control structures
  virtio-snd: Add device implementation structures
  virtio-snd: Add PCI wrapper code for VirtIOSound
  virtio-snd: Add properties for class init
  virtio-snd: Add code for get config function
  virtio-snd: Add code for set config function
  virtio-snd: Add code for the realize function
  virtio-snd: Add macros for logging
  virtio-snd: Add control virtqueue handler
  virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
  virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler
  virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
  virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle
  virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler
  virtio-snd: Add default configs to realize fn
  virtio-snd: Add callback for SWVoiceOut
  virtio-snd: Add VIRITO_SND_R_PCM_START handler
  virtio-snd: Add VIRTIO_SND_R_PCM_STOP handler
  virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler
  virtio-snd: Replaced goto with if else
  virtio-snd: Add code to device unrealize function
  virtio-snd: Add tx vq and handler
  virtio-snd: Add event vq and a handler stub
  virtio-snd: Add rx vq and stub handler

 hw/audio/Kconfig   |5 +
 hw/audio/meson.build   |1 +
 hw/audio/virtio-snd.c  | 1168 
 hw/virtio/meson.build  |1 +
 hw/virtio/virtio-snd-pci.c |   72 ++
 include/hw/virtio/virtio-snd.h |  393 +++
 6 files changed, 1640 insertions(+)
 create mode 100644 hw/audio/virtio-snd.c
 create mode 100644 hw/virtio/virtio-snd-pci.c
 create mode 100644 include/hw/virtio/virtio-snd.h

-- 
2.25.1




[RFC PATCH 01/27] virtio-snd: Add virtio sound header file

2021-04-29 Thread Shreyansh Chouhan
Added device configuration and common definitions to the header
file.

Signed-off-by: Shreyansh Chouhan 
---
 include/hw/virtio/virtio-snd.h | 97 ++
 1 file changed, 97 insertions(+)
 create mode 100644 include/hw/virtio/virtio-snd.h

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
new file mode 100644
index 00..bbbf174c51
--- /dev/null
+++ b/include/hw/virtio/virtio-snd.h
@@ -0,0 +1,97 @@
+/*
+ * Virtio Sound Device
+ */
+
+#ifndef QEMU_VIRTIO_SOUND_H
+#define QEMU_VIRTIO_SOUND_H
+
+#include "qemu/units.h"
+#include "hw/virtio/virtio.h"
+#include "qemu/queue.h"
+#include "audio/audio.h"
+#include "audio/audio_int.h"
+
+#define VIRTIO_ID_SOUND 25
+
+/* CONFIGURATION SPACE */
+
+typedef struct virtio_snd_config {
+/* # of jacks available */
+uint32_t jacks;
+/* # of streams avalable */
+uint32_t streams;
+/* # chmaps available */
+uint32_t chmaps;
+} virtio_snd_config;
+
+/* COMMON DEFINITIONS */
+
+/* supported sample data directions. */
+enum {
+VIRTIO_SND_D_OUTPUT = 0,
+VIRTIO_SND_D_INPUT
+};
+
+enum {
+/* jack control request types */
+VIRTIO_SND_R_JACK_INFO = 1,
+VIRTIO_SND_R_JACK_REMAP,
+
+/* PCM control request types */
+VIRTIO_SND_R_PCM_INFO = 0x0100,
+VIRTIO_SND_R_PCM_SET_PARAMS,
+VIRTIO_SND_R_PCM_PREPARE,
+VIRTIO_SND_R_PCM_RELEASE,
+VIRTIO_SND_R_PCM_START,
+VIRTIO_SND_R_PCM_STOP,
+
+/* channel map control request type */
+VIRTIO_SND_R_CHMAP_INFO = 0x200,
+
+/* jack event types */
+VIRTIO_SND_EVT_JACK_CONNECTED = 0x1000,
+VIRTIO_SND_EVT_JACK_DISCONNECTED,
+
+/* PCM event types */
+VIRTIO_SND_EVT_PCM_PERIOD_ELAPSED = 0x1100,
+VIRTIO_SND_EVT_PCM_XRUN,
+
+/* common status codes */
+VIRTIO_SND_S_OK = 0x8000,
+VIRTIO_SND_S_BAD_MSG,
+VIRTIO_SND_S_NOT_SUPP,
+VIRTIO_SND_S_IO_ERR
+};
+
+/* common header for request/response*/
+typedef struct virtio_snd_hdr {
+uint32_t code;
+} virtio_snd_hdr;
+
+/* event notification */
+typedef struct virtio_snd_event {
+/* VIRTIO_SND_EVT_* */
+virtio_snd_hdr hdr;
+/* Optional event data */
+uint32_t data;
+} virtio_snd_event;
+
+/* common control request to query an item information */
+typedef struct virtio_snd_query_info {
+/* VIRTIO_SND_R_*_INFO */
+struct virtio_snd_hdr hdr;
+/* item start identifier */
+uint32_t start_id;
+/* # of items to query */
+uint32_t count;
+/* size of a single item information in bytes */
+uint32_t size;
+} virtio_snd_query_info;
+
+/* common item information header */
+typedef struct virtio_snd_info {
+/* functional group node id (HDA Spec 7.1.2) */
+uint32_t hda_fn_nid;
+} virtio_snd_info;
+
+#endif
-- 
2.25.1




[RFC PATCH 05/27] virtio-snd: Add device implementation structures

2021-04-29 Thread Shreyansh Chouhan
Added jacks, pcm streams and the VirtIOSound structure for actual
device implementation.

Signed-off-by: Shreyansh Chouhan 
---
 include/hw/virtio/virtio-snd.h | 64 ++
 1 file changed, 64 insertions(+)

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
index ad068e5893..6ab131db50 100644
--- a/include/hw/virtio/virtio-snd.h
+++ b/include/hw/virtio/virtio-snd.h
@@ -13,6 +13,9 @@
 
 #define VIRTIO_ID_SOUND 25
 
+#define TYPE_VIRTIO_SOUND "virtio-sound-device"
+OBJECT_DECLARE_SIMPLE_TYPE(VirtIOSound, VIRTIO_SOUND)
+
 /* CONFIGURATION SPACE */
 
 typedef struct virtio_snd_config {
@@ -326,4 +329,65 @@ typedef struct virtio_snd_chmap_info {
 uint8_t positions[VIRTIO_SND_CHMAP_MAX_SIZE];
 } virtio_snd_chmap_info;
 
+/* VIRTIO SOUND DEVICE */
+
+/* Jacks */
+typedef struct virtio_snd_jack {
+uint32_t features; /* 1 << VIRTIO_SND_JACK_F_XXX */
+uint32_t hda_fn_nid;
+uint32_t hda_reg_defconf;
+uint32_t hda_reg_caps;
+uint8_t connected;
+} virtio_snd_jack;
+
+/* Streams */
+typedef struct virtio_snd_pcm_stream {
+uint32_t hda_fn_nid;
+uint32_t buffer_bytes;
+uint32_t period_bytes;
+uint32_t features; /* 1 << VIRTIO_SND_PCM_F_XXX */
+uint32_t flags; /* 1 << VIRTIO_SND_PCM_FL_XXX */
+uint32_t direction;
+uint8_t channels_min;
+uint8_t channels_max;
+uint64_t formats; /* 1 << VIRTIO_SND_PCM_FMT_XXX */
+uint64_t rates; /* 1 << VIRTIO_SND_PCM_RATE_XXX */
+int tail, r_pos, w_pos;
+VirtQueueElement **elems;
+VirtIOSound *s;
+union {
+SWVoiceIn *in;
+SWVoiceOut *out;
+} voice;
+} virtio_snd_pcm_stream;
+
+/* Stream params */
+typedef struct virtio_snd_pcm_params {
+uint32_t features;
+uint32_t buffer_bytes;  /* size of hardware buffer in bytes */
+uint32_t period_bytes;  /* size of hardware period in bytes */
+uint8_t channel;
+uint8_t format;
+uint8_t rate;
+} virtio_snd_pcm_params;
+
+/* Sound device */
+struct VirtIOSound {
+/* Parent VirtIODevice object */
+VirtIODevice parent_obj;
+virtio_snd_config snd_conf;
+
+VirtQueue *ctrl_vq;
+VirtQueue *event_vq;
+VirtQueue *tx_vq;
+VirtQueue *rx_vq;
+
+QEMUSoundCard card;
+size_t config_size;
+
+virtio_snd_pcm_params **pcm_params;
+virtio_snd_pcm_stream **streams;
+virtio_snd_jack **jacks;
+};
+
 #endif
-- 
2.25.1




[RFC PATCH 13/27] virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler

2021-04-29 Thread Shreyansh Chouhan
Signed-off-by: Shreyansh Chouhan 
---
 hw/audio/virtio-snd.c | 81 +--
 1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 435870e3ba..d50234f9a8 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -100,6 +100,80 @@ static uint64_t virtio_snd_get_features(VirtIODevice 
*vdev, uint64_t features,
 {
 return vdev->host_features;
 }
+/*
+ * Get a specific jack from the VirtIOSound card.
+ *
+ * @s: VirtIOSound card device.
+ * @id: Jack id
+ */
+static virtio_snd_jack *virtio_snd_get_jack(VirtIOSound *s, uint32_t id)
+{
+if (id >= s->snd_conf.jacks) {
+return NULL;
+}
+return s->jacks[id];
+}
+
+/*
+ * Handles VIRTIO_SND_R_JACK_INFO.
+ * The function writes the info structs and response to the virtqueue element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_jack_info(VirtIOSound *s,
+VirtQueueElement *elem)
+{
+virtio_snd_query_info req;
+size_t sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+assert(sz == sizeof(virtio_snd_query_info));
+
+virtio_snd_hdr resp;
+
+if (iov_size(elem->in_sg, elem->in_num) <
+sizeof(virtio_snd_hdr) + req.count * req.size) {
+virtio_snd_err("jack info: buffer too small got: %lu needed: %lu\n",
+   iov_size(elem->in_sg, elem->in_num),
+   sizeof(virtio_snd_hdr) + req.count * req.size);
+resp.code = VIRTIO_SND_S_BAD_MSG;
+goto done;
+}
+
+virtio_snd_jack_info *jack_info = g_new0(virtio_snd_jack_info, req.count);
+for (int i = req.start_id; i < req.count + req.start_id; i++) {
+virtio_snd_jack *jack = virtio_snd_get_jack(s, i);
+if (!jack) {
+virtio_snd_err("Invalid jack id: %d\n", i);
+resp.code = VIRTIO_SND_S_BAD_MSG;
+goto done;
+}
+
+jack_info[i - req.start_id].hdr.hda_fn_nid = jack->hda_fn_nid;
+jack_info[i - req.start_id].features = jack->features;
+jack_info[i - req.start_id].hda_reg_defconf = jack->hda_reg_defconf;
+jack_info[i - req.start_id].hda_reg_caps = jack->hda_reg_caps;
+jack_info[i - req.start_id].connected = jack->connected;
+memset(jack_info[i - req.start_id].padding, 0,
+   sizeof(jack_info[i - req.start_id].padding));
+}
+
+resp.code = VIRTIO_SND_S_OK;
+done:
+sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+assert(sz == sizeof(virtio_snd_hdr));
+
+if (resp.code == VIRTIO_SND_S_BAD_MSG) {
+g_free(jack_info);
+return sz;
+}
+
+sz = iov_from_buf(elem->in_sg, elem->in_num, sizeof(virtio_snd_hdr),
+  jack_info, sizeof(virtio_snd_jack_info) * req.count);
+assert(sz == req.count * req.size);
+g_free(jack_info);
+return sizeof(virtio_snd_hdr) + sz;
+}
 
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
@@ -110,6 +184,7 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev, 
uint64_t features,
  */
 static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
+VirtIOSound *s = VIRTIO_SOUND(vdev);
 virtio_snd_hdr ctrl;
 
 VirtQueueElement *elem = NULL;
@@ -139,7 +214,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 /* error */
 virtio_snd_err("virtio snd ctrl could not read header\n");
 } else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
-virtio_snd_log("VIRTIO_SND_R_JACK_INFO");
+sz = virtio_snd_handle_jack_info(s, elem);
+goto done;
 } else if (ctrl.code == VIRTIO_SND_R_JACK_REMAP) {
 virtio_snd_log("VIRTIO_SND_R_JACK_REMAP");
 } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
@@ -162,8 +238,9 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 virtio_snd_hdr resp;
 resp.code = VIRTIO_SND_S_OK;
 sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
-virtqueue_push(vq, elem, sz);
 
+done:
+virtqueue_push(vq, elem, sz);
 virtio_notify(vdev, vq);
 g_free(iov2);
 g_free(elem);
-- 
2.25.1




[RFC PATCH 04/27] virtio-snd: Add chmap control structures

2021-04-29 Thread Shreyansh Chouhan
Added structures for handling channel map control
requests to the header file.

Signed-off-by: Shreyansh Chouhan 
---
 include/hw/virtio/virtio-snd.h | 64 ++
 1 file changed, 64 insertions(+)

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
index e9a4fe3c5d..ad068e5893 100644
--- a/include/hw/virtio/virtio-snd.h
+++ b/include/hw/virtio/virtio-snd.h
@@ -262,4 +262,68 @@ typedef struct virtio_snd_pcm_status {
 uint32_t latency_bytes;
 } virtio_snd_pcm_status;
 
+/* CHANNEL MAP CONTROL MESSAGES */
+
+typedef struct virtio_snd_chmap_hdr {
+/* .code = VIRTIO_SND_R_CHMAP_* */
+virtio_snd_hdr hdr;
+/* 0 to (virtio_snd_config.chmaps - 1) */
+uint32_t chmap_id;
+} virtio_snd_chmap_hdr;
+
+/* standard channel position definition */
+enum {
+VIRTIO_SND_CHMAP_NONE = 0,  /* undefined */
+VIRTIO_SND_CHMAP_NA,/* silent */
+VIRTIO_SND_CHMAP_MONO,  /* mono stream */
+VIRTIO_SND_CHMAP_FL,/* front left */
+VIRTIO_SND_CHMAP_FR,/* front right */
+VIRTIO_SND_CHMAP_RL,/* rear left */
+VIRTIO_SND_CHMAP_RR,/* rear right */
+VIRTIO_SND_CHMAP_FC,/* front center */
+VIRTIO_SND_CHMAP_LFE,   /* low frequency (LFE) */
+VIRTIO_SND_CHMAP_SL,/* side left */
+VIRTIO_SND_CHMAP_SR,/* side right */
+VIRTIO_SND_CHMAP_RC,/* rear center */
+VIRTIO_SND_CHMAP_FLC,   /* front left center */
+VIRTIO_SND_CHMAP_FRC,   /* front right center */
+VIRTIO_SND_CHMAP_RLC,   /* rear left center */
+VIRTIO_SND_CHMAP_RRC,   /* rear right center */
+VIRTIO_SND_CHMAP_FLW,   /* front left wide */
+VIRTIO_SND_CHMAP_FRW,   /* front right wide */
+VIRTIO_SND_CHMAP_FLH,   /* front left high */
+VIRTIO_SND_CHMAP_FCH,   /* front center high */
+VIRTIO_SND_CHMAP_FRH,   /* front right high */
+VIRTIO_SND_CHMAP_TC,/* top center */
+VIRTIO_SND_CHMAP_TFL,   /* top front left */
+VIRTIO_SND_CHMAP_TFR,   /* top front right */
+VIRTIO_SND_CHMAP_TFC,   /* top front center */
+VIRTIO_SND_CHMAP_TRL,   /* top rear left */
+VIRTIO_SND_CHMAP_TRR,   /* top rear right */
+VIRTIO_SND_CHMAP_TRC,   /* top rear center */
+VIRTIO_SND_CHMAP_TFLC,  /* top front left center */
+VIRTIO_SND_CHMAP_TFRC,  /* top front right center */
+VIRTIO_SND_CHMAP_TSL,   /* top side left */
+VIRTIO_SND_CHMAP_TSR,   /* top side right */
+VIRTIO_SND_CHMAP_LLFE,  /* left LFE */
+VIRTIO_SND_CHMAP_RLFE,  /* right LFE */
+VIRTIO_SND_CHMAP_BC,/* bottom center */
+VIRTIO_SND_CHMAP_BLC,   /* bottom left center */
+VIRTIO_SND_CHMAP_BRC/* bottom right center */
+};
+
+/* maximum possible number of channels */
+#define VIRTIO_SND_CHMAP_MAX_SIZE   18
+
+typedef struct virtio_snd_chmap_info {
+/* common header */
+virtio_snd_info hdr;
+/* direction */
+uint8_t direction;
+/* # of valid channel position values */
+uint8_t channels;
+/* channel position values (VIRTIO_SND_CHMAP_*) */
+uint8_t positions[VIRTIO_SND_CHMAP_MAX_SIZE];
+} virtio_snd_chmap_info;
+
 #endif
-- 
2.25.1




  1   2   3   4   >