Re: [PATCH v3 1/1] os-posix: asynchronous teardown for shutdown on Linux

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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 --disable-guest-agent-msi --disable-hax --disab

Re: [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set

[Murilo Opsfelder Araújo]

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-l2tpv3 --disable-libdaxctl --disable-libiscsi -

Re: [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set

[Murilo Opsfelder Araújo]

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 

Re: [PATCH v2] block-qdict: Fix -Werror=maybe-uninitialized build failure

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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 

[Bug 1866962] Re: [Regression]Powerpc kvm guest unable to start with hugepage backed memory

[Murilo Opsfelder Araújo]

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 

Re: [PATCH v4 10/15] util/mmap-alloc: Prepare for resizeable mmaps

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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, >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, )) {
>  error_report("VFIO_UNMAP_DMA failed: %s", strerror(errno));
>  }
> -memmove(mapping, >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(>lock);
>  }
> @@ -698,7 +706,10 @@ void qemu_vfio_close(QEMUVFIOState *s)
>  return;
>  }
>  while (s->nr_mappings) {
> -qemu_vfio_undo_mapping(s, >mappings[s->nr_mappings - 1]);
> +IOVAMapping *m = >mappings[s->nr_mappings - 1];
> +
> +qemu_vfio_undo_mapping(s, m->size, m->iova);
> +qemu_vfio_remove_mapping(s, m);
>  }
>  ram_block_notifier_remove(>ram_notifier);
>  qemu_vfio_reset(s);


--
Murilo

Re: [PATCH v4 11/15] util/mmap-alloc: Implement resizeable mmaps

[Murilo Opsfelder Araújo]

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, 

Re: [PATCH v4 07/15] util/mmap-alloc: Factor out calculation of the pagesize for the guard page

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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, 

Re: [PATCH v4 13/15] util: oslib: Resizeable anonymous allocations under POSIX

[Murilo Opsfelder Araújo]

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 

Re: [PATCH v4 12/15] util: vfio-helpers: Implement ram_block_resized()

[Murilo Opsfelder Araújo]

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(>lock);
>  s->ram_notifier.ram_block_added = qemu_vfio_ram_block_added;
>  

Re: [PATCH v4 02/15] util: vfio-helpers: Remove Error parameter from qemu_vfio_undo_mapping()

[Murilo Opsfelder Araújo]

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, )) {
> -error_setg_errno(errp, errno, "VFIO_UNMAP_DMA failed");
> +error_report("VFIO_UNMAP_DMA failed: %s", strerror(errno));
>  }
>  memmove(mapping, >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(>lock);
>  }
> @@ -699,7 +698,7 @@ void qemu_vfio_close(QEMUVFIOState *s)
>  return;
>  }
>  while (s->nr_mappings) {
> -qemu_vfio_undo_mapping(s, >mappings[s->nr_mappings - 1], NULL);
> +qemu_vfio_undo_mapping(s, >mappings[s->nr_mappings - 1]);
>  }
>  ram_block_notifier_remove(>ram_notifier);
>  qemu_vfio_reset(s);


--
Murilo

Re: [PATCH v2 fixed 08/16] util/mmap-alloc: Factor out calculation of pagesize to mmap_pagesize()

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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)
>  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)>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)
>  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)>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)_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)_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)_cpu->dtl_addr;
>  ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );

[Qemu-devel] [Bug 1776096] Re: qemu 2.12.0 qemu-system-ppc illegal instruction on ppc64le, crashes emulator

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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)

[Murilo Opsfelder Araújo]

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)

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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 

[Qemu-devel] [Bug 1727259] Re: qemu-io-test 58 segfaults when configured with gcov

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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"' % 

Re: [Qemu-devel] [Qemu-ppc] [QEMU-PPC] [RFC 3/3] target/ppc: Add H-Call H_GET_CPU_CHARACTERISTICS

[Murilo Opsfelder Araújo]

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]

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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 <s1see...@linux.vnet.ibm.com>
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 <raise+56>
  msr0x9000d033 10376293541461676083
  cr 0x42244842 1109674050
  lr 0x3fffb796be9c 0x3fffb796be9c <_g_log_abort+60>
  ctr0x00
  xer  

[Qemu-devel] [Bug 1727250] Re: qemu-io-test 147 segfaults when configured with gcov

[Murilo Opsfelder Araújo]

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 path traversal for "%s

Re: [Qemu-devel] [PATCH 1/1] block/nbd: fix segmentation fault when .desc is not null-terminated

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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'

[Murilo Opsfelder Araújo]

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

[Murilo Opsfelder Araújo]

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