[Bug 1914870] Re: libvixl compilation failure on Debian unstable
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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/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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
* 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
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
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
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.
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
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
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
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
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
** 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
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
** 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
** 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
** 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
** 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
** 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
** 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
** 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
** 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]
** 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
** 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
** 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'"
** 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
** 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
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
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
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
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
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
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
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
** 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
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
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
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
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
** 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
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
** 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
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
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
** 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
** 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
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
** 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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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()
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()
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
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
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
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
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
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
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
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