Re: [PATCH v3 1/1] os-posix: asynchronous teardown for shutdown on Linux
On 8/12/22 04:26, Claudio Imbrenda wrote: On Thu, 11 Aug 2022 23:05:52 -0300 Murilo Opsfelder Araújo wrote: On 8/11/22 11:02, Daniel P. Berrangé wrote: [...] Hmm, I was hoping you could just use SIGKILL to guarantee that this gets killed off. Is SIGKILL delivered too soon to allow for the main QEMU process to have exited quickly ? yes, I tried. qemu has not finished exiting when the signal is delivered, the cleanup process dies before qemu, which defeats the purpose Ok, too bad. If so I wonder what happens when systemd just delivers SIGKILL to all processes in the cgroup - I'm not sure there's a guarantee it will SIGKILL the main qemu before it SIGKILLs this helper I'm afraid in that case there is no guarantee. for what it's worth, both virsh shutdown and destroy seem to do things properly. Hmm, probably because libvirt tells QEMU to exit before systemd comes along and tells everything in the cgroup to die with SIGKILL. It seems Libvirt sends SIGKILL if qemu process doesn't terminate within 10 seconds after Libvirt sent SIGTERM: https://gitlab.com/libvirt/libvirt/-/blob/0615df084ec9996b5df88d6a1b59c557e22f3a12/src/util/virprocess.c#L375 but this is fine. with asynchronous teardown, qemu will exit almost immediately when receiving SIGTERM, and the cleanup process will start cleaning up. Under normal and orderly conditions, yes. So I guess this patch happened to work with Libvirt because the main qemu process terminated before the timeout and before SIGKILL was delivered. it seems so The cleanup process is trying to solve the problem where the main qemu process takes too long to terminate. However, if the cleanup process itself takes too long, SIGKILL will be sent by Libvirt anyway. but that is not a problem, the sole purpose of the cleanup process is to terminate _after_ qemu. it doesn't matter what happens after qemu has terminated. if you look at the patch, after going to great lengths to assure that qemu has terminated, all the child process does is _exit(0). Perhaps we can describe this situation in the parameter help, e.g.: If management layer decides to send SIGKILL (e.g.: due to timeout or deliberate decision), the cleanup process can exit before the main process, deceiving its purpose. if the management layer (or the user) decides to send SIGKILL immediately to the whole cgroup without sending SIGTERM first, then this whole asynchronous teardown mechanism is defeated, yes. This situation is what we likely want to describe in the parameter help. I don't want to give users the false impression that this option will *always* behave the manner we expect it to work *most* of the time. -- Murilo
Re: [PATCH v3 1/1] os-posix: asynchronous teardown for shutdown on Linux
On 8/11/22 11:02, Daniel P. Berrangé wrote: [...] Hmm, I was hoping you could just use SIGKILL to guarantee that this gets killed off. Is SIGKILL delivered too soon to allow for the main QEMU process to have exited quickly ? yes, I tried. qemu has not finished exiting when the signal is delivered, the cleanup process dies before qemu, which defeats the purpose Ok, too bad. If so I wonder what happens when systemd just delivers SIGKILL to all processes in the cgroup - I'm not sure there's a guarantee it will SIGKILL the main qemu before it SIGKILLs this helper I'm afraid in that case there is no guarantee. for what it's worth, both virsh shutdown and destroy seem to do things properly. Hmm, probably because libvirt tells QEMU to exit before systemd comes along and tells everything in the cgroup to die with SIGKILL. It seems Libvirt sends SIGKILL if qemu process doesn't terminate within 10 seconds after Libvirt sent SIGTERM: https://gitlab.com/libvirt/libvirt/-/blob/0615df084ec9996b5df88d6a1b59c557e22f3a12/src/util/virprocess.c#L375 So I guess this patch happened to work with Libvirt because the main qemu process terminated before the timeout and before SIGKILL was delivered. The cleanup process is trying to solve the problem where the main qemu process takes too long to terminate. However, if the cleanup process itself takes too long, SIGKILL will be sent by Libvirt anyway. Perhaps we can describe this situation in the parameter help, e.g.: If management layer decides to send SIGKILL (e.g.: due to timeout or deliberate decision), the cleanup process can exit before the main process, deceiving its purpose. -- Murilo
Re: [PATCH v3 1/1] os-posix: asynchronous teardown for shutdown on Linux
Hi, Claudio. On 8/9/22 03:40, Claudio Imbrenda wrote: This patch adds support for asynchronously tearing down a VM on Linux. When qemu terminates, either naturally or because of a fatal signal, the VM is torn down. If the VM is huge, it can take a considerable amount of time for it to be cleaned up. In case of a protected VM, it might take even longer than a non-protected VM (this is the case on s390x, for example). Some users might want to shut down a VM and restart it immediately, without having to wait. This is especially true if management infrastructure like libvirt is used. This patch implements a simple trick on Linux to allow qemu to return immediately, with the teardown of the VM being performed asynchronously. If the new commandline option -async-teardown is used, a new process is spawned from qemu at startup, using the clone syscall, in such way that it will share its address space with qemu. The new process will have the name "cleanup/". It will wait until qemu terminates, and then it will exit itself. This allows qemu to terminate quickly, without having to wait for the whole address space to be torn down. The teardown process will exit after qemu, so it will be the last user of the address space, and therefore it will take care of the actual teardown. The teardown process will share the same cgroups as qemu, so both memory usage and cpu time will be accounted properly. This feature can already be used with libvirt by adding the following to the XML domain definition to pass the parameter to qemu directly: http://libvirt.org/schemas/domain/qemu/1.0";> More advanced interfaces like pidfd or close_range have intentionally been avoided in order to be more compatible with older kernels. Signed-off-by: Claudio Imbrenda I've smoke-tested this on ppc and everything looks fine. For what's worth: Reviewed-by: Murilo Opsfelder Araujo Tested-by: Murilo Opsfelder Araujo Have you measured the benefits of using -async-teardown vs. not using it? If so, can you please share the details so I can give it a try on ppc, too? The wall-clock perception is that nothing has changed, for better or worse. My tests used mid-sized VMs, like 128 vCPUs, 64GB RAM. Cheers! --- include/qemu/async-teardown.h | 22 ++ os-posix.c| 6 ++ qemu-options.hx | 17 + util/async-teardown.c | 123 ++ util/meson.build | 1 + 5 files changed, 169 insertions(+) create mode 100644 include/qemu/async-teardown.h create mode 100644 util/async-teardown.c diff --git a/include/qemu/async-teardown.h b/include/qemu/async-teardown.h new file mode 100644 index 00..092e7a37e7 --- /dev/null +++ b/include/qemu/async-teardown.h @@ -0,0 +1,22 @@ +/* + * Asynchronous teardown + * + * Copyright IBM, Corp. 2022 + * + * Authors: + * Claudio Imbrenda + * + * This work is licensed under the terms of the GNU GPL, version 2 or (at your + * option) any later version. See the COPYING file in the top-level directory. + * + */ +#ifndef QEMU_ASYNC_TEARDOWN_H +#define QEMU_ASYNC_TEARDOWN_H + +#include "config-host.h" + +#ifdef CONFIG_LINUX +void init_async_teardown(void); +#endif + +#endif diff --git a/os-posix.c b/os-posix.c index 321fc4bd13..4858650c3e 100644 --- a/os-posix.c +++ b/os-posix.c @@ -39,6 +39,7 @@ #ifdef CONFIG_LINUX #include +#include "qemu/async-teardown.h" #endif /* @@ -150,6 +151,11 @@ int os_parse_cmd_args(int index, const char *optarg) case QEMU_OPTION_daemonize: daemonize = 1; break; +#if defined(CONFIG_LINUX) +case QEMU_OPTION_asyncteardown: +init_async_teardown(); +break; +#endif default: return -1; } diff --git a/qemu-options.hx b/qemu-options.hx index 3f23a42fa8..d434353159 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4743,6 +4743,23 @@ HXCOMM Internal use DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL) DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL) +#ifdef __linux__ +DEF("async-teardown", 0, QEMU_OPTION_asyncteardown, +"-async-teardown enable asynchronous teardown\n", +QEMU_ARCH_ALL) +#endif +SRST +``-async-teardown`` +Enable asynchronous teardown. A new teardown process will be +created at startup, using clone. The teardown process will share +the address space of the main qemu process, and wait for the main +process to terminate. At that point, the teardown process will +also exit. This allows qemu to terminate quickly if the guest was +huge, leaving the teardown of the address space to the teardown +process. Since the teardown process shares the same cgroups as the +main qemu process, accounting is performed correctly. +ERST + DEF("msg", HAS_ARG, QEMU_OPTION_msg, "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n" "control error message format\n" diff --git a/util/async-teardown.c
Re: [PATCH] target/ppc/kvm: Skip ".." directory in kvmppc_find_cpu_dt
Hi, Daniel, David. On 7/12/22 10:03, Daniel Henrique Barboza wrote: On 7/12/22 00:46, David Gibson wrote: On Mon, Jul 11, 2022 at 04:37:43PM -0300, Murilo Opsfelder Araujo wrote: Some systems have /proc/device-tree/cpus/../clock-frequency. However, this is not the expected path for a CPU device tree directory. Signed-off-by: Murilo Opsfelder Araujo Signed-off-by: Fabiano Rosas --- target/ppc/kvm.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 6eed466f80..c8485a5cc0 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -1877,6 +1877,12 @@ static int kvmppc_find_cpu_dt(char *buf, int buf_len) buf[0] = '\0'; while ((dirp = readdir(dp)) != NULL) { FILE *f; + + /* Don't accidentally read from the upper directory */ + if (strcmp(dirp->d_name, "..") == 0) { It might not be causing problems now, but it would be technically more correct to also skip ".", wouldn't it? Given that the use of this function is inside kvmppc_read_int_cpu_dt(), which is used to read a property that belongs to a CPU node, I believe you're right. It's better to avoid returning "PROC_DEVTREE_CPU" as well. Murilo, can you please re-send it skipping both ".." and "." ? Better be on the safe side. Daniel I've sent v2: https://lore.kernel.org/qemu-devel/20220712210810.35514-1-muri...@linux.ibm.com/ Thank you for reviewing. -- Murilo
Re: [PATCH] target/ppc/cpu-models: Update max alias to power10
Hi, Matheus. On 5/31/22 15:04, Matheus K. Ferst wrote: On 31/05/2022 14:27, Murilo Opsfelder Araujo wrote: Update max alias to power10 so users can take advantage of a more recent CPU model when '-cpu max' is provided. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038 Cc: Daniel P. Berrangé Cc: Thomas Huth Cc: Cédric Le Goater Cc: Daniel Henrique Barboza Cc: Fabiano Rosas Signed-off-by: Murilo Opsfelder Araujo --- target/ppc/cpu-models.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c index 976be5e0d1..c15fcb43a1 100644 --- a/target/ppc/cpu-models.c +++ b/target/ppc/cpu-models.c @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "755", "755_v2.8" }, { "goldfinger", "755_v2.8" }, { "7400", "7400_v2.9" }, -{ "max", "7400_v2.9" }, { "g4", "7400_v2.9" }, { "7410", "7410_v1.4" }, { "nitro", "7410_v1.4" }, @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "power8nvl", "power8nvl_v1.0" }, { "power9", "power9_v2.0" }, { "power10", "power10_v2.0" }, +/* Update the 'max' alias to the latest CPU model */ +{ "max", "power10_v2.0" }, #endif /* Generic PowerPCs */ -- 2.36.1 Hi Murilo, I guess we need a "max" for qemu-system-ppc too, so maybe something like > /* Update the 'max' alias to the latest CPU model */ > #if defined(TARGET_PPC64) > { "max", "power10_v2.0" }, > #else > { "max", "7457a_v1.2" }, > #endif Thanks for reviewing. I'm more inclined to the idea of selecting the default CPU type of the machine, as other folks suggested in the replies, instead of hard-coding an alias. Cheers! -- Murilo
Re: [PATCH] target/ppc/cpu-models: Update max alias to power10
Hi, Daniel. On 6/1/22 06:59, Daniel Henrique Barboza wrote: On 6/1/22 06:25, Thomas Huth wrote: On 01/06/2022 10.38, Greg Kurz wrote: On Wed, 1 Jun 2022 09:27:31 +0200 Thomas Huth wrote: On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote: Update max alias to power10 so users can take advantage of a more recent CPU model when '-cpu max' is provided. ... We already have the concept of default CPU for the spapr machine types, that is usually popped up to the newer CPU model that is going to be supported in production. This goes with a bump of the machine type version as well for the sake of migration. This seems a lot more reliable than the "max" thingy IMHO. Unless there's a very important use case I'm missing, I'd rather kill the thing instead of trying to resurrect it. It's about making ppc similar to other architectures, which have "-cpu max" as well, see: https://gitlab.com/qemu-project/qemu/-/issues/1038 It would be nice to get something similar on ppc. I agree that it's preferable to fix it. This is how I would implement -cpu max today: pseries (default ppc64 machine): - kvm: equal to -cpu host - tcg: the latest IBM chip available (POWER10 today) powernv8: POWER8E powernv9: POWER9 powernv10: POWER10 pseries requires more work because the -cpu max varies with the host CPU when running with KVM. About the implementation, for the bug fix it's fine to just hardcode the alias for each machine-CPU pair. In the long run I would add more code to make -cpu max always point to the current default CPU of the chosen machine by default, with each machine overwriting it if needed. This would prevent this alias to be deprecated over time because we forgot to change it after adding new CPUs. I agree with using the default CPU type of the machine as the selected CPU for "-cpu max". Anyone disagree? For qemu-system-ppc the default machine seems to be g3beige and its default CPU is PowerPC 750. I would set -cpu max to this CPU in this case. Matter of fact I would attempt to set -cpu max = default cpu for all 32 bits CPUs for simplicity. This is also outside of gitlab 1038 as well since the bug isn't mentioning 32 bit machines, hence can be done later. Thanks, Daniel Cheeers! -- Murilo
Re: [PATCH] target/ppc/cpu-models: Update max alias to power10
Hi, Cédric. On 6/1/22 04:44, Cédric Le Goater wrote: On 6/1/22 09:27, Thomas Huth wrote: On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote: Update max alias to power10 so users can take advantage of a more recent CPU model when '-cpu max' is provided. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038 Cc: Daniel P. Berrangé Cc: Thomas Huth Cc: Cédric Le Goater Cc: Daniel Henrique Barboza Cc: Fabiano Rosas Signed-off-by: Murilo Opsfelder Araujo --- target/ppc/cpu-models.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c index 976be5e0d1..c15fcb43a1 100644 --- a/target/ppc/cpu-models.c +++ b/target/ppc/cpu-models.c @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "755", "755_v2.8" }, { "goldfinger", "755_v2.8" }, { "7400", "7400_v2.9" }, -{ "max", "7400_v2.9" }, { "g4", "7400_v2.9" }, { "7410", "7410_v1.4" }, { "nitro", "7410_v1.4" }, @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "power8nvl", "power8nvl_v1.0" }, { "power9", "power9_v2.0" }, { "power10", "power10_v2.0" }, +/* Update the 'max' alias to the latest CPU model */ +{ "max", "power10_v2.0" }, #endif I'm not sure whether "max" should really be fixed alias in this list here? What if a user runs with KVM on a POWER8 host - then "max" won't work this way, will it? And in the long run, it would also be good if this would work with other machines like the "g3beige", too (which don't support the new 64-bit POWER CPUs), so you should at least mention in the commit description that this is only a temporary hack for the pseries machine, I think. Yes. You are right, Thomas. s390 and x86 have a nice way to address "max". If I understood the code correctly, they implement "-cpu max" based on a CPU model with additional CPU features enabled. The resulting emulated CPU is not necessarily a CPU model that exists as a hardware. So, the "-cpu max" never gets any CPU feature dropped. Features are only added in. I'm not keen on this idea of having a CPU model that doesn't even exist as a hardware. -- Murilo
Re: [PATCH] target/ppc/cpu-models: Update max alias to power10
Hi, Greg. On 6/1/22 05:38, Greg Kurz wrote: On Wed, 1 Jun 2022 09:27:31 +0200 Thomas Huth wrote: On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote: Update max alias to power10 so users can take advantage of a more recent CPU model when '-cpu max' is provided. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038 Cc: Daniel P. Berrangé Cc: Thomas Huth Cc: Cédric Le Goater Cc: Daniel Henrique Barboza Cc: Fabiano Rosas Signed-off-by: Murilo Opsfelder Araujo --- target/ppc/cpu-models.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c index 976be5e0d1..c15fcb43a1 100644 --- a/target/ppc/cpu-models.c +++ b/target/ppc/cpu-models.c @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "755", "755_v2.8" }, { "goldfinger", "755_v2.8" }, { "7400", "7400_v2.9" }, -{ "max", "7400_v2.9" }, { "g4", "7400_v2.9" }, { "7410", "7410_v1.4" }, { "nitro", "7410_v1.4" }, @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "power8nvl", "power8nvl_v1.0" }, { "power9", "power9_v2.0" }, { "power10", "power10_v2.0" }, +/* Update the 'max' alias to the latest CPU model */ +{ "max", "power10_v2.0" }, #endif I'm not sure whether "max" should really be fixed alias in this list here? What if a user runs with KVM on a POWER8 host - then "max" won't work this way, will it? And in the long run, it would also be good if this would work with other machines like the "g3beige", too (which don't support the new 64-bit POWER CPUs), so you should at least mention in the commit description that this is only a temporary hack for the pseries machine, I think. I didn't even know there was a "max" alias :-) Me too. :) This was introduced by commit: commit 3fc6c082e3aad85addf25d36740030982963c0c8 Author: Fabrice Bellard Date: Sat Jul 2 20:59:34 2005 + preliminary patch to support more PowerPC CPUs (Jocelyn Mayer) This was already a 7400 model at the time. Obviously we've never maintained that and I hardly see how it is useful... As Thomas noted, "max" has implicit semantics that depend on the host. This isn't migration friendly and I'm pretty sure libvirt doesn't use it (Daniel ?) We already have the concept of default CPU for the spapr machine types, that is usually popped up to the newer CPU model that is going to be supported in production. This goes with a bump of the machine type version as well for the sake of migration. This seems a lot more reliable than the "max" thingy IMHO. We can use the default CPU type of the sPAPR machine as the "-cpu max". That would be a safer choice, I think. Cheers! -- Murilo
Re: [PATCH] target/ppc/cpu-models: Update max alias to power10
Hi, Thomas. On 6/1/22 04:27, Thomas Huth wrote: On 31/05/2022 19.27, Murilo Opsfelder Araujo wrote: Update max alias to power10 so users can take advantage of a more recent CPU model when '-cpu max' is provided. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1038 Cc: Daniel P. Berrangé Cc: Thomas Huth Cc: Cédric Le Goater Cc: Daniel Henrique Barboza Cc: Fabiano Rosas Signed-off-by: Murilo Opsfelder Araujo --- target/ppc/cpu-models.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c index 976be5e0d1..c15fcb43a1 100644 --- a/target/ppc/cpu-models.c +++ b/target/ppc/cpu-models.c @@ -879,7 +879,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "755", "755_v2.8" }, { "goldfinger", "755_v2.8" }, { "7400", "7400_v2.9" }, -{ "max", "7400_v2.9" }, { "g4", "7400_v2.9" }, { "7410", "7410_v1.4" }, { "nitro", "7410_v1.4" }, @@ -910,6 +909,8 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "power8nvl", "power8nvl_v1.0" }, { "power9", "power9_v2.0" }, { "power10", "power10_v2.0" }, +/* Update the 'max' alias to the latest CPU model */ +{ "max", "power10_v2.0" }, #endif I'm not sure whether "max" should really be fixed alias in this list here? What if a user runs with KVM on a POWER8 host - then "max" won't work this way, will it? "-cpu max" as an alias to power10 running with KVM on a P8 host won't work. It's already broken with the current 7400_v2.9, anyway. And in the long run, it would also be good if this would work with other machines like the "g3beige", too (which don't support the new 64-bit POWER CPUs), so you should at least mention in the commit description that this is only a temporary hack for the pseries machine, I think. I agree. I'll mention that if I end up respining the patch. Thank you! -- Murilo
Re: [PATCH v2] mos6522: fix linking error when CONFIG_MOS6522 is not set
Hi, Thomas. On 5/10/22 04:24, Thomas Huth wrote: On 06/05/2022 03.16, Murilo Opsfelder Araujo wrote: When CONFIG_MOS6522 is not set, building ppc64-softmmu target fails: /usr/bin/ld: libqemu-ppc64-softmmu.fa.p/monitor_misc.c.o:(.data+0x1158): undefined reference to `hmp_info_via' Make devices configuration available in hmp-commands*.hx and check for CONFIG_MOS6522. Fixes: 409e9f7131e5 (mos6522: add "info via" HMP command for debugging) Signed-off-by: Murilo Opsfelder Araujo Cc: Mark Cave-Ayland Cc: Fabiano Rosas --- v2: - Included devices configuration in monitor/misc.c v1: - https://lore.kernel.org/qemu-devel/20220429233146.29662-1-muri...@linux.ibm.com/ hmp-commands-info.hx | 2 ++ monitor/misc.c | 3 +++ 2 files changed, 5 insertions(+) diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index adfa085a9b..9ad784dd9f 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -881,6 +881,7 @@ SRST ERST #if defined(TARGET_M68K) || defined(TARGET_PPC) +#if defined(CONFIG_MOS6522) I think you could even get rid of the TARGET_ stuff now that the CONFIG line works! { .name = "via", .args_type = "", @@ -889,6 +890,7 @@ ERST .cmd = hmp_info_via, }, #endif +#endif SRST ``info via`` diff --git a/monitor/misc.c b/monitor/misc.c index 6c5bb82d3b..3d2312ba8d 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -84,6 +84,9 @@ #include "hw/s390x/storage-attributes.h" #endif +/* Make devices configuration available for use in hmp-commands*.hx templates */ +#include CONFIG_DEVICES Looks reasonable to me. So with the "#if defined(TARGET_M68K) || defined(TARGET_PPC)" removed: Reviewed-by: Thomas Huth I've sent v3 with your suggestion: https://lore.kernel.org/qemu-devel/20220510235439.54775-1-muri...@linux.ibm.com/ Thank you for your review. -- Murilo
Re: [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set
On 5/2/22 06:43, Mark Cave-Ayland wrote: On 30/04/2022 00:31, Murilo Opsfelder Araujo wrote: When CONFIG_MOS6522 is not set, building ppc64-softmmu target fails: /usr/bin/ld: libqemu-ppc64-softmmu.fa.p/monitor_misc.c.o:(.data+0x1158): undefined reference to `hmp_info_via' clang-13: error: linker command failed with exit code 1 (use -v to see invocation) Add CONFIG_MOS6522 check for hmp_info_via in hmp-commands-info.hx to fix such linking error. Fixes: 409e9f7131e5 (mos6522: add "info via" HMP command for debugging) Signed-off-by: Murilo Opsfelder Araujo Cc: Mark Cave-Ayland Cc: Fabiano Rosas --- hmp-commands-info.hx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index adfa085a9b..9ad784dd9f 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -881,6 +881,7 @@ SRST ERST #if defined(TARGET_M68K) || defined(TARGET_PPC) +#if defined(CONFIG_MOS6522) { .name = "via", .args_type = "", @@ -889,6 +890,7 @@ ERST .cmd = hmp_info_via, }, #endif +#endif SRST ``info via`` Hmmm. The patch in its proposed form isn't correct, since device CONFIG_* defines aren't declared when processing hmp-commands-info.hx. This was something that was discovered and discussed in the original thread for which the current workaround is to use the per-target TARGET_* defines instead. I've sent a v2 of this patch that addresses this, i.e.: the CONFIG_* options are available in hmp-commands-info.hx: https://lore.kernel.org/qemu-devel/20220506011632.183257-1-muri...@linux.ibm.com/ I hope it can resolve the build issue in the short-term. I'd appreciate if you or anyone on this thread could review it. Thank you, Mark, for the discussion and knowledge sharing! Cheers! -- Murilo
Re: [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set
Hi, Mark. On 5/4/22 11:32, Mark Cave-Ayland wrote: On 04/05/2022 14:16, Murilo Opsfelder Araújo wrote: Hi, Mark. On 5/4/22 04:10, Mark Cave-Ayland wrote: On 02/05/2022 14:36, Murilo Opsfelder Araújo wrote: Hi, Mark. Thanks for reviewing. Comments below. On 5/2/22 06:43, Mark Cave-Ayland wrote: On 30/04/2022 00:31, Murilo Opsfelder Araujo wrote: When CONFIG_MOS6522 is not set, building ppc64-softmmu target fails: /usr/bin/ld: libqemu-ppc64-softmmu.fa.p/monitor_misc.c.o:(.data+0x1158): undefined reference to `hmp_info_via' clang-13: error: linker command failed with exit code 1 (use -v to see invocation) Add CONFIG_MOS6522 check for hmp_info_via in hmp-commands-info.hx to fix such linking error. Fixes: 409e9f7131e5 (mos6522: add "info via" HMP command for debugging) Signed-off-by: Murilo Opsfelder Araujo Cc: Mark Cave-Ayland Cc: Fabiano Rosas --- hmp-commands-info.hx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index adfa085a9b..9ad784dd9f 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -881,6 +881,7 @@ SRST ERST #if defined(TARGET_M68K) || defined(TARGET_PPC) +#if defined(CONFIG_MOS6522) { .name = "via", .args_type = "", @@ -889,6 +890,7 @@ ERST .cmd = hmp_info_via, }, #endif +#endif SRST ``info via`` Hmmm. The patch in its proposed form isn't correct, since device CONFIG_* defines aren't declared when processing hmp-commands-info.hx. This was something that was discovered and discussed in the original thread for which the current workaround is to use the per-target TARGET_* defines instead. So my proposed fix worked just by coincidence. Thanks for providing the background. Given that the g3beige and mac99 machines are included by default in qemu-system-ppc64 which both contain the MOS6522 device, I can't quite understand how CONFIG_MOS6522 isn't being selected. Can you give more information about how you are building QEMU including your configure command line? Here is a reproducer adapted from CentOS 9 Stream qemu-kvm[0] package (build failed on c9s ppc64le with QEMU at commit f5643914a9e8f79c606a76e6a9d7ea82a3fc3e65): $ cat > configs/devices/rh-virtio.mak <<"EOF" CONFIG_VIRTIO=y CONFIG_VIRTIO_BALLOON=y CONFIG_VIRTIO_BLK=y CONFIG_VIRTIO_GPU=y CONFIG_VIRTIO_INPUT=y CONFIG_VIRTIO_INPUT_HOST=y CONFIG_VIRTIO_NET=y CONFIG_VIRTIO_RNG=y CONFIG_VIRTIO_SCSI=y CONFIG_VIRTIO_SERIAL=y EOF $ cat > configs/devices/ppc64-softmmu/ppc64-rh-devices.mak <<"EOF" include ../rh-virtio.mak CONFIG_DIMM=y CONFIG_MEM_DEVICE=y CONFIG_NVDIMM=y CONFIG_PCI=y CONFIG_PCI_DEVICES=y CONFIG_PCI_TESTDEV=y CONFIG_PCI_EXPRESS=y CONFIG_PSERIES=y CONFIG_SCSI=y CONFIG_SPAPR_VSCSI=y CONFIG_TEST_DEVICES=y CONFIG_USB=y CONFIG_USB_OHCI=y CONFIG_USB_OHCI_PCI=y CONFIG_USB_SMARTCARD=y CONFIG_USB_STORAGE_CORE=y CONFIG_USB_STORAGE_CLASSIC=y CONFIG_USB_XHCI=y CONFIG_USB_XHCI_NEC=y CONFIG_USB_XHCI_PCI=y CONFIG_VFIO=y CONFIG_VFIO_PCI=y CONFIG_VGA=y CONFIG_VGA_PCI=y CONFIG_VHOST_USER=y CONFIG_VIRTIO_PCI=y CONFIG_VIRTIO_VGA=y CONFIG_WDT_IB6300ESB=y CONFIG_XICS=y CONFIG_XIVE=y CONFIG_TPM=y CONFIG_TPM_SPAPR=y CONFIG_TPM_EMULATOR=y EOF $ mkdir build $ cd build $ ../configure --cc=clang --cxx=/bin/false --prefix=/usr --libdir=/usr/lib64 --datadir=/usr/share --sysconfdir=/etc --interp-prefix=/usr/qemu-%M --localstatedir=/var --docdir=/usr/share/doc --libexecdir=/usr/libexec '--extra-ldflags=-Wl,-z,relro -Wl,--as-needed -Wl,-z,now ' '--extra-cflags=-O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS --config /usr/lib/rpm/redhat/redhat-hardened-clang.cfg -fstack-protector-strong -m64 -mcpu=power9 -mtune=power9 -fasynchronous-unwind-tables -fstack-clash-protection -Wno-string-plus-int' --with-pkgversion=qemu-kvm-7.0.0-1.el9 --with-suffix=qemu-kvm --firmwarepath=/usr/share/qemu-firmware:/usr/share/ipxe/qemu:/usr/share/seavgabios:/usr/share/seabios --meson=internal --enable-trace-backend=dtrace --with-coroutine=ucontext --with-git=git --tls-priority=@QEMU,SYSTEM --audio-drv-list= --disable-alsa --disable-attr --disable-auth-pam --disable-avx2 --disable-avx512f --disable-block-drv-whitelist-in-tools --disable-bochs --disable-bpf --disable-brlapi --disable-bsd-user --disable-bzip2 --disable-cap-ng --disable-capstone --disable-cfi --disable-cfi-debug --disable-cloop --disable-cocoa --disable-coreaudio --disable-coroutine-pool --disable-crypto-afalg --disable-curl --disable-curses --disable-dbus-display --disable-debug-info --disable-debug-mutex --disable-debug-tcg --disable-dmg --disable-docs --disable-dsound --disable-fdt --disable-fuse --disable-fuse-lseek --disable-gcrypt --disable-gettext --disable-gio --disable-glusterfs --disable-gnutls --disable-gtk --disable-guest-agent --
Re: [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set
Hi, Mark. On 5/4/22 04:10, Mark Cave-Ayland wrote: On 02/05/2022 14:36, Murilo Opsfelder Araújo wrote: Hi, Mark. Thanks for reviewing. Comments below. On 5/2/22 06:43, Mark Cave-Ayland wrote: On 30/04/2022 00:31, Murilo Opsfelder Araujo wrote: When CONFIG_MOS6522 is not set, building ppc64-softmmu target fails: /usr/bin/ld: libqemu-ppc64-softmmu.fa.p/monitor_misc.c.o:(.data+0x1158): undefined reference to `hmp_info_via' clang-13: error: linker command failed with exit code 1 (use -v to see invocation) Add CONFIG_MOS6522 check for hmp_info_via in hmp-commands-info.hx to fix such linking error. Fixes: 409e9f7131e5 (mos6522: add "info via" HMP command for debugging) Signed-off-by: Murilo Opsfelder Araujo Cc: Mark Cave-Ayland Cc: Fabiano Rosas --- hmp-commands-info.hx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index adfa085a9b..9ad784dd9f 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -881,6 +881,7 @@ SRST ERST #if defined(TARGET_M68K) || defined(TARGET_PPC) +#if defined(CONFIG_MOS6522) { .name = "via", .args_type = "", @@ -889,6 +890,7 @@ ERST .cmd = hmp_info_via, }, #endif +#endif SRST ``info via`` Hmmm. The patch in its proposed form isn't correct, since device CONFIG_* defines aren't declared when processing hmp-commands-info.hx. This was something that was discovered and discussed in the original thread for which the current workaround is to use the per-target TARGET_* defines instead. So my proposed fix worked just by coincidence. Thanks for providing the background. Given that the g3beige and mac99 machines are included by default in qemu-system-ppc64 which both contain the MOS6522 device, I can't quite understand how CONFIG_MOS6522 isn't being selected. Can you give more information about how you are building QEMU including your configure command line? Here is a reproducer adapted from CentOS 9 Stream qemu-kvm[0] package (build failed on c9s ppc64le with QEMU at commit f5643914a9e8f79c606a76e6a9d7ea82a3fc3e65): $ cat > configs/devices/rh-virtio.mak <<"EOF" CONFIG_VIRTIO=y CONFIG_VIRTIO_BALLOON=y CONFIG_VIRTIO_BLK=y CONFIG_VIRTIO_GPU=y CONFIG_VIRTIO_INPUT=y CONFIG_VIRTIO_INPUT_HOST=y CONFIG_VIRTIO_NET=y CONFIG_VIRTIO_RNG=y CONFIG_VIRTIO_SCSI=y CONFIG_VIRTIO_SERIAL=y EOF $ cat > configs/devices/ppc64-softmmu/ppc64-rh-devices.mak <<"EOF" include ../rh-virtio.mak CONFIG_DIMM=y CONFIG_MEM_DEVICE=y CONFIG_NVDIMM=y CONFIG_PCI=y CONFIG_PCI_DEVICES=y CONFIG_PCI_TESTDEV=y CONFIG_PCI_EXPRESS=y CONFIG_PSERIES=y CONFIG_SCSI=y CONFIG_SPAPR_VSCSI=y CONFIG_TEST_DEVICES=y CONFIG_USB=y CONFIG_USB_OHCI=y CONFIG_USB_OHCI_PCI=y CONFIG_USB_SMARTCARD=y CONFIG_USB_STORAGE_CORE=y CONFIG_USB_STORAGE_CLASSIC=y CONFIG_USB_XHCI=y CONFIG_USB_XHCI_NEC=y CONFIG_USB_XHCI_PCI=y CONFIG_VFIO=y CONFIG_VFIO_PCI=y CONFIG_VGA=y CONFIG_VGA_PCI=y CONFIG_VHOST_USER=y CONFIG_VIRTIO_PCI=y CONFIG_VIRTIO_VGA=y CONFIG_WDT_IB6300ESB=y CONFIG_XICS=y CONFIG_XIVE=y CONFIG_TPM=y CONFIG_TPM_SPAPR=y CONFIG_TPM_EMULATOR=y EOF $ mkdir build $ cd build $ ../configure --cc=clang --cxx=/bin/false --prefix=/usr --libdir=/usr/lib64 --datadir=/usr/share --sysconfdir=/etc --interp-prefix=/usr/qemu-%M --localstatedir=/var --docdir=/usr/share/doc --libexecdir=/usr/libexec '--extra-ldflags=-Wl,-z,relro -Wl,--as-needed -Wl,-z,now ' '--extra-cflags=-O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS --config /usr/lib/rpm/redhat/redhat-hardened-clang.cfg -fstack-protector-strong -m64 -mcpu=power9 -mtune=power9 -fasynchronous-unwind-tables -fstack-clash-protection -Wno-string-plus-int' --with-pkgversion=qemu-kvm-7.0.0-1.el9 --with-suffix=qemu-kvm --firmwarepath=/usr/share/qemu-firmware:/usr/share/ipxe/qemu:/usr/share/seavgabios:/usr/share/seabios --meson=internal --enable-trace-backend=dtrace --with-coroutine=ucontext --with-git=git --tls-priority=@QEMU,SYSTEM --audio-drv-list= --disable-alsa --disable-attr --disable-auth-pam --disable-avx2 --disable-avx512f --disable-block-drv-whitelist-in-tools --disable-bochs --disable-bpf --disable-brlapi --disable-bsd-user --disable-bzip2 --disable-cap-ng --disable-capstone --disable-cfi --disable-cfi-debug --disable-cloop --disable-cocoa --disable-coreaudio --disable-coroutine-pool --disable-crypto-afalg --disable-curl --disable-curses --disable-dbus-display --disable-debug-info --disable-debug-mutex --disable-debug-tcg --disable-dmg --disable-docs --disable-dsound --disable-fdt --disable-fuse --disable-fuse-lseek --disable-gcrypt --disable-gettext --disable-gio --disable-glusterfs --disable-gnutls --disable-gtk --disable-guest-agent --disable-guest-agent-msi --disable-hax --disable-hvf --disable-iconv --disable-jack --disable-kvm --disable-l2
Re: [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set
Hi, Mark. Thanks for reviewing. Comments below. On 5/2/22 06:43, Mark Cave-Ayland wrote: On 30/04/2022 00:31, Murilo Opsfelder Araujo wrote: When CONFIG_MOS6522 is not set, building ppc64-softmmu target fails: /usr/bin/ld: libqemu-ppc64-softmmu.fa.p/monitor_misc.c.o:(.data+0x1158): undefined reference to `hmp_info_via' clang-13: error: linker command failed with exit code 1 (use -v to see invocation) Add CONFIG_MOS6522 check for hmp_info_via in hmp-commands-info.hx to fix such linking error. Fixes: 409e9f7131e5 (mos6522: add "info via" HMP command for debugging) Signed-off-by: Murilo Opsfelder Araujo Cc: Mark Cave-Ayland Cc: Fabiano Rosas --- hmp-commands-info.hx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index adfa085a9b..9ad784dd9f 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -881,6 +881,7 @@ SRST ERST #if defined(TARGET_M68K) || defined(TARGET_PPC) +#if defined(CONFIG_MOS6522) { .name = "via", .args_type = "", @@ -889,6 +890,7 @@ ERST .cmd = hmp_info_via, }, #endif +#endif SRST ``info via`` Hmmm. The patch in its proposed form isn't correct, since device CONFIG_* defines aren't declared when processing hmp-commands-info.hx. This was something that was discovered and discussed in the original thread for which the current workaround is to use the per-target TARGET_* defines instead. So my proposed fix worked just by coincidence. Thanks for providing the background. Given that the g3beige and mac99 machines are included by default in qemu-system-ppc64 which both contain the MOS6522 device, I can't quite understand how CONFIG_MOS6522 isn't being selected. Can you give more information about how you are building QEMU including your configure command line? Here is a reproducer adapted from CentOS 9 Stream qemu-kvm[0] package (build failed on c9s ppc64le with QEMU at commit f5643914a9e8f79c606a76e6a9d7ea82a3fc3e65): $ cat > configs/devices/rh-virtio.mak <<"EOF" CONFIG_VIRTIO=y CONFIG_VIRTIO_BALLOON=y CONFIG_VIRTIO_BLK=y CONFIG_VIRTIO_GPU=y CONFIG_VIRTIO_INPUT=y CONFIG_VIRTIO_INPUT_HOST=y CONFIG_VIRTIO_NET=y CONFIG_VIRTIO_RNG=y CONFIG_VIRTIO_SCSI=y CONFIG_VIRTIO_SERIAL=y EOF $ cat > configs/devices/ppc64-softmmu/ppc64-rh-devices.mak <<"EOF" include ../rh-virtio.mak CONFIG_DIMM=y CONFIG_MEM_DEVICE=y CONFIG_NVDIMM=y CONFIG_PCI=y CONFIG_PCI_DEVICES=y CONFIG_PCI_TESTDEV=y CONFIG_PCI_EXPRESS=y CONFIG_PSERIES=y CONFIG_SCSI=y CONFIG_SPAPR_VSCSI=y CONFIG_TEST_DEVICES=y CONFIG_USB=y CONFIG_USB_OHCI=y CONFIG_USB_OHCI_PCI=y CONFIG_USB_SMARTCARD=y CONFIG_USB_STORAGE_CORE=y CONFIG_USB_STORAGE_CLASSIC=y CONFIG_USB_XHCI=y CONFIG_USB_XHCI_NEC=y CONFIG_USB_XHCI_PCI=y CONFIG_VFIO=y CONFIG_VFIO_PCI=y CONFIG_VGA=y CONFIG_VGA_PCI=y CONFIG_VHOST_USER=y CONFIG_VIRTIO_PCI=y CONFIG_VIRTIO_VGA=y CONFIG_WDT_IB6300ESB=y CONFIG_XICS=y CONFIG_XIVE=y CONFIG_TPM=y CONFIG_TPM_SPAPR=y CONFIG_TPM_EMULATOR=y EOF $ mkdir build $ cd build $ ../configure --cc=clang --cxx=/bin/false --prefix=/usr --libdir=/usr/lib64 --datadir=/usr/share --sysconfdir=/etc --interp-prefix=/usr/qemu-%M --localstatedir=/var --docdir=/usr/share/doc --libexecdir=/usr/libexec '--extra-ldflags=-Wl,-z,relro -Wl,--as-needed -Wl,-z,now ' '--extra-cflags=-O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS --config /usr/lib/rpm/redhat/redhat-hardened-clang.cfg -fstack-protector-strong -m64 -mcpu=power9 -mtune=power9 -fasynchronous-unwind-tables -fstack-clash-protection -Wno-string-plus-int' --with-pkgversion=qemu-kvm-7.0.0-1.el9 --with-suffix=qemu-kvm --firmwarepath=/usr/share/qemu-firmware:/usr/share/ipxe/qemu:/usr/share/seavgabios:/usr/share/seabios --meson=internal --enable-trace-backend=dtrace --with-coroutine=ucontext --with-git=git --tls-priority=@QEMU,SYSTEM --audio-drv-list= --disable-alsa --disable-attr --disable-auth-pam --disable-avx2 --disable-avx512f --disable-block-drv-whitelist-in-tools --disable-bochs --disable-bpf --disable-brlapi --disable-bsd-user --disable-bzip2 --disable-cap-ng --disable-capstone --disable-cfi --disable-cfi-debug --disable-cloop --disable-cocoa --disable-coreaudio --disable-coroutine-pool --disable-crypto-afalg --disable-curl --disable-curses --disable-dbus-display --disable-debug-info --disable-debug-mutex --disable-debug-tcg --disable-dmg --disable-docs --disable-dsound --disable-fdt --disable-fuse --disable-fuse-lseek --disable-gcrypt --disable-gettext --disable-gio --disable-glusterfs --disable-gnutls --disable-gtk --disable-guest-agent --disable-guest-agent-msi --disable-hax --disable-hvf --disable-iconv --disable-jack --disable-kvm --disable-l2tpv3 --disable-libdaxctl --disable-libiscsi --disable-libnfs --disable-libpmem --disable-libssh --disable-libudev --disable-libusb --disable-linux-aio --disable-linux-io-uring --disable-linux-us
Re: [PATCH v2] block-qdict: Fix -Werror=maybe-uninitialized build failure
Hi, Philippe. On Monday, March 14, 2022 10:47:11 AM -03 Philippe Mathieu-Daudé wrote: > On 11/3/22 23:16, Murilo Opsfelder Araujo wrote: > > Building QEMU on Fedora 37 (Rawhide Prerelease) ppc64le failed with the > > following error: > > > > $ ../configure --prefix=/usr/local/qemu-disabletcg > > --target-list=ppc-softmmu,ppc64-softmmu --disable-tcg --disable-linux-user > > ... > > $ make -j$(nproc) > > ... > > FAILED: libqemuutil.a.p/qobject_block-qdict.c.o > > This part >>> > > > cc -m64 -mlittle-endian -Ilibqemuutil.a.p -I. -I.. > > -Isubprojects/libvhost-user -I../subprojects/libvhost-user -Iqapi -Itrace > > -Iui -Iui/shader -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include > > -I/usr/include/sysprof-4 -I/usr/include/lib > > mount -I/usr/include/blkid -I/usr/include/gio-unix-2.0 > > -I/usr/include/p11-kit-1 -I/usr/include/pixman-1 -fdiagnostics-color=auto > > -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem > > /root/qemu/linux-headers -isystem linux-headers -iquote > > . -iquote /root/qemu -iquote /root/qemu/include -iquote > > /root/qemu/disas/libvixl -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 > > -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE > > -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite > > -strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv > > -Wold-style-declaration -Wold-style-definition -Wtype-limits > > -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers > > -Wempty-body -Wnested-externs -Wendif-label > > s -Wexpansion-to-defined -Wimplicit-fallthrough=2 > > -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi > > -fstack-protector-strong -fPIE -MD -MQ > > libqemuutil.a.p/qobject_block-qdict.c.o -MF > > libqemuutil.a.p/qobject_block-qdict.c.o.d - > > o libqemuutil.a.p/qobject_block-qdict.c.o -c ../qobject/block-qdict.c > > <<< is noise (doesn't provide any value) and could be stripped. Is this something the committer/maintainer could edit when applying the commit or do you need I resend the v3? Cheers! > > > In file included from /root/qemu/include/qapi/qmp/qdict.h:16, > > from /root/qemu/include/block/qdict.h:13, > > from ../qobject/block-qdict.c:11: > > /root/qemu/include/qapi/qmp/qobject.h: In function > > âqdict_array_splitâ: > > /root/qemu/include/qapi/qmp/qobject.h:49:17: error: âsubqdictâ may > > be used uninitialized [-Werror=maybe-uninitialized] > > 49 | typeof(obj) _obj = (obj); > > \ > >| ^~~~ > > ../qobject/block-qdict.c:227:16: note: âsubqdictâ declared here > >227 | QDict *subqdict; > >|^~~~ > > cc1: all warnings being treated as errors > > > > Fix build failure by expanding the ternary operation. > > Tested with `make check-unit` (the check-block-qdict test passed). > > > > Signed-off-by: Murilo Opsfelder Araujo > > Cc: Kevin Wolf > > Cc: Hanna Reitz > > Cc: Markus Armbruster > > --- > > v1: https://lists.nongnu.org/archive/html/qemu-devel/2022-03/msg03224.html > > > > qobject/block-qdict.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/qobject/block-qdict.c b/qobject/block-qdict.c > > index 1487cc5dd8..4a83bda2c3 100644 > > --- a/qobject/block-qdict.c > > +++ b/qobject/block-qdict.c > > @@ -251,12 +251,12 @@ void qdict_array_split(QDict *src, QList **dst) > > if (is_subqdict) { > > qdict_extract_subqdict(src, &subqdict, prefix); > > assert(qdict_size(subqdict) > 0); > > +qlist_append_obj(*dst, QOBJECT(subqdict)); > > } else { > > qobject_ref(subqobj); > > qdict_del(src, indexstr); > > +qlist_append_obj(*dst, subqobj); > > } > > - > > -qlist_append_obj(*dst, subqobj ?: QOBJECT(subqdict)); > > } > > } > > > > > > -- Murilo
Re: [PATCH] block-qdict: Fix -Werror=maybe-uninitialized build failure
Hi, Markus. On 3/11/22 06:33, Markus Armbruster wrote: Murilo Opsfelder Araujo writes: Building QEMU on Fedora 37 (Rawhide Prerelease) ppc64le failed with the following error: $ ../configure --prefix=/usr/local/qemu-disabletcg --target-list=ppc-softmmu,ppc64-softmmu --disable-tcg --disable-linux-user ... $ make -j$(nproc) ... FAILED: libqemuutil.a.p/qobject_block-qdict.c.o cc -m64 -mlittle-endian -Ilibqemuutil.a.p -I. -I.. -Isubprojects/libvhost-user -I../subprojects/libvhost-user -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -I/usr/include/lib mount -I/usr/include/blkid -I/usr/include/gio-unix-2.0 -I/usr/include/p11-kit-1 -I/usr/include/pixman-1 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /root/qemu/linux-headers -isystem linux-headers -iquote . -iquote /root/qemu -iquote /root/qemu/include -iquote /root/qemu/disas/libvixl -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite -strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-label s -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -MD -MQ libqemuutil.a.p/qobject_block-qdict.c.o -MF libqemuutil.a.p/qobject_block-qdict.c.o.d - o libqemuutil.a.p/qobject_block-qdict.c.o -c ../qobject/block-qdict.c In file included from /root/qemu/include/qapi/qmp/qdict.h:16, from /root/qemu/include/block/qdict.h:13, from ../qobject/block-qdict.c:11: /root/qemu/include/qapi/qmp/qobject.h: In function ‘qdict_array_split’: /root/qemu/include/qapi/qmp/qobject.h:49:17: error: ‘subqdict’ may be used uninitialized [-Werror=maybe-uninitialized] 49 | typeof(obj) _obj = (obj); \ | ^~~~ ../qobject/block-qdict.c:227:16: note: ‘subqdict’ declared here 227 | QDict *subqdict; |^~~~ cc1: all warnings being treated as errors Fix build failure by initializing the QDict variable with NULL. Signed-off-by: Murilo Opsfelder Araujo Cc: Kevin Wolf Cc: Hanna Reitz Cc: Markus Armbruster --- qobject/block-qdict.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qobject/block-qdict.c b/qobject/block-qdict.c index 1487cc5dd8..b26524429c 100644 --- a/qobject/block-qdict.c +++ b/qobject/block-qdict.c @@ -224,7 +224,7 @@ void qdict_array_split(QDict *src, QList **dst) for (i = 0; i < UINT_MAX; i++) { QObject *subqobj; bool is_subqdict; -QDict *subqdict; +QDict *subqdict = NULL; char indexstr[32], prefix[32]; size_t snprintf_ret; The compiler's warning is actually spurious. Your patch is the minimally invasive way to shut it up. But I wonder whether we can make the code clearer instead. Let's have a look: /* * There may be either a single subordinate object (named * "%u") or multiple objects (each with a key prefixed "%u."), * but not both. */ if (!subqobj == !is_subqdict) { break; Because of this, ... } if (is_subqdict) { ... subqobj is non-null here, and ... qdict_extract_subqdict(src, &subqdict, prefix); assert(qdict_size(subqdict) > 0); } else { ... null here. qobject_ref(subqobj); qdict_del(src, indexstr); } qlist_append_obj(*dst, subqobj ?: QOBJECT(subqdict)); What about this: if (is_subqdict) { qdict_extract_subqdict(src, &subqdict, prefix); assert(qdict_size(subqdict) > 0); qlist_append_obj(*dst, subqobj); } else { qobject_ref(subqobj); qdict_del(src, indexstr); qlist_append_obj(*dst, QOBJECT(subqdict)); } The logic looks inverted but I think I got what you meant. I've sent a v2 with changes that made the compiler happy and also passed check-unit tests. Thank you! -- Murilo
Re: [PATCH 0/9] --disable-tcg avocado fixes for ppc-softmmu
On 3/10/22 15:30, Daniel Henrique Barboza wrote: Hi, These are more test fixes that I missed from my first series [1]. Thanks Murilo Opsfelder and Fabiano for letting me know that we still had broken tests to deal with. All these tests were either a case of 'this needs kvm_pr' or 'this needs kvm_hv'. Since avocado doesn't have yet a way of detecting if the host is running kvm_hv or kvm_pr, the workaround for now is to skip them if TCG isn't available. [1] https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg00866.html Daniel Henrique Barboza (9): avocado/boot_linux_console.py: check TCG accel in test_ppc_g3beige() avocado/boot_linux_console.py: check TCG accel in test_ppc_mac99() avocado/ppc_405.py: remove test_ppc_taihu() avocado/ppc_405.py: check TCG accel in test_ppc_ref405ep() avocado/ppc_74xx.py: check TCG accel for all tests avocado/ppc_bamboo.py: check TCG accel in test_ppc_bamboo() avocado/ppc_mpc8544ds.py: check TCG accel in test_ppc_mpc8544ds() avocado/ppc_prep_40p.py: check TCG accel in all tests avocado/ppc_virtex_ml507.py: check TCG accel in test_ppc_virtex_ml507() tests/avocado/boot_linux_console.py | 12 tests/avocado/ppc_405.py| 10 ++ tests/avocado/ppc_74xx.py | 13 + tests/avocado/ppc_bamboo.py | 2 ++ tests/avocado/ppc_mpc8544ds.py | 2 ++ tests/avocado/ppc_prep_40p.py | 6 ++ tests/avocado/ppc_virtex_ml507.py | 2 ++ 7 files changed, 39 insertions(+), 8 deletions(-) Hi, Daniel. With this series and series "--disable-tcg qtest/avocado fixes for ppc64" [0] applied, check-avocado passed for QEMU built with --disable-tcg --disable-linux-user. [0] https://lists.nongnu.org/archive/html/qemu-devel/2022-03/msg00866.html So: Tested-by: Murilo Opsfelder Araujo -- Murilo
Re: [PATCH 3/7] ppc/pnv: Use skiboot addresses to load kernel and ramfs
On Tuesday, January 26, 2021 2:10:55 PM -03 Cédric Le Goater wrote: > The current settings are useful to load large kernels (with debug) but > it moves the initrd image in a memory region not protected by > skiboot. If skiboot is compiled with DEBUG=1, memory poisoning will > corrupt the initrd. > > Cc: Murilo Opsfelder Araujo > Signed-off-by: Cédric Le Goater Reviewed-by: Murilo Opsfelder Araujo > --- > > If we want to increase the kernel size limit as commit b45b56baeecd > ("ppc/pnv: increase kernel size limit to 256MiB") intented to do, I > think we should add a machine option. > > hw/ppc/pnv.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 14fc9758a973..e500c2e2437e 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -65,9 +65,9 @@ > #define FW_MAX_SIZE (16 * MiB) > > #define KERNEL_LOAD_ADDR0x2000 > -#define KERNEL_MAX_SIZE (256 * MiB) > -#define INITRD_LOAD_ADDR0x6000 > -#define INITRD_MAX_SIZE (256 * MiB) > +#define KERNEL_MAX_SIZE (128 * MiB) > +#define INITRD_LOAD_ADDR0x2800 > +#define INITRD_MAX_SIZE (128 * MiB) > > static const char *pnv_chip_core_typename(const PnvChip *o) > { -- Murilo
Re: [PATCH 3/7] ppc/pnv: Use skiboot addresses to load kernel and ramfs
Bonjour, Cédric. On Tuesday, January 26, 2021 2:10:55 PM -03 Cédric Le Goater wrote: > The current settings are useful to load large kernels (with debug) but > it moves the initrd image in a memory region not protected by > skiboot. If skiboot is compiled with DEBUG=1, memory poisoning will > corrupt the initrd. > > Cc: Murilo Opsfelder Araujo > Signed-off-by: Cédric Le Goater > --- > > If we want to increase the kernel size limit as commit b45b56baeecd > ("ppc/pnv: increase kernel size limit to 256MiB") intented to do, I > think we should add a machine option. Is this a problem on bare-metal as well? I'm wondering if we should address this the other way around by increasing KERNEL_LOAD_SIZE and INITRAMFS_LOAD_SIZE in skiboot to accomodate large kernel and initramfs images. I think Linux debuginfo images won't get smaller with time and, assuming this also happens on bare-metal (I haven't verified), updating skiboot looks more appropriate. Bear in mind that I'm not an skiboot expert, I'm just considering the possibilities. > > hw/ppc/pnv.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 14fc9758a973..e500c2e2437e 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -65,9 +65,9 @@ > #define FW_MAX_SIZE (16 * MiB) > > #define KERNEL_LOAD_ADDR0x2000 > -#define KERNEL_MAX_SIZE (256 * MiB) > -#define INITRD_LOAD_ADDR0x6000 > -#define INITRD_MAX_SIZE (256 * MiB) > +#define KERNEL_MAX_SIZE (128 * MiB) > +#define INITRD_LOAD_ADDR0x2800 > +#define INITRD_MAX_SIZE (128 * MiB) > > static const char *pnv_chip_core_typename(const PnvChip *o) > { Cheers! -- Murilo
[Bug 1866962] Re: [Regression]Powerpc kvm guest unable to start with hugepage backed memory
Thank you for verifying, Satheesh. ** Changed in: qemu Status: New => 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/1866962 Title: [Regression]Powerpc kvm guest unable to start with hugepage backed memory Status in QEMU: Fix Released Bug description: Current upstream qemu master does not boot a powerpc kvm guest backed by hugepage. HW: Power9 (DD2.3) Host Kernel: 5.6.0-rc5 Guest Kernel: 5.6.0-rc5 Qemu: ba29883206d92a29ad5a466e679ccfc2ee6132ef Steps to reproduce: 1. Allocate enough hugepage to boot a KVM guest # cat /proc/meminfo |grep ^HugePages HugePages_Total:5000 HugePages_Free: 5000 HugePages_Rsvd:0 HugePages_Surp:0 2. Define and boot a guest /usr/bin/virt-install --connect=qemu:///system --hvm --accelerate --name 'vm1' --machine pseries --memory=8192,hugepages=yes --vcpu=8,maxvcpus=8,sockets=1,cores=8,threads=1 --import --nographics --serial pty --memballoon model=virtio --controller type=scsi,model=virtio-scsi --disk path=/home/kvmci/tests/data/avocado-vt/images/f31-ppc64le.qcow2,bus=scsi,size=10,format=qcow2 --network=bridge=virbr0,model=virtio,mac=52:54:00:5f:82:83 --mac=52:54:00:5f:82:83 --boot emulator=/home/sath/qemu/ppc64-softmmu/qemu-system-ppc64,kernel=/home/kvmci/linux/vmlinux,kernel_args="root=/dev/sda5 rw console=tty0 console=ttyS0,115200 init=/sbin/init initcall_debug selinux=0" --noautoconsole Starting install... ERRORinternal error: qemu unexpectedly closed the monitor: qemu-system-ppc64: util/qemu-thread-posix.c:76: qemu_mutex_lock_impl: Assertion `mutex->initialized' failed. qemu-system-ppc64: util/qemu-thread-posix.c:76: qemu_mutex_lock_impl: Assertion `mutex->initialized' failed. ---NOK Bisected the issue to below commit. 037fb5eb3941c80a2b7c36a843e47207ddb004d4 is the first bad commit commit 037fb5eb3941c80a2b7c36a843e47207ddb004d4 Author: bauerchen Date: Tue Feb 11 17:10:35 2020 +0800 mem-prealloc: optimize large guest startup [desc]: Large memory VM starts slowly when using -mem-prealloc, and there are some areas to optimize in current method; 1、mmap will be used to alloc threads stack during create page clearing threads, and it will attempt mm->mmap_sem for write lock, but clearing threads have hold read lock, this competition will cause threads createion very slow; 2、methods of calcuating pages for per threads is not well;if we use 64 threads to split 160 hugepage,63 threads clear 2page,1 thread clear 34 page,so the entire speed is very slow; to solve the first problem,we add a mutex in thread function,and start all threads when all threads finished createion; and the second problem, we spread remainder to other threads,in situation that 160 hugepage and 64 threads, there are 32 threads clear 3 pages,and 32 threads clear 2 pages. [test]: 320G 84c VM start time can be reduced to 10s 680G 84c VM start time can be reduced to 18s Signed-off-by: bauerchen Reviewed-by: Pan Rui Reviewed-by: Ivan Ren [Simplify computation of the number of pages per thread. - Paolo] Signed-off-by: Paolo Bonzini util/oslib-posix.c | 32 1 file changed, 24 insertions(+), 8 deletions(-) bisect log: # git bisect log git bisect start # good: [52901abf94477b400cf88c1f70bb305e690ba2de] Update version for v4.2.0-rc5 release git bisect good 52901abf94477b400cf88c1f70bb305e690ba2de # bad: [ba29883206d92a29ad5a466e679ccfc2ee6132ef] Merge remote-tracking branch 'remotes/borntraeger/tags/s390x-20200310' into staging git bisect bad ba29883206d92a29ad5a466e679ccfc2ee6132ef # good: [d1ebbc9d16297b54b153ee33abe05eb4f1df0c66] target/arm/kvm: trivial: Clean up header documentation git bisect good d1ebbc9d16297b54b153ee33abe05eb4f1df0c66 # good: [87b74e8b6edd287ea2160caa0ebea725fa8f1ca1] target/arm: Vectorize USHL and SSHL git bisect good 87b74e8b6edd287ea2160caa0ebea725fa8f1ca1 # bad: [e0175b71638cf4398903c0d25f93fe62e0606389] Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200228' into staging git bisect bad e0175b71638cf4398903c0d25f93fe62e0606389 # bad: [ca6155c0f2bd39b4b4162533be401c98bd960820] Merge tag 'patchew/20200219160953.13771-1-imamm...@redhat.com' of https://github.com/patchew-project/qemu into HEAD git bisect bad ca6155c0f2bd39b4b4162533be401c98bd960820 # good: [ab74e543112957696f7c79b0c33ecebd18b52af5] ppc/spapr: use memdev for RAM git bisect good ab74e543112957696f7c79b0c33ecebd18b52af5 # good: [cb06fdad05f3e546a4e20f1f3c0127f9ae53de1a] fuzz: support for fork-based fuzzing. git bisect good cb06fdad05f3e546a4e20f1
[Bug 1866962] Re: [Regression]Powerpc kvm guest unable to start with hugepage backed memory
This issue seems to be fixed by commit 78b3f67acdf0f646d35ebdf98b9e91fb04ab9a07 Author: Paolo Bonzini Date: Tue Mar 10 18:58:30 2020 +0100 oslib-posix: initialize mutex and condition variable The mutex and condition variable were never initialized, causing -mem-prealloc to abort with an assertion failure. Fixes: 037fb5eb3941c80a2b7c36a843e47207ddb004d4 Reported-by: Marc Hartmayer Cc: bauerchen Signed-off-by: Paolo Bonzini -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1866962 Title: [Regression]Powerpc kvm guest unable to start with hugepage backed memory Status in QEMU: New Bug description: Current upstream qemu master does not boot a powerpc kvm guest backed by hugepage. HW: Power9 (DD2.3) Host Kernel: 5.6.0-rc5 Guest Kernel: 5.6.0-rc5 Qemu: ba29883206d92a29ad5a466e679ccfc2ee6132ef Steps to reproduce: 1. Allocate enough hugepage to boot a KVM guest # cat /proc/meminfo |grep ^HugePages HugePages_Total:5000 HugePages_Free: 5000 HugePages_Rsvd:0 HugePages_Surp:0 2. Define and boot a guest /usr/bin/virt-install --connect=qemu:///system --hvm --accelerate --name 'vm1' --machine pseries --memory=8192,hugepages=yes --vcpu=8,maxvcpus=8,sockets=1,cores=8,threads=1 --import --nographics --serial pty --memballoon model=virtio --controller type=scsi,model=virtio-scsi --disk path=/home/kvmci/tests/data/avocado-vt/images/f31-ppc64le.qcow2,bus=scsi,size=10,format=qcow2 --network=bridge=virbr0,model=virtio,mac=52:54:00:5f:82:83 --mac=52:54:00:5f:82:83 --boot emulator=/home/sath/qemu/ppc64-softmmu/qemu-system-ppc64,kernel=/home/kvmci/linux/vmlinux,kernel_args="root=/dev/sda5 rw console=tty0 console=ttyS0,115200 init=/sbin/init initcall_debug selinux=0" --noautoconsole Starting install... ERRORinternal error: qemu unexpectedly closed the monitor: qemu-system-ppc64: util/qemu-thread-posix.c:76: qemu_mutex_lock_impl: Assertion `mutex->initialized' failed. qemu-system-ppc64: util/qemu-thread-posix.c:76: qemu_mutex_lock_impl: Assertion `mutex->initialized' failed. ---NOK Bisected the issue to below commit. 037fb5eb3941c80a2b7c36a843e47207ddb004d4 is the first bad commit commit 037fb5eb3941c80a2b7c36a843e47207ddb004d4 Author: bauerchen Date: Tue Feb 11 17:10:35 2020 +0800 mem-prealloc: optimize large guest startup [desc]: Large memory VM starts slowly when using -mem-prealloc, and there are some areas to optimize in current method; 1、mmap will be used to alloc threads stack during create page clearing threads, and it will attempt mm->mmap_sem for write lock, but clearing threads have hold read lock, this competition will cause threads createion very slow; 2、methods of calcuating pages for per threads is not well;if we use 64 threads to split 160 hugepage,63 threads clear 2page,1 thread clear 34 page,so the entire speed is very slow; to solve the first problem,we add a mutex in thread function,and start all threads when all threads finished createion; and the second problem, we spread remainder to other threads,in situation that 160 hugepage and 64 threads, there are 32 threads clear 3 pages,and 32 threads clear 2 pages. [test]: 320G 84c VM start time can be reduced to 10s 680G 84c VM start time can be reduced to 18s Signed-off-by: bauerchen Reviewed-by: Pan Rui Reviewed-by: Ivan Ren [Simplify computation of the number of pages per thread. - Paolo] Signed-off-by: Paolo Bonzini util/oslib-posix.c | 32 1 file changed, 24 insertions(+), 8 deletions(-) bisect log: # git bisect log git bisect start # good: [52901abf94477b400cf88c1f70bb305e690ba2de] Update version for v4.2.0-rc5 release git bisect good 52901abf94477b400cf88c1f70bb305e690ba2de # bad: [ba29883206d92a29ad5a466e679ccfc2ee6132ef] Merge remote-tracking branch 'remotes/borntraeger/tags/s390x-20200310' into staging git bisect bad ba29883206d92a29ad5a466e679ccfc2ee6132ef # good: [d1ebbc9d16297b54b153ee33abe05eb4f1df0c66] target/arm/kvm: trivial: Clean up header documentation git bisect good d1ebbc9d16297b54b153ee33abe05eb4f1df0c66 # good: [87b74e8b6edd287ea2160caa0ebea725fa8f1ca1] target/arm: Vectorize USHL and SSHL git bisect good 87b74e8b6edd287ea2160caa0ebea725fa8f1ca1 # bad: [e0175b71638cf4398903c0d25f93fe62e0606389] Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200228' into staging git bisect bad e0175b71638cf4398903c0d25f93fe62e0606389 # bad: [ca6155c0f2bd39b4b4162533be401c98bd960820] Merge tag 'patchew/20200219160953.13771-1-imamm...@redhat.com' of https://github.com/patchew-project/qemu into HEA
Re: [PATCH v4 10/15] util/mmap-alloc: Prepare for resizeable mmaps
On Thursday, March 5, 2020 11:29:40 AM -03 David Hildenbrand wrote: > When shrinking a mmap we want to re-reserve the already activated area. > When growing a memory region, we want to activate starting with a given > fd_offset. Prepare by allowing to pass these parameters. > > Also, let's make sure we always process full pages, to avoid > unmapping/remapping pages that are already in use when > growing/shrinking. Add some asserts. > > Reviewed-by: Richard Henderson > Reviewed-by: Peter Xu > Cc: Igor Kotrasinski > Cc: Murilo Opsfelder Araujo > Cc: "Michael S. Tsirkin" > Cc: Greg Kurz > Cc: Murilo Opsfelder Araujo > Cc: Eduardo Habkost > Cc: "Dr. David Alan Gilbert" > Cc: Igor Mammedov > Signed-off-by: David Hildenbrand > --- Acked-by: Murilo Opsfelder Araujo > util/mmap-alloc.c | 34 +++--- > 1 file changed, 23 insertions(+), 11 deletions(-) > > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c > index 8f40ef4fed..2767caa33b 100644 > --- a/util/mmap-alloc.c > +++ b/util/mmap-alloc.c > @@ -83,12 +83,12 @@ size_t qemu_mempath_getpagesize(const char *mem_path) > } > > /* > - * Reserve a new memory region of the requested size to be used for mapping > - * from the given fd (if any). > + * Reserve a new memory region of the requested size or re-reserve parts > + * of an activated region to be used for mapping from the given fd (if > any). */ > -static void *mmap_reserve(size_t size, int fd) > +static void *mmap_reserve(void *ptr, size_t size, int fd) > { > -int flags = MAP_PRIVATE; > +int flags = MAP_PRIVATE | (ptr ? MAP_FIXED : 0); > > #if defined(__powerpc64__) && defined(__linux__) > /* > @@ -111,20 +111,24 @@ static void *mmap_reserve(size_t size, int fd) > flags |= MAP_ANONYMOUS; > #endif > > -return mmap(0, size, PROT_NONE, flags, fd, 0); > +return mmap(ptr, size, PROT_NONE, flags, fd, 0); > } > > /* > * Activate memory in a reserved region from the given fd (if any), to make > * it accessible. > */ > -static void *mmap_activate(void *ptr, size_t size, int fd, bool shared, > - bool is_pmem) > +static void *mmap_activate(void *ptr, size_t size, int fd, size_t > fd_offset, + bool shared, bool is_pmem) > { > int map_sync_flags = 0; > int flags = MAP_FIXED; > void *activated_ptr; > > +if (fd == -1) { > +fd_offset = 0; > +} > + > flags |= fd == -1 ? MAP_ANONYMOUS : 0; > flags |= shared ? MAP_SHARED : MAP_PRIVATE; > if (shared && is_pmem) { > @@ -132,7 +136,7 @@ static void *mmap_activate(void *ptr, size_t size, int > fd, bool shared, } > > activated_ptr = mmap(ptr, size, PROT_READ | PROT_WRITE, > - flags | map_sync_flags, fd, 0); > + flags | map_sync_flags, fd, fd_offset); > if (activated_ptr == MAP_FAILED && map_sync_flags) { > if (errno == ENOTSUP) { > char *proc_link = g_strdup_printf("/proc/self/fd/%d", fd); > @@ -154,7 +158,8 @@ static void *mmap_activate(void *ptr, size_t size, int > fd, bool shared, * If mmap failed with MAP_SHARED_VALIDATE | MAP_SYNC, we > will try * again without these flags to handle backwards compatibility. */ > -activated_ptr = mmap(ptr, size, PROT_READ | PROT_WRITE, flags, fd, > 0); +activated_ptr = mmap(ptr, size, PROT_READ | PROT_WRITE, flags, > fd, + fd_offset); > } > return activated_ptr; > } > @@ -176,16 +181,19 @@ void *qemu_ram_mmap(int fd, > bool is_pmem) > { > const size_t guard_pagesize = mmap_guard_pagesize(fd); > +const size_t pagesize = qemu_fd_getpagesize(fd); > size_t offset, total; > void *ptr, *guardptr; > > +g_assert(QEMU_IS_ALIGNED(size, pagesize)); > + > /* > * Note: this always allocates at least one extra page of virtual > address * space, even if size is already aligned. > */ > total = size + align; > > -guardptr = mmap_reserve(total, fd); > +guardptr = mmap_reserve(NULL, total, fd); > if (guardptr == MAP_FAILED) { > return MAP_FAILED; > } > @@ -196,7 +204,7 @@ void *qemu_ram_mmap(int fd, > > offset = QEMU_ALIGN_UP((uintptr_t)guardptr, align) - > (uintptr_t)guardptr; > > -ptr = mmap_activate(guardptr + offset, size, fd, shared, is_pmem); > +ptr = mmap_activate(guardptr + offset, size, fd, 0, shared, is_pmem); > if (ptr == MAP_FAILED) { > munmap(guardptr, total); > return MAP_FAILED; > @@ -220,6 +228,10 @@ void *qemu_ram_mmap(int fd, > > void qemu_ram_munmap(int fd, void *ptr, size_t size) > { > +const size_t pagesize = qemu_fd_getpagesize(fd); > + > +g_assert(QEMU_IS_ALIGNED(size, pagesize)); > + > if (ptr) { > /* Unmap both the RAM block and the guard page */ > munmap(ptr, size + mmap_guard_pagesize(fd)); -- Murilo
Re: [PATCH v4 03/15] util: vfio-helpers: Factor out removal from qemu_vfio_undo_mapping()
On Thursday, March 5, 2020 11:29:33 AM -03 David Hildenbrand wrote: > Factor it out and properly use it where applicable. Make > qemu_vfio_undo_mapping() look like qemu_vfio_do_mapping(), passing the > size and iova, not the mapping. > > Reviewed-by: Peter Xu > Cc: Richard Henderson > Cc: Paolo Bonzini > Cc: Eduardo Habkost > Cc: Marcel Apfelbaum > Cc: Alex Williamson > Cc: Stefan Hajnoczi > Signed-off-by: David Hildenbrand > --- Acked-by: Murilo Opsfelder Araujo > util/vfio-helpers.c | 43 +++ > 1 file changed, 27 insertions(+), 16 deletions(-) > > diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c > index 7085ca702c..f0c77f0d69 100644 > --- a/util/vfio-helpers.c > +++ b/util/vfio-helpers.c > @@ -518,6 +518,20 @@ static IOVAMapping *qemu_vfio_add_mapping(QEMUVFIOState > *s, return insert; > } > > +/** > + * Remove the mapping from @s and free it. > + */ > +static void qemu_vfio_remove_mapping(QEMUVFIOState *s, IOVAMapping > *mapping) +{ > +const int index = mapping - s->mappings; > + > +assert(index >= 0 && index < s->nr_mappings); > +memmove(mapping, &s->mappings[index + 1], > +sizeof(s->mappings[0]) * (s->nr_mappings - index - 1)); > +s->nr_mappings--; > +s->mappings = g_renew(IOVAMapping, s->mappings, s->nr_mappings); > +} > + > /* Do the DMA mapping with VFIO. */ > static int qemu_vfio_do_mapping(QEMUVFIOState *s, void *host, size_t size, > uint64_t iova) > @@ -539,29 +553,22 @@ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void > *host, size_t size, } > > /** > - * Undo the DMA mapping from @s with VFIO, and remove from mapping list. > + * Undo the DMA mapping from @s with VFIO. > */ > -static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping) > +static void qemu_vfio_undo_mapping(QEMUVFIOState *s, size_t size, uint64_t > iova) { > -int index; > struct vfio_iommu_type1_dma_unmap unmap = { > .argsz = sizeof(unmap), > .flags = 0, > -.iova = mapping->iova, > -.size = mapping->size, > +.iova = iova, > +.size = size, > }; > > -index = mapping - s->mappings; > -assert(mapping->size > 0); > -assert(QEMU_IS_ALIGNED(mapping->size, qemu_real_host_page_size)); > -assert(index >= 0 && index < s->nr_mappings); > +assert(size > 0); > +assert(QEMU_IS_ALIGNED(size, qemu_real_host_page_size)); > if (ioctl(s->container, VFIO_IOMMU_UNMAP_DMA, &unmap)) { > error_report("VFIO_UNMAP_DMA failed: %s", strerror(errno)); > } > -memmove(mapping, &s->mappings[index + 1], > -sizeof(s->mappings[0]) * (s->nr_mappings - index - 1)); > -s->nr_mappings--; > -s->mappings = g_renew(IOVAMapping, s->mappings, s->nr_mappings); > } > > /* Check if the mapping list is (ascending) ordered. */ > @@ -621,7 +628,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, > size_t size, assert(qemu_vfio_verify_mappings(s)); > ret = qemu_vfio_do_mapping(s, host, size, iova0); > if (ret) { > -qemu_vfio_undo_mapping(s, mapping); > +qemu_vfio_remove_mapping(s, mapping); > goto out; > } > s->low_water_mark += size; > @@ -681,7 +688,8 @@ void qemu_vfio_dma_unmap(QEMUVFIOState *s, void *host) > if (!m) { > goto out; > } > -qemu_vfio_undo_mapping(s, m); > +qemu_vfio_undo_mapping(s, m->size, m->iova); > +qemu_vfio_remove_mapping(s, m); > out: > qemu_mutex_unlock(&s->lock); > } > @@ -698,7 +706,10 @@ void qemu_vfio_close(QEMUVFIOState *s) > return; > } > while (s->nr_mappings) { > -qemu_vfio_undo_mapping(s, &s->mappings[s->nr_mappings - 1]); > +IOVAMapping *m = &s->mappings[s->nr_mappings - 1]; > + > +qemu_vfio_undo_mapping(s, m->size, m->iova); > +qemu_vfio_remove_mapping(s, m); > } > ram_block_notifier_remove(&s->ram_notifier); > qemu_vfio_reset(s); -- Murilo
Re: [PATCH v4 11/15] util/mmap-alloc: Implement resizeable mmaps
On Thursday, March 5, 2020 11:29:41 AM -03 David Hildenbrand wrote: > Implement resizeable mmaps. For now, the actual resizing is not wired up. > Introduce qemu_ram_mmap_resizeable() and qemu_ram_mmap_resize(). Make > qemu_ram_mmap() a wrapper of qemu_ram_mmap_resizeable(). > > Reviewed-by: Peter Xu > Cc: Richard Henderson > Cc: Igor Kotrasinski > Cc: Murilo Opsfelder Araujo > Cc: "Michael S. Tsirkin" > Cc: Greg Kurz > Cc: Eduardo Habkost > Cc: "Dr. David Alan Gilbert" > Cc: Igor Mammedov > Signed-off-by: David Hildenbrand > --- Acked-by: Murilo Opsfelder Araujo > include/qemu/mmap-alloc.h | 21 +++ > util/mmap-alloc.c | 43 --- > 2 files changed, 44 insertions(+), 20 deletions(-) > > diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h > index e786266b92..ca8f7edf70 100644 > --- a/include/qemu/mmap-alloc.h > +++ b/include/qemu/mmap-alloc.h > @@ -7,11 +7,13 @@ size_t qemu_fd_getpagesize(int fd); > size_t qemu_mempath_getpagesize(const char *mem_path); > > /** > - * qemu_ram_mmap: mmap the specified file or device. > + * qemu_ram_mmap_resizeable: reserve a memory region of @max_size to mmap > the + * specified file or device and mmap @size > of it. * > * Parameters: > * @fd: the file or the device to mmap > * @size: the number of bytes to be mmaped > + * @max_size: the number of bytes to be reserved > * @align: if not zero, specify the alignment of the starting mapping > address; * otherwise, the alignment in use will be determined by > QEMU. * @shared: map has RAM_SHARED flag. > @@ -21,12 +23,15 @@ size_t qemu_mempath_getpagesize(const char *mem_path); > * On success, return a pointer to the mapped area. > * On failure, return MAP_FAILED. > */ > -void *qemu_ram_mmap(int fd, > -size_t size, > -size_t align, > -bool shared, > -bool is_pmem); > - > -void qemu_ram_munmap(int fd, void *ptr, size_t size); > +void *qemu_ram_mmap_resizeable(int fd, size_t size, size_t max_size, > + size_t align, bool shared, bool is_pmem); > +bool qemu_ram_mmap_resize(void *ptr, int fd, size_t old_size, size_t > new_size, + bool shared, bool is_pmem); > +static inline void *qemu_ram_mmap(int fd, size_t size, size_t align, > + bool shared, bool is_pmem) > +{ > +return qemu_ram_mmap_resizeable(fd, size, size, align, shared, > is_pmem); +} > +void qemu_ram_munmap(int fd, void *ptr, size_t max_size); > > #endif > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c > index 2767caa33b..7ed85f16d3 100644 > --- a/util/mmap-alloc.c > +++ b/util/mmap-alloc.c > @@ -174,11 +174,8 @@ static inline size_t mmap_guard_pagesize(int fd) > #endif > } > > -void *qemu_ram_mmap(int fd, > -size_t size, > -size_t align, > -bool shared, > -bool is_pmem) > +void *qemu_ram_mmap_resizeable(int fd, size_t size, size_t max_size, > + size_t align, bool shared, bool is_pmem) > { > const size_t guard_pagesize = mmap_guard_pagesize(fd); > const size_t pagesize = qemu_fd_getpagesize(fd); > @@ -186,12 +183,14 @@ void *qemu_ram_mmap(int fd, > void *ptr, *guardptr; > > g_assert(QEMU_IS_ALIGNED(size, pagesize)); > +g_assert(QEMU_IS_ALIGNED(max_size, pagesize)); > > /* > * Note: this always allocates at least one extra page of virtual > address - * space, even if size is already aligned. > + * space, even if the size is already aligned. We will reserve an area > of + * at least max_size, but only activate the requested part of it. > */ > -total = size + align; > +total = max_size + align; > > guardptr = mmap_reserve(NULL, total, fd); > if (guardptr == MAP_FAILED) { > @@ -219,21 +218,41 @@ void *qemu_ram_mmap(int fd, > * a guard page guarding against potential buffer overflows. > */ > total -= offset; > -if (total > size + guard_pagesize) { > -munmap(ptr + size + guard_pagesize, total - size - guard_pagesize); > +if (total > max_size + guard_pagesize) { > +munmap(ptr + max_size + guard_pagesize, > + total - max_size - guard_pagesize); > } > > return ptr; > } > > -void qemu_ram_munmap(int fd, void *ptr, size_t size) > +bool qemu_ram_mmap_resize(void *ptr, int fd, size_t old_size, size_t > new_size, + bool shared, bool is_pmem) > { > const size_t pagesize = qemu_fd_getpagesize(fd); > > -g_assert(QEMU_IS_ALIGNED(size, pagesize)); > +g_assert(QEMU_IS_ALIGNED(old_size, pagesize)); > +g_assert(QEMU_IS_ALIGNED(new_size, pagesize)); > + > +if (old_size < new_size) { > +/* activate the missing piece in the reserved area */ > +ptr = mmap_activate(ptr + old_size, new_si
Re: [PATCH v4 07/15] util/mmap-alloc: Factor out calculation of the pagesize for the guard page
On Thursday, March 5, 2020 11:29:37 AM -03 David Hildenbrand wrote: > Let's factor out calculating the size of the guard page and rename the > variable to make it clearer that this pagesize only applies to the > guard page. > > Reviewed-by: Peter Xu > Cc: "Michael S. Tsirkin" > Cc: Murilo Opsfelder Araujo > Cc: Greg Kurz > Cc: Eduardo Habkost > Cc: "Dr. David Alan Gilbert" > Cc: Igor Mammedov > Signed-off-by: David Hildenbrand > --- Acked-by: Murilo Opsfelder Araujo > util/mmap-alloc.c | 31 --- > 1 file changed, 16 insertions(+), 15 deletions(-) > > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c > index 27dcccd8ec..f0277f9fad 100644 > --- a/util/mmap-alloc.c > +++ b/util/mmap-alloc.c > @@ -82,17 +82,27 @@ size_t qemu_mempath_getpagesize(const char *mem_path) > return qemu_real_host_page_size; > } > > +static inline size_t mmap_guard_pagesize(int fd) > +{ > +#if defined(__powerpc64__) && defined(__linux__) > +/* Mappings in the same segment must share the same page size */ > +return qemu_fd_getpagesize(fd); > +#else > +return qemu_real_host_page_size; > +#endif > +} > + > void *qemu_ram_mmap(int fd, > size_t size, > size_t align, > bool shared, > bool is_pmem) > { > +const size_t guard_pagesize = mmap_guard_pagesize(fd); > int flags; > int map_sync_flags = 0; > int guardfd; > size_t offset; > -size_t pagesize; > size_t total; > void *guardptr; > void *ptr; > @@ -113,8 +123,7 @@ void *qemu_ram_mmap(int fd, > * anonymous memory is OK. > */ > flags = MAP_PRIVATE; > -pagesize = qemu_fd_getpagesize(fd); > -if (fd == -1 || pagesize == qemu_real_host_page_size) { > +if (fd == -1 || guard_pagesize == qemu_real_host_page_size) { > guardfd = -1; > flags |= MAP_ANONYMOUS; > } else { > @@ -123,7 +132,6 @@ void *qemu_ram_mmap(int fd, > } > #else > guardfd = -1; > -pagesize = qemu_real_host_page_size; > flags = MAP_PRIVATE | MAP_ANONYMOUS; > #endif > > @@ -135,7 +143,7 @@ void *qemu_ram_mmap(int fd, > > assert(is_power_of_2(align)); > /* Always align to host page size */ > -assert(align >= pagesize); > +assert(align >= guard_pagesize); > > flags = MAP_FIXED; > flags |= fd == -1 ? MAP_ANONYMOUS : 0; > @@ -189,8 +197,8 @@ void *qemu_ram_mmap(int fd, > * a guard page guarding against potential buffer overflows. > */ > total -= offset; > -if (total > size + pagesize) { > -munmap(ptr + size + pagesize, total - size - pagesize); > +if (total > size + guard_pagesize) { > +munmap(ptr + size + guard_pagesize, total - size - guard_pagesize); > } > > return ptr; > @@ -198,15 +206,8 @@ void *qemu_ram_mmap(int fd, > > void qemu_ram_munmap(int fd, void *ptr, size_t size) > { > -size_t pagesize; > - > if (ptr) { > /* Unmap both the RAM block and the guard page */ > -#if defined(__powerpc64__) && defined(__linux__) > -pagesize = qemu_fd_getpagesize(fd); > -#else > -pagesize = qemu_real_host_page_size; > -#endif > -munmap(ptr, size + pagesize); > +munmap(ptr, size + mmap_guard_pagesize(fd)); > } > } -- Murilo
Re: [PATCH v4 14/15] numa: Introduce ram_block_notifiers_support_resize()
On Thursday, March 5, 2020 11:29:44 AM -03 David Hildenbrand wrote: > We want to actually use resizeable allocations in resizeable ram blocks > (IOW, make everything between used_length and max_length inaccessible) - > however, not all ram block notifiers can support that. > > Introduce a way to detect if any registered notifier does not > support resizes - ram_block_notifiers_support_resize() - which we can later > use to fallback to legacy handling if a registered notifier (esp., SEV and > HAX) does not support actual resizes. > > Reviewed-by: Peter Xu > Cc: Richard Henderson > Cc: Paolo Bonzini > Cc: "Dr. David Alan Gilbert" > Cc: Eduardo Habkost > Cc: Marcel Apfelbaum > Cc: "Michael S. Tsirkin" > Cc: Igor Mammedov > Signed-off-by: David Hildenbrand > --- Acked-by: Murilo Opsfelder Araujo > hw/core/numa.c | 12 > include/exec/ramlist.h | 1 + > 2 files changed, 13 insertions(+) > > diff --git a/hw/core/numa.c b/hw/core/numa.c > index 37ce175e13..1d5288c22c 100644 > --- a/hw/core/numa.c > +++ b/hw/core/numa.c > @@ -914,3 +914,15 @@ void ram_block_notify_resize(void *host, size_t > old_size, size_t new_size) } > } > } > + > +bool ram_block_notifiers_support_resize(void) > +{ > +RAMBlockNotifier *notifier; > + > +QLIST_FOREACH(notifier, &ram_list.ramblock_notifiers, next) { > +if (!notifier->ram_block_resized) { > +return false; > +} > +} > +return true; > +} > diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h > index 293c0ddabe..ac5811be96 100644 > --- a/include/exec/ramlist.h > +++ b/include/exec/ramlist.h > @@ -79,6 +79,7 @@ void ram_block_notifier_remove(RAMBlockNotifier *n); > void ram_block_notify_add(void *host, size_t size, size_t max_size); > void ram_block_notify_remove(void *host, size_t size, size_t max_size); > void ram_block_notify_resize(void *host, size_t old_size, size_t new_size); > +bool ram_block_notifiers_support_resize(void); > > void ram_block_dump(Monitor *mon); -- Murilo
Re: [PATCH v4 15/15] exec: Ram blocks with resizeable anonymous allocations under POSIX
On Thursday, March 5, 2020 11:29:45 AM -03 David Hildenbrand wrote: > We can now make use of resizeable anonymous allocations to implement > actually resizeable ram blocks. Resizeable anonymous allocations are > not implemented under WIN32 yet and are not available when using > alternative allocators. Fall back to the existing handling. > > We also have to fallback to the existing handling in case any ram block > notifier does not support resizing (esp., AMD SEV, HAX) yet. Remember > in RAM_RESIZEABLE_ALLOC if we are using resizeable anonymous allocations. > > Try to grow early, as that can easily fail if out of memory. Shrink late > and ignore errors (nothing will actually break). Warn only. > > The benefit of actually resizeable ram blocks is that e.g., under Linux, > only the actual size will be reserved (even if > "/proc/sys/vm/overcommit_memory" is set to "never"). Additional memory will > be reserved when trying to resize, which allows to have ram blocks that > start small but can theoretically grow very large. > > Note1: We are not able to create resizeable ram blocks with pre-allocated >memory yet, so prealloc is not affected. > Note2: mlock should work as it used to as os_mlock() does a >mlockall(MCL_CURRENT | MCL_FUTURE), which includes future >mappings. > Note3: Nobody should access memory beyond used_length. Memory notifiers >already properly take care of this, only ram block notifiers >violate this constraint and, therefore, have to be special-cased. >Especially, any ram block notifier that might dynamically >register at runtime (e.g., vfio) has to support resizes. Add an >assert for that. Both, HAX and SEV register early, so they are >fine. > > Reviewed-by: Peter Xu > Cc: Richard Henderson > Cc: Paolo Bonzini > Cc: "Dr. David Alan Gilbert" > Cc: Eduardo Habkost > Cc: Marcel Apfelbaum > Cc: Stefan Weil > Cc: Igor Mammedov > Cc: Shameerali Kolothum Thodi > Signed-off-by: David Hildenbrand > --- > exec.c| 65 --- > hw/core/numa.c| 7 + > include/exec/cpu-common.h | 2 ++ > include/exec/memory.h | 8 + > 4 files changed, 77 insertions(+), 5 deletions(-) > > diff --git a/exec.c b/exec.c > index 9c3cc79193..6c6b6e12d2 100644 > --- a/exec.c > +++ b/exec.c > @@ -2001,6 +2001,16 @@ void qemu_ram_unset_migratable(RAMBlock *rb) > rb->flags &= ~RAM_MIGRATABLE; > } > > +bool qemu_ram_is_resizeable(RAMBlock *rb) > +{ > +return rb->flags & RAM_RESIZEABLE; > +} > + > +bool qemu_ram_is_resizeable_alloc(RAMBlock *rb) > +{ > +return rb->flags & RAM_RESIZEABLE_ALLOC; > +} > + > /* Called with iothread lock held. */ > void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState > *dev) { > @@ -2094,6 +2104,7 @@ static void qemu_ram_apply_settings(void *host, size_t > length) */ > int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp) > { > +const bool shared = block->flags & RAM_SHARED; Do you think a new function, for example, qemu_ram_is_shared() would be welcome to check for RAM_SHARED flag as well? Similar to what is done in qemu_ram_is_resizeable() and qemu_ram_is_resizeable_alloc(). Apart from that, Acked-by: Murilo Opsfelder Araujo > const ram_addr_t oldsize = block->used_length; > > assert(block); > @@ -2104,7 +2115,7 @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t > newsize, Error **errp) return 0; > } > > -if (!(block->flags & RAM_RESIZEABLE)) { > +if (!qemu_ram_is_resizeable(block)) { > error_setg_errno(errp, EINVAL, > "Length mismatch: %s: 0x" RAM_ADDR_FMT > " in != 0x" RAM_ADDR_FMT, block->idstr, > @@ -2120,6 +2131,15 @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t > newsize, Error **errp) return -EINVAL; > } > > +if (oldsize < newsize && qemu_ram_is_resizeable_alloc(block)) { > +if (!qemu_anon_ram_resize(block->host, oldsize, newsize, shared)) { > +error_setg_errno(errp, -ENOMEM, "Cannot allocate enough > memory."); +return -ENOMEM; > +} > +/* apply settings for the newly accessible memory */ > +qemu_ram_apply_settings(block->host + oldsize, newsize - oldsize); > +} > + > /* Notify before modifying the ram block and touching the bitmaps. */ > if (block->host) { > ram_block_notify_resize(block->host, oldsize, newsize); > @@ -2133,6 +2153,16 @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t > newsize, Error **errp) if (block->resized) { > block->resized(block->idstr, newsize, block->host); > } > + > +/* > + * Shrinking will only fail in rare scenarios (e.g., maximum number of > + * mappings reached), and can be ignored. Warn only. > + */ > +if (newsize < oldsize && qemu_ram_is_resizeable_alloc(block) && > +!qemu_anon_ram_resize(block->host, oldsize, newsize, share
Re: [PATCH v4 13/15] util: oslib: Resizeable anonymous allocations under POSIX
On Thursday, March 5, 2020 11:29:43 AM -03 David Hildenbrand wrote: > Introduce qemu_anon_ram_alloc_resizeable() and qemu_anon_ram_resize(). > Implement them under POSIX and make them return NULL under WIN32. > > Under POSIX, we make use of resizeable mmaps. An implementation under > WIN32 is theoretically possible AFAIK and can be added later. > > In qemu_anon_ram_free(), rename the size parameter to max_size, to make > it clearer that we have to use the maximum size when freeing resizeable > anonymous allocations. > > Reviewed-by: Peter Xu > Cc: Richard Henderson > Cc: Paolo Bonzini > Cc: "Dr. David Alan Gilbert" > Cc: Eduardo Habkost > Cc: Marcel Apfelbaum > Cc: Stefan Weil > Cc: Igor Mammedov > Signed-off-by: David Hildenbrand > --- Acked-by: Murilo Opsfelder Araujo > include/qemu/osdep.h | 6 +- > util/oslib-posix.c | 37 ++--- > util/oslib-win32.c | 14 ++ > util/trace-events| 4 +++- > 4 files changed, 56 insertions(+), 5 deletions(-) > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 9bd3dcfd13..a1ea9e043d 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -311,8 +311,12 @@ int qemu_daemon(int nochdir, int noclose); > void *qemu_try_memalign(size_t alignment, size_t size); > void *qemu_memalign(size_t alignment, size_t size); > void *qemu_anon_ram_alloc(size_t size, uint64_t *align, bool shared); > +void *qemu_anon_ram_alloc_resizeable(size_t size, size_t max_size, > + uint64_t *align, bool shared); > +bool qemu_anon_ram_resize(void *ptr, size_t old_size, size_t new_size, > + bool shared); > void qemu_vfree(void *ptr); > -void qemu_anon_ram_free(void *ptr, size_t size); > +void qemu_anon_ram_free(void *ptr, size_t max_size); > > #define QEMU_MADV_INVALID -1 > > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > index 897e8f3ba6..34b1ce74b7 100644 > --- a/util/oslib-posix.c > +++ b/util/oslib-posix.c > @@ -223,16 +223,47 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t > *alignment, bool shared) return ptr; > } > > +void *qemu_anon_ram_alloc_resizeable(size_t size, size_t max_size, > + uint64_t *alignment, bool shared) > +{ > +size_t align = QEMU_VMALLOC_ALIGN; > +void *ptr = qemu_ram_mmap_resizeable(-1, size, max_size, align, shared, > + false); > + > +if (ptr == MAP_FAILED) { > +return NULL; > +} > + > +if (alignment) { > +*alignment = align; > +} > + > +trace_qemu_anon_ram_alloc_resizeable(size, max_size, ptr); > +return ptr; > +} > + > +bool qemu_anon_ram_resize(void *ptr, size_t old_size, size_t new_size, > + bool shared) > +{ > +bool resized = qemu_ram_mmap_resize(ptr, -1, old_size, new_size, > shared, +false); > + > +if (resized) { > +trace_qemu_anon_ram_resize(old_size, new_size, ptr); > +} > +return resized; > +} > + > void qemu_vfree(void *ptr) > { > trace_qemu_vfree(ptr); > free(ptr); > } > > -void qemu_anon_ram_free(void *ptr, size_t size) > +void qemu_anon_ram_free(void *ptr, size_t max_size) > { > -trace_qemu_anon_ram_free(ptr, size); > -qemu_ram_munmap(-1, ptr, size); > +trace_qemu_anon_ram_free(ptr, max_size); > +qemu_ram_munmap(-1, ptr, max_size); > } > > void qemu_set_block(int fd) > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index e9b14ab178..c034fdfe01 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -90,6 +90,20 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t *align, > bool shared) return ptr; > } > > +void *qemu_anon_ram_alloc_resizeable(size_t size, size_t max_size, > + uint64_t *align, bool shared) > +{ > +/* resizeable ram not implemented yet */ > +return NULL; > +} > + > +bool qemu_anon_ram_resize(void *ptr, size_t old_size, size_t new_size, > + bool shared) > +{ > +/* resizeable ram not implemented yet */ > +return false; > +} > + > void qemu_vfree(void *ptr) > { > trace_qemu_vfree(ptr); > diff --git a/util/trace-events b/util/trace-events > index a4d39eca5e..24a6f1a1e1 100644 > --- a/util/trace-events > +++ b/util/trace-events > @@ -46,8 +46,10 @@ qemu_co_mutex_unlock_return(void *mutex, void *self) > "mutex %p self %p" # oslib-posix.c > qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size > %zu ptr %p" qemu_anon_ram_alloc(size_t size, void *ptr) "size %zu ptr %p" > +qemu_anon_ram_alloc_resizeable(size_t size, size_t max_size, void *ptr) > "size %zu max_size %zu ptr %p" +qemu_anon_ram_resize(size_t old_size, > size_t new_size, void *ptr) "old_size %zu new_size %zu ptr %p" > qemu_vfree(void *ptr) "ptr %p" > -qemu_anon_ram_free(void *ptr, size_t size) "ptr %p size %zu" > +qemu_anon_ram_free(void *ptr, size_t max_size)
Re: [PATCH v4 12/15] util: vfio-helpers: Implement ram_block_resized()
On Thursday, March 5, 2020 11:29:42 AM -03 David Hildenbrand wrote: > Let's implement ram_block_resized(), allowing resizeable mappings. > > For resizeable mappings, we reserve $max_size IOVA address space, but only > map $size of it. When resizing, unmap the old part and remap the new > part. We'll need e.g., new ioctl to do this atomically (e.g., to resize > while the guest is running). > > Right now, we only resize RAM blocks during incoming migration (when > syncing RAM block sizes during the precopy phase) or after guest > resets when building acpi tables. Any future user of resizeable RAM has to > be aware that vfio has to be treated with care. > > Reviewed-by: Peter Xu > Cc: Richard Henderson > Cc: Paolo Bonzini > Cc: Eduardo Habkost > Cc: Marcel Apfelbaum > Cc: Alex Williamson > Cc: Stefan Hajnoczi > Cc: "Dr. David Alan Gilbert" > Signed-off-by: David Hildenbrand > --- Acked-by: Murilo Opsfelder Araujo > util/trace-events | 7 ++-- > util/vfio-helpers.c | 95 +++-- > 2 files changed, 88 insertions(+), 14 deletions(-) > > diff --git a/util/trace-events b/util/trace-events > index 83b6639018..a4d39eca5e 100644 > --- a/util/trace-events > +++ b/util/trace-events > @@ -74,10 +74,11 @@ qemu_mutex_unlock(void *mutex, const char *file, const > int line) "released mutex > > # vfio-helpers.c > qemu_vfio_dma_reset_temporary(void *s) "s %p" > -qemu_vfio_ram_block_added(void *s, void *p, size_t size) "s %p host %p size > 0x%zx" -qemu_vfio_ram_block_removed(void *s, void *p, size_t size) "s %p > host %p size 0x%zx" +qemu_vfio_ram_block_added(void *s, void *p, size_t > size, size_t max_size) "s %p host %p size 0x%zx max_size 0x%zx" > +qemu_vfio_ram_block_removed(void *s, void *p, size_t size, size_t > max_size) "s %p host %p size 0x%zx max_size 0x%zx" > +qemu_vfio_ram_block_resized(void *s, void *p, size_t old_size, size_t > new_sizze) "s %p host %p old_size 0x%zx new_size 0x%zx" > qemu_vfio_find_mapping(void *s, void *p) "s %p host %p" > -qemu_vfio_new_mapping(void *s, void *host, size_t size, int index, uint64_t > iova) "s %p host %p size %zu index %d iova 0x%"PRIx64 > +qemu_vfio_new_mapping(void *s, void *host, size_t size, size_t max_size, > int index, uint64_t iova) "s %p host %p size %zu max_size %zu index %d iova > 0x%"PRIx64 qemu_vfio_do_mapping(void *s, void *host, size_t size, uint64_t > iova) "s %p host %p size %zu iova 0x%"PRIx64 qemu_vfio_dma_map(void *s, > void *host, size_t size, bool temporary, uint64_t *iova) "s %p host %p size > %zu temporary %d iova %p" qemu_vfio_dma_unmap(void *s, void *host) "s %p > host %p" > diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c > index f0c77f0d69..789faf38bd 100644 > --- a/util/vfio-helpers.c > +++ b/util/vfio-helpers.c > @@ -36,6 +36,7 @@ typedef struct { > /* Page aligned addr. */ > void *host; > size_t size; > +size_t max_size; > uint64_t iova; > } IOVAMapping; > > @@ -372,14 +373,20 @@ fail_container: > return ret; > } > > +static int qemu_vfio_dma_map_resizeable(QEMUVFIOState *s, void *host, > +size_t size, size_t max_size, > +bool temporary, uint64_t *iova); > +static void qemu_vfio_dma_map_resize(QEMUVFIOState *s, void *host, > + size_t old_size, size_t new_size); > + > static void qemu_vfio_ram_block_added(RAMBlockNotifier *n, void *host, >size_t size, size_t max_size) > { > QEMUVFIOState *s = container_of(n, QEMUVFIOState, ram_notifier); > int ret; > > -trace_qemu_vfio_ram_block_added(s, host, max_size); > -ret = qemu_vfio_dma_map(s, host, max_size, false, NULL); > +trace_qemu_vfio_ram_block_added(s, host, size, max_size); > +ret = qemu_vfio_dma_map_resizeable(s, host, size, max_size, false, > NULL); if (ret) { > error_report("qemu_vfio_dma_map(%p, %zu) failed: %s", host, > max_size, strerror(-ret)); > @@ -391,16 +398,28 @@ static void > qemu_vfio_ram_block_removed(RAMBlockNotifier *n, void *host, { > QEMUVFIOState *s = container_of(n, QEMUVFIOState, ram_notifier); > if (host) { > -trace_qemu_vfio_ram_block_removed(s, host, max_size); > +trace_qemu_vfio_ram_block_removed(s, host, size, max_size); > qemu_vfio_dma_unmap(s, host); > } > } > > +static void qemu_vfio_ram_block_resized(RAMBlockNotifier *n, void *host, > +size_t old_size, size_t new_size) > +{ > +QEMUVFIOState *s = container_of(n, QEMUVFIOState, ram_notifier); > + > +if (host) { > +trace_qemu_vfio_ram_block_resized(s, host, old_size, new_size); > +qemu_vfio_dma_map_resize(s, host, old_size, new_size); > +} > +} > + > 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_bl
Re: [PATCH v4 02/15] util: vfio-helpers: Remove Error parameter from qemu_vfio_undo_mapping()
On Thursday, March 5, 2020 11:29:32 AM -03 David Hildenbrand wrote: > Everybody discards the error. Let's error_report() instead so this error > doesn't get lost. > > This is now the same error handling as in qemu_vfio_do_mapping(). However, > we don't report any errors via the return value to the caller. This seems > to be one of these "will never happen, but let's better print an error > because the caller can't handle it either way" cases. > > Cc: Richard Henderson > Cc: Paolo Bonzini > Cc: Eduardo Habkost > Cc: Marcel Apfelbaum > Cc: Alex Williamson > Cc: Stefan Hajnoczi > Signed-off-by: David Hildenbrand > --- Acked-by: Murilo Opsfelder Araujo > util/vfio-helpers.c | 11 +-- > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c > index f31aa77ffe..7085ca702c 100644 > --- a/util/vfio-helpers.c > +++ b/util/vfio-helpers.c > @@ -541,8 +541,7 @@ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void > *host, size_t size, /** > * Undo the DMA mapping from @s with VFIO, and remove from mapping list. > */ > -static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping, > - Error **errp) > +static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping) > { > int index; > struct vfio_iommu_type1_dma_unmap unmap = { > @@ -557,7 +556,7 @@ static void qemu_vfio_undo_mapping(QEMUVFIOState *s, > IOVAMapping *mapping, assert(QEMU_IS_ALIGNED(mapping->size, > qemu_real_host_page_size)); assert(index >= 0 && index < s->nr_mappings); > if (ioctl(s->container, VFIO_IOMMU_UNMAP_DMA, &unmap)) { > -error_setg_errno(errp, errno, "VFIO_UNMAP_DMA failed"); > +error_report("VFIO_UNMAP_DMA failed: %s", strerror(errno)); > } > memmove(mapping, &s->mappings[index + 1], > sizeof(s->mappings[0]) * (s->nr_mappings - index - 1)); > @@ -622,7 +621,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, > size_t size, assert(qemu_vfio_verify_mappings(s)); > ret = qemu_vfio_do_mapping(s, host, size, iova0); > if (ret) { > -qemu_vfio_undo_mapping(s, mapping, NULL); > +qemu_vfio_undo_mapping(s, mapping); > goto out; > } > s->low_water_mark += size; > @@ -682,7 +681,7 @@ void qemu_vfio_dma_unmap(QEMUVFIOState *s, void *host) > if (!m) { > goto out; > } > -qemu_vfio_undo_mapping(s, m, NULL); > +qemu_vfio_undo_mapping(s, m); > out: > qemu_mutex_unlock(&s->lock); > } > @@ -699,7 +698,7 @@ void qemu_vfio_close(QEMUVFIOState *s) > return; > } > while (s->nr_mappings) { > -qemu_vfio_undo_mapping(s, &s->mappings[s->nr_mappings - 1], NULL); > +qemu_vfio_undo_mapping(s, &s->mappings[s->nr_mappings - 1]); > } > ram_block_notifier_remove(&s->ram_notifier); > qemu_vfio_reset(s); -- Murilo
Re: [PATCH v2 fixed 08/16] util/mmap-alloc: Factor out calculation of pagesize to mmap_pagesize()
On Monday, February 24, 2020 11:16:16 AM -03 Murilo Opsfelder Araújo wrote: > On Monday, February 24, 2020 7:57:03 AM -03 David Hildenbrand wrote: > > On 24.02.20 11:50, David Hildenbrand wrote: > > > On 19.02.20 23:46, Peter Xu wrote: > > >> On Wed, Feb 12, 2020 at 02:42:46PM +0100, David Hildenbrand wrote: > > >>> Factor it out and add a comment. > > >>> > > >>> Reviewed-by: Igor Kotrasinski > > >>> Acked-by: Murilo Opsfelder Araujo > > >>> Reviewed-by: Richard Henderson > > >>> Cc: "Michael S. Tsirkin" > > >>> Cc: Murilo Opsfelder Araujo > > >>> Cc: Greg Kurz > > >>> Cc: Eduardo Habkost > > >>> Cc: "Dr. David Alan Gilbert" > > >>> Cc: Igor Mammedov > > >>> Signed-off-by: David Hildenbrand > > >>> --- > > >>> > > >>> util/mmap-alloc.c | 21 - > > >>> 1 file changed, 12 insertions(+), 9 deletions(-) > > >>> > > >>> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c > > >>> index 27dcccd8ec..82f02a2cec 100644 > > >>> --- a/util/mmap-alloc.c > > >>> +++ b/util/mmap-alloc.c > > >>> @@ -82,17 +82,27 @@ size_t qemu_mempath_getpagesize(const char > > >>> *mem_path) > > >>> > > >>> return qemu_real_host_page_size; > > >>> > > >>> } > > >>> > > >>> +static inline size_t mmap_pagesize(int fd) > > >>> +{ > > >>> +#if defined(__powerpc64__) && defined(__linux__) > > >>> +/* Mappings in the same segment must share the same page size */ > > >>> +return qemu_fd_getpagesize(fd); > > >>> +#else > > >>> +return qemu_real_host_page_size; > > >>> +#endif > > >>> +} > > >> > > >> Pure question: This will return 4K even for huge pages on x86, is this > > >> what we want? > > > > > > (was asking myself the same question) I *think* it's intended. It's > > > mainly only used to allocate one additional guard page. The callers of > > > qemu_ram_mmap() make sure that the size is properly aligned (e.g., to > > > huge pages). > > > > > > Of course, a 4k guard page is sufficient - unless we can't use that > > > (special case for ppc64 here). > > > > > > Thanks! > > > > We could rename the function to mmap_guard_pagesize(), thoughts? > > The existing qemu_fd_getpagesize() already returns qemu_real_host_page_size > for non-anonymous mappings (when fd == -1). I think this new > mmap_pagesize() could be dropped in favor of qemu_fd_getpagesize(). s/non-// I mean "for anonymous mappings". > > A side effect of this change would be guard page using a bit more memory for > non-anonymous mapping. Could that be a problem? > > What do you think? > > -- > Murilo -- Murilo
Re: [PATCH v2 fixed 08/16] util/mmap-alloc: Factor out calculation of pagesize to mmap_pagesize()
On Monday, February 24, 2020 7:57:03 AM -03 David Hildenbrand wrote: > On 24.02.20 11:50, David Hildenbrand wrote: > > On 19.02.20 23:46, Peter Xu wrote: > >> On Wed, Feb 12, 2020 at 02:42:46PM +0100, David Hildenbrand wrote: > >>> Factor it out and add a comment. > >>> > >>> Reviewed-by: Igor Kotrasinski > >>> Acked-by: Murilo Opsfelder Araujo > >>> Reviewed-by: Richard Henderson > >>> Cc: "Michael S. Tsirkin" > >>> Cc: Murilo Opsfelder Araujo > >>> Cc: Greg Kurz > >>> Cc: Eduardo Habkost > >>> Cc: "Dr. David Alan Gilbert" > >>> Cc: Igor Mammedov > >>> Signed-off-by: David Hildenbrand > >>> --- > >>> > >>> util/mmap-alloc.c | 21 - > >>> 1 file changed, 12 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c > >>> index 27dcccd8ec..82f02a2cec 100644 > >>> --- a/util/mmap-alloc.c > >>> +++ b/util/mmap-alloc.c > >>> @@ -82,17 +82,27 @@ size_t qemu_mempath_getpagesize(const char > >>> *mem_path) > >>> > >>> return qemu_real_host_page_size; > >>> > >>> } > >>> > >>> +static inline size_t mmap_pagesize(int fd) > >>> +{ > >>> +#if defined(__powerpc64__) && defined(__linux__) > >>> +/* Mappings in the same segment must share the same page size */ > >>> +return qemu_fd_getpagesize(fd); > >>> +#else > >>> +return qemu_real_host_page_size; > >>> +#endif > >>> +} > >> > >> Pure question: This will return 4K even for huge pages on x86, is this > >> what we want? > > > > (was asking myself the same question) I *think* it's intended. It's > > mainly only used to allocate one additional guard page. The callers of > > qemu_ram_mmap() make sure that the size is properly aligned (e.g., to > > huge pages). > > > > Of course, a 4k guard page is sufficient - unless we can't use that > > (special case for ppc64 here). > > > > Thanks! > > We could rename the function to mmap_guard_pagesize(), thoughts? The existing qemu_fd_getpagesize() already returns qemu_real_host_page_size for non-anonymous mappings (when fd == -1). I think this new mmap_pagesize() could be dropped in favor of qemu_fd_getpagesize(). A side effect of this change would be guard page using a bit more memory for non-anonymous mapping. Could that be a problem? What do you think? -- Murilo
Re: [PATCH v1 09/13] util/mmap-alloc: Implement resizable mmaps
Hello, David. On Monday, February 3, 2020 3:31:21 PM -03 David Hildenbrand wrote: > Implement resizable mmaps. For now, the actual resizing is not wired up. > Introduce qemu_ram_mmap_resizable() and qemu_ram_mmap_resize(). Make > qemu_ram_mmap() a wrapper of qemu_ram_mmap_resizable(). > > Cc: "Michael S. Tsirkin" > Cc: Greg Kurz > Cc: Murilo Opsfelder Araujo > Cc: Eduardo Habkost > Cc: "Dr. David Alan Gilbert" > Signed-off-by: David Hildenbrand > --- > include/qemu/mmap-alloc.h | 21 --- > util/mmap-alloc.c | 44 --- > 2 files changed, 45 insertions(+), 20 deletions(-) > > diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h > index e786266b92..70bc8e9637 100644 > --- a/include/qemu/mmap-alloc.h > +++ b/include/qemu/mmap-alloc.h > @@ -7,11 +7,13 @@ size_t qemu_fd_getpagesize(int fd); > size_t qemu_mempath_getpagesize(const char *mem_path); > > /** > - * qemu_ram_mmap: mmap the specified file or device. > + * qemu_ram_mmap_resizable: reserve a memory region of @max_size to mmap > the + * specified file or device and mmap @size of > it. * > * Parameters: > * @fd: the file or the device to mmap > * @size: the number of bytes to be mmaped > + * @max_size: the number of bytes to be reserved > * @align: if not zero, specify the alignment of the starting mapping > address; * otherwise, the alignment in use will be determined by > QEMU. * @shared: map has RAM_SHARED flag. > @@ -21,12 +23,15 @@ size_t qemu_mempath_getpagesize(const char *mem_path); > * On success, return a pointer to the mapped area. > * On failure, return MAP_FAILED. > */ > -void *qemu_ram_mmap(int fd, > -size_t size, > -size_t align, > -bool shared, > -bool is_pmem); > - > -void qemu_ram_munmap(int fd, void *ptr, size_t size); > +void *qemu_ram_mmap_resizable(int fd, size_t size, size_t max_size, > + size_t align, bool shared, bool is_pmem); > +void *qemu_ram_mmap_resize(void *ptr, int fd, size_t old_size, size_t > new_size, + bool shared, bool is_pmem); > +static inline void *qemu_ram_mmap(int fd, size_t size, size_t align, > + bool shared, bool is_pmem) > +{ > +return qemu_ram_mmap_resizable(fd, size, size, align, shared, is_pmem); > +} > +void qemu_ram_munmap(int fd, void *ptr, size_t max_size); > > #endif > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c > index 63ad6893b7..2d562145e9 100644 > --- a/util/mmap-alloc.c > +++ b/util/mmap-alloc.c > @@ -172,11 +172,8 @@ static inline size_t mmap_pagesize(int fd) > #endif > } > > -void *qemu_ram_mmap(int fd, > -size_t size, > -size_t align, > -bool shared, > -bool is_pmem) > +void *qemu_ram_mmap_resizable(int fd, size_t size, size_t max_size, > + size_t align, bool shared, bool is_pmem) > { > const size_t pagesize = mmap_pagesize(fd); > size_t offset, total; > @@ -184,12 +181,14 @@ void *qemu_ram_mmap(int fd, > > /* we can only map whole pages */ > size = QEMU_ALIGN_UP(size, pagesize); > +max_size = QEMU_ALIGN_UP(max_size, pagesize); > > /* > * Note: this always allocates at least one extra page of virtual > address - * space, even if size is already aligned. > + * space, even if the size is already aligned. We will reserve an area > of + * at least max_size, but only populate the requested part of it. > */ > -total = size + align; > +total = max_size + align; > > guardptr = mmap_reserve(0, total, fd); > if (guardptr == MAP_FAILED) { > @@ -217,22 +216,43 @@ void *qemu_ram_mmap(int fd, > * a guard page guarding against potential buffer overflows. > */ > total -= offset; > -if (total > size + pagesize) { > -munmap(ptr + size + pagesize, total - size - pagesize); > +if (total > max_size + pagesize) { > +munmap(ptr + max_size + pagesize, total - max_size - pagesize); > } > > return ptr; > } > > -void qemu_ram_munmap(int fd, void *ptr, size_t size) > +void *qemu_ram_mmap_resize(void *ptr, int fd, size_t old_size, size_t > new_size, + bool shared, bool is_pmem) > { > const size_t pagesize = mmap_pagesize(fd); > > /* we can only map whole pages */ > -size = QEMU_ALIGN_UP(size, pagesize); > +old_size = QEMU_ALIGN_UP(old_size, pagesize); > +new_size = QEMU_ALIGN_UP(new_size, pagesize); Shouldn't we just assert old_size and new_size are aligned with pagesize? > + > +/* we support actually resizable memory regions only on Linux */ > +if (old_size < new_size) { > +/* populate the missing piece into the reserved area */ > +ptr = mmap_populate(ptr + old_size, new_size - old_size, fd, > old_size, +
Re: [PATCH v1 08/13] util/mmap-alloc: Prepare for resizable mmaps
Hello, David. On Thursday, February 6, 2020 5:52:26 AM -03 David Hildenbrand wrote: > On 06.02.20 00:00, Murilo Opsfelder Araújo wrote: > > Hello, David. > > > > On Monday, February 3, 2020 3:31:20 PM -03 David Hildenbrand wrote: > >> When shrinking a mmap we want to re-reserve the already populated area. > >> When growing a memory region, we want to populate starting with a given > >> fd_offset. Prepare by allowing to pass these parameters. > >> > >> Also, let's make sure we always process full pages, to avoid > >> unmapping/remapping pages that are already in use when > >> growing/shrinking. (existing callers seem to guarantee this, but that's > >> not obvious) > >> > >> Cc: "Michael S. Tsirkin" > >> Cc: Greg Kurz > >> Cc: Murilo Opsfelder Araujo > >> Cc: Eduardo Habkost > >> Cc: "Dr. David Alan Gilbert" > >> Signed-off-by: David Hildenbrand > >> --- > >> > >> util/mmap-alloc.c | 32 +--- > >> 1 file changed, 21 insertions(+), 11 deletions(-) > >> > >> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c > >> index f043ccb0ab..63ad6893b7 100644 > >> --- a/util/mmap-alloc.c > >> +++ b/util/mmap-alloc.c > >> @@ -83,12 +83,12 @@ size_t qemu_mempath_getpagesize(const char *mem_path) > >> > >> } > >> > >> /* > >> > >> - * Reserve a new memory region of the requested size to be used for > >> mapping - * from the given fd (if any). > >> + * Reserve a new memory region of the requested size or re-reserve parts > >> + * of an existing region to be used for mapping from the given fd (if > >> any). */ > >> -static void *mmap_reserve(size_t size, int fd) > >> +static void *mmap_reserve(void *ptr, size_t size, int fd) > >> > >> { > >> > >> -int flags = MAP_PRIVATE; > >> +int flags = MAP_PRIVATE | (ptr ? MAP_FIXED : 0); > >> > >> #if defined(__powerpc64__) && defined(__linux__) > >> > >> /* > >> > >> @@ -111,19 +111,23 @@ static void *mmap_reserve(size_t size, int fd) > >> > >> flags |= MAP_ANONYMOUS; > >> > >> #endif > >> > >> -return mmap(0, size, PROT_NONE, flags, fd, 0); > >> +return mmap(ptr, size, PROT_NONE, flags, fd, 0); > >> > >> } > >> > >> /* > >> > >> * Populate memory in a reserved region from the given fd (if any). > >> */ > >> > >> -static void *mmap_populate(void *ptr, size_t size, int fd, bool shared, > >> - bool is_pmem) > >> +static void *mmap_populate(void *ptr, size_t size, int fd, size_t > >> fd_offset, + bool shared, bool is_pmem) > >> > >> { > >> > >> int map_sync_flags = 0; > >> int flags = MAP_FIXED; > >> void *new_ptr; > >> > >> +if (fd == -1) { > >> +fd_offset = 0; > >> +} > >> + > >> > >> flags |= fd == -1 ? MAP_ANONYMOUS : 0; > >> flags |= shared ? MAP_SHARED : MAP_PRIVATE; > >> if (shared && is_pmem) { > >> > >> @@ -131,7 +135,7 @@ static void *mmap_populate(void *ptr, size_t size, > >> int > >> fd, bool shared, } > >> > >> new_ptr = mmap(ptr, size, PROT_READ | PROT_WRITE, flags | > >> > >> map_sync_flags, - fd, 0); > >> + fd, fd_offset); > >> > >> if (new_ptr == MAP_FAILED && map_sync_flags) { > >> > >> if (errno == ENOTSUP) { > >> > >> char *proc_link = g_strdup_printf("/proc/self/fd/%d", fd); > >> > >> @@ -153,7 +157,7 @@ static void *mmap_populate(void *ptr, size_t size, > >> int > >> fd, bool shared, * If mmap failed with MAP_SHARED_VALIDATE | MAP_SYNC, we > >> will try * again without these flags to handle backwards compatibility. > >> */ > >> -new_ptr = mmap(ptr, size, PROT_READ | PROT_WRITE, flags, fd, 0); > >> +new_ptr = mmap(ptr, size, PROT_READ | PROT_WRITE, flags, fd, > >> fd_offset); } > >> > >> return new_ptr; > >> > >> } > >> > >> @@ -178,13 +182,16 @@ void *qemu_ram_mmap(int fd, > >> > >> size_t offset, total; > >> void *ptr, *guardptr; > >> > >> +/* we can only map whole pages */ > >> +size = QEMU_ALIGN_UP(size, pagesize); > >> + > > > > Caller already rounds up size to block->page_size. > > > > Why this QEMU_ALIGN_UP is necessary? > > Thanks for having a look > > I guess you read the patch description, right? :) > > "(existing callers seem to guarantee this, but that's > not obvious)" > > Do you prefer a g_assert(IS_ALIGNED()) instead? I guess you mean QEMU_IS_ALIGNED(). But yes, I think we could just check alignments here, so callers do the alignments first. We could have, for example, a new helper function mmap_align(int size) that returned size already aligned. > > Thanks! -- Murilo
Re: [PATCH v1 08/13] util/mmap-alloc: Prepare for resizable mmaps
Hello, David. On Monday, February 3, 2020 3:31:20 PM -03 David Hildenbrand wrote: > When shrinking a mmap we want to re-reserve the already populated area. > When growing a memory region, we want to populate starting with a given > fd_offset. Prepare by allowing to pass these parameters. > > Also, let's make sure we always process full pages, to avoid > unmapping/remapping pages that are already in use when > growing/shrinking. (existing callers seem to guarantee this, but that's > not obvious) > > Cc: "Michael S. Tsirkin" > Cc: Greg Kurz > Cc: Murilo Opsfelder Araujo > Cc: Eduardo Habkost > Cc: "Dr. David Alan Gilbert" > Signed-off-by: David Hildenbrand > --- > util/mmap-alloc.c | 32 +--- > 1 file changed, 21 insertions(+), 11 deletions(-) > > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c > index f043ccb0ab..63ad6893b7 100644 > --- a/util/mmap-alloc.c > +++ b/util/mmap-alloc.c > @@ -83,12 +83,12 @@ size_t qemu_mempath_getpagesize(const char *mem_path) > } > > /* > - * Reserve a new memory region of the requested size to be used for mapping > - * from the given fd (if any). > + * Reserve a new memory region of the requested size or re-reserve parts > + * of an existing region to be used for mapping from the given fd (if any). > */ > -static void *mmap_reserve(size_t size, int fd) > +static void *mmap_reserve(void *ptr, size_t size, int fd) > { > -int flags = MAP_PRIVATE; > +int flags = MAP_PRIVATE | (ptr ? MAP_FIXED : 0); > > #if defined(__powerpc64__) && defined(__linux__) > /* > @@ -111,19 +111,23 @@ static void *mmap_reserve(size_t size, int fd) > flags |= MAP_ANONYMOUS; > #endif > > -return mmap(0, size, PROT_NONE, flags, fd, 0); > +return mmap(ptr, size, PROT_NONE, flags, fd, 0); > } > > /* > * Populate memory in a reserved region from the given fd (if any). > */ > -static void *mmap_populate(void *ptr, size_t size, int fd, bool shared, > - bool is_pmem) > +static void *mmap_populate(void *ptr, size_t size, int fd, size_t > fd_offset, + bool shared, bool is_pmem) > { > int map_sync_flags = 0; > int flags = MAP_FIXED; > void *new_ptr; > > +if (fd == -1) { > +fd_offset = 0; > +} > + > flags |= fd == -1 ? MAP_ANONYMOUS : 0; > flags |= shared ? MAP_SHARED : MAP_PRIVATE; > if (shared && is_pmem) { > @@ -131,7 +135,7 @@ static void *mmap_populate(void *ptr, size_t size, int > fd, bool shared, } > > new_ptr = mmap(ptr, size, PROT_READ | PROT_WRITE, flags | > map_sync_flags, - fd, 0); > + fd, fd_offset); > if (new_ptr == MAP_FAILED && map_sync_flags) { > if (errno == ENOTSUP) { > char *proc_link = g_strdup_printf("/proc/self/fd/%d", fd); > @@ -153,7 +157,7 @@ static void *mmap_populate(void *ptr, size_t size, int > fd, bool shared, * If mmap failed with MAP_SHARED_VALIDATE | MAP_SYNC, we > will try * again without these flags to handle backwards compatibility. */ > -new_ptr = mmap(ptr, size, PROT_READ | PROT_WRITE, flags, fd, 0); > +new_ptr = mmap(ptr, size, PROT_READ | PROT_WRITE, flags, fd, > fd_offset); } > return new_ptr; > } > @@ -178,13 +182,16 @@ void *qemu_ram_mmap(int fd, > size_t offset, total; > void *ptr, *guardptr; > > +/* we can only map whole pages */ > +size = QEMU_ALIGN_UP(size, pagesize); > + Caller already rounds up size to block->page_size. Why this QEMU_ALIGN_UP is necessary? > /* > * Note: this always allocates at least one extra page of virtual > address * space, even if size is already aligned. > */ > total = size + align; If size was aligned above with pagesize boundary, why would this align be necessary? Can the pagesize differ from memory region align? > > -guardptr = mmap_reserve(total, fd); > +guardptr = mmap_reserve(0, total, fd); > if (guardptr == MAP_FAILED) { > return MAP_FAILED; > } > @@ -195,7 +202,7 @@ void *qemu_ram_mmap(int fd, > > offset = QEMU_ALIGN_UP((uintptr_t)guardptr, align) - > (uintptr_t)guardptr; > > -ptr = mmap_populate(guardptr + offset, size, fd, shared, is_pmem); > +ptr = mmap_populate(guardptr + offset, size, fd, 0, shared, is_pmem); > if (ptr == MAP_FAILED) { > munmap(guardptr, total); > return MAP_FAILED; > @@ -221,6 +228,9 @@ void qemu_ram_munmap(int fd, void *ptr, size_t size) > { > const size_t pagesize = mmap_pagesize(fd); > > +/* we can only map whole pages */ > +size = QEMU_ALIGN_UP(size, pagesize); > + I'm trying to understand why this align is necessary, too. > if (ptr) { > /* Unmap both the RAM block and the guard page */ > munmap(ptr, size + pagesize); -- Murilo
Re: [PATCH v1 07/13] util/mmap-alloc: Factor out populating of memory to mmap_populate()
Hello, David. On 2/3/20 3:31 PM, David Hildenbrand wrote: We want to populate memory within a reserved memory region. Let's factor that out. Cc: "Michael S. Tsirkin" Cc: Greg Kurz Cc: Murilo Opsfelder Araujo Cc: Eduardo Habkost Cc: "Dr. David Alan Gilbert" Signed-off-by: David Hildenbrand --- Acked-by: Murilo Opsfelder Araujo A minor comment below. util/mmap-alloc.c | 89 +-- 1 file changed, 47 insertions(+), 42 deletions(-) diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c index 43a26f38a8..f043ccb0ab 100644 --- a/util/mmap-alloc.c +++ b/util/mmap-alloc.c @@ -114,6 +114,50 @@ static void *mmap_reserve(size_t size, int fd) return mmap(0, size, PROT_NONE, flags, fd, 0); } +/* + * Populate memory in a reserved region from the given fd (if any). + */ +static void *mmap_populate(void *ptr, size_t size, int fd, bool shared, + bool is_pmem) +{ +int map_sync_flags = 0; +int flags = MAP_FIXED; +void *new_ptr; Do you think another name would be welcome here, e.g.: "populated_ptr" or "populated_memptr" or just "populated"? + +flags |= fd == -1 ? MAP_ANONYMOUS : 0; +flags |= shared ? MAP_SHARED : MAP_PRIVATE; +if (shared && is_pmem) { +map_sync_flags = MAP_SYNC | MAP_SHARED_VALIDATE; +} + +new_ptr = mmap(ptr, size, PROT_READ | PROT_WRITE, flags | map_sync_flags, + fd, 0); +if (new_ptr == MAP_FAILED && map_sync_flags) { +if (errno == ENOTSUP) { +char *proc_link = g_strdup_printf("/proc/self/fd/%d", fd); +char *file_name = g_malloc0(PATH_MAX); +int len = readlink(proc_link, file_name, PATH_MAX - 1); + +if (len < 0) { +len = 0; +} +file_name[len] = '\0'; +fprintf(stderr, "Warning: requesting persistence across crashes " +"for backend file %s failed. Proceeding without " +"persistence, data might become corrupted in case of host " +"crash.\n", file_name); +g_free(proc_link); +g_free(file_name); +} +/* + * If mmap failed with MAP_SHARED_VALIDATE | MAP_SYNC, we will try + * again without these flags to handle backwards compatibility. + */ +new_ptr = mmap(ptr, size, PROT_READ | PROT_WRITE, flags, fd, 0); +} +return new_ptr; +} + static inline size_t mmap_pagesize(int fd) { #if defined(__powerpc64__) && defined(__linux__) @@ -131,12 +175,8 @@ void *qemu_ram_mmap(int fd, bool is_pmem) { const size_t pagesize = mmap_pagesize(fd); -int flags; -int map_sync_flags = 0; -size_t offset; -size_t total; -void *guardptr; -void *ptr; +size_t offset, total; +void *ptr, *guardptr; /* * Note: this always allocates at least one extra page of virtual address @@ -153,44 +193,9 @@ void *qemu_ram_mmap(int fd, /* Always align to host page size */ assert(align >= pagesize); -flags = MAP_FIXED; -flags |= fd == -1 ? MAP_ANONYMOUS : 0; -flags |= shared ? MAP_SHARED : MAP_PRIVATE; -if (shared && is_pmem) { -map_sync_flags = MAP_SYNC | MAP_SHARED_VALIDATE; -} - offset = QEMU_ALIGN_UP((uintptr_t)guardptr, align) - (uintptr_t)guardptr; -ptr = mmap(guardptr + offset, size, PROT_READ | PROT_WRITE, - flags | map_sync_flags, fd, 0); - -if (ptr == MAP_FAILED && map_sync_flags) { -if (errno == ENOTSUP) { -char *proc_link, *file_name; -int len; -proc_link = g_strdup_printf("/proc/self/fd/%d", fd); -file_name = g_malloc0(PATH_MAX); -len = readlink(proc_link, file_name, PATH_MAX - 1); -if (len < 0) { -len = 0; -} -file_name[len] = '\0'; -fprintf(stderr, "Warning: requesting persistence across crashes " -"for backend file %s failed. Proceeding without " -"persistence, data might become corrupted in case of host " -"crash.\n", file_name); -g_free(proc_link); -g_free(file_name); -} -/* - * if map failed with MAP_SHARED_VALIDATE | MAP_SYNC, - * we will remove these flags to handle compatibility. - */ -ptr = mmap(guardptr + offset, size, PROT_READ | PROT_WRITE, - flags, fd, 0); -} - +ptr = mmap_populate(guardptr + offset, size, fd, shared, is_pmem); if (ptr == MAP_FAILED) { munmap(guardptr, total); return MAP_FAILED; -- Murilo
Re: [PATCH v1 06/13] util/mmap-alloc: Factor out reserving of a memory region to mmap_reserve()
Hello, David. On 2/3/20 3:31 PM, David Hildenbrand wrote: We want to reserve a memory region without actually populating memory. Let's factor that out. Cc: "Michael S. Tsirkin" Cc: Greg Kurz Cc: Murilo Opsfelder Araujo Cc: Eduardo Habkost Cc: "Dr. David Alan Gilbert" Signed-off-by: David Hildenbrand Acked-by: Murilo Opsfelder Araujo --- util/mmap-alloc.c | 58 +++ 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c index 82f02a2cec..43a26f38a8 100644 --- a/util/mmap-alloc.c +++ b/util/mmap-alloc.c @@ -82,6 +82,38 @@ size_t qemu_mempath_getpagesize(const char *mem_path) return qemu_real_host_page_size; } +/* + * Reserve a new memory region of the requested size to be used for mapping + * from the given fd (if any). + */ +static void *mmap_reserve(size_t size, int fd) +{ +int flags = MAP_PRIVATE; + +#if defined(__powerpc64__) && defined(__linux__) +/* + * On ppc64 mappings in the same segment (aka slice) must share the same + * page size. Since we will be re-allocating part of this segment + * from the supplied fd, we should make sure to use the same page size, to + * this end we mmap the supplied fd. In this case, set MAP_NORESERVE to + * avoid allocating backing store memory. + * We do this unless we are using the system page size, in which case + * anonymous memory is OK. + */ +if (fd == -1 || qemu_fd_getpagesize(fd) == qemu_real_host_page_size) { +fd = -1; +flags |= MAP_ANONYMOUS; +} else { +flags |= MAP_NORESERVE; +} +#else +fd = -1; +flags |= MAP_ANONYMOUS; +#endif + +return mmap(0, size, PROT_NONE, flags, fd, 0); +} + static inline size_t mmap_pagesize(int fd) { #if defined(__powerpc64__) && defined(__linux__) @@ -101,7 +133,6 @@ void *qemu_ram_mmap(int fd, const size_t pagesize = mmap_pagesize(fd); int flags; int map_sync_flags = 0; -int guardfd; size_t offset; size_t total; void *guardptr; @@ -113,30 +144,7 @@ void *qemu_ram_mmap(int fd, */ total = size + align; -#if defined(__powerpc64__) && defined(__linux__) -/* On ppc64 mappings in the same segment (aka slice) must share the same - * page size. Since we will be re-allocating part of this segment - * from the supplied fd, we should make sure to use the same page size, to - * this end we mmap the supplied fd. In this case, set MAP_NORESERVE to - * avoid allocating backing store memory. - * We do this unless we are using the system page size, in which case - * anonymous memory is OK. - */ -flags = MAP_PRIVATE; -if (fd == -1 || pagesize == qemu_real_host_page_size) { -guardfd = -1; -flags |= MAP_ANONYMOUS; -} else { -guardfd = fd; -flags |= MAP_NORESERVE; -} -#else -guardfd = -1; -flags = MAP_PRIVATE | MAP_ANONYMOUS; -#endif - -guardptr = mmap(0, total, PROT_NONE, flags, guardfd, 0); - +guardptr = mmap_reserve(total, fd); if (guardptr == MAP_FAILED) { return MAP_FAILED; } -- Murilo
Re: [PATCH v1 05/13] util/mmap-alloc: Factor out calculation of pagesize to mmap_pagesize()
Hello, David. On 2/3/20 3:31 PM, David Hildenbrand wrote: Factor it out and add a comment. Cc: "Michael S. Tsirkin" Cc: Murilo Opsfelder Araujo Cc: Greg Kurz Cc: Eduardo Habkost Cc: "Dr. David Alan Gilbert" Signed-off-by: David Hildenbrand Acked-by: Murilo Opsfelder Araujo --- util/mmap-alloc.c | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c index 27dcccd8ec..82f02a2cec 100644 --- a/util/mmap-alloc.c +++ b/util/mmap-alloc.c @@ -82,17 +82,27 @@ size_t qemu_mempath_getpagesize(const char *mem_path) return qemu_real_host_page_size; } +static inline size_t mmap_pagesize(int fd) +{ +#if defined(__powerpc64__) && defined(__linux__) +/* Mappings in the same segment must share the same page size */ +return qemu_fd_getpagesize(fd); +#else +return qemu_real_host_page_size; +#endif +} + void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared, bool is_pmem) { +const size_t pagesize = mmap_pagesize(fd); int flags; int map_sync_flags = 0; int guardfd; size_t offset; -size_t pagesize; size_t total; void *guardptr; void *ptr; @@ -113,7 +123,6 @@ void *qemu_ram_mmap(int fd, * anonymous memory is OK. */ flags = MAP_PRIVATE; -pagesize = qemu_fd_getpagesize(fd); if (fd == -1 || pagesize == qemu_real_host_page_size) { guardfd = -1; flags |= MAP_ANONYMOUS; @@ -123,7 +132,6 @@ void *qemu_ram_mmap(int fd, } #else guardfd = -1; -pagesize = qemu_real_host_page_size; flags = MAP_PRIVATE | MAP_ANONYMOUS; #endif @@ -198,15 +206,10 @@ void *qemu_ram_mmap(int fd, void qemu_ram_munmap(int fd, void *ptr, size_t size) { -size_t pagesize; +const size_t pagesize = mmap_pagesize(fd); if (ptr) { /* Unmap both the RAM block and the guard page */ -#if defined(__powerpc64__) && defined(__linux__) -pagesize = qemu_fd_getpagesize(fd); -#else -pagesize = qemu_real_host_page_size; -#endif munmap(ptr, size + pagesize); } } -- Murilo
Re: [Qemu-devel] [PATCH 2/2] target/ppc/kvm: Convert DPRINTF to traces
Hi, Greg. Greg Kurz writes: > Signed-off-by: Greg Kurz > --- > target/ppc/kvm.c| 68 > +++ > target/ppc/trace-events | 25 + > 2 files changed, 52 insertions(+), 41 deletions(-) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 2427c8ee13ae..3a11d2e1060c 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -49,16 +49,6 @@ > #include "elf.h" > #include "sysemu/kvm_int.h" > > -//#define DEBUG_KVM > - > -#ifdef DEBUG_KVM > -#define DPRINTF(fmt, ...) \ > -do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) > -#else > -#define DPRINTF(fmt, ...) \ > -do { } while (0) > -#endif > - > #define PROC_DEVTREE_CPU "/proc/device-tree/cpus/" > > const KVMCapabilityInfo kvm_arch_required_capabilities[] = { > @@ -626,7 +616,7 @@ static int kvm_put_fp(CPUState *cs) > reg.addr = (uintptr_t)&fpscr; > ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > if (ret < 0) { > -DPRINTF("Unable to set FPSCR to KVM: %s\n", strerror(errno)); > +trace_kvm_failed_fpscr_set(strerror(errno)); > return ret; > } > > @@ -647,8 +637,8 @@ static int kvm_put_fp(CPUState *cs) > > ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > if (ret < 0) { > -DPRINTF("Unable to set %s%d to KVM: %s\n", vsx ? "VSR" : > "FPR", > -i, strerror(errno)); > +trace_kvm_failed_fp_set(vsx ? "VSR" : "FPR", i, > +strerror(errno)); > return ret; > } > } > @@ -659,7 +649,7 @@ static int kvm_put_fp(CPUState *cs) > reg.addr = (uintptr_t)&env->vscr; > ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > if (ret < 0) { > -DPRINTF("Unable to set VSCR to KVM: %s\n", strerror(errno)); > +trace_kvm_failed_vscr_set(strerror(errno)); > return ret; > } > > @@ -668,7 +658,7 @@ static int kvm_put_fp(CPUState *cs) > reg.addr = (uintptr_t)cpu_avr_ptr(env, i); > ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > if (ret < 0) { > -DPRINTF("Unable to set VR%d to KVM: %s\n", i, > strerror(errno)); > +trace_kvm_failed_vr_set(i, strerror(errno)); > return ret; > } > } > @@ -693,7 +683,7 @@ static int kvm_get_fp(CPUState *cs) > reg.addr = (uintptr_t)&fpscr; > ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); > if (ret < 0) { > -DPRINTF("Unable to get FPSCR from KVM: %s\n", strerror(errno)); > +trace_kvm_failed_fpscr_get(strerror(errno)); > return ret; > } else { > env->fpscr = fpscr; > @@ -709,8 +699,8 @@ static int kvm_get_fp(CPUState *cs) > > ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); > if (ret < 0) { > -DPRINTF("Unable to get %s%d from KVM: %s\n", > -vsx ? "VSR" : "FPR", i, strerror(errno)); > +trace_kvm_failed_fp_get(vsx ? "VSR" : "FPR", i, > +strerror(errno)); > return ret; > } else { > #ifdef HOST_WORDS_BIGENDIAN > @@ -733,7 +723,7 @@ static int kvm_get_fp(CPUState *cs) > reg.addr = (uintptr_t)&env->vscr; > ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); > if (ret < 0) { > -DPRINTF("Unable to get VSCR from KVM: %s\n", strerror(errno)); > +trace_kvm_failed_vscr_get(strerror(errno)); > return ret; > } > > @@ -742,8 +732,7 @@ static int kvm_get_fp(CPUState *cs) > reg.addr = (uintptr_t)cpu_avr_ptr(env, i); > ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); > if (ret < 0) { > -DPRINTF("Unable to get VR%d from KVM: %s\n", > -i, strerror(errno)); > +trace_kvm_failed_vr_get(i, strerror(errno)); > return ret; > } > } > @@ -764,7 +753,7 @@ static int kvm_get_vpa(CPUState *cs) > reg.addr = (uintptr_t)&spapr_cpu->vpa_addr; > ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); > if (ret < 0) { > -DPRINTF("Unable to get VPA address from KVM: %s\n", strerror(errno)); > +trace_kvm_failed_vpa_addr_get(strerror(errno)); > return ret; > } > > @@ -774,8 +763,7 @@ static int kvm_get_vpa(CPUState *cs) > reg.addr = (uintptr_t)&spapr_cpu->slb_shadow_addr; > ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); > if (ret < 0) { > -DPRINTF("Unable to get SLB shadow state from KVM: %s\n", > -strerror(errno)); > +trace_kvm_failed_slb_get(strerror(errno)); > return ret; > } > > @@ -785,8 +773,7 @@ static int kvm_get_vpa(CPUState *cs) > reg.addr = (uintptr_t)&spapr_cpu->dtl_addr; >
[Qemu-devel] [Bug 1776096] Re: qemu 2.12.0 qemu-system-ppc illegal instruction on ppc64le, crashes emulator
Hi, Cameron. The step "Start QEMU and boot Mac OS X 10.4.11" is not clear to me. Is there a location where one could download such image and boot? I wonder how one without access to a Mac image can reproduce this issue. Cheers Murilo -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1776096 Title: qemu 2.12.0 qemu-system-ppc illegal instruction on ppc64le, crashes emulator Status in QEMU: New Bug description: % uname -a Linux tim.floodgap.com 4.16.14-300.fc28.ppc64le #1 SMP Tue Jun 5 15:59:48 UTC 2018 ppc64le ppc64le ppc64le GNU/Linux STR: Start QEMU and boot Mac OS X 10.4.11. Download the current version of TenFourFox (I used G3 so that AltiVec was not a confounder). Try to start TenFourFox in safe mode (hold down Option as you double-click while the icon bounces in the Dock). Expected: TenFourFox starts. Actual: The entire emulator exits with an illegal instruction error. Trace of session (including some disassembly so you can see where TCG went wrong): tim:/home/spectre/src/qemu-2.12.0/ppc-softmmu/% gdb --args ./qemu- system-ppc -M mac99,accel=tcg -m 2048 -prom-env boot-args=-v -boot c -drive file=tigerhd.img,format=raw,cache=none -netdev user,id=mynet0 -device usb-net,netdev=mynet0 -usb -device usb-tablet GNU gdb (GDB) Fedora 8.1-15.fc28 [...] Reading symbols from ./qemu-system-ppc...done. (gdb) run [...] Thread 6 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7242ea30 (LWP 7017)] 0xfffc in ?? () #0 0xfffc in () #1 0x7fffd4edec00 in code_gen_buffer () #2 0x100c9e20 in cpu_tb_exec (itb=, cpu=) at /home/spectre/src/qemu-2.12.0/accel/tcg/cpu-exec.c:169 #3 0x100c9e20 in cpu_loop_exec_tb (tb_exit=, last_tb=, tb=, cpu=) at /home/spectre/src/qemu-2.12.0/accel/tcg/cpu-exec.c:626 #4 0x100c9e20 in cpu_exec (cpu=) at /home/spectre/src/qemu-2.12.0/accel/tcg/cpu-exec.c:734 #5 0x1007decc in tcg_cpu_exec (cpu=0x11774e10) at /home/spectre/src/qemu-2.12.0/cpus.c:1362 (gdb) disas 0x7fffd4edebf0, 0x7fffd4edec10 Dump of assembler code from 0x7fffd4edebf0 to 0x7fffd4edec10: 0x7fffd4edebf0 :addir0,r4,3 0x7fffd4edebf4 :rlwinm r0,r0,0,0,19 0x7fffd4edebf8 :cmplw cr7,r0,r12 0x7fffd4edebfc :bnel cr7,0x7fffd4ed8b64 0x7fffd4edec00 :lwbrx r14,r3,r4 0x7fffd4edec04 :stw r14,40(r27) 0x7fffd4edec08 :clrldi r4,r14,32 0x7fffd4edec0c :rlwinm r3,r4,25,19,26 End of assembler dump. (gdb) disas 0x7fffd4ed8b60, 0x7fffd4ed8b70 Dump of assembler code from 0x7fffd4ed8b60 to 0x7fffd4ed8b70: 0x7fffd4ed8b60 :bctrl 0x7fffd4ed8b64 :mtctr r3 0x7fffd4ed8b68 :mr r31,r3 0x7fffd4ed8b6c :li r3,0 End of assembler dump. (gdb) i reg ctr ctr0x 18446744073709551615 It appears that the branch at 0x7fffd4edebfc caused a jump back (a return?) through CTR, but CTR has -1 in it, hence setting PC to 0xfffc. I am not sure how to debug this further. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1776096/+subscriptions
[Qemu-devel] [Bug 1786343] Re: QEMU v3.0.0-rc4 configure fails with --enable-mpath on CentOS 7.5
I'll work on a fix for configure. ** Changed in: qemu Status: New => Confirmed ** Changed in: qemu Assignee: (unassigned) => Murilo Opsfelder Araújo (mopsfelder) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1786343 Title: QEMU v3.0.0-rc4 configure fails with --enable-mpath on CentOS 7.5 Status in QEMU: Confirmed Bug description: QEMU v3.0.0-rc4 configure fails with --enable-mpath on CentOS 7.5. After commit b3f1c8c413bc83e4a2cc7a63e4eddf9fe6449052 "qemu-pr-helper: use new libmultipath API", QEMU started using new libmultipath API, which is not available on CentOS 7.5. Reverting this commit, configure passes. Steps to reproduce (fails on x86_64 and ppc64le architectures): $ git clone git://git.qemu.org/qemu.git $ mkdir -p qemu/build && cd qemu/build $ ../configure --enable-mpath ERROR: Multipath requires libmpathpersist devel $ rpm -qa | grep device-mapper | sort device-mapper-1.02.146-4.el7.ppc64le device-mapper-devel-1.02.146-4.el7.ppc64le device-mapper-libs-1.02.146-4.el7.ppc64le device-mapper-multipath-0.4.9-119.el7.ppc64le device-mapper-multipath-devel-0.4.9-119.el7.ppc64le device-mapper-multipath-libs-0.4.9-119.el7.ppc64le Snippet from config.log: funcs: do_compiler do_cc compile_prog main lines: 92 125 3580 0 cc -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-strong -Wno-missing-braces -I/usr/include/p11-kit-1 -I/usr/include/libpng15 -o config-temp/qemu-conf.exe config-temp/qemu-conf.c -m64 -g -ludev -lmultipath -lmpathpersist config-temp/qemu-conf.c: In function ‘main’: config-temp/qemu-conf.c:15:5: error: too few arguments to function ‘mpath_lib_init’ multipath_conf = mpath_lib_init(); ^ In file included from config-temp/qemu-conf.c:2:0: /usr/include/mpath_persist.h:179:12: note: declared here extern int mpath_lib_init (struct udev *udev); ^ To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1786343/+subscriptions
[Qemu-devel] [Bug 1786343] [NEW] QEMU v3.0.0-rc4 configure fails with --enable-mpath on CentOS 7.5
Public bug reported: QEMU v3.0.0-rc4 configure fails with --enable-mpath on CentOS 7.5. After commit b3f1c8c413bc83e4a2cc7a63e4eddf9fe6449052 "qemu-pr-helper: use new libmultipath API", QEMU started using new libmultipath API, which is not available on CentOS 7.5. Reverting this commit, configure passes. Steps to reproduce (fails on x86_64 and ppc64le architectures): $ git clone git://git.qemu.org/qemu.git $ mkdir -p qemu/build && cd qemu/build $ ../configure --enable-mpath ERROR: Multipath requires libmpathpersist devel $ rpm -qa | grep device-mapper | sort device-mapper-1.02.146-4.el7.ppc64le device-mapper-devel-1.02.146-4.el7.ppc64le device-mapper-libs-1.02.146-4.el7.ppc64le device-mapper-multipath-0.4.9-119.el7.ppc64le device-mapper-multipath-devel-0.4.9-119.el7.ppc64le device-mapper-multipath-libs-0.4.9-119.el7.ppc64le Snippet from config.log: funcs: do_compiler do_cc compile_prog main lines: 92 125 3580 0 cc -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-strong -Wno-missing-braces -I/usr/include/p11-kit-1 -I/usr/include/libpng15 -o config-temp/qemu-conf.exe config-temp/qemu-conf.c -m64 -g -ludev -lmultipath -lmpathpersist config-temp/qemu-conf.c: In function ‘main’: config-temp/qemu-conf.c:15:5: error: too few arguments to function ‘mpath_lib_init’ multipath_conf = mpath_lib_init(); ^ In file included from config-temp/qemu-conf.c:2:0: /usr/include/mpath_persist.h:179:12: note: declared here extern int mpath_lib_init (struct udev *udev); ^ ** 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/1786343 Title: QEMU v3.0.0-rc4 configure fails with --enable-mpath on CentOS 7.5 Status in QEMU: New Bug description: QEMU v3.0.0-rc4 configure fails with --enable-mpath on CentOS 7.5. After commit b3f1c8c413bc83e4a2cc7a63e4eddf9fe6449052 "qemu-pr-helper: use new libmultipath API", QEMU started using new libmultipath API, which is not available on CentOS 7.5. Reverting this commit, configure passes. Steps to reproduce (fails on x86_64 and ppc64le architectures): $ git clone git://git.qemu.org/qemu.git $ mkdir -p qemu/build && cd qemu/build $ ../configure --enable-mpath ERROR: Multipath requires libmpathpersist devel $ rpm -qa | grep device-mapper | sort device-mapper-1.02.146-4.el7.ppc64le device-mapper-devel-1.02.146-4.el7.ppc64le device-mapper-libs-1.02.146-4.el7.ppc64le device-mapper-multipath-0.4.9-119.el7.ppc64le device-mapper-multipath-devel-0.4.9-119.el7.ppc64le device-mapper-multipath-libs-0.4.9-119.el7.ppc64le Snippet from config.log: funcs: do_compiler do_cc compile_prog main lines: 92 125 3580 0 cc -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-strong -Wno-missing-braces -I/usr/include/p11-kit-1 -I/usr/include/libpng15 -o config-temp/qemu-conf.exe config-temp/qemu-conf.c -m64 -g -ludev -lmultipath -lmpathpersist config-temp/qemu-conf.c: In function ‘main’: config-temp/qemu-conf.c:15:5: error: too few arguments to function ‘mpath_lib_init’ multipath_conf = mpath_lib_init(); ^ In file included from config-temp/qemu-conf.c:2:0: /usr/include/mpath_persist.h:179:12: note: declared here extern int mpath_lib_init (struct udev *udev); ^ To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1786343/+subscriptions
[Qemu-devel] [Bug 623852] Re: PPC emulation loops on booting a FreeBSD kernel
Hi, Nigel. Support for powerpc64 is available since FreeBSD 9.0-RELEASE, I think. FreeBSD 11.2-RC2 boots fine in QEMU (at commit 46012db666990ff2eed1d3dc) running on an x86 host with accel=tcg. Below are the steps I have followed to boot it. Build QEMU: $ mkdir build && cd build $ ../configure --target-list=ppc64-softmmu $ make -j$(nproc) Boot FreeBSD: $ wget http://ftp.freebsd.org/pub/FreeBSD/releases/powerpc/powerpc64/ISO-IMAGES/11.2/FreeBSD-11.2-RC2-powerpc-powerpc64-disc1.iso $ ./qemu-img create -f qcow2 freebsd.qcow2 10G $ ./ppc64-softmmu/qemu-system-ppc64 -name freebsd -machine pseries,accel=tcg,usb=off -m 1024 -realtime mlock=off -smp 4,sockets=4,cores=1,threads=1 -nographic -no-user-config -nodefaults -rtc base=utc -no-shutdown -boot strict=on -device pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x1 -device pci-ohci,id=usb,bus=pci.0,addr=0x2 -device spapr-vscsi,id=scsi0,reg=0x2000 -drive file=freebsd.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -drive file=FreeBSD-11.2-RC2-powerpc-powerpc64-disc1.iso,format=raw,if=none,id=drive-scsi0-0-1-0,readonly=on -device scsi-cd,bus=scsi0.0,channel=0,scsi-id=1,lun=0,drive=drive-scsi0-0-1-0,id=scsi0-0-1-0,bootindex=2 -netdev user,id=hostnet0 -device spapr-vlan,netdev=hostnet0,id=net0,mac=4c:45:42:45:01:18,reg=0x1000 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 -msg timestamp=on -serial mon:stdio Since this bug is almost 8 years old and FreeBSD powerpc64 seems to be working just fine, I will close it. Feel free to submit a new one if needed. Cheers Murilo ** Changed in: qemu Status: New => Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/623852 Title: PPC emulation loops on booting a FreeBSD kernel Status in QEMU: Invalid Bug description: Has anyone tried booting FreeBSD8.1-ppc under QEMU (Linux x86_64 host; PPC guest)? I can get Linux/PPC to run fine, and FreeBSD8.1-i386 as well; but there seems to be a problem with whatever the FreeBSD8.1 kernel does, that QEMU's PPC emulation can't handle. I am using the latest version of QEMU from GIT as of 25/8/10. I don't know how to get a "git commit hash", so I can't quote it. The kernel starts OK then loops after "Kernel entry at 0x100100 ...". The command I am running is qemu-system-ppc -cdrom FreeBSD-8.1-RELEASE-powerpc-disc1.iso -hda freebsd8.1-ppc -m 94 -boot d" I obtained the kernel from ftp://ftp.freebsd.org/pub/FreeBSD/releases/powerpc/ISO- IMAGES/8.1/FreeBSD-8.1-RELEASE-powerpc-disc1.iso. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/623852/+subscriptions
[Qemu-devel] [Bug 1763536] Re: go build fails under qemu-ppc64le-static (qemu-user)
With QEMU from tag v2.12.0-rc4 on Fedora 27 x86_64, it works too. muriloo@laptop$ docker run --rm -it qemutest /go # qemu-ppc64le-static --version qemu-ppc64le version 2.11.94 (v2.12.0-rc4) Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers /go # go version go version go1.10.1 linux/ppc64le /go # go build hello.go /go # ./hello hello world /go # muriloo@laptop$ uname -a Linux laptop 4.15.17-300.fc27.x86_64 #1 SMP Thu Apr 12 18:19:17 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux muriloo@laptop$ rpm -q docker docker-1.13.1-51.git4032bd5.fc27.x86_64 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1763536 Title: go build fails under qemu-ppc64le-static (qemu-user) Status in QEMU: New Bug description: I am using qemu-user (built static) in a docker container environment. When running multi-threaded go commands in the container (go build for example) the process may hang, report segfaults or other errors. I built qemu-ppc64le from the upstream git (master). I see the problem running on a multi core system with Intel i7 processors. # cat /proc/cpuinfo | grep "model name" model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz Steps to reproduce: 1) Build qemu-ppc64le as static and copy into docker build directory named it qemu-ppc64le-static. 2) Add hello.go to docker build dir. package main import "fmt" func main() { fmt.Println("hello world") } 3) Create the Dockerfile from below: FROM ppc64le/golang:1.10.1-alpine3. COPY qemu-ppc64le-static /usr/bin/ COPY hello.go /go 4) Build container $ docker build -t qemutest -f Dockerfile ./go 5) Run test $ docker run -it qemutest /go # /usr/bin/qemu-ppc64le-static --version qemu-ppc64le version 2.11.93 (v2.12.0-rc3-dirty) Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers /go # go version go version go1.10.1 linux/ppc64le /go # go build hello.go fatal error: fatal error: stopm holding locksunexpected signal during runtime execution panic during panic [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1003528c] runtime stack: runtime: unexpected return pc for syscall.Syscall6 called from 0xc42007f500 stack: frame={sp:0xc4203be840, fp:0xc4203be860} stack=[0x4000b7ecf0,0x4000b928f0) syscall.Syscall6(0x100744e8, 0x3d, 0xc42050c140, 0x20, 0x18, 0x10422b80, 0xc4203be968[signal , 0x10012d88SIGSEGV: segmentation violation, 0xc420594000 code=, 0x00x1 addr=0x0 pc=0x1003528c) ] runtime stack: /usr/local/go/src/syscall/asm_linux_ppc64x.s:61runtime.throw(0x10472d19, 0x13) +/usr/local/go/src/runtime/panic.go:0x6c616 +0x68 runtime.stopm() /usr/local/go/src/runtime/proc.go:1939goroutine +10x158 [runtime.exitsyscall0semacquire(0xc42007f500) /usr/local/go/src/runtime/proc.go:3129 +]: 0x130 runtime.mcall(0xc42007f500) /usr/local/go/src/runtime/asm_ppc64x.s:183 +0x58sync.runtime_Semacquire (0xc4201fab1c) /usr/local/go/src/runtime/sema.go:56 +0x38 Note the results may differ between attempts, hangs and other faults sometimes happen. If I run "go: single threaded I don't see the problem, for example: /go # GOMAXPROCS=1 go build -p 1 hello.go /go # ./hello hello world I see the same issue with arm64. I don't think this is a go issue, but don't have a real evidence to prove that. This problem looks similar to other problem I have seen reported against qemu running multi-threaded applications. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1763536/+subscriptions
[Qemu-devel] [Bug 1763536] Re: go build fails under qemu-ppc64le-static (qemu-user)
Using QEMU from tag v2.12.0-rc4 on Ubuntu Xenial ppc64el, it works. muriloo@jaspion1:~/go-docker$ sudo docker run --rm -it qemutest /go # /usr/bin/qemu-ppc64le-static --version qemu-ppc64 version 2.11.94 (v2.12.0-rc4-dirty) Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers /go # go version go version go1.10.1 linux/ppc64le /go # go build hello.go /go # ./hello hello world /go # Here is how I configured QEMU: muriloo@jaspion1:~/sources/qemu$ ./configure --target-list=ppc64-linux- user --disable-system --disable-tools --static muriloo@jaspion1:~$ uname -a Linux jaspion1 4.4.0-119-generic #143-Ubuntu SMP Mon Apr 2 16:08:02 UTC 2018 ppc64le ppc64le ppc64le GNU/Linux -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1763536 Title: go build fails under qemu-ppc64le-static (qemu-user) Status in QEMU: New Bug description: I am using qemu-user (built static) in a docker container environment. When running multi-threaded go commands in the container (go build for example) the process may hang, report segfaults or other errors. I built qemu-ppc64le from the upstream git (master). I see the problem running on a multi core system with Intel i7 processors. # cat /proc/cpuinfo | grep "model name" model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz Steps to reproduce: 1) Build qemu-ppc64le as static and copy into docker build directory named it qemu-ppc64le-static. 2) Add hello.go to docker build dir. package main import "fmt" func main() { fmt.Println("hello world") } 3) Create the Dockerfile from below: FROM ppc64le/golang:1.10.1-alpine3. COPY qemu-ppc64le-static /usr/bin/ COPY hello.go /go 4) Build container $ docker build -t qemutest -f Dockerfile ./go 5) Run test $ docker run -it qemutest /go # /usr/bin/qemu-ppc64le-static --version qemu-ppc64le version 2.11.93 (v2.12.0-rc3-dirty) Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers /go # go version go version go1.10.1 linux/ppc64le /go # go build hello.go fatal error: fatal error: stopm holding locksunexpected signal during runtime execution panic during panic [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1003528c] runtime stack: runtime: unexpected return pc for syscall.Syscall6 called from 0xc42007f500 stack: frame={sp:0xc4203be840, fp:0xc4203be860} stack=[0x4000b7ecf0,0x4000b928f0) syscall.Syscall6(0x100744e8, 0x3d, 0xc42050c140, 0x20, 0x18, 0x10422b80, 0xc4203be968[signal , 0x10012d88SIGSEGV: segmentation violation, 0xc420594000 code=, 0x00x1 addr=0x0 pc=0x1003528c) ] runtime stack: /usr/local/go/src/syscall/asm_linux_ppc64x.s:61runtime.throw(0x10472d19, 0x13) +/usr/local/go/src/runtime/panic.go:0x6c616 +0x68 runtime.stopm() /usr/local/go/src/runtime/proc.go:1939goroutine +10x158 [runtime.exitsyscall0semacquire(0xc42007f500) /usr/local/go/src/runtime/proc.go:3129 +]: 0x130 runtime.mcall(0xc42007f500) /usr/local/go/src/runtime/asm_ppc64x.s:183 +0x58sync.runtime_Semacquire (0xc4201fab1c) /usr/local/go/src/runtime/sema.go:56 +0x38 Note the results may differ between attempts, hangs and other faults sometimes happen. If I run "go: single threaded I don't see the problem, for example: /go # GOMAXPROCS=1 go build -p 1 hello.go /go # ./hello hello world I see the same issue with arm64. I don't think this is a go issue, but don't have a real evidence to prove that. This problem looks similar to other problem I have seen reported against qemu running multi-threaded applications. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1763536/+subscriptions
[Qemu-devel] [Bug 1738691] Re: Guest kernel crashes with kvm_pr on POWER8
Hi, Timothy. I tried to reproduce this issue on a POWER8 box and couldn't reproduce it. Whatever the issue was, it seems to be fixed on kernel v4.16-rc4 with qemu 2.11.50. I downloaded vmlinux/initrd.gz from Ubuntu 18.04 to boot guest. It booted fine up to the installer initial screen. Please find my environment information listed below. I'm closing this bug but feel free to reopen it or file a new one. Cheers Murilo Machine type/model: 8247-22L [muriloo@baratheon ~]$ uname -a Linux localhost.localdomain 4.16.0-rc4+ #1 SMP Thu Mar 8 22:54:31 UTC 2018 ppc64le ppc64le ppc64le GNU/Linux [muriloo@baratheon ~]$ lsmod | grep kvm kvm_pr100276 0 kvm 217753 1 kvm_pr [muriloo@baratheon ~]$ ~/qemu/build/ppc64-softmmu/qemu-system-ppc64 --version QEMU emulator version 2.11.50 (v2.11.0-2108-g83d2e94) Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers [muriloo@baratheon ~]$ ~/qemu/build/ppc64-softmmu/qemu-system-ppc64 -kernel ~/ubuntu/18.04/vmlinux -initrd ~/ubuntu/18.04/initrd.gz -append "console=hvc0 verbose" -nodefaults -nographic -serial mon:stdio -accel kvm vmlinux: http://ports.ubuntu.com/ubuntu-ports/dists/bionic/main/installer-ppc64el/current/images/netboot/ubuntu-installer/ppc64el/vmlinux initrd.gz: http://ports.ubuntu.com/ubuntu-ports/dists/bionic/main/installer-ppc64el/current/images/netboot/ubuntu-installer/ppc64el/initrd.gz ** Changed in: qemu Status: New => Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1738691 Title: Guest kernel crashes with kvm_pr on POWER8 Status in QEMU: Invalid Bug description: When attempting to use the kvm_pr module with QEMU 2.10 on a POWER8 host, Debian and Ubuntu guests hang and show crashes. Host kernel is 4.14. Issue is observed with host kernels 4.9 and 4.13 as well; no other host kernels were tested. Is this the correct place to report a kvm_pr bug? Output from Ubuntu 17.10 guest: Quiescing Open Firmware ... Booting Linux via __start() @ 0x0200 ... [0.00] Page sizes from device-tree: [0.00] base_shift=12: shift=12, sllp=0x, avpnm=0x, tlbiel=1, penc=0 [0.00] base_shift=16: shift=16, sllp=0x0110, avpnm=0x, tlbiel=1, penc=1 [0.00] base_shift=24: shift=24, sllp=0x0100, avpnm=0x0001, tlbiel=0, penc=0 [0.00] Using 1TB segments [0.00] Initializing hash mmu with SLB [0.00] Linux version 4.13.0-16-generic (buildd@bos01-ppc64el-029) (gcc version 7.2.0 (Ubuntu 7.2.0-8ubuntu2)) #19-Ubuntu SMP Wed Oct 11 18:37:02 UTC 2017 (Ubuntu 4.13.0-16.19-generic 4.13.4) [0.00] Found initrd at 0xc3b0:0xc48cf68b [0.00] Using pSeries machine description [0.00] bootconsole [udbg0] enabled [0.00] Partition configured for 2 cpus. [0.00] CPU maps initialized for 1 thread per core -> smp_release_cpus() spinning_secondaries = 1 <- smp_release_cpus() [0.00] - [0.00] ppc64_pft_size= 0x19 [0.00] phys_mem_size = 0x1 [0.00] dcache_bsize = 0x80 [0.00] icache_bsize = 0x80 [0.00] cpu_features = 0x077c7a6c18500249 [0.00] possible= 0x5fff18500649 [0.00] always = 0x18100040 [0.00] cpu_user_features = 0xdc0065c2 0xae00 [0.00] mmu_features = 0x7c006001 [0.00] firmware_features = 0x415a445f [0.00] htab_hash_mask= 0x3 [0.00] - [0.00] numa: NODE_DATA [mem 0xfffd7c80-0xfffe3fff] [0.00] PCI host bridge /pci@8002000 ranges: [0.00] IO 0x2000..0x2000 -> 0x [0.00] MEM 0x20008000..0x2000 -> 0x8000 [0.00] MEM 0x2100..0x21ff -> 0x2100 [0.00] PPC64 nvram contains 65536 bytes [0.00] Zone ranges: [0.00] DMA [mem 0x-0x] [0.00] DMA32empty [0.00] Normal empty [0.00] Device empty [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x-0x] [0.00] Initmem setup node 0 [mem 0x-0x] [0.00] percpu: Embedded 4 pages/cpu @c000ffe0 s162840 r0 d99304 u524288 [0.00] Built 1 zonelists in Node order, mobility grouping on. Total pages: 65472 [0.00] Policy zone: DMA [0.00] Kernel command line: BOOT_IMAGE=/install/vmlinux file=/cdrom/preseed/ubuntu-server.seed no_
[Qemu-devel] [Bug 1727259] Re: qemu-io-test 58 segfaults when configured with gcov
The fix was committed: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=c4365735a7d38f4355c6f77e6670d3972315f7c2 commit c4365735a7d38f4355c6f77e6670d3972315f7c2 Author: Murilo Opsfelder Araujo Date: Fri Jan 5 11:32:41 2018 -0200 block/nbd: fix segmentation fault when .desc is not null-terminated ** Changed in: qemu Status: In Progress => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1727259 Title: qemu-io-test 58 segfaults when configured with gcov Status in QEMU: Fix Committed Bug description: Head is at 3d7196d43bfe12efe98568cb60057e273652b99b Steps to re-produce: 1. git clone ./configure --enable-gcov --target-list=ppc64-softmmu make cd tests/qemu-iotests 2. export qemu binary, in my environment export QEMU_PROG=/home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 3. Run test 58 with format qcow2 ./check -qcow2 58 QEMU -- "/home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64" -nodefaults -machine accel=qtest QEMU_IMG -- "/home/nasastry/qemu_gcov/qemu-img" QEMU_IO -- "/home/nasastry/qemu_gcov/qemu-io" --cache writeback -f qcow2 QEMU_NBD -- "/home/nasastry/qemu_gcov/qemu-nbd" IMGFMT-- qcow2 (compat=1.1) IMGPROTO -- file PLATFORM -- Linux/ppc64le zzfp365-lp1 4.13.0-4.rel.git49564cb.el7.centos.ppc64le TEST_DIR -- /home/nasastry/qemu_gcov/tests/qemu-iotests/scratch SOCKET_SCM_HELPER -- /home/nasastry/qemu_gcov/tests/qemu-iotests/socket_scm_helper 058 1s ... - output mismatch (see 058.out.bad) --- /home/nasastry/qemu_gcov/tests/qemu-iotests/058.out 2017-10-09 14:09:04.262726912 +0530 +++ /home/nasastry/qemu_gcov/tests/qemu-iotests/058.out.bad 2017-10-25 15:00:52.037515025 +0530 @@ -19,16 +19,28 @@ 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) == verifying the exported snapshot with patterns, method 1 == -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 8192 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +./common.rc: line 66: 36255 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) +./common.rc: line 66: 36262 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) == verifying the exported snapshot with patterns, method 2 == -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 8192 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +./common.rc: line 66: 36274 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) +./common.rc: line 66: 36282 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) == verifying the converted snapshot with patterns, method 1 == read 4096/4096 bytes at offset 4096 Failures: 058 Failed 1 of 1 tests with out gcov configured this test case is pass. # ./check -qcow2 58 QEMU -- "/home/nasastry/qemu/ppc64-softmmu/qemu-system-ppc64" -nodefaults -machine accel=qtest QEMU_IMG -- "/home/nasastry/qemu/qemu-img" QEMU_IO -- "/home/nasastry/qemu/qemu-io" --cache writeback -f qcow2 QEMU_NBD -- "/home/nasastry/qemu/qemu-nbd" IMGFMT-- qcow2 (compat=1.1) IMGPROTO -- file PLATFORM -- Linux/ppc64le zzfp365-lp1 4.13.0-4.rel.git49564cb.el7.centos.ppc64le TEST_DIR -- /home/nasastry/qemu/tests/qemu-iotests/scratch SOCKET_SCM_HELPER -- /home/nasastry/qemu/tests/qemu-iotests/socket_scm_helper 058 0s ... Passed all 1 tests To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1727259/+subscriptions
[Qemu-devel] [Bug 1727250] Re: qemu-io-test 147 segfaults when configured with gcov
The fix was committed: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=c4365735a7d38f4355c6f77e6670d3972315f7c2 commit c4365735a7d38f4355c6f77e6670d3972315f7c2 Author: Murilo Opsfelder Araujo Date: Fri Jan 5 11:32:41 2018 -0200 block/nbd: fix segmentation fault when .desc is not null-terminated ** Changed in: qemu Status: In Progress => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1727250 Title: qemu-io-test 147 segfaults when configured with gcov Status in QEMU: Fix Committed Bug description: Head is at 3d7196d43bfe12efe98568cb60057e273652b99b Steps to re-produce: 1. git clone ./configure --enable-gcov --target-list=ppc64-softmmu make cd tests/qemu-iotests 2. export qemu binary, in my environment export QEMU_PROG=/home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 3. Run test 147 with format qcow2 ./check -qcow2 147 QEMU -- "/home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64" -nodefaults -machine accel=qtest QEMU_IMG -- "/home/nasastry/qemu/qemu-img" QEMU_IO -- "/home/nasastry/qemu/qemu-io" --cache writeback -f qcow2 QEMU_NBD -- "/home/nasastry/qemu/qemu-nbd" IMGFMT-- qcow2 (compat=1.1) IMGPROTO -- file PLATFORM -- Linux/ppc64le zzfp365-lp1 4.13.0-4.rel.git49564cb.el7.centos.ppc64le TEST_DIR -- /home/nasastry/qemu/tests/qemu-iotests/scratch SOCKET_SCM_HELPER -- /home/nasastry/qemu/tests/qemu-iotests/socket_scm_helper 147 0s ... [failed, exit status 1] - output mismatch (see 147.out.bad) --- /home/nasastry/qemu/tests/qemu-iotests/147.out2017-10-25 14:04:54.978600753 +0530 +++ /home/nasastry/qemu/tests/qemu-iotests/147.out.bad2017-10-25 14:09:53.769783770 +0530 @@ -1,5 +1,95 @@ -.. +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +FF +== +FAIL: test_fd (__main__.BuiltinNBD) +-- +Traceback (most recent call last): + File "147", line 203, in test_fd +self.client_test(filename, flatten_sock_addr(address), 'nbd-export') + File "147", line 55, in client_test +self.assert_qmp(result, 'return', {}) + File "/home/nasastry/qemu/tests/qemu-iotests/iotests.py", line 315, in assert_qmp +result = self.dictpath(d, path) + File "/home/nasastry/qemu/tests/qemu-iotests/iotests.py", line 274, in dictpath +self.fail('failed path traversal for "%s" in "%s"' % (path, str(d))) +AssertionE
Re: [Qemu-devel] [Qemu-ppc] [QEMU-PPC] [RFC 3/3] target/ppc: Add H-Call H_GET_CPU_CHARACTERISTICS
On 01/09/2018 07:21 AM, Suraj Jitindar Singh wrote: > The new H-Call H_GET_CPU_CHARACTERISTICS is used by the guest to query > behaviours and available characteristics of the cpu. > > Implement the handler for this new H-Call which formulates its response > based on the setting of the new capabilities added in the previous > patch. > > Note: Currently we return H_FUNCTION under TCG which will direct the > guest to fall back to doing a displacement flush > > Discussion: > Is TCG affected? > Is there any point in telling the guest to do these workarounds on TCG > given they're unlikely to translate to host instructions which have the > desired effect? Hi, Suraj. What if this is left to the cover letter? > --- > hw/ppc/spapr_hcall.c | 81 > ++ > include/hw/ppc/spapr.h | 1 + > 2 files changed, 82 insertions(+) > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index 51eba52e86..b62b47c8d9 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -1654,6 +1654,84 @@ static target_ulong > h_client_architecture_support(PowerPCCPU *cpu, > return H_SUCCESS; > } > > +#define CPU_CHARACTERISTIC_SPEC_BARRIER (1ULL << (63 - 0)) > +#define CPU_CHARACTERISTIC_BCCTR_SERIAL (1ULL << (63 - 1)) > +#define CPU_CHARACTERISTIC_ORI_L1_CACHE (1ULL << (63 - 2)) > +#define CPU_CHARACTERISTIC_MTTRIG_L1_CACHE (1ULL << (63 - 3)) > +#define CPU_CHARACTERISTIC_L1_CACHE_PRIV(1ULL << (63 - 4)) > +#define CPU_CHARACTERISTIC_BRANCH_HINTS (1ULL << (63 - 5)) > +#define CPU_CHARACTERISTIC_MTTRIG_THR_RECONF(1ULL << (63 - 6)) > +#define CPU_BEHAVIOUR_FAVOUR_SECURITY (1ULL << (63 - 0)) > +#define CPU_BEHAVIOUR_L1_CACHE_FLUSH(1ULL << (63 - 1)) > +#define CPU_BEHAVIOUR_SPEC_BARRIER (1ULL << (63 - 2)) Can PPC_BIT be used here? Cheers Murilo
Re: [Qemu-devel] [Qemu-ppc] [QEMU-PPC] [RFC 2/3] hw/spapr/spapr_caps: Add new caps safe_[cache/bounds_check/indirect_branch]
On 01/09/2018 07:21 AM, Suraj Jitindar Singh wrote: > This patch adds three new capabilities: > cap-cfpc -> safe_cache > cap-sbbc -> safe_bounds_check > cap-ibs -> safe_indirect_branch Hi, Suraj. What about splitting this into smaller patches, one per capability? > Each capability is tristate with the possible values "broken", > "workaround" or "fixed". Add generic getter and setter functions for > this new capability type. Add these new capabilities to the capabilities > list. The maximum value for the capabilities is queried from kvm through > new kvm capabilities. The requested values are considered to be > compatible if kvm can support an equal or higher value for each > capability. > > Discussion: > Currently these new capabilities default to broken to allow for > backwards compatibility, is this the best option? This could be placed in the cover letter, not in the commit message. Cheers Murilo
Re: [Qemu-devel] [Qemu-ppc] [QEMU-PPC] [RFC 1/3] hw/ppc/spapr_caps: Rework spapr_caps to use uint8 internal representation
On 01/09/2018 07:21 AM, Suraj Jitindar Singh wrote: > Currently spapr_caps are tied to boolean values (on or off). This patch > reworks the caps so that they can have any value between 0 and 127, > inclusive. This allows more capabilities with various values to be > represented in the same way internally. Capabilities are numbered in > ascending order. The internal representation of capability values is an > array of uint8s in the sPAPRMachineState, indexed by capability number. > Note: The MSB (0x80) of a capability is reserved to track whether the > capability was set from the command line. > > Capabilities can have their own name, description, options, getter and > setter functions, type and allow functions. They also each have their own > section in the migration stream. Capabilities are only migrated if they > were explictly set on the command line, with the assumption that > otherwise the default will match. > > On migration we ensure that the capability value on the destination > is greater than or equal to the capability value from the source. So > long at this remains the case then the migration is considered > compatible and allowed to continue. > > This patch implements generic getter and setter functions for boolean > capabilities. It also converts the existings cap-htm, cap-vsx and > cap-dfp capabilities to this new format. Hi, Suraj. I've got the impression that this patch does a lot of things. What about splitting this patch into the following? - rename spapr_has_cap() -> spapr_get_cap() - introduce each spapr_cap_[gs]et_bool() in separate patches - make use of spapr_cap[gs]et_bool() - convert capabilities internal representation to uint8 - add each new capability separately Perhaps it can be broken into even smaller changes. Cheers Murilo
[Qemu-devel] [Bug 1721220] Re: qemu crashes with assertion error `!mr->container' failed
As per previous comments, this bug was fixed by commit https://git.qemu.org/?p=qemu.git;a=commitdiff;h=d659d94013390238961fac741572306c95496bf5 (released in QEMU v2.11.0): commit d659d94013390238961fac741572306c95496bf5 Author: Aleksandr Bezzubikov Date: Mon Sep 25 02:21:58 2017 +0300 hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case ** Changed in: qemu Status: New => 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/1721220 Title: qemu crashes with assertion error `!mr->container' failed Status in QEMU: Fix Released Bug description: Re-production steps: git clone today's qemu git tree (4th Oct 2017) ./configure --target-list=ppc64-softmmu && make -j 8 Run the device-crash-test from scripts folder, seeing the following error INFO: running test case: machine=bamboo binary=ppc64-softmmu/qemu-system-ppc64 device=pcie-pci-bridge accel=kvm WARNING: qemu received signal -6: ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/var/tmp/qemu-30972-monitor.sock -mon chardev=mon,mode=control -display none -vga none -S -machine bamboo,accel=kvm -device pcie-pci-bridge CRITICAL: failed: machine=bamboo binary=ppc64-softmmu/qemu-system-ppc64 device=pcie-pci-bridge accel=kvm CRITICAL: cmdline: ppc64-softmmu/qemu-system-ppc64 -S -machine bamboo,accel=kvm -device pcie-pci-bridge CRITICAL: log: qemu-system-ppc64: /home/nasastry/qemu/memory.c:1699: memory_region_finalize: Assertion `!mr->container' failed. CRITICAL: log: warning: KVM does not support watchdog CRITICAL: exit code: -6 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1721220/+subscriptions
[Qemu-devel] [Bug 1713516] Re: qemu crashes with "GLib-ERROR **: gmem.c" error when a negative value passed to smp threads, cores
I've confirmed with Seeteena and this bug was fixed by commit https://git.qemu.org/?p=qemu.git;a=commit;h=c0dd10991903c552811d8cbe9231055b1b3a7ebd commit c0dd10991903c552811d8cbe9231055b1b3a7ebd Author: Seeteena Thoufeek Date: Mon Sep 4 13:13:51 2017 +0530 vl: exit if maxcpus is negative ** Changed in: qemu Status: New => Fix Released ** Changed in: qemu Assignee: (unassigned) => Murilo Opsfelder Araújo (mopsfelder) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1713516 Title: qemu crashes with "GLib-ERROR **: gmem.c" error when a negative value passed to smp threads, cores Status in QEMU: Fix Released Bug description: After fixing other bug, https://bugs.launchpad.net/qemu/+bug/1713408 with the proposed patch http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg05357.html When tried smp core and thread as negative numbers seeing the following similar error. There is a need to fix for the following. Instead of fixing it for every variable/argument. Is there a common place to fix all of these issues? cloned today's git (i.e. 28th Aug 2017) # ppc64-softmmu/qemu-system-ppc64 --nographic -vga none -machine pseries,accel=kvm,kvm-type=HV -m size=200g -device virtio-blk- pci,drive=rootdisk -drive file=/home/nasastry/avocado-fvt-wrapper/data /avocado- vt/images/pegas-1.0-ppc64le.qcow2,if=none,cache=none,id=rootdisk,format=qcow2 -monitor telnet:127.0.0.1:1234,server,nowait -net nic,model=virtio -net user -device nec-usb-xhci -smp 8,cores=-1,threads=-1,maxcpus=12 (process:27477): GLib-ERROR **: gmem.c:130: failed to allocate 18446744073709550568 bytes Trace/breakpoint trap [New Thread 0x3fffb63deb60 (LWP 27731)] [New Thread 0x3fffb5aceb60 (LWP 27734)] (process:27726): GLib-ERROR **: gmem.c:130: failed to allocate 18446744073709550568 bytes Program received signal SIGTRAP, Trace/breakpoint trap. 0x3fffb75e5408 in raise () from /lib64/libpthread.so.0 Missing separate debuginfos, use: debuginfo-install glib2-2.50.3-3.el7.ppc64le glibc-2.17-196.el7.ppc64le gnutls-3.3.26-9.el7.ppc64le krb5-libs-1.15.1-8.el7.ppc64le libgcc-4.8.5-16.el7.ppc64le libstdc++-4.8.5-16.el7.ppc64le ncurses-libs-5.9-13.20130511.el7.ppc64le nss-3.28.4-8.el7.ppc64le nss-softokn-freebl-3.28.3-6.el7.ppc64le nss-util-3.28.4-3.el7.ppc64le openldap-2.4.44-5.el7.ppc64le openssl-libs-1.0.2k-8.el7.ppc64le p11-kit-0.23.5-3.el7.ppc64le (gdb) bt #0 0x3fffb75e5408 in raise () from /lib64/libpthread.so.0 #1 0x3fffb796be9c in _g_log_abort () from /lib64/libglib-2.0.so.0 #2 0x3fffb796d4c4 in g_log_default_handler () from /lib64/libglib-2.0.so.0 #3 0x3fffb796d86c in g_logv () from /lib64/libglib-2.0.so.0 #4 0x3fffb796db00 in g_log () from /lib64/libglib-2.0.so.0 #5 0x3fffb796b694 in g_malloc0 () from /lib64/libglib-2.0.so.0 #6 0x1018fa84 in spapr_possible_cpu_arch_ids (machine=0x111651e0) at /home/nasastry/upstream/qemu/hw/ppc/spapr.c:3322 #7 0x1018b444 in spapr_init_cpus (spapr=0x111651e0) at /home/nasastry/upstream/qemu/hw/ppc/spapr.c:2096 #8 0x1018bc6c in ppc_spapr_init (machine=0x111651e0) at /home/nasastry/upstream/qemu/hw/ppc/spapr.c:2275 #9 0x1041ca80 in machine_run_board_init (machine=0x111651e0) at hw/core/machine.c:760 #10 0x10377284 in main (argc=22, argv=0x3128, envp=0x31e0) at vl.c:4638 (gdb) i r r0 0xfa 250 r1 0x3fffe470 70368744170608 r2 0x3fffb7608100 70367525765376 r3 0x00 r4 0x6c4e 27726 r5 0x55 r6 0x00 r7 0x3fffa820 70367267782688 r8 0x6c4e 27726 r9 0x00 r100x00 r110x00 r120x00 r130x3fffb64fccb0 70367507893424 r140x00 r150x00 r160x00 r170x00 r180x11 r190x00 r200x3fffb796d3f0 70367529325552 r210x00 r220x2000 536870912 r230x11 r240x3fffb7a61498 70367530325144 r250x3fffb7a614e8 70367530325224 r260x3fffb7a61488 70367530325128 r270x3fffa80008c0 70367267784896 r280x3fffb79cd2a8 70367529718440 r290x3fffb79cd2a8 70367529718440 r300x 18446744073709551615 r310x11 pc 0x3fffb75e5408 0x3fffb75e5408 msr0x9000d033 10376293541461676083 cr 0x42244842 1109674050 lr 0x3fffb796be9c 0x3fffb796be9c <_g_log_abort+60> ctr0x00 xer0x00 orig_r30x6c4e 27726
[Qemu-devel] [Bug 1727250] Re: qemu-io-test 147 segfaults when configured with gcov
I confirmed that my patch http://lists.nongnu.org/archive/html/qemu- devel/2018-01/msg00883.html fixes this bug too. ** Changed in: qemu Status: New => In Progress ** Changed in: qemu Assignee: (unassigned) => Murilo Opsfelder Araújo (mopsfelder) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1727250 Title: qemu-io-test 147 segfaults when configured with gcov Status in QEMU: In Progress Bug description: Head is at 3d7196d43bfe12efe98568cb60057e273652b99b Steps to re-produce: 1. git clone ./configure --enable-gcov --target-list=ppc64-softmmu make cd tests/qemu-iotests 2. export qemu binary, in my environment export QEMU_PROG=/home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 3. Run test 147 with format qcow2 ./check -qcow2 147 QEMU -- "/home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64" -nodefaults -machine accel=qtest QEMU_IMG -- "/home/nasastry/qemu/qemu-img" QEMU_IO -- "/home/nasastry/qemu/qemu-io" --cache writeback -f qcow2 QEMU_NBD -- "/home/nasastry/qemu/qemu-nbd" IMGFMT-- qcow2 (compat=1.1) IMGPROTO -- file PLATFORM -- Linux/ppc64le zzfp365-lp1 4.13.0-4.rel.git49564cb.el7.centos.ppc64le TEST_DIR -- /home/nasastry/qemu/tests/qemu-iotests/scratch SOCKET_SCM_HELPER -- /home/nasastry/qemu/tests/qemu-iotests/socket_scm_helper 147 0s ... [failed, exit status 1] - output mismatch (see 147.out.bad) --- /home/nasastry/qemu/tests/qemu-iotests/147.out2017-10-25 14:04:54.978600753 +0530 +++ /home/nasastry/qemu/tests/qemu-iotests/147.out.bad2017-10-25 14:09:53.769783770 +0530 @@ -1,5 +1,95 @@ -.. +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +WARNING:qemu:qemu received signal -11: /home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 -chardev socket,id=mon,path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/home/nasastry/qemu/tests/qemu-iotests/scratch/qemu-28636-qtest.sock -machine accel=qtest -nodefaults -machine accel=qtest +FF +== +FAIL: test_fd (__main__.BuiltinNBD) +-- +Traceback (most recent call last): + File "147", line 203, in test_fd +self.client_test(filename, flatten_sock_addr(address), 'nbd-export') + File "147", line 55, in client_test +self.assert_qmp(result, 'return', {}) + File "/home/nasastry/qemu/tests/qemu-iotests/iotests.py", line 315, in assert_qmp +result = self.dictpath(d, path) + File "/home/nasastry/qemu/tests/qemu-iotests/iotests.py", line 274, in dictpath +self.fail('failed pa
Re: [Qemu-devel] [PATCH 1/1] block/nbd: fix segmentation fault when .desc is not null-terminated
On 01/05/2018 11:57 AM, Eric Blake wrote: > On 01/05/2018 07:32 AM, Murilo Opsfelder Araujo wrote: >> The find_desc_by_name() from util/qemu-option.c relies on the .name not being >> NULL to call strcmp(). This check becomes unsafe when the list is not >> NULL-terminated, which is the case of nbd_runtime_opts in block/nbd.c, and >> can >> result in segmentation fault when strcmp() tries to access an invalid memory: > > Thanks for the report and patch. Adding qemu-stable in cc. > >> >> This patch fixes the segmentation fault in strcmp() by adding a NULL element >> at >> the end of nbd_runtime_opts.desc list, which is the common practice to most >> of >> other structs like runtime_opts in block/null.c. Thus, the desc[i].name != >> NULL >> check becomes safe because it will not evaluate to true when .desc list >> reached >> its end. >> >> Reported-by: R. Nageswara Sastry >> Buglink: https://bugs.launchpad.net/qemu/+bug/1727259 >> Signed-off-by: Murilo Opsfelder Araujo > > I'll update the commit message to add in the commit id that introduced > the problem, as well as check that other QemuOptsList do not have a > similar problem; I'm queueing this on the NBD tree and will submit a > pull request soon. > > Reviewed-by: Eric Blake Hi, Eric. A quick look brought my attention to: block/ssh.c 530:static QemuOptsList ssh_runtime_opts = { I've sent a patch to fix it too. Thanks. -- Murilo
[Qemu-devel] [Bug 1727259] Re: qemu-io-test 58 segfaults when configured with gcov
Patch sent: http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00883.html ** Changed in: qemu Status: New => In Progress -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1727259 Title: qemu-io-test 58 segfaults when configured with gcov Status in QEMU: In Progress Bug description: Head is at 3d7196d43bfe12efe98568cb60057e273652b99b Steps to re-produce: 1. git clone ./configure --enable-gcov --target-list=ppc64-softmmu make cd tests/qemu-iotests 2. export qemu binary, in my environment export QEMU_PROG=/home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 3. Run test 58 with format qcow2 ./check -qcow2 58 QEMU -- "/home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64" -nodefaults -machine accel=qtest QEMU_IMG -- "/home/nasastry/qemu_gcov/qemu-img" QEMU_IO -- "/home/nasastry/qemu_gcov/qemu-io" --cache writeback -f qcow2 QEMU_NBD -- "/home/nasastry/qemu_gcov/qemu-nbd" IMGFMT-- qcow2 (compat=1.1) IMGPROTO -- file PLATFORM -- Linux/ppc64le zzfp365-lp1 4.13.0-4.rel.git49564cb.el7.centos.ppc64le TEST_DIR -- /home/nasastry/qemu_gcov/tests/qemu-iotests/scratch SOCKET_SCM_HELPER -- /home/nasastry/qemu_gcov/tests/qemu-iotests/socket_scm_helper 058 1s ... - output mismatch (see 058.out.bad) --- /home/nasastry/qemu_gcov/tests/qemu-iotests/058.out 2017-10-09 14:09:04.262726912 +0530 +++ /home/nasastry/qemu_gcov/tests/qemu-iotests/058.out.bad 2017-10-25 15:00:52.037515025 +0530 @@ -19,16 +19,28 @@ 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) == verifying the exported snapshot with patterns, method 1 == -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 8192 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +./common.rc: line 66: 36255 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) +./common.rc: line 66: 36262 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) == verifying the exported snapshot with patterns, method 2 == -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 8192 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +./common.rc: line 66: 36274 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) +./common.rc: line 66: 36282 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) == verifying the converted snapshot with patterns, method 1 == read 4096/4096 bytes at offset 4096 Failures: 058 Failed 1 of 1 tests with out gcov configured this test case is pass. # ./check -qcow2 58 QEMU -- "/home/nasastry/qemu/ppc64-softmmu/qemu-system-ppc64" -nodefaults -machine accel=qtest QEMU_IMG -- "/home/nasastry/qemu/qemu-img" QEMU_IO -- "/home/nasastry/qemu/qemu-io" --cache writeback -f qcow2 QEMU_NBD -- "/home/nasastry/qemu/qemu-nbd" IMGFMT-- qcow2 (compat=1.1) IMGPROTO -- file PLATFORM -- Linux/ppc64le zzfp365-lp1 4.13.0-4.rel.git49564cb.el7.centos.ppc64le TEST_DIR -- /home/nasastry/qemu/tests/qemu-iotests/scratch SOCKET_SCM_HELPER -- /home/nasastry/qemu/tests/qemu-iotests/socket_scm_helper 058 0s ... Passed all 1 tests To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1727259/+subscriptions
[Qemu-devel] [Bug 1727259] Re: qemu-io-test 58 segfaults when configured with gcov
I'll work on this. ** Changed in: qemu Assignee: (unassigned) => Murilo Opsfelder Araújo (mopsfelder) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1727259 Title: qemu-io-test 58 segfaults when configured with gcov Status in QEMU: New Bug description: Head is at 3d7196d43bfe12efe98568cb60057e273652b99b Steps to re-produce: 1. git clone ./configure --enable-gcov --target-list=ppc64-softmmu make cd tests/qemu-iotests 2. export qemu binary, in my environment export QEMU_PROG=/home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64 3. Run test 58 with format qcow2 ./check -qcow2 58 QEMU -- "/home/nasastry/qemu_gcov/ppc64-softmmu/qemu-system-ppc64" -nodefaults -machine accel=qtest QEMU_IMG -- "/home/nasastry/qemu_gcov/qemu-img" QEMU_IO -- "/home/nasastry/qemu_gcov/qemu-io" --cache writeback -f qcow2 QEMU_NBD -- "/home/nasastry/qemu_gcov/qemu-nbd" IMGFMT-- qcow2 (compat=1.1) IMGPROTO -- file PLATFORM -- Linux/ppc64le zzfp365-lp1 4.13.0-4.rel.git49564cb.el7.centos.ppc64le TEST_DIR -- /home/nasastry/qemu_gcov/tests/qemu-iotests/scratch SOCKET_SCM_HELPER -- /home/nasastry/qemu_gcov/tests/qemu-iotests/socket_scm_helper 058 1s ... - output mismatch (see 058.out.bad) --- /home/nasastry/qemu_gcov/tests/qemu-iotests/058.out 2017-10-09 14:09:04.262726912 +0530 +++ /home/nasastry/qemu_gcov/tests/qemu-iotests/058.out.bad 2017-10-25 15:00:52.037515025 +0530 @@ -19,16 +19,28 @@ 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) == verifying the exported snapshot with patterns, method 1 == -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 8192 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +./common.rc: line 66: 36255 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) +./common.rc: line 66: 36262 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) == verifying the exported snapshot with patterns, method 2 == -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 8192 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +./common.rc: line 66: 36274 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) +./common.rc: line 66: 36282 Segmentation fault (core dumped) ( if [ "${VALGRIND_QEMU}" == "y" ]; then +exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +else +exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"; +fi ) == verifying the converted snapshot with patterns, method 1 == read 4096/4096 bytes at offset 4096 Failures: 058 Failed 1 of 1 tests with out gcov configured this test case is pass. # ./check -qcow2 58 QEMU -- "/home/nasastry/qemu/ppc64-softmmu/qemu-system-ppc64" -nodefaults -machine accel=qtest QEMU_IMG -- "/home/nasastry/qemu/qemu-img" QEMU_IO -- "/home/nasastry/qemu/qemu-io" --cache writeback -f qcow2 QEMU_NBD -- "/home/nasastry/qemu/qemu-nbd" IMGFMT-- qcow2 (compat=1.1) IMGPROTO -- file PLATFORM -- Linux/ppc64le zzfp365-lp1 4.13.0-4.rel.git49564cb.el7.centos.ppc64le TEST_DIR -- /home/nasastry/qemu/tests/qemu-iotests/scratch SOCKET_SCM_HELPER -- /home/nasastry/qemu/tests/qemu-iotests/socket_scm_helper 058 0s ... Passed all 1 tests To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1727259/+subscriptions
[Qemu-devel] [Bug 1729623] Re: test-aio-multithread fails with 'Co-routine re-entered recursively'
I confirmed with Stefan and this bug was fixed by https://git.qemu.org/?p=qemu.git;a=commitdiff;h=fb0c43f34eed8b18678c6e1f481d8564b35c99ed commit fb0c43f34eed8b18678c6e1f481d8564b35c99ed Author: Stefan Hajnoczi Date: Mon Nov 6 19:02:33 2017 + tests-aio-multithread: fix /aio/multi/schedule race condition ** Changed in: qemu Status: New => 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/1729623 Title: test-aio-multithread fails with 'Co-routine re-entered recursively' Status in QEMU: Fix Released Bug description: git head is at fa73e146250181852c0915aa65df8d54d35485fa configure with the following ./configure --enable-attr --enable-bsd-user --enable-cap-ng\ --enable-coroutine-pool --enable-crypto-afalg --enable-curl\ --enable-curses --enable-debug --enable-debug-info\ --enable-debug-tcg --enable-fdt --enable-gcrypt \ --enable-gnutls --enable-gprof --enable-gtk \ --enable-guest-agent --enable-kvm --enable-libiscsi \ --enable-libssh2 --enable-linux-aio --enable-linux-user \ --enable-live-block-migration --enable-modules \ --enable-numa --enable-pie --enable-profiler \ --enable-qom-cast-debug --enable-rbd --enable-replication \ --enable-seccomp --enable-smartcard --enable-stack-protector \ --enable-system --enable-tcg --enable-tcg-interpreter \ --enable-tools --enable-tpm --enable-trace-backend=ftrace \ --enable-user --enable-vhost-net --enable-vhost-scsi \ --enable-vhost-user --enable-vhost-vsock --enable-virtfs \ --enable-vnc --enable-tpm --enable-vnc-png \ --enable-vnc-sasl --enable-werror --enable-xfsctl \ --enable-gcov --enable-debug-stack-usage make -j 32 make test-aio-multithread V=1 ... File '/home/nasastry/qemu/include/qapi/qmp/qobject.h' No executable lines MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} gtester -k --verbose -m=quick tests/test-aio-multithread TEST: tests/test-aio-multithread... (pid=86877) /aio/multi/lifecycle:OK /aio/multi/schedule: Co-routine re-entered recursively FAIL GTester: last random seed: R02S681209ce87fc22715b41223212d9f6f0 (pid=86891) /aio/multi/mutex/contended: OK /aio/multi/mutex/handoff:OK /aio/multi/mutex/mcs:OK /aio/multi/mutex/pthread:OK FAIL: tests/test-aio-multithread make: *** [check-tests/test-aio-multithread] Error 1 Full log will be attached. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1729623/+subscriptions
Re: [Qemu-devel] [PATCH v9 2/8] qemu.py: better control of created files
On 11/13/2017 07:39 PM, Amador Pahim wrote: > To launch a VM, we need to create basically two files: the monitor > socket (if it's a UNIX socket) and the qemu log file. > > For the qemu log file, we currently just open the path, which will > create the file if it does not exist or overwrite the file if it does > exist. > > For the monitor socket, if it already exists, we are currently removing > it, even if it's not created by us. > > This patch moves to pre_launch() the responsibility to create a > temporary directory to host the files so we can remove the whole > directory on post_shutdown(). s/pre_launch()/_pre_launch()/ s/post_shutdown()/_post_shutdown()/ > Signed-off-by: Amador Pahim > --- > scripts/qemu.py | 42 -- > 1 file changed, 24 insertions(+), 18 deletions(-) > > diff --git a/scripts/qemu.py b/scripts/qemu.py > index 65d9ad688c..58f00bdd64 100644 > --- a/scripts/qemu.py > +++ b/scripts/qemu.py > @@ -17,6 +17,8 @@ import logging > import os > import subprocess > import qmp.qmp > +import shutil > +import tempfile > > > LOG = logging.getLogger(__name__) > @@ -72,10 +74,10 @@ class QEMUMachine(object): > wrapper = [] > if name is None: > name = "qemu-%d" % os.getpid() > -if monitor_address is None: > -monitor_address = os.path.join(test_dir, name + "-monitor.sock") > +self._name = name > self._monitor_address = monitor_address > -self._qemu_log_path = os.path.join(test_dir, name + ".log") > +self._qemu_log_path = None > +self._qemu_log_fd = None Is our intent to hold an integer file descriptor in self._qemu_log_fd? Python's built-in open() returns a file object, which is what we're using here. Do you think it shall be named, for example, _qemu_log_file to avoid confusion with integer file descriptors, usually denoted by "fd"? > self._popen = None > self._binary = binary > self._args = list(args) # Force copy args in case we modify them > @@ -85,6 +87,8 @@ class QEMUMachine(object): > self._socket_scm_helper = socket_scm_helper > self._qmp = None > self._qemu_full_args = None > +self._test_dir = test_dir > +self._temp_dir = None > > # just in case logging wasn't configured by the main script: > logging.basicConfig() > @@ -134,16 +138,6 @@ class QEMUMachine(object): > > return proc.returncode > > -@staticmethod > -def _remove_if_exists(path): > -'''Remove file object at path if it exists''' > -try: > -os.remove(path) > -except OSError as exception: > -if exception.errno == errno.ENOENT: > -return > -raise > - > def is_running(self): > return self._popen is not None and self._popen.returncode is None > > @@ -173,6 +167,13 @@ class QEMUMachine(object): > '-display', 'none', '-vga', 'none'] > > def _pre_launch(self): > +self._temp_dir = tempfile.mkdtemp(dir=self._test_dir) > +if not isinstance(self._monitor_address, tuple): > +self._monitor_address = os.path.join(self._temp_dir, > + self._name + > "-monitor.sock") > +self._qemu_log_path = os.path.join(self._temp_dir, self._name + > ".log") > +self._qemu_log_fd = open(self._qemu_log_path, 'wb') > + > self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, > server=True) > > @@ -180,23 +181,28 @@ class QEMUMachine(object): > self._qmp.accept() > > def _post_shutdown(self): > -if not isinstance(self._monitor_address, tuple): > -self._remove_if_exists(self._monitor_address) > -self._remove_if_exists(self._qemu_log_path) > +if self._qemu_log_fd is not None: > +self._qemu_log_fd.close() > +self._qemu_log_fd = None > + > +self._qemu_log_path = None > + > +if self._temp_dir is not None: > +shutil.rmtree(self._temp_dir) > +self._temp_dir = None > > def launch(self): > '''Launch the VM and establish a QMP connection''' > self._iolog = None > self._qemu_full_args = None > devnull = open(os.path.devnull, 'rb') > -qemulog = open(self._qemu_log_path, 'wb') > try: > self._pre_launch() > self._qemu_full_args = (self._wrapper + [self._binary] + > self._base_args() + self._args) > self._popen = subprocess.Popen(self._qemu_full_args, > stdin=devnull, > - stdout=qemulog, > + stdout=self._qemu_log_fd, > stderr=subprocess.STDOUT,