[PATCH v1 3/3] hw/intc: ibex_plic: Honour source priorities

[Alistair Francis]

This patch follows what commit aa4d30f6618dc "riscv: plic: Honour source
priorities" does and ensures that the highest priority interrupt will be
serviced first.

Signed-off-by: Alistair Francis 
Cc: Jessica Clarke 
---
 hw/intc/ibex_plic.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/intc/ibex_plic.c b/hw/intc/ibex_plic.c
index 669247ef08..f49fa67c91 100644
--- a/hw/intc/ibex_plic.c
+++ b/hw/intc/ibex_plic.c
@@ -57,6 +57,8 @@ static void ibex_plic_irqs_set_pending(IbexPlicState *s, int 
irq, bool level)
 static bool ibex_plic_irqs_pending(IbexPlicState *s, uint32_t context)
 {
 int i;
+uint32_t max_irq = 0;
+uint32_t max_prio = s->threshold;
 
 for (i = 0; i < s->pending_num; i++) {
 uint32_t irq_num = ctz64(s->pending[i]) + (i * 32);
@@ -66,14 +68,17 @@ static bool ibex_plic_irqs_pending(IbexPlicState *s, 
uint32_t context)
 continue;
 }
 
-if (s->priority[irq_num] > s->threshold) {
-if (!s->claim) {
-s->claim = irq_num;
-}
-return true;
+if (s->priority[irq_num] > max_prio) {
+max_irq = irq_num;
+max_prio = s->priority[irq_num];
 }
 }
 
+if (max_irq) {
+s->claim = max_irq;
+return true;
+}
+
 return false;
 }
 
-- 
2.27.0

[PATCH v1 1/3] hw/intc: ibex_plic: Update the pending irqs

[Alistair Francis]

After a claim or a priority change we need to update the pending
interrupts. This is based on the same patch for the SiFive PLIC:
55765822804f5a58594e "riscv: plic: Add a couple of mising
sifive_plic_update calls"

Signed-off-by: Alistair Francis 
Cc: Jessica Clarke 
---
 hw/intc/ibex_plic.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/intc/ibex_plic.c b/hw/intc/ibex_plic.c
index 41079518c6..578edd2ce0 100644
--- a/hw/intc/ibex_plic.c
+++ b/hw/intc/ibex_plic.c
@@ -121,6 +121,9 @@ static uint64_t ibex_plic_read(void *opaque, hwaddr addr,
 s->pending[pending_num] &= ~(1 << (s->claim % 32));
 
 ret = s->claim;
+
+/* Update the interrupt status after the claim */
+ibex_plic_update(s);
 }
 
 return ret;
@@ -140,6 +143,7 @@ static void ibex_plic_write(void *opaque, hwaddr addr,
 } else if (addr_between(addr, s->priority_base, s->priority_num)) {
 uint32_t irq = ((addr - s->priority_base) >> 2) + 1;
 s->priority[irq] = value & 7;
+ibex_plic_update(s);
 } else if (addr_between(addr, s->enable_base, s->enable_num)) {
 uint32_t enable_reg = (addr - s->enable_base) / 4;
 
-- 
2.27.0

[PATCH v1 0/3] hw/intc: A few fixes for the Ibex PLIC

[Alistair Francis]

Recently some SiFive PLIC fixes have been merged into QEMU, this applies
those fixes to the Ibex PLIC as well as an update on how claiming
interrupts works.

Alistair Francis (3):
  hw/intc: ibex_plic: Update the pending irqs
  hw/intc: ibex_plic: Don't allow repeat interrupts on claimed lines
  hw/intc: ibex_plic: Honour source priorities

 include/hw/intc/ibex_plic.h |  1 +
 hw/intc/ibex_plic.c | 36 +++-
 2 files changed, 32 insertions(+), 5 deletions(-)

-- 
2.27.0

[PATCH v1 2/3] hw/intc: ibex_plic: Don't allow repeat interrupts on claimed lines

[Alistair Francis]

Once an interrupt has been claimed, but before it has been compelted we
shouldn't receive any more pending interrupts. This patche keeps track
of this to ensure that we don't see any more interrupts until it is
completed.

Signed-off-by: Alistair Francis 
---
 include/hw/intc/ibex_plic.h |  1 +
 hw/intc/ibex_plic.c | 17 +
 2 files changed, 18 insertions(+)

diff --git a/include/hw/intc/ibex_plic.h b/include/hw/intc/ibex_plic.h
index ddc7909903..d8eb09b258 100644
--- a/include/hw/intc/ibex_plic.h
+++ b/include/hw/intc/ibex_plic.h
@@ -33,6 +33,7 @@ typedef struct IbexPlicState {
 MemoryRegion mmio;
 
 uint32_t *pending;
+uint32_t *claimed;
 uint32_t *source;
 uint32_t *priority;
 uint32_t *enable;
diff --git a/hw/intc/ibex_plic.c b/hw/intc/ibex_plic.c
index 578edd2ce0..669247ef08 100644
--- a/hw/intc/ibex_plic.c
+++ b/hw/intc/ibex_plic.c
@@ -43,6 +43,14 @@ static void ibex_plic_irqs_set_pending(IbexPlicState *s, int 
irq, bool level)
 {
 int pending_num = irq / 32;
 
+if (s->claimed[pending_num] & 1 << (irq % 32)) {
+/*
+ * The interrupt has been claimed, but not compelted.
+ * The pending bit can't be set.
+ */
+return;
+}
+
 s->pending[pending_num] |= level << (irq % 32);
 }
 
@@ -120,6 +128,10 @@ static uint64_t ibex_plic_read(void *opaque, hwaddr addr,
 int pending_num = s->claim / 32;
 s->pending[pending_num] &= ~(1 << (s->claim % 32));
 
+/* Set the interrupt as claimed, but not compelted */
+s->claimed[pending_num] |= 1 << (s->claim % 32);
+
+/* Return the current claimed interrupt */
 ret = s->claim;
 
 /* Update the interrupt status after the claim */
@@ -155,6 +167,10 @@ static void ibex_plic_write(void *opaque, hwaddr addr,
 /* Interrupt was completed */
 s->claim = 0;
 }
+if (s->claimed[value / 32] & 1 << (value % 32)) {
+/* This value was already claimed, clear it. */
+s->claimed[value / 32] &= ~(1 << (value % 32));
+}
 }
 
 ibex_plic_update(s);
@@ -215,6 +231,7 @@ static void ibex_plic_realize(DeviceState *dev, Error 
**errp)
 int i;
 
 s->pending = g_new0(uint32_t, s->pending_num);
+s->claimed = g_new0(uint32_t, s->pending_num);
 s->source = g_new0(uint32_t, s->source_num);
 s->priority = g_new0(uint32_t, s->priority_num);
 s->enable = g_new0(uint32_t, s->enable_num);
-- 
2.27.0

[Bug 1888918] Re: qemu-system-ppc: Floating point instructions do not properly generate the SPE/Embedded Floating-Point Unavailable interrupt

[Matthieu Bucchianeri]

** Summary changed:

- qemu-ppc: Floating point instructions do not properly generate the 
SPE/Embedded Floating-Point Unavailable interrupt
+ qemu-system-ppc: Floating point instructions do not properly generate the 
SPE/Embedded Floating-Point Unavailable interrupt

** Description changed:

  When emulating certain floating point instructions or vector
  instructions on PowerPC machines, QEMU does not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt.
  
  As described in the Signal Processing Engine (SPE) Programming
  Environments Manual, Rev. 0, available at https://www.nxp.com/docs/en
  /reference-manual/SPEPEM.pdf:
  
  > An SPE/embedded floating-point unavailable exception occurs on an attempt 
to execute any of the
  > following instructions and MSR[SPV] is not set:
  > * SPE instruction (except brinc)
  > * An embedded scalar double-precision instruction
  > * A vector single-precision floating-point instructions
  > It is not used by embedded scalar single-precision floating-point 
instructions
  
  This behavior was partially reported in Bug #1611394, however the issue
  is larger than what is described in that bug. As mentioned in that bug,
  some single-precision instructions generate the exception (while they
  should not), which is incorrect but does not typically produce an
  incorrect output. What is more of an issue is that several double-
  precision and vector instructions do not generate the exception (while
- they should), and this break support for lazy FPU/vector context
+ they should), and this breaks support for lazy FPU/vector context
  switching in Linux (for example).
  
  The upper 32-bit of the double-precision/vector registers (which are in
  fact hidden in the general purpose registers) is not properly
  saved/restored, and this causes arithmetic errors. This was observed
  very frequently on a commercial project that does a lot of double-
  precision computations. The application works perfectly fine on an
  MPC8548 CPU, but fails often with QEMU.
+ 
+ This is only an issue with full platform emulation - the SPE/Embedded
+ Floating-Point Unavailable interrupt is not relevant for application
+ emulation.
  
  The issue can be reproduced using the attached Linux program "spe-
  bug.c". This program properly prints the number 42 (as the result of
  some very simple double-precision computation) on real PowerPC hardware,
  but prints an incorrect result (typically 0) on QEMU.
  
  This issue was first discovered in an older version of QEMU, but is also
  reproduced in the latest:
  
  # git rev-parse HEAD
  7adfbea8fd1efce36019a0c2f198ca73be9d3f18
  # ppc-softmmu/qemu-system-ppc --version
  QEMU emulator version 5.0.91 (v5.1.0-rc1-28-g7adfbea8fd-dirty)
  Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers
  
  Upon further analysis a total of 39 instructions are misbehaving:
  
  efsabs: raised: 1, expected: 0
  efsnabs: raised: 1, expected: 0
  efsneg: raised: 1, expected: 0
  efdcfs: raised: 0, expected: 1
  efdcfsf: raised: 0, expected: 1
  efdcfsi: raised: 0, expected: 1
  efdcfuf: raised: 0, expected: 1
  efdcfui: raised: 0, expected: 1
  efdctsf: raised: 0, expected: 1
  efdctsi: raised: 0, expected: 1
  efdctsiz: raised: 0, expected: 1
  efdctuf: raised: 0, expected: 1
  efdctui: raised: 0, expected: 1
  efdctuiz: raised: 0, expected: 1
  efscfd: raised: 0, expected: 1
  evfscfsf: raised: 0, expected: 1
  evfscfsi: raised: 0, expected: 1
  evfscfuf: raised: 0, expected: 1
  evfscfui: raised: 0, expected: 1
  evfsctsf: raised: 0, expected: 1
  evfsctsi: raised: 0, expected: 1
  evfsctsiz: raised: 0, expected: 1
  evfsctuf: raised: 0, expected: 1
  evfsctui: raised: 0, expected: 1
  evfsctuiz: raised: 0, expected: 1
  brinc: raised: 0, expected: 1
  efsadd: raised: 1, expected: 0
  efsdiv: raised: 1, expected: 0
  efsmul: raised: 1, expected: 0
  efssub: raised: 1, expected: 0
  evsplatfi: raised: 0, expected: 1
  evsplati: raised: 0, expected: 1
  efscmpeq: raised: 1, expected: 0
  efscmpgt: raised: 1, expected: 0
  efscmplt: raised: 1, expected: 0
  efststeq: raised: 1, expected: 0
  efststgt: raised: 1, expected: 0
  efststlt: raised: 1, expected: 0
  evsel: raised: 0, expected: 1
  
  When "raised" is 0 and "expected" is 1, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was not generated while it should have.
  When "raised" is 1 and "expected" is 0, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was generated while it should not have 
(Bug #1611394).
  
  A comprehensive program testing all the instructions listed in the
  Signal Processing Engine (SPE) Programming Environments Manual, Rev. 0
  is posted in the comments of this ticket, and can be used to reproduce
  the issue, and validate the future fix.

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

Title:
  

[Bug 1888918] Re: qemu-ppc: Floating point instructions do not properly generate the SPE/Embedded Floating-Point Unavailable interrupt

[Matthieu Bucchianeri]

** Changed in: qemu
 Assignee: (unassigned) => Matthieu Bucchianeri (matthieu-bucchianeri)

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

Title:
  qemu-ppc: Floating point instructions do not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt

Status in QEMU:
  New

Bug description:
  When emulating certain floating point instructions or vector
  instructions on PowerPC machines, QEMU does not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt.

  As described in the Signal Processing Engine (SPE) Programming
  Environments Manual, Rev. 0, available at https://www.nxp.com/docs/en
  /reference-manual/SPEPEM.pdf:

  > An SPE/embedded floating-point unavailable exception occurs on an attempt 
to execute any of the
  > following instructions and MSR[SPV] is not set:
  > * SPE instruction (except brinc)
  > * An embedded scalar double-precision instruction
  > * A vector single-precision floating-point instructions
  > It is not used by embedded scalar single-precision floating-point 
instructions

  This behavior was partially reported in Bug #1611394, however the
  issue is larger than what is described in that bug. As mentioned in
  that bug, some single-precision instructions generate the exception
  (while they should not), which is incorrect but does not typically
  produce an incorrect output. What is more of an issue is that several
  double-precision and vector instructions do not generate the exception
  (while they should), and this break support for lazy FPU/vector
  context switching in Linux (for example).

  The upper 32-bit of the double-precision/vector registers (which are
  in fact hidden in the general purpose registers) is not properly
  saved/restored, and this causes arithmetic errors. This was observed
  very frequently on a commercial project that does a lot of double-
  precision computations. The application works perfectly fine on an
  MPC8548 CPU, but fails often with QEMU.

  The issue can be reproduced using the attached Linux program "spe-
  bug.c". This program properly prints the number 42 (as the result of
  some very simple double-precision computation) on real PowerPC
  hardware, but prints an incorrect result (typically 0) on QEMU.

  This issue was first discovered in an older version of QEMU, but is
  also reproduced in the latest:

  # git rev-parse HEAD
  7adfbea8fd1efce36019a0c2f198ca73be9d3f18
  # ppc-softmmu/qemu-system-ppc --version
  QEMU emulator version 5.0.91 (v5.1.0-rc1-28-g7adfbea8fd-dirty)
  Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers

  Upon further analysis a total of 39 instructions are misbehaving:

  efsabs: raised: 1, expected: 0
  efsnabs: raised: 1, expected: 0
  efsneg: raised: 1, expected: 0
  efdcfs: raised: 0, expected: 1
  efdcfsf: raised: 0, expected: 1
  efdcfsi: raised: 0, expected: 1
  efdcfuf: raised: 0, expected: 1
  efdcfui: raised: 0, expected: 1
  efdctsf: raised: 0, expected: 1
  efdctsi: raised: 0, expected: 1
  efdctsiz: raised: 0, expected: 1
  efdctuf: raised: 0, expected: 1
  efdctui: raised: 0, expected: 1
  efdctuiz: raised: 0, expected: 1
  efscfd: raised: 0, expected: 1
  evfscfsf: raised: 0, expected: 1
  evfscfsi: raised: 0, expected: 1
  evfscfuf: raised: 0, expected: 1
  evfscfui: raised: 0, expected: 1
  evfsctsf: raised: 0, expected: 1
  evfsctsi: raised: 0, expected: 1
  evfsctsiz: raised: 0, expected: 1
  evfsctuf: raised: 0, expected: 1
  evfsctui: raised: 0, expected: 1
  evfsctuiz: raised: 0, expected: 1
  brinc: raised: 0, expected: 1
  efsadd: raised: 1, expected: 0
  efsdiv: raised: 1, expected: 0
  efsmul: raised: 1, expected: 0
  efssub: raised: 1, expected: 0
  evsplatfi: raised: 0, expected: 1
  evsplati: raised: 0, expected: 1
  efscmpeq: raised: 1, expected: 0
  efscmpgt: raised: 1, expected: 0
  efscmplt: raised: 1, expected: 0
  efststeq: raised: 1, expected: 0
  efststgt: raised: 1, expected: 0
  efststlt: raised: 1, expected: 0
  evsel: raised: 0, expected: 1

  When "raised" is 0 and "expected" is 1, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was not generated while it should have.
  When "raised" is 1 and "expected" is 0, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was generated while it should not have 
(Bug #1611394).

  A comprehensive program testing all the instructions listed in the
  Signal Processing Engine (SPE) Programming Environments Manual, Rev. 0
  is posted in the comments of this ticket, and can be used to reproduce
  the issue, and validate the future fix.

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

[Bug 1611394] Re: qemu-ppc: Scalar Single-Precision Floating-Point instructions should not test MSR[SPV]

[Matthieu Bucchianeri]

I have filed a broader ticket, Bug #1888918, reporting a very similar
issue that leads to corruption/bad arithmetic when using double-
precision & vector instructions.

I will be submitting a patch in the next few days that will also address
this ticket.

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

Title:
  qemu-ppc: Scalar Single-Precision Floating-Point instructions should
  not test  MSR[SPV]

Status in QEMU:
  New

Bug description:
  According to "Signal Processing Engine (SPE) Programming Environments Manual" 
at
  
http://cache.nxp.com/files/32bit/doc/ref_manual/SPEPEM.pdf?fsrch=1=1=1

  c.f section 4.2.3  and also Figure 2-2.

  When MSR[SPV] is _NOT_ set, then the embedded scalar single-precision 
floating-point instructions
  should _NOT_ generate an Embedded Floating-Point Unavailable Interrupt.

  
  Hence, some tests for MSR[SPV] in file target-ppc/translate.c need to be 
removed.
  Namely, in the definitions of 
  1. GEN_SPEFPUOP_ARITH2_32_32
  2. gen_efsabs
  3. gen_efsnabs
  4. gen_efsneg
  5. GEN_SPEFPUOP_COMP_32

  Note, the macro GEN_SPEFPUOP_CONV_32_32 is already correct.

  One more thing, afaict the macro GEN_SPEFPUOP_CONV_32_64 is used by both
  efs- and efd- instructions, and will need to be split in two versions.
  The efs-use (i.e for efscfd) should be as it is today, but the use by 
efd-instructions 
  (e.g efdctui) will need to add a test for MSR[SPV].


  (I've looked at today's HEAD-revision of target-ppc/translate.c).

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

[Bug 1888918] Re: qemu-ppc: Floating point instructions do not properly generate the SPE/Embedded Floating-Point Unavailable interrupt

[Matthieu Bucchianeri]

Attaching the output of the test module run on QEMU. As shown in the
description, 39 instructions do not comply with the rule described in
the Signal Processing Engine (SPE) Programming Environments Manual, Rev.
0.

QEMU was run as follows:

# qemu/ppc-softmmu/qemu-system-ppc -nographic -machine mpc8544ds -kernel
buildroot/output/images/uImage -hda buildroot/output/images/rootfs.cpio
-append "root=/dev/sda rw single"

The Linux image is built using buildroot, and the selected configuration
is qemu_ppc_mpc8544ds_defconfig with some very mild changes to setup an
overlay and initramfs.

Then, the module is loaded, output can be observed directly in the
console, or using dmesg:

# insmod spe-test.ko


** Attachment added: "Results on QEMU"
   
https://bugs.launchpad.net/qemu/+bug/1888918/+attachment/5395641/+files/result_qemu_no_fix.txt

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

Title:
  qemu-ppc: Floating point instructions do not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt

Status in QEMU:
  New

Bug description:
  When emulating certain floating point instructions or vector
  instructions on PowerPC machines, QEMU does not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt.

  As described in the Signal Processing Engine (SPE) Programming
  Environments Manual, Rev. 0, available at https://www.nxp.com/docs/en
  /reference-manual/SPEPEM.pdf:

  > An SPE/embedded floating-point unavailable exception occurs on an attempt 
to execute any of the
  > following instructions and MSR[SPV] is not set:
  > * SPE instruction (except brinc)
  > * An embedded scalar double-precision instruction
  > * A vector single-precision floating-point instructions
  > It is not used by embedded scalar single-precision floating-point 
instructions

  This behavior was partially reported in Bug #1611394, however the
  issue is larger than what is described in that bug. As mentioned in
  that bug, some single-precision instructions generate the exception
  (while they should not), which is incorrect but does not typically
  produce an incorrect output. What is more of an issue is that several
  double-precision and vector instructions do not generate the exception
  (while they should), and this break support for lazy FPU/vector
  context switching in Linux (for example).

  The upper 32-bit of the double-precision/vector registers (which are
  in fact hidden in the general purpose registers) is not properly
  saved/restored, and this causes arithmetic errors. This was observed
  very frequently on a commercial project that does a lot of double-
  precision computations. The application works perfectly fine on an
  MPC8548 CPU, but fails often with QEMU.

  The issue can be reproduced using the attached Linux program "spe-
  bug.c". This program properly prints the number 42 (as the result of
  some very simple double-precision computation) on real PowerPC
  hardware, but prints an incorrect result (typically 0) on QEMU.

  This issue was first discovered in an older version of QEMU, but is
  also reproduced in the latest:

  # git rev-parse HEAD
  7adfbea8fd1efce36019a0c2f198ca73be9d3f18
  # ppc-softmmu/qemu-system-ppc --version
  QEMU emulator version 5.0.91 (v5.1.0-rc1-28-g7adfbea8fd-dirty)
  Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers

  Upon further analysis a total of 39 instructions are misbehaving:

  efsabs: raised: 1, expected: 0
  efsnabs: raised: 1, expected: 0
  efsneg: raised: 1, expected: 0
  efdcfs: raised: 0, expected: 1
  efdcfsf: raised: 0, expected: 1
  efdcfsi: raised: 0, expected: 1
  efdcfuf: raised: 0, expected: 1
  efdcfui: raised: 0, expected: 1
  efdctsf: raised: 0, expected: 1
  efdctsi: raised: 0, expected: 1
  efdctsiz: raised: 0, expected: 1
  efdctuf: raised: 0, expected: 1
  efdctui: raised: 0, expected: 1
  efdctuiz: raised: 0, expected: 1
  efscfd: raised: 0, expected: 1
  evfscfsf: raised: 0, expected: 1
  evfscfsi: raised: 0, expected: 1
  evfscfuf: raised: 0, expected: 1
  evfscfui: raised: 0, expected: 1
  evfsctsf: raised: 0, expected: 1
  evfsctsi: raised: 0, expected: 1
  evfsctsiz: raised: 0, expected: 1
  evfsctuf: raised: 0, expected: 1
  evfsctui: raised: 0, expected: 1
  evfsctuiz: raised: 0, expected: 1
  brinc: raised: 0, expected: 1
  efsadd: raised: 1, expected: 0
  efsdiv: raised: 1, expected: 0
  efsmul: raised: 1, expected: 0
  efssub: raised: 1, expected: 0
  evsplatfi: raised: 0, expected: 1
  evsplati: raised: 0, expected: 1
  efscmpeq: raised: 1, expected: 0
  efscmpgt: raised: 1, expected: 0
  efscmplt: raised: 1, expected: 0
  efststeq: raised: 1, expected: 0
  efststgt: raised: 1, expected: 0
  efststlt: raised: 1, expected: 0
  evsel: raised: 0, expected: 1

  When "raised" is 0 and "expected" is 1, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was not 

[Bug 1888918] Re: qemu-ppc: Floating point instructions do not properly generate the SPE/Embedded Floating-Point Unavailable interrupt

[Matthieu Bucchianeri]

I have already written a patch for this issue, that I will be submitting
to the community in the next few days.

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

Title:
  qemu-ppc: Floating point instructions do not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt

Status in QEMU:
  New

Bug description:
  When emulating certain floating point instructions or vector
  instructions on PowerPC machines, QEMU does not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt.

  As described in the Signal Processing Engine (SPE) Programming
  Environments Manual, Rev. 0, available at https://www.nxp.com/docs/en
  /reference-manual/SPEPEM.pdf:

  > An SPE/embedded floating-point unavailable exception occurs on an attempt 
to execute any of the
  > following instructions and MSR[SPV] is not set:
  > * SPE instruction (except brinc)
  > * An embedded scalar double-precision instruction
  > * A vector single-precision floating-point instructions
  > It is not used by embedded scalar single-precision floating-point 
instructions

  This behavior was partially reported in Bug #1611394, however the
  issue is larger than what is described in that bug. As mentioned in
  that bug, some single-precision instructions generate the exception
  (while they should not), which is incorrect but does not typically
  produce an incorrect output. What is more of an issue is that several
  double-precision and vector instructions do not generate the exception
  (while they should), and this break support for lazy FPU/vector
  context switching in Linux (for example).

  The upper 32-bit of the double-precision/vector registers (which are
  in fact hidden in the general purpose registers) is not properly
  saved/restored, and this causes arithmetic errors. This was observed
  very frequently on a commercial project that does a lot of double-
  precision computations. The application works perfectly fine on an
  MPC8548 CPU, but fails often with QEMU.

  The issue can be reproduced using the attached Linux program "spe-
  bug.c". This program properly prints the number 42 (as the result of
  some very simple double-precision computation) on real PowerPC
  hardware, but prints an incorrect result (typically 0) on QEMU.

  This issue was first discovered in an older version of QEMU, but is
  also reproduced in the latest:

  # git rev-parse HEAD
  7adfbea8fd1efce36019a0c2f198ca73be9d3f18
  # ppc-softmmu/qemu-system-ppc --version
  QEMU emulator version 5.0.91 (v5.1.0-rc1-28-g7adfbea8fd-dirty)
  Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers

  Upon further analysis a total of 39 instructions are misbehaving:

  efsabs: raised: 1, expected: 0
  efsnabs: raised: 1, expected: 0
  efsneg: raised: 1, expected: 0
  efdcfs: raised: 0, expected: 1
  efdcfsf: raised: 0, expected: 1
  efdcfsi: raised: 0, expected: 1
  efdcfuf: raised: 0, expected: 1
  efdcfui: raised: 0, expected: 1
  efdctsf: raised: 0, expected: 1
  efdctsi: raised: 0, expected: 1
  efdctsiz: raised: 0, expected: 1
  efdctuf: raised: 0, expected: 1
  efdctui: raised: 0, expected: 1
  efdctuiz: raised: 0, expected: 1
  efscfd: raised: 0, expected: 1
  evfscfsf: raised: 0, expected: 1
  evfscfsi: raised: 0, expected: 1
  evfscfuf: raised: 0, expected: 1
  evfscfui: raised: 0, expected: 1
  evfsctsf: raised: 0, expected: 1
  evfsctsi: raised: 0, expected: 1
  evfsctsiz: raised: 0, expected: 1
  evfsctuf: raised: 0, expected: 1
  evfsctui: raised: 0, expected: 1
  evfsctuiz: raised: 0, expected: 1
  brinc: raised: 0, expected: 1
  efsadd: raised: 1, expected: 0
  efsdiv: raised: 1, expected: 0
  efsmul: raised: 1, expected: 0
  efssub: raised: 1, expected: 0
  evsplatfi: raised: 0, expected: 1
  evsplati: raised: 0, expected: 1
  efscmpeq: raised: 1, expected: 0
  efscmpgt: raised: 1, expected: 0
  efscmplt: raised: 1, expected: 0
  efststeq: raised: 1, expected: 0
  efststgt: raised: 1, expected: 0
  efststlt: raised: 1, expected: 0
  evsel: raised: 0, expected: 1

  When "raised" is 0 and "expected" is 1, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was not generated while it should have.
  When "raised" is 1 and "expected" is 0, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was generated while it should not have 
(Bug #1611394).

  A comprehensive program testing all the instructions listed in the
  Signal Processing Engine (SPE) Programming Environments Manual, Rev. 0
  is posted in the comments of this ticket, and can be used to reproduce
  the issue, and validate the future fix.

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

[RFC PATCH 6/8] migration/dirtyrate: Implement get_sample_gap_period() and block_sample_gap_period()

[Chuan Zheng]

From: Zheng Chuan 

Implement get_sample_gap_period() and block_sample_gap_period() to
sleep specific time between sample actions.

Signed-off-by: Zheng Chuan 
Signed-off-by: YanYing Zhang 
---
 migration/dirtyrate.c | 28 
 migration/dirtyrate.h |  6 +-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 7badc53..00abfa7 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -295,10 +295,38 @@ static void 
set_dirty_rate_stage(CalculatingDirtyRateStage ratestage)
 calculating_dirty_rate_stage = ratestage;
 }
 
+static int64_t block_sample_gap_period(int64_t msec, int64_t initial_time)
+{
+int64_t current_time;
+
+current_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+if ((current_time - initial_time) >= msec) {
+msec = current_time - initial_time;
+} else {
+g_usleep((msec + initial_time - current_time) * 1000);
+}
+
+return msec;
+}
+
+static int64_t get_sample_gap_period(struct dirtyrate_config config)
+{
+int64_t msec;
+
+msec = config.sample_period_seconds * 1000;
+if (msec <= MIN_FETCH_DIRTYRATE_TIME_MSEC || msec > 
MAX_FETCH_DIRTYRATE_TIME_MSEC) {
+msec = DEFAULT_FETCH_DIRTYRATE_TIME_MSEC;
+}
+return msec;
+}
+
 void *get_dirtyrate_thread(void *arg)
 {
 struct dirtyrate_config config = *(struct dirtyrate_config *)arg;
 int64_t msec = 0;
+
+/* max period is 60 seconds */
+msec = get_sample_gap_period(config);
  
 set_dirty_rate_stage(CAL_DIRTY_RATE_ING);
 
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index 4d9b3b8..5aef2d7 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -14,12 +14,16 @@
 #define QEMU_MIGRATION_DIRTYRATE_H
 
 /* take 256 pages per GB for cal dirty rate */
-#define DIRTYRATE_DEFAULT_SAMPLE_PAGES256
+#define DIRTYRATE_DEFAULT_SAMPLE_PAGES  256
 #define DIRTYRATE_SAMPLE_PAGE_SIZE  4096
 #define DIRTYRATE_PAGE_SIZE_SHIFT   12
 #define BLOCK_INFO_MAX_LEN  256
 #define PAGE_SIZE_SHIFT 20
 
+#define MIN_FETCH_DIRTYRATE_TIME_MSEC0
+#define MAX_FETCH_DIRTYRATE_TIME_MSEC6
+#define DEFAULT_FETCH_DIRTYRATE_TIME_MSEC1000
+
 struct dirtyrate_config {
 uint64_t sample_pages_per_gigabytes;
 int64_t sample_period_seconds;
-- 
1.8.3.1

[RFC PATCH 7/8] migration/dirtyrate: Implement calculate_dirtyrate() function

[Chuan Zheng]

From: Zheng Chuan 

Implement calculate_dirtyrate() function.

Signed-off-by: Zheng Chuan 
Signed-off-by: YanYing Zhang 
---
 migration/dirtyrate.c | 53 +--
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 00abfa7..d87e16d 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -161,6 +161,21 @@ alloc_block_dirty_info(int *block_index,
 return block_dinfo;
 }
 
+static void free_block_dirty_info(struct block_dirty_info *infos, int count)
+{
+int i;
+
+if (!infos) {
+return;
+}
+
+for (i = 0; i < count; i++) {
+g_free(infos[i].sample_page_vfn);
+g_free(infos[i].hash_result);
+}
+g_free(infos);
+}
+
 static int ram_block_skip(RAMBlock *block)
 {
 if (!strstr(qemu_ram_get_idstr(block), "ram-node") &&
@@ -278,12 +293,6 @@ static int compare_block_hash_info(struct block_dirty_info 
*info, int block_inde
 return 0;
 }
 
-
-static void calculate_dirtyrate(struct dirtyrate_config config, int64_t time)
-{
-/* todo */
-}
-
 /*
  * There are multithread will write/read *calculating_dirty_rate_stage*,
  * we can protect only one thread write/read it by libvirt api.
@@ -320,6 +329,38 @@ static int64_t get_sample_gap_period(struct 
dirtyrate_config config)
 return msec;
 }
 
+static void calculate_dirtyrate(struct dirtyrate_config config, int64_t time)
+{
+struct block_dirty_info *block_dinfo = NULL;
+int block_index = 0;
+int64_t msec = time;
+int64_t initial_time;
+
+rcu_register_thread();
+reset_dirtyrate_stat();
+initial_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+rcu_read_lock();
+if (record_block_hash_info(config, _dinfo, _index) < 0) {
+goto out;
+}
+rcu_read_unlock();
+
+msec = block_sample_gap_period(msec, initial_time);
+
+rcu_read_lock();
+if (compare_block_hash_info(block_dinfo, block_index) < 0) {
+goto out;
+}
+
+update_dirtyrate(msec);
+
+out:
+rcu_read_unlock();
+free_block_dirty_info(block_dinfo, block_index + 1);
+rcu_unregister_thread();
+}
+
+
 void *get_dirtyrate_thread(void *arg)
 {
 struct dirtyrate_config config = *(struct dirtyrate_config *)arg;
-- 
1.8.3.1

[RFC PATCH 8/8] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function

[Chuan Zheng]

From: Zheng Chuan 

Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be 
called
by libvirt api.

Signed-off-by: Zheng Chuan 
Signed-off-by: YanYing Zhang 
---
 migration/Makefile.objs |  1 +
 migration/dirtyrate.c   | 45 +
 qapi/migration.json | 24 
 qapi/pragma.json|  3 ++-
 4 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 0fc619e..12ae98c 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -6,6 +6,7 @@ common-obj-y += qemu-file.o global_state.o
 common-obj-y += qemu-file-channel.o
 common-obj-y += xbzrle.o postcopy-ram.o
 common-obj-y += qjson.o
+common-obj-y += dirtyrate.o
 common-obj-y += block-dirty-bitmap.o
 common-obj-y += multifd.o
 common-obj-y += multifd-zlib.o
diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index d87e16d..5717521 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -31,6 +31,21 @@ CalculatingDirtyRateStage calculating_dirty_rate_stage = 
CAL_DIRTY_RATE_INIT;
 INTERNAL_RAMBLOCK_FOREACH(block)   \
 if (!qemu_ram_is_migratable(block)) {} else
 
+static int64_t get_dirty_rate_info(void)
+{
+int64_t dirty_rate = dirty_stat.dirty_rate;
+/*
+ *Return dirty_rate only when we get CAL_DIRTY_RATE_END.
+ *And we must initial calculating_dirty_rate_stage.
+ */
+if (calculating_dirty_rate_stage == CAL_DIRTY_RATE_END) {
+calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
+return dirty_rate;
+}  else {
+return -1;
+}
+}
+
 static void reset_dirtyrate_stat(void)
 {
 dirty_stat.total_dirty_samples = 0;
@@ -377,3 +392,33 @@ void *get_dirtyrate_thread(void *arg)
 
 return NULL;
 }
+
+void qmp_cal_dirty_rate(int64_t value, Error **errp)
+{
+static struct dirtyrate_config dirtyrate_config;
+QemuThread thread;
+
+/*
+ * We don't begin calculating thread only when it's in calculating status.
+ */
+if (calculating_dirty_rate_stage == CAL_DIRTY_RATE_ING) {
+return;
+}
+
+/*
+ * If we get CAL_DIRTY_RATE_END here, libvirtd may be restarted at last 
round,
+ * we need to initial it.
+ */
+if (calculating_dirty_rate_stage == CAL_DIRTY_RATE_END)
+calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
+
+dirtyrate_config.sample_period_seconds = value;
+dirtyrate_config.sample_pages_per_gigabytes = sample_pages_per_gigabytes;
+qemu_thread_create(, "get_dirtyrate", get_dirtyrate_thread,
+   (void *)_config, QEMU_THREAD_DETACHED);
+}
+
+int64_t qmp_get_dirty_rate(Error **errp)
+{
+return get_dirty_rate_info();
+}
diff --git a/qapi/migration.json b/qapi/migration.json
index d500055..59f35bb 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1621,3 +1621,27 @@
 ##
 { 'event': 'UNPLUG_PRIMARY',
   'data': { 'device-id': 'str' } }
+
+##
+# @cal_dirty_rate:
+#
+# start calculating dirty rate for vm
+#
+# @value: time for sample dirty pages
+#
+# Since: 5.1
+#
+# Example:
+#   {"command": "cal_dirty_rate", "data": {"value": 1} }
+#
+##
+{ 'command': 'cal_dirty_rate', 'data': {'value': 'int64'} }
+
+##
+# @get_dirty_rate:
+#
+# get dirty rate for vm
+#
+# Since: 5.1
+##
+{ 'command': 'get_dirty_rate', 'returns': 'int64' }
diff --git a/qapi/pragma.json b/qapi/pragma.json
index cffae27..ecd294b 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -10,7 +10,8 @@
 'query-migrate-cache-size',
 'query-tpm-models',
 'query-tpm-types',
-'ringbuf-read' ],
+'ringbuf-read',
+'get_dirty_rate' ],
 'name-case-whitelist': [
 'ACPISlotType', # DIMM, visible through 
query-acpi-ospm-status
 'CpuInfoMIPS',  # PC, visible through query-cpu
-- 
1.8.3.1

[RFC PATCH 4/8] migration/dirtyrate: Record hash results for each ramblock

[Chuan Zheng]

From: Zheng Chuan 

Record hash results for each ramblock.

Signed-off-by: Zheng Chuan 
Signed-off-by: YanYing Zhang 
---
 migration/dirtyrate.c | 157 ++
 migration/dirtyrate.h |   1 +
 2 files changed, 158 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 6baf674..45cfc91 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -10,12 +10,27 @@
  * See the COPYING file in the top-level directory.
  */
 
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "crypto/hash.h"
+#include "crypto/random.h"
+#include "qemu/config-file.h"
+#include "exec/memory.h"
+#include "exec/ramblock.h"
+#include "exec/target_page.h"
+#include "qemu/rcu_queue.h"
+#include "qapi/qapi-commands-migration.h"
+#include "migration.h"
 #include "dirtyrate.h"
 
 static uint64_t sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
 static struct dirtyrate_statistics dirty_stat;
 CalculatingDirtyRateStage calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
 
+#define RAMBLOCK_FOREACH_MIGRATABLE(block) \
+INTERNAL_RAMBLOCK_FOREACH(block)   \
+if (!qemu_ram_is_migratable(block)) {} else
+
 static void reset_dirtyrate_stat(void)
 {
 dirty_stat.total_dirty_samples = 0;
@@ -44,6 +59,148 @@ static void update_dirtyrate(int64_t msec)
 dirty_stat.dirty_rate = dirty_rate;
 }
 
+static int get_block_vfn_hash(struct block_dirty_info *info, unsigned long vfn,
+  uint8_t **md, size_t *hash_len)
+{
+struct iovec iov_array;
+int ret = 0;
+int nkey = 1;
+
+iov_array.iov_base = info->block_addr +
+ vfn * DIRTYRATE_SAMPLE_PAGE_SIZE;
+iov_array.iov_len = DIRTYRATE_SAMPLE_PAGE_SIZE;
+
+if (qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_MD5,
+_array, nkey,
+md, hash_len, NULL) < 0) {
+ret = -1;
+}
+
+return ret;
+}
+
+static int save_block_hash(struct block_dirty_info *info)
+{
+unsigned long *rand_buf = NULL;
+unsigned int sample_pages_count;
+uint8_t *md = NULL;
+size_t hash_len;
+int i;
+int ret = -1;
+
+sample_pages_count = info->sample_pages_count;
+/* block size less than one page, return success to skip this block */
+if (unlikely(info->block_pages == 0 || sample_pages_count == 0)) {
+ret = 0;
+goto out;
+}
+
+/* use random bytes to pick sample page vfn */
+rand_buf = g_malloc0_n(sample_pages_count, sizeof(unsigned long));
+/* DEFAULT_READ_RANDOM_MAX_LIMIT 32M,
+ * can support 4T vm 1024 sample_pages_per_gigabytes
+ */
+ret = qcrypto_random_bytes((unsigned char *)rand_buf,
+   sample_pages_count * sizeof(unsigned long),
+   NULL);
+if (ret) {
+ret = -1;
+goto out;
+}
+
+hash_len = qcrypto_hash_digest_len(QCRYPTO_HASH_ALG_MD5);
+info->hash_result = g_malloc0_n(sample_pages_count, sizeof(uint8_t) * 
hash_len);
+info->sample_page_vfn = g_malloc0_n(sample_pages_count, sizeof(unsigned 
long));
+
+for (i = 0; i < sample_pages_count; i++) {
+md = info->hash_result + i * hash_len;
+info->sample_page_vfn[i] = rand_buf[i] % info->block_pages;
+ret = get_block_vfn_hash(info, info->sample_page_vfn[i], , 
_len);
+if (ret < 0) {
+goto out;
+}
+}
+ret = 0;
+out:
+g_free(rand_buf);
+return ret;
+}
+
+static void get_block_dirty_info(RAMBlock *block, struct block_dirty_info 
*info,
+ struct dirtyrate_config *config)
+{
+uint64_t sample_pages_per_gigabytes = config->sample_pages_per_gigabytes;
+
+/* Right shift 30 bits to calc block size in GB */
+info->sample_pages_count = (qemu_ram_get_used_length(block) * 
sample_pages_per_gigabytes) >> 30;
+
+info->block_pages = qemu_ram_get_used_length(block) >> 
DIRTYRATE_PAGE_SIZE_SHIFT;
+info->block_addr = qemu_ram_get_host_addr(block);
+strcpy(info->idstr, qemu_ram_get_idstr(block));
+}
+
+static struct block_dirty_info *
+alloc_block_dirty_info(int *block_index,
+   struct block_dirty_info *block_dinfo)
+{
+struct block_dirty_info *info = NULL;
+int index = *block_index;
+
+if (!block_dinfo) {
+block_dinfo = g_new(struct block_dirty_info, 1);
+index = 0;
+} else {
+block_dinfo = g_realloc(block_dinfo, (index + 1) *
+sizeof(struct block_dirty_info));
+index++;
+}
+info = _dinfo[index];
+memset(info, 0, sizeof(struct block_dirty_info));
+
+*block_index = index;
+return block_dinfo;
+}
+
+static int ram_block_skip(RAMBlock *block)
+{
+if (!strstr(qemu_ram_get_idstr(block), "ram-node") &&
+!strstr(qemu_ram_get_idstr(block), "memdimm")) {
+if (strcmp(qemu_ram_get_idstr(block), "mach-virt.ram") ||
+

[RFC PATCH 5/8] migration/dirtyrate: Compare hash results for recorded ramblock

[Chuan Zheng]

From: Zheng Chuan 

Compare hash results for recorded ramblock.

Signed-off-by: Zheng Chuan 
Signed-off-by: YanYing Zhang 
---
 migration/dirtyrate.c | 77 +++
 1 file changed, 77 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 45cfc91..7badc53 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -202,6 +202,83 @@ static int record_block_hash_info(struct dirtyrate_config 
config,
 return 0;
 }
 
+static int cal_block_dirty_rate(struct block_dirty_info *info)
+{
+uint8_t *md = NULL;
+size_t hash_len;
+int i;
+int ret = 0;
+
+hash_len = qcrypto_hash_digest_len(QCRYPTO_HASH_ALG_MD5);
+md = g_new0(uint8_t, hash_len);
+
+for (i = 0; i < info->sample_pages_count; i++) {
+ret = get_block_vfn_hash(info, info->sample_page_vfn[i], , 
_len);
+if (ret < 0) {
+goto out;
+}
+
+if (memcmp(md, info->hash_result + i * hash_len, hash_len) != 0) {
+info->sample_dirty_count++;
+}
+}
+
+out:
+g_free(md);
+return ret;
+}
+
+static bool find_block_matched(RAMBlock *block, struct block_dirty_info *infos,
+   int count, struct block_dirty_info **matched)
+{
+int i;
+
+for (i = 0; i < count; i++) {
+if (!strcmp(infos[i].idstr, qemu_ram_get_idstr(block))) {
+break;
+}
+}
+
+if (i == count) {
+return false;
+}
+
+if (infos[i].block_addr != qemu_ram_get_host_addr(block) ||
+infos[i].block_pages !=
+(qemu_ram_get_used_length(block) >> DIRTYRATE_PAGE_SIZE_SHIFT)) {
+return false;
+}
+
+*matched = [i];
+return true;
+}
+
+static int compare_block_hash_info(struct block_dirty_info *info, int 
block_index)
+{
+struct block_dirty_info *block_dinfo = NULL;
+RAMBlock *block = NULL;
+
+RAMBLOCK_FOREACH_MIGRATABLE(block) {
+if (ram_block_skip(block) < 0) {
+continue;
+}
+block_dinfo = NULL;
+if (!find_block_matched(block, info, block_index + 1, _dinfo)) {
+continue;
+}
+if (cal_block_dirty_rate(block_dinfo) < 0) {
+return -1;
+}
+update_dirtyrate_stat(block_dinfo);
+}
+if (!dirty_stat.total_sample_count) {
+return -1;
+}
+
+return 0;
+}
+
+
 static void calculate_dirtyrate(struct dirtyrate_config config, int64_t time)
 {
 /* todo */
-- 
1.8.3.1

[RFC PATCH 1/8] migration/dirtyrate: Add get_dirtyrate_thread() function

[Chuan Zheng]

From: Zheng Chuan 

Add get_dirtyrate_thread() functions

Signed-off-by: Zheng Chuan 
Signed-off-by: YanYing Zhang 
---
 migration/dirtyrate.c | 63 +++
 migration/dirtyrate.h | 38 +++
 2 files changed, 101 insertions(+)
 create mode 100644 migration/dirtyrate.c
 create mode 100644 migration/dirtyrate.h

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
new file mode 100644
index 000..fc652fb
--- /dev/null
+++ b/migration/dirtyrate.c
@@ -0,0 +1,63 @@
+/*
+ * Dirtyrate implement code
+ *
+ * Copyright (c) 2017-2020 HUAWEI TECHNOLOGIES CO.,LTD.
+ *
+ * Authors:
+ *  Chuan Zheng 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "dirtyrate.h"
+
+static uint64_t sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
+static uint64_t dirty_rate; /* MB/s */
+CalculatingDirtyRateStage calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
+
+static bool calculate_dirtyrate(struct dirtyrate_config config,
+uint64_t *dirty_rate, int64_t time)
+{
+/* todo */
+return true;
+}
+
+static void set_dirty_rate(uint64_t drate)
+{
+dirty_rate = drate;
+}
+
+/*
+ * There are multithread will write/read *calculating_dirty_rate_stage*,
+ * we can protect only one thread write/read it by libvirt api.
+ * So we don't add mutex_lock to protect it here, but we must calculate
+ * dirty_rate by libvirt api.
+ */
+static void set_dirty_rate_stage(CalculatingDirtyRateStage ratestage)
+{
+calculating_dirty_rate_stage = ratestage;
+}
+
+void *get_dirtyrate_thread(void *arg)
+{
+struct dirtyrate_config config = *(struct dirtyrate_config *)arg;
+uint64_t dirty_rate;
+uint64_t hash_dirty_rate;
+bool query_succ;
+int64_t msec = 0;
+ 
+set_dirty_rate_stage(CAL_DIRTY_RATE_ING);
+
+query_succ = calculate_dirtyrate(config, _dirty_rate, msec);
+if (!query_succ) {
+dirty_rate = 0;
+} else {
+dirty_rate = hash_dirty_rate;
+}
+
+set_dirty_rate(dirty_rate);
+set_dirty_rate_stage(CAL_DIRTY_RATE_END);
+
+return NULL;
+}
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
new file mode 100644
index 000..9a5c228
--- /dev/null
+++ b/migration/dirtyrate.h
@@ -0,0 +1,38 @@
+/*
+ *  Dirtyrate common functions
+ *
+ *  Copyright (c) 2020 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ *  Authors:
+ *  Chuan Zheng 
+ *
+ *  This work is licensed under the terms of the GNU GPL, version 2 or later.
+ *  See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_MIGRATION_DIRTYRATE_H
+#define QEMU_MIGRATION_DIRTYRATE_H
+
+/* take 256 pages per GB for cal dirty rate */
+#define DIRTYRATE_DEFAULT_SAMPLE_PAGES256
+
+struct dirtyrate_config {
+uint64_t sample_pages_per_gigabytes;
+int64_t sample_period_seconds;
+};
+
+/*
+ *  To record calculate dirty_rate status:
+ *  0: initial status, calculating thread is not be created here.
+ *  1: calculating thread is created.
+ *  2: calculating thread is end, we can get result.
+ */
+typedef enum {
+CAL_DIRTY_RATE_INIT  = 0,
+CAL_DIRTY_RATE_ING   = 1,
+CAL_DIRTY_RATE_END   = 2,
+} CalculatingDirtyRateStage;
+
+void *get_dirtyrate_thread(void *arg);
+#endif
+
-- 
1.8.3.1

[RFC PATCH 3/8] migration/dirtyrate: Add dirtyrate statistics series functions

[Chuan Zheng]

From: Zheng Chuan 

Add dirtyrate statistics to record/update dirtyrate info.

Signed-off-by: Zheng Chuan 
Signed-off-by: YanYing Zhang 
---
 migration/dirtyrate.c | 47 ++-
 migration/dirtyrate.h | 11 +++
 2 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index fc652fb..6baf674 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -13,19 +13,41 @@
 #include "dirtyrate.h"
 
 static uint64_t sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
-static uint64_t dirty_rate; /* MB/s */
+static struct dirtyrate_statistics dirty_stat;
 CalculatingDirtyRateStage calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
 
-static bool calculate_dirtyrate(struct dirtyrate_config config,
-uint64_t *dirty_rate, int64_t time)
+static void reset_dirtyrate_stat(void)
 {
-/* todo */
-return true;
+dirty_stat.total_dirty_samples = 0;
+dirty_stat.total_sample_count = 0;
+dirty_stat.total_block_mem_MB = 0;
+dirty_stat.dirty_rate = 0;
+}
+
+static void update_dirtyrate_stat(struct block_dirty_info *info)
+{
+dirty_stat.total_dirty_samples += info->sample_dirty_count;
+dirty_stat.total_sample_count += info->sample_pages_count;
+dirty_stat.total_block_mem_MB += (info->block_pages << 
DIRTYRATE_PAGE_SIZE_SHIFT) >> PAGE_SIZE_SHIFT;
 }
 
-static void set_dirty_rate(uint64_t drate)
+static void update_dirtyrate(int64_t msec)
 {
-dirty_rate = drate;
+uint64_t dirty_rate;
+unsigned int total_dirty_samples = dirty_stat.total_dirty_samples;
+unsigned int total_sample_count = dirty_stat.total_sample_count;
+unsigned long total_block_mem_MB = dirty_stat.total_block_mem_MB;
+
+dirty_rate = total_dirty_samples * total_block_mem_MB *
+ 1000 / (total_sample_count * msec);
+
+dirty_stat.dirty_rate = dirty_rate;
+}
+
+
+static void calculate_dirtyrate(struct dirtyrate_config config, int64_t time)
+{
+/* todo */
 }
 
 /*
@@ -42,21 +64,12 @@ static void set_dirty_rate_stage(CalculatingDirtyRateStage 
ratestage)
 void *get_dirtyrate_thread(void *arg)
 {
 struct dirtyrate_config config = *(struct dirtyrate_config *)arg;
-uint64_t dirty_rate;
-uint64_t hash_dirty_rate;
-bool query_succ;
 int64_t msec = 0;
  
 set_dirty_rate_stage(CAL_DIRTY_RATE_ING);
 
-query_succ = calculate_dirtyrate(config, _dirty_rate, msec);
-if (!query_succ) {
-dirty_rate = 0;
-} else {
-dirty_rate = hash_dirty_rate;
-}
+calculate_dirtyrate(config, msec);
 
-set_dirty_rate(dirty_rate);
 set_dirty_rate_stage(CAL_DIRTY_RATE_END);
 
 return NULL;
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index 342b89f..2994535 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -15,6 +15,9 @@
 
 /* take 256 pages per GB for cal dirty rate */
 #define DIRTYRATE_DEFAULT_SAMPLE_PAGES256
+#define DIRTYRATE_PAGE_SIZE_SHIFT   12
+#define BLOCK_INFO_MAX_LEN  256
+#define PAGE_SIZE_SHIFT 20
 
 struct dirtyrate_config {
 uint64_t sample_pages_per_gigabytes;
@@ -33,6 +36,14 @@ typedef enum {
 CAL_DIRTY_RATE_END   = 2,
 } CalculatingDirtyRateStage;
 
+struct dirtyrate_statistics {
+unsigned int total_dirty_samples;
+unsigned int total_sample_count;
+unsigned long total_block_mem_MB;
+int64_t dirty_rate;
+};
+
+
 /* 
  * Store dirtypage info for each block.
  */
-- 
1.8.3.1

[RFC PATCH 0/8] *** A Method for evaluating dirty page rate ***

[Chuan Zheng]

From: Zheng Chuan 

Sometimes it is neccessary to evaluate dirty page rate before migration.
Users could decide whether to proceed migration based on the evaluation
in case of vm performance loss due to heavy workload.
Unlikey simulating dirtylog sync which could do harm on runnning vm,
we provide a sample-hash method to compare hash results for samping page.
In this way, it would have hardly no impact on vm performance.

We evaluate the dirtypage rate on running vm.
The VM specifications for migration are as follows:
- VM use 4-K page;
- the number of VCPU is 32;
- the total memory is 32Gigabit;
- use 'mempress' tool to pressurize VM(mempress 4096 1024);

++
|  |dirtyrate|
++
| no mempress  | 4MB/s   |
--
| mempress 4096 1024   |1204MB/s |
++
| mempress 4096 4096   |4000Mb/s |
++

Test dirtyrate by qmp command like this:
1.  virsh qemu-monitor-command [vmname] '{"execute":"cal_dirty_rate", 
"arguments": {"value": [sampletime]}}'
2.  virsh qemu-monitor-command [vmname] '{"execute":"get_dirty_rate"}'

Further test dirtyrate by libvirt api like this:
virsh getdirtyrate [vmname] [sampletime]

Zheng Chuan (8):
  migration/dirtyrate: Add get_dirtyrate_thread() function
  migration/dirtyrate: Add block_dirty_info to store dirtypage info
  migration/dirtyrate: Add dirtyrate statistics series functions
  migration/dirtyrate: Record hash results for each ramblock
  migration/dirtyrate: Compare hash results for recorded ramblock
  migration/dirtyrate: Implement get_sample_gap_period() and
block_sample_gap_period()
  migration/dirtyrate: Implement calculate_dirtyrate() function
  migration/dirtyrate: Implement
qmp_cal_dirty_rate()/qmp_get_dirty_rate() function

 migration/Makefile.objs |   1 +
 migration/dirtyrate.c   | 424 
 migration/dirtyrate.h   |  67 
 qapi/migration.json |  24 +++
 qapi/pragma.json|   3 +-
 5 files changed, 518 insertions(+), 1 deletion(-)
 create mode 100644 migration/dirtyrate.c
 create mode 100644 migration/dirtyrate.h

-- 
1.8.3.1

[RFC PATCH 2/8] migration/dirtyrate: Add block_dirty_info to store dirtypage info

[Chuan Zheng]

From: Zheng Chuan 

Add block_dirty_info to store dirtypage info for each ramblock

Signed-off-by: Zheng Chuan 
Signed-off-by: YanYing Zhang 
---
 migration/dirtyrate.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index 9a5c228..342b89f 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -33,6 +33,19 @@ typedef enum {
 CAL_DIRTY_RATE_END   = 2,
 } CalculatingDirtyRateStage;
 
+/* 
+ * Store dirtypage info for each block.
+ */
+struct block_dirty_info {
+char idstr[BLOCK_INFO_MAX_LEN];
+uint8_t *block_addr;
+unsigned long block_pages;
+unsigned long *sample_page_vfn;
+unsigned int sample_pages_count;
+unsigned int sample_dirty_count;
+uint8_t *hash_result;
+};
+
 void *get_dirtyrate_thread(void *arg);
 #endif
 
-- 
1.8.3.1

[Bug 1888918] Re: qemu-ppc: Floating point instructions do not properly generate the SPE/Embedded Floating-Point Unavailable interrupt

[Matthieu Bucchianeri]

Attaching the test program mentioned in the description. This program (a
kernel module, in fact) can be loaded on a Linux system both in QEMU or
on real hardware.

In the next comments, I will attach the detailed output of the test
program.

** Attachment added: "SPE test module"
   
https://bugs.launchpad.net/qemu/+bug/1888918/+attachment/5395639/+files/spe-test.tar.gz

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

Title:
  qemu-ppc: Floating point instructions do not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt

Status in QEMU:
  New

Bug description:
  When emulating certain floating point instructions or vector
  instructions on PowerPC machines, QEMU does not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt.

  As described in the Signal Processing Engine (SPE) Programming
  Environments Manual, Rev. 0, available at https://www.nxp.com/docs/en
  /reference-manual/SPEPEM.pdf:

  > An SPE/embedded floating-point unavailable exception occurs on an attempt 
to execute any of the
  > following instructions and MSR[SPV] is not set:
  > * SPE instruction (except brinc)
  > * An embedded scalar double-precision instruction
  > * A vector single-precision floating-point instructions
  > It is not used by embedded scalar single-precision floating-point 
instructions

  This behavior was partially reported in Bug #1611394, however the
  issue is larger than what is described in that bug. As mentioned in
  that bug, some single-precision instructions generate the exception
  (while they should not), which is incorrect but does not typically
  produce an incorrect output. What is more of an issue is that several
  double-precision and vector instructions do not generate the exception
  (while they should), and this break support for lazy FPU/vector
  context switching in Linux (for example).

  The upper 32-bit of the double-precision/vector registers (which are
  in fact hidden in the general purpose registers) is not properly
  saved/restored, and this causes arithmetic errors. This was observed
  very frequently on a commercial project that does a lot of double-
  precision computations. The application works perfectly fine on an
  MPC8548 CPU, but fails often with QEMU.

  The issue can be reproduced using the attached Linux program "spe-
  bug.c". This program properly prints the number 42 (as the result of
  some very simple double-precision computation) on real PowerPC
  hardware, but prints an incorrect result (typically 0) on QEMU.

  This issue was first discovered in an older version of QEMU, but is
  also reproduced in the latest:

  # git rev-parse HEAD
  7adfbea8fd1efce36019a0c2f198ca73be9d3f18
  # ppc-softmmu/qemu-system-ppc --version
  QEMU emulator version 5.0.91 (v5.1.0-rc1-28-g7adfbea8fd-dirty)
  Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers

  Upon further analysis a total of 39 instructions are misbehaving:

  efsabs: raised: 1, expected: 0
  efsnabs: raised: 1, expected: 0
  efsneg: raised: 1, expected: 0
  efdcfs: raised: 0, expected: 1
  efdcfsf: raised: 0, expected: 1
  efdcfsi: raised: 0, expected: 1
  efdcfuf: raised: 0, expected: 1
  efdcfui: raised: 0, expected: 1
  efdctsf: raised: 0, expected: 1
  efdctsi: raised: 0, expected: 1
  efdctsiz: raised: 0, expected: 1
  efdctuf: raised: 0, expected: 1
  efdctui: raised: 0, expected: 1
  efdctuiz: raised: 0, expected: 1
  efscfd: raised: 0, expected: 1
  evfscfsf: raised: 0, expected: 1
  evfscfsi: raised: 0, expected: 1
  evfscfuf: raised: 0, expected: 1
  evfscfui: raised: 0, expected: 1
  evfsctsf: raised: 0, expected: 1
  evfsctsi: raised: 0, expected: 1
  evfsctsiz: raised: 0, expected: 1
  evfsctuf: raised: 0, expected: 1
  evfsctui: raised: 0, expected: 1
  evfsctuiz: raised: 0, expected: 1
  brinc: raised: 0, expected: 1
  efsadd: raised: 1, expected: 0
  efsdiv: raised: 1, expected: 0
  efsmul: raised: 1, expected: 0
  efssub: raised: 1, expected: 0
  evsplatfi: raised: 0, expected: 1
  evsplati: raised: 0, expected: 1
  efscmpeq: raised: 1, expected: 0
  efscmpgt: raised: 1, expected: 0
  efscmplt: raised: 1, expected: 0
  efststeq: raised: 1, expected: 0
  efststgt: raised: 1, expected: 0
  efststlt: raised: 1, expected: 0
  evsel: raised: 0, expected: 1

  When "raised" is 0 and "expected" is 1, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was not generated while it should have.
  When "raised" is 1 and "expected" is 0, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was generated while it should not have 
(Bug #1611394).

  A comprehensive program testing all the instructions listed in the
  Signal Processing Engine (SPE) Programming Environments Manual, Rev. 0
  is posted in the comments of this ticket, and can be used to reproduce
  the issue, and validate the future fix.

To manage 

[Bug 1888918] Re: qemu-ppc: Floating point instructions do not properly generate the SPE/Embedded Floating-Point Unavailable interrupt

[Matthieu Bucchianeri]

Attaching the output of the test module (above) when run on a real
MPC8548. This is to establish a baseline validating which instructions
shall or shall not generate the SPE/Embedded Floating-Point Unavailable
interrupt.

Note that on the MPC8548, it is observed that the "brinc" instruction
does generate the interrupt, which contradicts section 4.2.3
SPE/Embedded Floating-Point Unavailable Interrupt of the Signal
Processing Engine (SPE) Programming Environments Manual, Rev. 0 (see the
quote in the description). The test program was modified to pass 100% on
real hardware, hence claiming that "brinc" shall generate the interrupt.

** Attachment added: "Results on real hardware (MPC8548)"
   
https://bugs.launchpad.net/qemu/+bug/1888918/+attachment/5395640/+files/result_mpc8548.txt

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

Title:
  qemu-ppc: Floating point instructions do not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt

Status in QEMU:
  New

Bug description:
  When emulating certain floating point instructions or vector
  instructions on PowerPC machines, QEMU does not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt.

  As described in the Signal Processing Engine (SPE) Programming
  Environments Manual, Rev. 0, available at https://www.nxp.com/docs/en
  /reference-manual/SPEPEM.pdf:

  > An SPE/embedded floating-point unavailable exception occurs on an attempt 
to execute any of the
  > following instructions and MSR[SPV] is not set:
  > * SPE instruction (except brinc)
  > * An embedded scalar double-precision instruction
  > * A vector single-precision floating-point instructions
  > It is not used by embedded scalar single-precision floating-point 
instructions

  This behavior was partially reported in Bug #1611394, however the
  issue is larger than what is described in that bug. As mentioned in
  that bug, some single-precision instructions generate the exception
  (while they should not), which is incorrect but does not typically
  produce an incorrect output. What is more of an issue is that several
  double-precision and vector instructions do not generate the exception
  (while they should), and this break support for lazy FPU/vector
  context switching in Linux (for example).

  The upper 32-bit of the double-precision/vector registers (which are
  in fact hidden in the general purpose registers) is not properly
  saved/restored, and this causes arithmetic errors. This was observed
  very frequently on a commercial project that does a lot of double-
  precision computations. The application works perfectly fine on an
  MPC8548 CPU, but fails often with QEMU.

  The issue can be reproduced using the attached Linux program "spe-
  bug.c". This program properly prints the number 42 (as the result of
  some very simple double-precision computation) on real PowerPC
  hardware, but prints an incorrect result (typically 0) on QEMU.

  This issue was first discovered in an older version of QEMU, but is
  also reproduced in the latest:

  # git rev-parse HEAD
  7adfbea8fd1efce36019a0c2f198ca73be9d3f18
  # ppc-softmmu/qemu-system-ppc --version
  QEMU emulator version 5.0.91 (v5.1.0-rc1-28-g7adfbea8fd-dirty)
  Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers

  Upon further analysis a total of 39 instructions are misbehaving:

  efsabs: raised: 1, expected: 0
  efsnabs: raised: 1, expected: 0
  efsneg: raised: 1, expected: 0
  efdcfs: raised: 0, expected: 1
  efdcfsf: raised: 0, expected: 1
  efdcfsi: raised: 0, expected: 1
  efdcfuf: raised: 0, expected: 1
  efdcfui: raised: 0, expected: 1
  efdctsf: raised: 0, expected: 1
  efdctsi: raised: 0, expected: 1
  efdctsiz: raised: 0, expected: 1
  efdctuf: raised: 0, expected: 1
  efdctui: raised: 0, expected: 1
  efdctuiz: raised: 0, expected: 1
  efscfd: raised: 0, expected: 1
  evfscfsf: raised: 0, expected: 1
  evfscfsi: raised: 0, expected: 1
  evfscfuf: raised: 0, expected: 1
  evfscfui: raised: 0, expected: 1
  evfsctsf: raised: 0, expected: 1
  evfsctsi: raised: 0, expected: 1
  evfsctsiz: raised: 0, expected: 1
  evfsctuf: raised: 0, expected: 1
  evfsctui: raised: 0, expected: 1
  evfsctuiz: raised: 0, expected: 1
  brinc: raised: 0, expected: 1
  efsadd: raised: 1, expected: 0
  efsdiv: raised: 1, expected: 0
  efsmul: raised: 1, expected: 0
  efssub: raised: 1, expected: 0
  evsplatfi: raised: 0, expected: 1
  evsplati: raised: 0, expected: 1
  efscmpeq: raised: 1, expected: 0
  efscmpgt: raised: 1, expected: 0
  efscmplt: raised: 1, expected: 0
  efststeq: raised: 1, expected: 0
  efststgt: raised: 1, expected: 0
  efststlt: raised: 1, expected: 0
  evsel: raised: 0, expected: 1

  When "raised" is 0 and "expected" is 1, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was not generated while it should have.
  When "raised" is 1 and 

[Bug 1888918] [NEW] qemu-ppc: Floating point instructions do not properly generate the SPE/Embedded Floating-Point Unavailable interrupt

[Matthieu Bucchianeri]

Public bug reported:

When emulating certain floating point instructions or vector
instructions on PowerPC machines, QEMU does not properly generate the
SPE/Embedded Floating-Point Unavailable interrupt.

As described in the Signal Processing Engine (SPE) Programming
Environments Manual, Rev. 0, available at https://www.nxp.com/docs/en
/reference-manual/SPEPEM.pdf:

> An SPE/embedded floating-point unavailable exception occurs on an attempt to 
> execute any of the
> following instructions and MSR[SPV] is not set:
> * SPE instruction (except brinc)
> * An embedded scalar double-precision instruction
> * A vector single-precision floating-point instructions
> It is not used by embedded scalar single-precision floating-point instructions

This behavior was partially reported in Bug #1611394, however the issue
is larger than what is described in that bug. As mentioned in that bug,
some single-precision instructions generate the exception (while they
should not), which is incorrect but does not typically produce an
incorrect output. What is more of an issue is that several double-
precision and vector instructions do not generate the exception (while
they should), and this break support for lazy FPU/vector context
switching in Linux (for example).

The upper 32-bit of the double-precision/vector registers (which are in
fact hidden in the general purpose registers) is not properly
saved/restored, and this causes arithmetic errors. This was observed
very frequently on a commercial project that does a lot of double-
precision computations. The application works perfectly fine on an
MPC8548 CPU, but fails often with QEMU.

The issue can be reproduced using the attached Linux program "spe-
bug.c". This program properly prints the number 42 (as the result of
some very simple double-precision computation) on real PowerPC hardware,
but prints an incorrect result (typically 0) on QEMU.

This issue was first discovered in an older version of QEMU, but is also
reproduced in the latest:

# git rev-parse HEAD
7adfbea8fd1efce36019a0c2f198ca73be9d3f18
# ppc-softmmu/qemu-system-ppc --version
QEMU emulator version 5.0.91 (v5.1.0-rc1-28-g7adfbea8fd-dirty)
Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers

Upon further analysis a total of 39 instructions are misbehaving:

efsabs: raised: 1, expected: 0
efsnabs: raised: 1, expected: 0
efsneg: raised: 1, expected: 0
efdcfs: raised: 0, expected: 1
efdcfsf: raised: 0, expected: 1
efdcfsi: raised: 0, expected: 1
efdcfuf: raised: 0, expected: 1
efdcfui: raised: 0, expected: 1
efdctsf: raised: 0, expected: 1
efdctsi: raised: 0, expected: 1
efdctsiz: raised: 0, expected: 1
efdctuf: raised: 0, expected: 1
efdctui: raised: 0, expected: 1
efdctuiz: raised: 0, expected: 1
efscfd: raised: 0, expected: 1
evfscfsf: raised: 0, expected: 1
evfscfsi: raised: 0, expected: 1
evfscfuf: raised: 0, expected: 1
evfscfui: raised: 0, expected: 1
evfsctsf: raised: 0, expected: 1
evfsctsi: raised: 0, expected: 1
evfsctsiz: raised: 0, expected: 1
evfsctuf: raised: 0, expected: 1
evfsctui: raised: 0, expected: 1
evfsctuiz: raised: 0, expected: 1
brinc: raised: 0, expected: 1
efsadd: raised: 1, expected: 0
efsdiv: raised: 1, expected: 0
efsmul: raised: 1, expected: 0
efssub: raised: 1, expected: 0
evsplatfi: raised: 0, expected: 1
evsplati: raised: 0, expected: 1
efscmpeq: raised: 1, expected: 0
efscmpgt: raised: 1, expected: 0
efscmplt: raised: 1, expected: 0
efststeq: raised: 1, expected: 0
efststgt: raised: 1, expected: 0
efststlt: raised: 1, expected: 0
evsel: raised: 0, expected: 1

When "raised" is 0 and "expected" is 1, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was not generated while it should have.
When "raised" is 1 and "expected" is 0, this means that the SPE/Embedded 
Floating-Point Unavailable interrupt was generated while it should not have 
(Bug #1611394).

A comprehensive program testing all the instructions listed in the
Signal Processing Engine (SPE) Programming Environments Manual, Rev. 0
is posted in the comments of this ticket, and can be used to reproduce
the issue, and validate the future fix.

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: floating ppc spe

** Attachment added: "Linux userspace application to reproduce the issue"
   https://bugs.launchpad.net/bugs/1888918/+attachment/5395638/+files/spe-bug.c

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

Title:
  qemu-ppc: Floating point instructions do not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt

Status in QEMU:
  New

Bug description:
  When emulating certain floating point instructions or vector
  instructions on PowerPC machines, QEMU does not properly generate the
  SPE/Embedded Floating-Point Unavailable interrupt.

  As described in the Signal Processing Engine (SPE) Programming
  Environments Manual, Rev. 0, 

Re: [PATCH 1/1] scripts/performance: Add bisect.py script

[John Snow]

On 7/21/20 7:15 PM, Ahmed Karaman wrote:

Python script that locates the commit that caused a performance
degradation or improvement in QEMU using the git bisect command
(binary search).

Syntax:
bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
--target TARGET --tool {perf,callgrind} -- \
 []

[-h] - Print the script arguments help message
-s,--start START - First commit hash in the search range
[-e,--end END] - Last commit hash in the search range
 (default: Latest commit)
[-q,--qemu QEMU] - QEMU path.
 (default: Path to a GitHub QEMU clone)
--target TARGET - QEMU target name
--tool {perf,callgrind} - Underlying tool used for measurements

Example of usage:
bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc \
--tool=perf -- coulomb_double-ppc -n 1000

Example output:
Start Commit Instructions: 12,710,790,060
End Commit Instructions:   13,031,083,512
Performance Change:-2.458%

Estimated Number of Steps: 10

*BISECT STEP 1*
Instructions:13,031,097,790
Status:  slow commit
*BISECT STEP 2*
Instructions:12,710,805,265
Status:  fast commit
*BISECT STEP 3*
Instructions:13,031,028,053
Status:  slow commit
*BISECT STEP 4*
Instructions:12,711,763,211
Status:  fast commit
*BISECT STEP 5*
Instructions:13,031,027,292
Status:  slow commit
*BISECT STEP 6*
Instructions:12,711,748,738
Status:  fast commit
*BISECT STEP 7*
Instructions:12,711,748,788
Status:  fast commit
*BISECT STEP 8*
Instructions:13,031,100,493
Status:  slow commit
*BISECT STEP 9*
Instructions:12,714,472,954
Status:  fast commit
BISECT STEP 10*
Instructions:12,715,409,153
Status:  fast commit
BISECT STEP 11*
Instructions:12,715,394,739
Status:  fast commit

*BISECT RESULT*
commit 0673ecdf6cb2b1445a85283db8cbacb251c46516
Author: Richard Henderson 
Date:   Tue May 5 10:40:23 2020 -0700

 softfloat: Inline float64 compare specializations

 Replace the float64 compare specializations with inline functions
 that call the standard float64_compare{,_quiet} functions.
 Use bool as the return type.
***

Signed-off-by: Ahmed Karaman 


Howdy!

Can you run this through some python linters?
Try pylint 2.4.4+ and flake8 3.8.3+.

`pylint --disable=too-many-locals,too-many-statements bisect.py` would 
be a good target to hit; there's not too many warnings. Mostly, it wants 
you to specify the check= parameter to subprocess.run().



---
  scripts/performance/bisect.py | 374 ++
  1 file changed, 374 insertions(+)
  create mode 100755 scripts/performance/bisect.py

diff --git a/scripts/performance/bisect.py b/scripts/performance/bisect.py
new file mode 100755
index 00..869cc69ef4
--- /dev/null
+++ b/scripts/performance/bisect.py
@@ -0,0 +1,374 @@
+#!/usr/bin/env python3
+


The following preamble could be a docstring:

"""


+#  Locate the commit that caused a performance degradation or improvement in
+#  QEMU using the git bisect command (binary search).
+#
+#  Syntax:
+#  bisect.py [-h] -s,--start START [-e,--end END] [-q,--qemu QEMU] \
+#  --target TARGET --tool {perf,callgrind} -- \
+#   []
+#
+#  [-h] - Print the script arguments help message
+#  -s,--start START - First commit hash in the search range
+#  [-e,--end END] - Last commit hash in the search range
+# (default: Latest commit)
+#  [-q,--qemu QEMU] - QEMU path.
+#  (default: Path to a GitHub QEMU clone)
+#  --target TARGET - QEMU target name
+#  --tool {perf,callgrind} - Underlying tool used for measurements
+
+#  Example of usage:
+#  bisect.py --start=fdd76fecdd --qemu=/path/to/qemu --target=ppc --tool=perf \
+#  -- coulomb_double-ppc -n 1000
+#
+#  This file is a part of the project "TCG Continuous Benchmarking".


"""


+#
+#  Copyright (C) 2020  Ahmed Karaman 
+#  Copyright (C) 2020  Aleksandar Markovic 
+#
+#  This program is free software: you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation, either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+#  GNU General Public License for more 

Re: [PATCH] target/unicore32: Remove CURSES stuff from the Makefile.objs

[Guan Xuetao]

That's OK for unicore32 target.

Acked-by: Guan Xuetao 


> -原始邮件-
> 发件人: "Thomas Huth" 
> 发送时间: 2020-07-23 21:22:19 (星期四)
> 收件人: "Guan Xuetao" , qemu-devel@nongnu.org
> 抄送: "Philippe Mathieu-Daudé" , qemu-triv...@nongnu.org, 
> "Richard Henderson" 
> 主题: [PATCH] target/unicore32: Remove CURSES stuff from the Makefile.objs
> 
> The dependency on curses has been removed in commit c7a856b42e403e2b
> ("target/unicore32: Prefer qemu_semihosting_log_out() over curses").
> So we can remove the related lines in the Makefile now, too.
> 
> Signed-off-by: Thomas Huth 
> ---
>  target/unicore32/Makefile.objs | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/target/unicore32/Makefile.objs b/target/unicore32/Makefile.objs
> index 35d8bf530d..6b41b1e9ef 100644
> --- a/target/unicore32/Makefile.objs
> +++ b/target/unicore32/Makefile.objs
> @@ -2,7 +2,3 @@ obj-y += translate.o op_helper.o helper.o cpu.o
>  obj-y += ucf64_helper.o
>  
>  obj-$(CONFIG_SOFTMMU) += softmmu.o
> -
> -# Huh? Uses curses directly instead of using ui/console.h interfaces ...
> -helper.o-cflags := $(CURSES_CFLAGS)
> -helper.o-libs := $(CURSES_LIBS)
> -- 
> 2.18.1

Re: [BUG] vhost-vdpa: qemu-system-s390x crashes with second virtio-net-ccw device

[Jason Wang]

On 2020/7/24 下午11:34, Cornelia Huck wrote:

On Fri, 24 Jul 2020 11:17:57 -0400
"Michael S. Tsirkin"  wrote:


On Fri, Jul 24, 2020 at 04:56:27PM +0200, Cornelia Huck wrote:

On Fri, 24 Jul 2020 09:30:58 -0400
"Michael S. Tsirkin"  wrote:
   

On Fri, Jul 24, 2020 at 03:27:18PM +0200, Cornelia Huck wrote:

When I start qemu with a second virtio-net-ccw device (i.e. adding
-device virtio-net-ccw in addition to the autogenerated device), I get
a segfault. gdb points to

#0  0x55d6ab52681d in virtio_net_get_config (vdev=,
 config=0x55d6ad9e3f80 "RT") at 
/home/cohuck/git/qemu/hw/net/virtio-net.c:146
146 if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {

(backtrace doesn't go further)

The core was incomplete, but running under gdb directly shows that it
is just a bog-standard config space access (first for that device).

The cause of the crash is that nc->peer is not set... no idea how that
can happen, not that familiar with that part of QEMU. (Should the code
check, or is that really something that should not happen?)

What I don't understand is why it is set correctly for the first,
autogenerated virtio-net-ccw device, but not for the second one, and
why virtio-net-pci doesn't show these problems. The only difference
between -ccw and -pci that comes to my mind here is that config space
accesses for ccw are done via an asynchronous operation, so timing
might be different.

Hopefully Jason has an idea. Could you post a full command line
please? Do you need a working guest to trigger this? Does this trigger
on an x86 host?

Yes, it does trigger with tcg-on-x86 as well. I've been using

s390x-softmmu/qemu-system-s390x -M s390-ccw-virtio,accel=tcg -cpu qemu,zpci=on
-m 1024 -nographic -device virtio-scsi-ccw,id=scsi0,devno=fe.0.0001
-drive file=/path/to/image,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
-device virtio-net-ccw

It seems it needs the guest actually doing something with the nics; I
cannot reproduce the crash if I use the old advent calendar moon buggy
image and just add a virtio-net-ccw device.

(I don't think it's a problem with my local build, as I see the problem
both on my laptop and on an LPAR.)



It looks to me we forget the check the existence of peer.

Please try the attached patch to see if it works.

Thanks

>From f6959056dcc65cbdc256c4af2b1a0eaee784c15f Mon Sep 17 00:00:00 2001
From: Jason Wang 
Date: Sat, 25 Jul 2020 08:13:17 +0800
Subject: [PATCH] virtio-net: check the existence of peer before accesing its
 config

We try to get config from peer unconditionally which may lead NULL
pointer dereference. Add a check before trying to access the config.

Fixes: 108a64818e69b ("vhost-vdpa: introduce vhost-vdpa backend")
Signed-off-by: Jason Wang 
---
 hw/net/virtio-net.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 4895af1cbe..935b9ef5c7 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -125,6 +125,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
 {
 VirtIONet *n = VIRTIO_NET(vdev);
 struct virtio_net_config netcfg;
+NetClientState *nc = qemu_get_queue(n->nic);
 
 int ret = 0;
 memset(, 0 , sizeof(struct virtio_net_config));
@@ -142,13 +143,12 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
  VIRTIO_NET_RSS_SUPPORTED_HASHES);
 memcpy(config, , n->config_size);
 
-NetClientState *nc = qemu_get_queue(n->nic);
-if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
 ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *),
- n->config_size);
-if (ret != -1) {
-memcpy(config, , n->config_size);
-}
+   n->config_size);
+if (ret != -1) {
+memcpy(config, , n->config_size);
+}
 }
 }
 
@@ -156,6 +156,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
 {
 VirtIONet *n = VIRTIO_NET(vdev);
 struct virtio_net_config netcfg = {};
+NetClientState *nc = qemu_get_queue(n->nic);
 
 memcpy(, config, n->config_size);
 
@@ -166,11 +167,10 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
 qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
 }
 
-NetClientState *nc = qemu_get_queue(n->nic);
-if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
-vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *),
-   0, n->config_size,
-VHOST_SET_CONFIG_TYPE_MASTER);
+if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+vhost_net_set_config(get_vhost_net(nc->peer),
+  

[PULL v1 3/3] tpm_emulator: Report an error if chardev is missing

[Stefan Berger]

This patch fixes the odd error reporting when trying to send a file
descriptor to the TPM emulator if one has not passed a valid chardev.

$ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Failed to send 
CMD_SET_DATAFD: Success
qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Could not cleanly 
shutdown the TPM: Success

This is the new error report:

$ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: parameter 'chardev' 
is missing

This change does not hide the display of supported TPM types if a non-existent 
type is passed:

$ x86_64-softmmu/qemu-system-x86_64 -tpmdev nonexistent,id=tpm0
qemu-system-x86_64: -tpmdev nonexistent,id=tpm0: Parameter 'type' expects a TPM 
backend type
Supported TPM types (choose only one):
 passthrough   Passthrough TPM backend driver
emulator   TPM emulator backend driver

Signed-off-by: Stefan Berger 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 backends/tpm/tpm_emulator.c | 38 ++---
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index 9605339f93..a9b0f55e67 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -549,27 +549,30 @@ err_exit:
 static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, QemuOpts 
*opts)
 {
 const char *value;
+Error *err = NULL;
+Chardev *dev;
 
 value = qemu_opt_get(opts, "chardev");
-if (value) {
-Error *err = NULL;
-Chardev *dev = qemu_chr_find(value);
-
-if (!dev) {
-error_report("tpm-emulator: tpm chardev '%s' not found.", value);
-goto err;
-}
+if (!value) {
+error_report("tpm-emulator: parameter 'chardev' is missing");
+goto err;
+}
 
-if (!qemu_chr_fe_init(_emu->ctrl_chr, dev, )) {
-error_prepend(, "tpm-emulator: No valid chardev found at 
'%s':",
-  value);
-error_report_err(err);
-goto err;
-}
+dev = qemu_chr_find(value);
+if (!dev) {
+error_report("tpm-emulator: tpm chardev '%s' not found", value);
+goto err;
+}
 
-tpm_emu->options->chardev = g_strdup(value);
+if (!qemu_chr_fe_init(_emu->ctrl_chr, dev, )) {
+error_prepend(, "tpm-emulator: No valid chardev found at '%s':",
+  value);
+error_report_err(err);
+goto err;
 }
 
+tpm_emu->options->chardev = g_strdup(value);
+
 if (tpm_emulator_prepare_data_fd(tpm_emu) < 0) {
 goto err;
 }
@@ -925,6 +928,11 @@ static void tpm_emulator_shutdown(TPMEmulator *tpm_emu)
 {
 ptm_res res;
 
+if (!tpm_emu->options->chardev) {
+/* was never properly initialized */
+return;
+}
+
 if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SHUTDOWN, , 0, sizeof(res)) < 0) 
{
 error_report("tpm-emulator: Could not cleanly shutdown the TPM: %s",
  strerror(errno));
-- 
2.24.1

[PULL v1 1/3] Revert "tpm: Clean up error reporting in tpm_init_tpmdev()"

[Stefan Berger]

From: Markus Armbruster 

This reverts commit d10e05f15d5c3dd5e5cc59c5dfff460d89d48580.

We report some -tpmdev failures, but then continue as if all was fine.
Reproducer:

$ qemu-system-x86_64 -nodefaults -S -display none -monitor stdio -chardev 
null,id=tpm0 -tpmdev emulator,id=tpm0,chardev=chrtpm -device tpm-tis,tpmdev=tpm0
qemu-system-x86_64: -tpmdev emulator,id=tpm0,chardev=chrtpm: tpm-emulator: 
tpm chardev 'chrtpm' not found.
qemu-system-x86_64: -tpmdev emulator,id=tpm0,chardev=chrtpm: tpm-emulator: 
Could not cleanly shutdown the TPM: No such file or directory
QEMU 5.0.90 monitor - type 'help' for more information
(qemu) qemu-system-x86_64: -device tpm-tis,tpmdev=tpm0: Property 
'tpm-tis.tpmdev' can't find value 'tpm0'
$ echo $?
1

This is a regression caused by commit d10e05f15d "tpm: Clean up error
reporting in tpm_init_tpmdev()".  It's incomplete: be->create(opts)
continues to use error_report(), and we don't set an error when it
fails.

I figure converting the create() methods to Error would make some
sense, but I'm not sure it's worth the effort right now.  Revert the
broken commit instead, and add a comment to tpm_init_tpmdev().

Straightforward conflict in tpm.c resolved.

Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Stefan Berger 
Signed-off-by: Stefan Berger 
---
 include/sysemu/tpm.h |  2 +-
 softmmu/vl.c |  4 +++-
 stubs/tpm.c  |  3 ++-
 tpm.c| 30 +-
 4 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 03fb25941c..730c61ac97 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -16,7 +16,7 @@
 #include "qom/object.h"
 
 int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
-void tpm_init(void);
+int tpm_init(void);
 void tpm_cleanup(void);
 
 typedef enum TPMVersion {
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 3416241557..660537a709 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -4258,7 +4258,9 @@ void qemu_init(int argc, char **argv, char **envp)
   user_creatable_add_opts_foreach,
   object_create_delayed, _fatal);
 
-tpm_init();
+if (tpm_init() < 0) {
+exit(1);
+}
 
 blk_mig_init();
 ram_mig_init();
diff --git a/stubs/tpm.c b/stubs/tpm.c
index 66c99d667d..9bded191d9 100644
--- a/stubs/tpm.c
+++ b/stubs/tpm.c
@@ -10,8 +10,9 @@
 #include "sysemu/tpm.h"
 #include "hw/acpi/tpm.h"
 
-void tpm_init(void)
+int tpm_init(void)
 {
+return 0;
 }
 
 void tpm_cleanup(void)
diff --git a/tpm.c b/tpm.c
index fe03b24858..f6045bb6da 100644
--- a/tpm.c
+++ b/tpm.c
@@ -81,26 +81,33 @@ TPMBackend *qemu_find_tpm_be(const char *id)
 
 static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
 {
+/*
+ * Use of error_report() in a function with an Error ** parameter
+ * is suspicious.  It is okay here.  The parameter only exists to
+ * make the function usable with qemu_opts_foreach().  It is not
+ * actually used.
+ */
 const char *value;
 const char *id;
 const TPMBackendClass *be;
 TPMBackend *drv;
+Error *local_err = NULL;
 int i;
 
 if (!QLIST_EMPTY(_backends)) {
-error_setg(errp, "Only one TPM is allowed.");
+error_report("Only one TPM is allowed.");
 return 1;
 }
 
 id = qemu_opts_id(opts);
 if (id == NULL) {
-error_setg(errp, QERR_MISSING_PARAMETER, "id");
+error_report(QERR_MISSING_PARAMETER, "id");
 return 1;
 }
 
 value = qemu_opt_get(opts, "type");
 if (!value) {
-error_setg(errp, QERR_MISSING_PARAMETER, "type");
+error_report(QERR_MISSING_PARAMETER, "type");
 tpm_display_backend_drivers();
 return 1;
 }
@@ -108,14 +115,15 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, 
Error **errp)
 i = qapi_enum_parse(_lookup, value, -1, NULL);
 be = i >= 0 ? tpm_be_find_by_type(i) : NULL;
 if (be == NULL) {
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
-   "a TPM backend type");
+error_report(QERR_INVALID_PARAMETER_VALUE,
+ "type", "a TPM backend type");
 tpm_display_backend_drivers();
 return 1;
 }
 
 /* validate backend specific opts */
-if (!qemu_opts_validate(opts, be->opts, errp)) {
+if (!qemu_opts_validate(opts, be->opts, _err)) {
+error_report_err(local_err);
 return 1;
 }
 
@@ -148,10 +156,14 @@ void tpm_cleanup(void)
  * Initialize the TPM. Process the tpmdev command line options describing the
  * TPM backend.
  */
-void tpm_init(void)
+int tpm_init(void)
 {
-qemu_opts_foreach(qemu_find_opts("tpmdev"),
-  tpm_init_tpmdev, NULL, _fatal);
+if (qemu_opts_foreach(qemu_find_opts("tpmdev"),
+  tpm_init_tpmdev, NULL, NULL)) {
+return -1;
+}
+
+return 

[PULL v1 2/3] tpm: Improve help on TPM types when none are available

[Stefan Berger]

From: Markus Armbruster 

Help is a bit awkward when no TPM types are built into QEMU:

$ qemu-system-x86_64 -tpmdev nonexistent,id=tpm0
qemu-system-x86_64: -tpmdev nonexistent,id=tpm0: Parameter 'type' expects a 
TPM backend type
Supported TPM types (choose only one):

Improve to

qemu-system-x86_64: -tpmdev nonexistent,id=tpm0: Parameter 'type' expects a 
TPM backend type
No TPM backend types are available

Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Stefan Berger 
Signed-off-by: Stefan Berger 
---
 tpm.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tpm.c b/tpm.c
index f6045bb6da..cab206355a 100644
--- a/tpm.c
+++ b/tpm.c
@@ -47,18 +47,23 @@ tpm_be_find_by_type(enum TpmType type)
  */
 static void tpm_display_backend_drivers(void)
 {
+bool got_one = false;
 int i;
 
-fprintf(stderr, "Supported TPM types (choose only one):\n");
-
 for (i = 0; i < TPM_TYPE__MAX; i++) {
 const TPMBackendClass *bc = tpm_be_find_by_type(i);
 if (!bc) {
 continue;
 }
-fprintf(stderr, "%12s   %s\n", TpmType_str(i), bc->desc);
+if (!got_one) {
+error_printf("Supported TPM types (choose only one):\n");
+got_one = true;
+}
+error_printf("%12s   %s\n", TpmType_str(i), bc->desc);
+}
+if (!got_one) {
+error_printf("No TPM backend types are available\n");
 }
-fprintf(stderr, "\n");
 }
 
 /*
-- 
2.24.1

[PULL v1 0/3] Merge tpm 2020/07/24 v1

[Stefan Berger]

This series of patches corrects bad error reporting due to erroneous
or missing TPM related command line parameters.

  Regards,
Stefan

The following changes since commit 7adfbea8fd1efce36019a0c2f198ca73be9d3f18:

  Merge remote-tracking branch 
'remotes/ehabkost/tags/x86-next-for-5.1-pull-request' into staging (2020-07-24 
10:52:20 +0100)

are available in the Git repository at:

  git://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2020-07-24-1

for you to fetch changes up to 88f830745721ba8c9e9d2831c01045a6f130c1a6:

  tpm_emulator: Report an error if chardev is missing (2020-07-24 12:44:13 
-0400)


Markus Armbruster (2):
  Revert "tpm: Clean up error reporting in tpm_init_tpmdev()"
  tpm: Improve help on TPM types when none are available

Stefan Berger (1):
  tpm_emulator: Report an error if chardev is missing

 backends/tpm/tpm_emulator.c | 38 +++---
 include/sysemu/tpm.h|  2 +-
 softmmu/vl.c|  4 +++-
 stubs/tpm.c |  3 ++-
 tpm.c   | 43 ++-
 5 files changed, 59 insertions(+), 31 deletions(-)

-- 
2.24.1

Re: [PATCH v5 6/6] target/ppc: add vmsumudm vmsumcud instructions

[Lijun Pan]

> On Jul 24, 2020, at 1:46 PM, Lijun Pan  wrote:
> 
> 
> 
>> On Jul 24, 2020, at 1:00 PM, Richard Henderson > > wrote:
>> 
>> On 7/23/20 9:58 PM, Lijun Pan wrote:
>>> vmsumudm (Power ISA 3.0) - Vector Multiply-Sum Unsigned Doubleword Modulo
>>> VA-form.
>>> vmsumcud (Power ISA 3.1) - Vector Multiply-Sum & write Carry-out Unsigned
>>> Doubleword VA-form.
>>> 
>>> Signed-off-by: Lijun Pan mailto:l...@linux.ibm.com>>
>>> ---
>>> v5: update instruction flag for vmsumcud.
>>>integrate into this isa3.1 patch series
>>> v3: implement vmsumudm/vmsumcud through int128 functions,
>>>suggested by Richard Henderson.
>>> 
>>> disas/ppc.c |  2 ++
>>> target/ppc/helper.h |  4 ++-
>>> target/ppc/int_helper.c | 49 -
>>> target/ppc/translate.c  |  1 -
>>> target/ppc/translate/vmx-impl.inc.c | 39 ---
>>> target/ppc/translate/vmx-ops.inc.c  |  2 ++
>>> 6 files changed, 76 insertions(+), 21 deletions(-)
>>> 
>>> diff --git a/disas/ppc.c b/disas/ppc.c
>>> index 63e97cfe1d..bd76fae4c4 100644
>>> --- a/disas/ppc.c
>>> +++ b/disas/ppc.c
>>> @@ -2261,7 +2261,9 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> { "vmsumshs",  VXA(4,  41), VXA_MASK,   PPCVEC, { VD, VA, VB, 
>>> VC } },
>>> { "vmsumubm",  VXA(4,  36), VXA_MASK,   PPCVEC, { VD, VA, VB, 
>>> VC } },
>>> { "vmsumuhm",  VXA(4,  38), VXA_MASK,   PPCVEC, { VD, VA, VB, 
>>> VC } },
>>> +{ "vmsumudm",  VXA(4,  35), VXA_MASK,   PPCVEC, { VD, VA, VB, VC } 
>>> },
>>> { "vmsumuhs",  VXA(4,  39), VXA_MASK,   PPCVEC, { VD, VA, VB, 
>>> VC } },
>>> +{ "vmsumcud",  VXA(4,  23), VXA_MASK,   PPCVEC, { VD, VA, VB, VC } 
>>> },
>>> { "vmulesb",   VX(4,  776), VX_MASK,PPCVEC, { VD, VA, VB } 
>>> },
>>> { "vmulesh",   VX(4,  840), VX_MASK,PPCVEC, { VD, VA, VB } 
>>> },
>>> { "vmuleub",   VX(4,  520), VX_MASK,PPCVEC, { VD, VA, VB } 
>>> },
>>> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
>>> index 70a14029ca..00a31d64bc 100644
>>> --- a/target/ppc/helper.h
>>> +++ b/target/ppc/helper.h
>>> @@ -274,10 +274,12 @@ DEF_HELPER_3(vpkpx, void, avr, avr, avr)
>>> DEF_HELPER_5(vmhaddshs, void, env, avr, avr, avr, avr)
>>> DEF_HELPER_5(vmhraddshs, void, env, avr, avr, avr, avr)
>>> DEF_HELPER_5(vmsumuhm, void, env, avr, avr, avr, avr)
>>> +DEF_HELPER_5(vmsumudm, void, env, avr, avr, avr, avr)
>>> DEF_HELPER_5(vmsumuhs, void, env, avr, avr, avr, avr)
>>> DEF_HELPER_5(vmsumshm, void, env, avr, avr, avr, avr)
>>> DEF_HELPER_5(vmsumshs, void, env, avr, avr, avr, avr)
>>> -DEF_HELPER_4(vmladduhm, void, avr, avr, avr, avr)
>>> +DEF_HELPER_5(vmsumcud, void, env, avr, avr, avr, avr)
>>> +DEF_HELPER_5(vmladduhm, void, env, avr, avr, avr, avr)
>>> DEF_HELPER_FLAGS_2(mtvscr, TCG_CALL_NO_RWG, void, env, i32)
>>> DEF_HELPER_FLAGS_1(mfvscr, TCG_CALL_NO_RWG, i32, env)
>>> DEF_HELPER_3(lvebx, void, env, avr, tl)
>>> diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
>>> index 62b93b4568..2e919a7b8e 100644
>>> --- a/target/ppc/int_helper.c
>>> +++ b/target/ppc/int_helper.c
>>> @@ -913,7 +913,8 @@ void helper_vmhraddshs(CPUPPCState *env, ppc_avr_t *r, 
>>> ppc_avr_t *a,
>>> }
>>> }
>>> 
>>> -void helper_vmladduhm(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t 
>>> *c)
>>> +void helper_vmladduhm(CPUPPCState *env, ppc_avr_t *r, ppc_avr_t *a,
>>> +  ppc_avr_t *b, ppc_avr_t *c)
>> 
>> Why?
>> 
>>> +void helper_vmsumudm(CPUPPCState *env, ppc_avr_t *r,
>>> + ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)
>>> +{
>> 
>> ...
>> 
>>> +void helper_vmsumcud(CPUPPCState *env, ppc_avr_t *r,
>>> + ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)
>> 
>> You don't actually use env in either helper, so you shouldn't pass it in.
>> 
>> 
> 
> I just wanted to reuse GEN_VAFORM_PAIRED which has env passed in, 
> rather than creating a GEN_VAFORM_PAIRED_NOENV which does not have env.
> I created GEN_VAFORM which includes env so that it can cover both env and 
> non-env case.
> 
> I re-look at the code, and find out that GEN_**FORM** differentiate in env 
> and non-env.
> So I can rename current GEN_VAFORM_PAIRED (env passed in) to 
> GEN_VAFORM_PAIRED_ENV,
> create a new GEN_VAFORM_PAIRED (no env) to cater vmladduhm and vmsumudm
> remove the env part code in GEN_VAFORM to have vmsumcud fit into.
> 

I take back what I said in the second paragraph.
I think we need to keep GEN_VAFORM_PAIRED having env to cover both env and 
non-env cases
because some pair has both non-env, some pair has both env, some pair has env 
and non-env.
e.g. GEN_VAFORM_PAIRED(vmsumuhm, vmsumuhs, 19)  vmsumuhm non-env, vmsumuhs env.

It is the same for GEN_VAFORM. though for now, no instructions uses env, adding 
env to
GEN_VAFORM saves for later changes.

Lijun

Re: Testing the virtio-vhost-user QEMU patch

[Alyssa Ross]

Stefan Hajnoczi  writes:

> On Fri, Jul 24, 2020 at 10:58:45AM +, Alyssa Ross wrote:
>> Alyssa Ross  writes:
>> 
>> > Stefan Hajnoczi  writes:
>> >
>> >> On Tue, Jul 21, 2020 at 07:14:38AM +, Alyssa Ross wrote:
>> >>> Hi -- I hope it's okay me reaching out like this.
>> >>> 
>> >>> I've been trying to test out the virtio-vhost-user implementation that's
>> >>> been posted to this list a couple of times, but have been unable to get
>> >>> it to boot a kernel following the steps listed either on
>> >>>  or
>> >>> .
>> >>> 
>> >>> Specifically, the kernel appears to be unable to write to the
>> >>> virtio-vhost-user device's PCI registers.  I've included the full panic
>> >>> output from the kernel at the end of this message.  The panic is
>> >>> reproducible with two different kernels I tried (with different configs
>> >>> and versions).  I tried both versions of the virtio-vhost-user I was
>> >>> able to find[1][2], and both exhibited the same behaviour.
>> >>> 
>> >>> Is this a known issue?  Am I doing something wrong?
>> >>
>> >> Hi,
>> >> Unfortunately I'm not sure what the issue is. This is an early
>> >> virtio-pci register access before a driver for any specific device type
>> >> (net, blk, vhost-user, etc) comes into play.
>> >
>> > Small update here: I tried on another computer, and it worked.  Made
>> > sure that it was exactly the same QEMU binary, command line, and VM
>> > disk/initrd/kernel, so I think I can fairly confidently say the panic
>> > depends on what hardware QEMU is running on.  I set -cpu value to the
>> > same on both as well (SandyBridge).
>> >
>> > I also discovered that it works on my primary computer (the one it
>> > panicked on before) with KVM disabled.
>> >
>> > Note that I've only got so far as finding that it boots on the other
>> > machine -- I haven't verified yet that it actually works.
>> >
>> > Bad host CPU:  Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz
>> > Good host CPU: AMD EPYC 7401P 24-Core Processor
>> >
>> > May I ask what host CPUs other people have tested this on?  Having more
>> > data would probably be useful.  Could it be an AMD vs. Intel thing?
>> 
>> I think I've figured it out!
>> 
>> Sandy Bridge and Ivy Bridge hosts encounter this panic because the
>> "additional resources" bar size is too big, at 1 << 36.  If I change
>> this to 1 << 35, no more kernel panic.
>> 
>> Skylake and later are fine with 1 << 36.  In between Ivy Bridge and
>> Skylake were Haswell and Broadwell, but I couldn't find anybody who was
>> able to help me test on either of those, so I don't know what they do.
>> 
>> Perhaps related, the hosts that produce panics all seem to have a
>> physical address size of 36 bits, while the hosts that work have larger
>> physical address sizes, as reported by lscpu.
>
> I have run it successfully on Broadwell but never tried 64GB or larger
> shared memory resources.

To clarify, I haven't been using big shared memory resources either --
this has all been about getting the backend VM to start at all.  The
panic happens at boot, and the 1 << 36 BAR allocation comes from here,
during realization:
https://github.com/ndragazis/qemu/blob/f9ab08c0c8/hw/virtio/virtio-vhost-user-pci.c#L291

[Bug 1888728] Re: Bare chroot in linux-user fails with pgb_reserved_va: Assertion `guest_base != 0' failed.

[Richard Henderson]

For the record, reproducing this problem requires root, thus
trying to reproduce it outside of a chroot is non-obvious.

https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg07224.html

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

** Changed in: qemu
 Assignee: (unassigned) => Richard Henderson (rth)

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

Title:
  Bare chroot in linux-user fails with pgb_reserved_va: Assertion
  `guest_base != 0' failed.

Status in QEMU:
  In Progress

Bug description:
  Trying to run a bare chroot with no additional bind mounts fails on
  git master (8ffa52c20d5693d454f65f2024a1494edfea65d4) with:

  root@nofan:~/qemu> chroot /local_scratch/sid-m68k-sbuild/
  qemu-m68k-static: /root/qemu/linux-user/elfload.c:2315: pgb_reserved_va: 
Assertion `guest_base != 0' failed.
  Aborted
  root@nofan:~/qemu>

  The problem can be worked around by bind-mounting /proc from the host
  system into the target chroot:

  root@nofan:~/qemu> mount -o bind /proc/ /local_scratch/sid-m68k-sbuild/proc/
  root@nofan:~/qemu> chroot /local_scratch/sid-m68k-sbuild/
  bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
  (sid-m68k-sbuild)root@nofan:/#

  Host system is an up-to-date Debian unstable (2020-07-23).

  I have not been able to bisect the issue yet since there is another
  annoying linux-user bug (virtual memory exhaustion) that was somewhere
  introduced and fixed between v5.0.0 and HEAD and overshadows the
  original Assertion failure bug.

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

Re: [PATCH for-5.1?] target/i386: Save cc_op before loop insns

[Richard Henderson]

On 7/24/20 11:53 AM, Paolo Bonzini wrote:
> Looks good, will queue when I am back---or just send a pull request yourself 
> if
> you prefer.

Ok, will do.  Thanks.


r~

> 
> Paolo
> 
> Il ven 24 lug 2020, 20:35 Richard Henderson  > ha scritto:
> 
> Ping?
> 
> On 7/20/20 8:40 AM, Richard Henderson wrote:
> > We forgot to update cc_op before these branch insns,
> > which lead to losing track of the current eflags.
> >
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1888165
> > Signed-off-by: Richard Henderson  >
> > ---
> >  target/i386/translate.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/target/i386/translate.c b/target/i386/translate.c
> > index a1d31f09c1..caea6f5fb1 100644
> > --- a/target/i386/translate.c
> > +++ b/target/i386/translate.c
> > @@ -7148,6 +7148,7 @@ static target_ulong disas_insn(DisasContext *s,
> CPUState *cpu)
> >              l1 = gen_new_label();
> >              l2 = gen_new_label();
> >              l3 = gen_new_label();
> > +            gen_update_cc_op(s);
> >              b &= 3;
> >              switch(b) {
> >              case 0: /* loopnz */
> >
> 

[PATCH] linux-user: Ensure mmap_min_addr is non-zero

[Richard Henderson]

When the chroot does not have /proc mounted, we can read neither
/proc/sys/vm/mmap_min_addr nor /proc/sys/maps.

The enforcement of mmap_min_addr in the host kernel is done by
the security module, and so does not apply to processes owned
by root.  Which leads pgd_find_hole_fallback to succeed in probing
a reservation at address 0.  Which confuses pgb_reserved_va to
believe that guest_base has not actually been initialized.

We don't actually want NULL addresses to become accessible, so
make sure that mmap_min_addr is initialized with a non-zero value.

Buglink: https://bugs.launchpad.net/qemu/+bug/1888728
Reported-by: John Paul Adrian Glaubitz 
Signed-off-by: Richard Henderson 
---
 linux-user/main.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 3597e99bb1..75c9785157 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -758,14 +758,26 @@ int main(int argc, char **argv, char **envp)
 
 if ((fp = fopen("/proc/sys/vm/mmap_min_addr", "r")) != NULL) {
 unsigned long tmp;
-if (fscanf(fp, "%lu", ) == 1) {
+if (fscanf(fp, "%lu", ) == 1 && tmp != 0) {
 mmap_min_addr = tmp;
-qemu_log_mask(CPU_LOG_PAGE, "host mmap_min_addr=0x%lx\n", 
mmap_min_addr);
+qemu_log_mask(CPU_LOG_PAGE, "host mmap_min_addr=0x%lx\n",
+  mmap_min_addr);
 }
 fclose(fp);
 }
 }
 
+/*
+ * We prefer to not make NULL pointers accessible to QEMU.
+ * If we're in a chroot with no /proc, fall back to 1 page.
+ */
+if (mmap_min_addr == 0) {
+mmap_min_addr = qemu_host_page_size;
+qemu_log_mask(CPU_LOG_PAGE,
+  "host mmap_min_addr=0x%lx (fallback)\n",
+  mmap_min_addr);
+}
+
 /*
  * Prepare copy of argv vector for target.
  */
-- 
2.25.1

Re: [PATCH v3 15/21] migration/block-dirty-bitmap: relax error handling in incoming part

[Vladimir Sementsov-Ogievskiy]

24.07.2020 20:35, Eric Blake wrote:

On 7/24/20 3:43 AM, Vladimir Sementsov-Ogievskiy wrote:

Bitmaps data is not critical, and we should not fail the migration (or
use postcopy recovering) because of dirty-bitmaps migration failure.
Instead we should just lose unfinished bitmaps.

Still we have to report io stream violation errors, as they affect the
whole migration stream.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  migration/block-dirty-bitmap.c | 152 +
  1 file changed, 117 insertions(+), 35 deletions(-)




@@ -650,15 +695,32 @@ static int dirty_bitmap_load_bits(QEMUFile *f, 
DBMLoadState *s)
  if (s->flags & DIRTY_BITMAP_MIG_FLAG_ZEROES) {
  trace_dirty_bitmap_load_bits_zeroes();
-    bdrv_dirty_bitmap_deserialize_zeroes(s->bitmap, first_byte, nr_bytes,
- false);
+    if (!s->cancelled) {
+    bdrv_dirty_bitmap_deserialize_zeroes(s->bitmap, first_byte,
+ nr_bytes, false);
+    }
  } else {
  size_t ret;
  uint8_t *buf;
  uint64_t buf_size = qemu_get_be64(f);


Pre-existing, but if I understand, we are reading a value from the migration 
stream...


Hmm, actually, this becomes worse after patch, as before patch we had the 
check, that the size at least corresponds to the bitmap.. But we want to relax 
things in cancelled mode (and we may not have any bitmap). Most correct thing 
is to use read in a loop to just skip the data from stream if we are in 
cancelled mode (something like nbd_drop()).

I can fix this with a follow-up patch.




-    uint64_t needed_size =
-    bdrv_dirty_bitmap_serialization_size(s->bitmap,
- first_byte, nr_bytes);
+    uint64_t needed_size;
+
+    buf = g_malloc(buf_size);
+    ret = qemu_get_buffer(f, buf, buf_size);


...and using it to malloc memory.  Is that a potential risk of a malicious 
stream causing us to allocate too much memory in relation to the guest's normal 
size?  If so, fixing that should be done separately.

I'm not a migration expert, but the patch looks reasonable to me.

Reviewed-by: Eric Blake 




--
Best regards,
Vladimir

[PATCH v2] cputlb: Make store_helper less fragile to compiler optimizations

[Shu-Chun Weng]

This change has no functional change.

There is a potential link error with "undefined symbol:
qemu_build_not_reached" due to how `store_helper` is structured.
This does not produce at current QEMU head, but was reproducible at
v4.2.0 with `clang-10 -O2 -fexperimental-new-pass-manager`.

The current function structure is:

inline QEMU_ALWAYSINLINE
store_memop() {
switch () {
...
default:
qemu_build_not_reached();
}
}
inline QEMU_ALWAYSINLINE
store_helper() {
...
if (span_two_pages_or_io) {
...
helper_rst_stb_mmu();
}
store_memop();
}
helper_rst_stb_mmu() {
store_helper();
}

Both `store_memop` and `store_helper` need to be inlined and allow
constant propogations to eliminate the `qemu_build_not_reached` call.
QEMU_ALWAYSINLINE is a valiant effort to make that happen, but it's
still not guaranteed to be inlined. What happens with the failing
combination is that the compiler decided to inline the
`helper_rst_stb_mmu` call inside `store_helper`, making the latter
self-recursive, thus prevents constant propagations.

The new structure is:

inline QEMU_ALWAYSINLINE
store_memop() {
switch () {
...
default:
qemu_build_not_reached();
}
}
inline QEMU_ALWAYSINLINE
store_helper_size_aligned()() {
...
if (span_two_pages_or_io) {
return false;
}
store_memoop();
return true;
}
inline QEMU_ALWAYSINLINE
store_helper() {
if (store_helper_size_aligned() {
return;
}
helper_rst_stb_mmu();
}
helper_rst_stb_mmu() {
store_helper_size_aligned()();
}

Since a byte store cannot span across more than one page, this is a
no-op. Moreover, there is no more recursion making it more robust
against potential optimizations.

Signed-off-by: Shu-Chun Weng 
---
v1: https://patchew.org/QEMU/20200318062057.224953-1-...@google.com/

v1 -> v2:
  Restructure the function instead of marking `helper_rst_stb_mmu` noinline.

 accel/tcg/cputlb.c | 158 ++---
 1 file changed, 92 insertions(+), 66 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index d370aedb47..14324f08d2 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2010,9 +2010,10 @@ store_memop(void *haddr, uint64_t val, MemOp op)
 }
 }
 
-static inline void QEMU_ALWAYS_INLINE
-store_helper(CPUArchState *env, target_ulong addr, uint64_t val,
- TCGMemOpIdx oi, uintptr_t retaddr, MemOp op)
+/* Returns false if the store is not size-aligned and no store is done. */
+static inline bool QEMU_ALWAYS_INLINE
+store_helper_size_aligned(CPUArchState *env, target_ulong addr, uint64_t val,
+  TCGMemOpIdx oi, uintptr_t retaddr, MemOp op)
 {
 uintptr_t mmu_idx = get_mmuidx(oi);
 uintptr_t index = tlb_index(env, mmu_idx, addr);
@@ -2048,7 +2049,7 @@ store_helper(CPUArchState *env, target_ulong addr, 
uint64_t val,
 
 /* For anything that is unaligned, recurse through byte stores.  */
 if ((addr & (size - 1)) != 0) {
-goto do_unaligned_access;
+return false;
 }
 
 iotlbentry = _tlb(env)->d[mmu_idx].iotlb[index];
@@ -2066,12 +2067,12 @@ store_helper(CPUArchState *env, target_ulong addr, 
uint64_t val,
 if (tlb_addr & TLB_MMIO) {
 io_writex(env, iotlbentry, mmu_idx, val, addr, retaddr,
   op ^ (need_swap * MO_BSWAP));
-return;
+return true;
 }
 
 /* Ignore writes to ROM.  */
 if (unlikely(tlb_addr & TLB_DISCARD_WRITE)) {
-return;
+return true;
 }
 
 /* Handle clean RAM pages.  */
@@ -2091,82 +2092,107 @@ store_helper(CPUArchState *env, target_ulong addr, 
uint64_t val,
 } else {
 store_memop(haddr, val, op);
 }
-return;
+return true;
 }
 
-/* Handle slow unaligned access (it spans two pages or IO).  */
 if (size > 1
 && unlikely((addr & ~TARGET_PAGE_MASK) + size - 1
  >= TARGET_PAGE_SIZE)) {
-int i;
-uintptr_t index2;
-CPUTLBEntry *entry2;
-target_ulong page2, tlb_addr2;
-size_t size2;
+/* Slow unaligned access (it spans two pages or IO).  */
+return false;
+}
 
-do_unaligned_access:
-/*
- * Ensure the second page is in the TLB.  Note that the first page
- * is already guaranteed to be filled, and that the second page
- * cannot evict the first.
- */
-page2 = (addr + size) & TARGET_PAGE_MASK;
-size2 = (addr + size) & ~TARGET_PAGE_MASK;
-index2 = tlb_index(env, mmu_idx, page2);
-entry2 = tlb_entry(env, mmu_idx, page2);
-tlb_addr2 = tlb_addr_write(entry2);
-if 

Re: [PATCH] linux-user: Fix syscall rt_sigtimedwait() implementation

[Laurent Vivier]

Le 24/07/2020 à 20:16, Filip Bozuta a écrit :
> Implementation of 'rt_sigtimedwait()' in 'syscall.c' uses the
> function 'target_to_host_timespec()' to transfer the value of
> 'struct timespec' from target to host. However, the implementation
> doesn't check whether this conversion succeeds and thus can cause
> an unaproppriate error instead of the 'EFAULT (Bad address)' which
> is supposed to be set if the conversion from target to host fails.
> 
> This was confirmed with the LTP test for rt_sigtimedwait:
> "/testcases/kernel/syscalls/rt_sigtimedwait/rt_sigtimedwait01.c"
> which causes an unapropriate error in test case "test_bad_adress3"
> which is run with a bad adress for the 'struct timespec' argument:
> 
> FAIL: test_bad_address3 (349): Unexpected failure: EAGAIN/EWOULDBLOCK (11)
> 
> The test fails with an unexptected errno 'EAGAIN/EWOULDBLOCK' instead
> of the expected EFAULT.
> 
> After the changes from this patch, the test case is executed successfully
> along with the other LTP test cases for 'rt_sigtimedwait()':
> 
> PASS: test_bad_address3 (349): Test passed
> 
> Signed-off-by: Filip Bozuta 
> ---
>  linux-user/syscall.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c2..72735682cb 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8868,7 +8868,9 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>  unlock_user(p, arg1, 0);
>  if (arg3) {
>  puts = 
> -target_to_host_timespec(puts, arg3);
> +if (target_to_host_timespec(puts, arg3)) {
> +return -TARGET_EFAULT;
> +}
>  } else {
>  puts = NULL;
>  }
> 

Reviewed-by: Laurent Vivier 

Re: [PATCH for-5.1?] target/i386: Save cc_op before loop insns

[Alex Bennée]

Richard Henderson  writes:

> We forgot to update cc_op before these branch insns,
> which lead to losing track of the current eflags.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1888165
> Signed-off-by: Richard Henderson 
> ---
>  target/i386/translate.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index a1d31f09c1..caea6f5fb1 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -7148,6 +7148,7 @@ static target_ulong disas_insn(DisasContext *s,
> CPUState *cpu)

At first I thought that was too broad to go in disas_insn and then I
realised it was one of those mega functions

>  l1 = gen_new_label();
>  l2 = gen_new_label();
>  l3 = gen_new_label();
> +gen_update_cc_op(s);


Seems legit:

Reviewed-by: Alex Bennée 


-- 
Alex Bennée

Re: [PATCH v3 20/21] qemu-iotests/199: add early shutdown case to bitmaps postcopy

[Eric Blake]

On 7/24/20 3:43 AM, Vladimir Sementsov-Ogievskiy wrote:

Previous patches fixed two crashes which may occur on shutdown prior to
bitmaps postcopy finished. Check that it works now.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Andrey Shinkevich 
---
  tests/qemu-iotests/199 | 24 
  tests/qemu-iotests/199.out |  4 ++--
  2 files changed, 26 insertions(+), 2 deletions(-)


I've now confirmed that 18,19 don't expose any failures (but I'll leave 
them where they are in the series), and 20 does expose a testsuite 
failure if it is applied early; 20 and 21 are not fixed until 17 is 
applied (although I did not try to further bisect if 20 in isolation 
gets fixed sooner in the series).


So I'm adding to 20 and 21:

Tested-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 1/2] tests: Add 'fetch-acceptance' rule

[Wainer dos Santos Moschetta]

Hello,

On 7/24/20 4:35 AM, Philippe Mathieu-Daudé wrote:

Add a rule to fetch acceptance test assets.

This is particularly useful in a CI context, when a single job
can fetch and save the cache so other jobs reuse it directly.

It is also useful to measure the time spent downloading the
assets versus the time spent running the tests.

Signed-off-by: Philippe Mathieu-Daudé 
---
  tests/Makefile.include | 9 +
  1 file changed, 9 insertions(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index c7e4646ded..238974d8da 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -944,6 +944,15 @@ get-vm-image-fedora-31-%: check-venv
  # download all vm images, according to defined targets
  get-vm-images: check-venv $(patsubst %,get-vm-image-fedora-31-%, 
$(FEDORA_31_DOWNLOAD))
  
+# fetch acceptance test assets

+fetch-acceptance: check-venv


This new target misses an entry on check-help.


+   $(call quiet-command, \
+$(TESTS_VENV_DIR)/bin/python -m avocado \
+$(if $(V),--show=$(AVOCADO_SHOW)) \
+assets fetch \


Perhaps pass '--ignore-errors' so that intermittent network failurse 
won't disturb the execution (the test will have a second chance to 
download the asset later when it executes).



+$(wildcard tests/acceptance/*.py), \
+"AVOCADO", "tests/acceptance")


nit: print "Downloading acceptance tests assets" (similar to 
get-vm-image-fedora-32-% target).


Talking about get-vm-images...that target is pre-req of 
check-acceptance, which makes me think that fetch-acceptance should be 
either (for the sake of consistency.) The downside is that - as a 
developer running it on my machine - `avocado assets fetch` will attempt 
to download artifacts even for those tests which I'm not going to run 
anyway. Any opinion?


Regards,

Wainer


+
  check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
$(call quiet-command, \
  $(TESTS_VENV_DIR)/bin/python -m avocado \

Re: [PATCH for-5.1?] target/i386: Save cc_op before loop insns

[Paolo Bonzini]

Looks good, will queue when I am back---or just send a pull request
yourself if you prefer.

Paolo

Il ven 24 lug 2020, 20:35 Richard Henderson 
ha scritto:

> Ping?
>
> On 7/20/20 8:40 AM, Richard Henderson wrote:
> > We forgot to update cc_op before these branch insns,
> > which lead to losing track of the current eflags.
> >
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1888165
> > Signed-off-by: Richard Henderson 
> > ---
> >  target/i386/translate.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/target/i386/translate.c b/target/i386/translate.c
> > index a1d31f09c1..caea6f5fb1 100644
> > --- a/target/i386/translate.c
> > +++ b/target/i386/translate.c
> > @@ -7148,6 +7148,7 @@ static target_ulong disas_insn(DisasContext *s,
> CPUState *cpu)
> >  l1 = gen_new_label();
> >  l2 = gen_new_label();
> >  l3 = gen_new_label();
> > +gen_update_cc_op(s);
> >  b &= 3;
> >  switch(b) {
> >  case 0: /* loopnz */
> >
>
>

Re: [PATCH 2/2] tests: Exclude 'boot_linux.py' from fetch-acceptance rule

[Willian Rampazzo]

On Fri, Jul 24, 2020 at 3:26 PM Wainer dos Santos Moschetta
 wrote:
>
> Hi Philippe,
>
> On 7/24/20 4:35 AM, Philippe Mathieu-Daudé wrote:
> > The boot_linux.py file triggers an exception:
> >
> >$ make fetch-acceptance
> >AVOCADO tests/acceptance
> >Fetching assets from tests/acceptance/empty_cpu_model.py.
> >Fetching assets from tests/acceptance/vnc.py.
> >Fetching assets from tests/acceptance/boot_linux_console.py.
> >Fetching assets from tests/acceptance/boot_linux.py.
> >Traceback (most recent call last):
> >  File 
> > "/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/__main__.py",
> >  line 11, in 
> >sys.exit(main.run())
> >  File 
> > "/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/core/app.py",
> >  line 91, in run
> >return method(self.parser.config)
> >  File 
> > "/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/plugins/assets.py",
> >  line 291, in run
> >success, fail = fetch_assets(test_file)
> >  File 
> > "/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/plugins/assets.py",
> >  line 200, in fetch_assets
> >handler = FetchAssetHandler(test_file, klass, method)
> >  File 
> > "/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/plugins/assets.py",
> >  line 65, in __init__
> >self.visit(self.tree)
> >  File "/usr/lib64/python3.7/ast.py", line 271, in visit
> >return visitor(node)
> >  File "/usr/lib64/python3.7/ast.py", line 279, in generic_visit
> >self.visit(item)
> >  File "/usr/lib64/python3.7/ast.py", line 271, in visit
> >return visitor(node)
> >  File 
> > "/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/plugins/assets.py",
> >  line 139, in visit_ClassDef
> >self.generic_visit(node)
> >  File "/usr/lib64/python3.7/ast.py", line 279, in generic_visit
> >self.visit(item)
> >  File "/usr/lib64/python3.7/ast.py", line 271, in visit
> >return visitor(node)
> >  File 
> > "/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/plugins/assets.py",
> >  line 171, in visit_Assign
> >self.asgmts[cur_klass][cur_method][name] = node.value.s
> >KeyError: 'launch_and_wait'
> >make: *** [tests/Makefile.include:949: fetch-acceptance] Error 1
>
> Currently the acceptance tests use Avocado 7.6. I bumped to 80.0 (latest
> released) here and that error is gone. Could you double check?
>
> Regards,
>
> Wainer

Hi Wainer, thanks for looking at this problem.

This bug was fixed here
https://github.com/avocado-framework/avocado/pull/3665, on release 78
of Avocado. It was reported by Philippe at that time. I think we
forgot to bump the Avocado version here.

>
> >
> > Exclude it for now. We will revert this commit once the script is
> > fixed.
> >
> > Signed-off-by: Philippe Mathieu-Daudé 
> > ---
> >   tests/Makefile.include | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 238974d8da..7c9cf7a818 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -950,7 +950,7 @@ fetch-acceptance: check-venv
> >   $(TESTS_VENV_DIR)/bin/python -m avocado \
> >   $(if $(V),--show=$(AVOCADO_SHOW)) \
> >   assets fetch \
> > -$(wildcard tests/acceptance/*.py), \
> > +$(filter-out tests/acceptance/boot_linux.py,$(wildcard 
> > tests/acceptance/*.py)), \
> >   "AVOCADO", "tests/acceptance")
> >
> >   check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
>

Re: [PATCH v5 6/6] target/ppc: add vmsumudm vmsumcud instructions

[Lijun Pan]

> On Jul 24, 2020, at 1:00 PM, Richard Henderson  
> wrote:
> 
> On 7/23/20 9:58 PM, Lijun Pan wrote:
>> vmsumudm (Power ISA 3.0) - Vector Multiply-Sum Unsigned Doubleword Modulo
>> VA-form.
>> vmsumcud (Power ISA 3.1) - Vector Multiply-Sum & write Carry-out Unsigned
>> Doubleword VA-form.
>> 
>> Signed-off-by: Lijun Pan 
>> ---
>> v5: update instruction flag for vmsumcud.
>>integrate into this isa3.1 patch series
>> v3: implement vmsumudm/vmsumcud through int128 functions,
>>suggested by Richard Henderson.
>> 
>> disas/ppc.c |  2 ++
>> target/ppc/helper.h |  4 ++-
>> target/ppc/int_helper.c | 49 -
>> target/ppc/translate.c  |  1 -
>> target/ppc/translate/vmx-impl.inc.c | 39 ---
>> target/ppc/translate/vmx-ops.inc.c  |  2 ++
>> 6 files changed, 76 insertions(+), 21 deletions(-)
>> 
>> diff --git a/disas/ppc.c b/disas/ppc.c
>> index 63e97cfe1d..bd76fae4c4 100644
>> --- a/disas/ppc.c
>> +++ b/disas/ppc.c
>> @@ -2261,7 +2261,9 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>> { "vmsumshs",  VXA(4,  41), VXA_MASK,PPCVEC, { VD, VA, VB, 
>> VC } },
>> { "vmsumubm",  VXA(4,  36), VXA_MASK,   PPCVEC,  { VD, VA, VB, 
>> VC } },
>> { "vmsumuhm",  VXA(4,  38), VXA_MASK,   PPCVEC,  { VD, VA, VB, 
>> VC } },
>> +{ "vmsumudm",  VXA(4,  35), VXA_MASK,   PPCVEC, { VD, VA, VB, VC } 
>> },
>> { "vmsumuhs",  VXA(4,  39), VXA_MASK,   PPCVEC,  { VD, VA, VB, 
>> VC } },
>> +{ "vmsumcud",  VXA(4,  23), VXA_MASK,   PPCVEC, { VD, VA, VB, VC } 
>> },
>> { "vmulesb",   VX(4,  776), VX_MASK, PPCVEC, { VD, VA, VB } },
>> { "vmulesh",   VX(4,  840), VX_MASK, PPCVEC, { VD, VA, VB } },
>> { "vmuleub",   VX(4,  520), VX_MASK, PPCVEC, { VD, VA, VB } },
>> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
>> index 70a14029ca..00a31d64bc 100644
>> --- a/target/ppc/helper.h
>> +++ b/target/ppc/helper.h
>> @@ -274,10 +274,12 @@ DEF_HELPER_3(vpkpx, void, avr, avr, avr)
>> DEF_HELPER_5(vmhaddshs, void, env, avr, avr, avr, avr)
>> DEF_HELPER_5(vmhraddshs, void, env, avr, avr, avr, avr)
>> DEF_HELPER_5(vmsumuhm, void, env, avr, avr, avr, avr)
>> +DEF_HELPER_5(vmsumudm, void, env, avr, avr, avr, avr)
>> DEF_HELPER_5(vmsumuhs, void, env, avr, avr, avr, avr)
>> DEF_HELPER_5(vmsumshm, void, env, avr, avr, avr, avr)
>> DEF_HELPER_5(vmsumshs, void, env, avr, avr, avr, avr)
>> -DEF_HELPER_4(vmladduhm, void, avr, avr, avr, avr)
>> +DEF_HELPER_5(vmsumcud, void, env, avr, avr, avr, avr)
>> +DEF_HELPER_5(vmladduhm, void, env, avr, avr, avr, avr)
>> DEF_HELPER_FLAGS_2(mtvscr, TCG_CALL_NO_RWG, void, env, i32)
>> DEF_HELPER_FLAGS_1(mfvscr, TCG_CALL_NO_RWG, i32, env)
>> DEF_HELPER_3(lvebx, void, env, avr, tl)
>> diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
>> index 62b93b4568..2e919a7b8e 100644
>> --- a/target/ppc/int_helper.c
>> +++ b/target/ppc/int_helper.c
>> @@ -913,7 +913,8 @@ void helper_vmhraddshs(CPUPPCState *env, ppc_avr_t *r, 
>> ppc_avr_t *a,
>> }
>> }
>> 
>> -void helper_vmladduhm(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t 
>> *c)
>> +void helper_vmladduhm(CPUPPCState *env, ppc_avr_t *r, ppc_avr_t *a,
>> +  ppc_avr_t *b, ppc_avr_t *c)
> 
> Why?
> 
>> +void helper_vmsumudm(CPUPPCState *env, ppc_avr_t *r,
>> + ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)
>> +{
> 
> ...
> 
>> +void helper_vmsumcud(CPUPPCState *env, ppc_avr_t *r,
>> + ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)
> 
> You don't actually use env in either helper, so you shouldn't pass it in.
> 
> 

I just wanted to reuse GEN_VAFORM_PAIRED which has env passed in, 
rather than creating a GEN_VAFORM_PAIRED_NOENV which does not have env.
I created GEN_VAFORM which includes env so that it can cover both env and 
non-env case.

I re-look at the code, and find out that GEN_**FORM** differentiate in env and 
non-env.
So I can rename current GEN_VAFORM_PAIRED (env passed in) to 
GEN_VAFORM_PAIRED_ENV,
create a new GEN_VAFORM_PAIRED (no env) to cater vmladduhm and vmsumudm
remove the env part code in GEN_VAFORM to have vmsumcud fit into.

What do you think, Richard?
 
Lijun

Re: [PATCH for-5.1?] target/i386: Save cc_op before loop insns

[Richard Henderson]

Ping?

On 7/20/20 8:40 AM, Richard Henderson wrote:
> We forgot to update cc_op before these branch insns,
> which lead to losing track of the current eflags.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1888165
> Signed-off-by: Richard Henderson 
> ---
>  target/i386/translate.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index a1d31f09c1..caea6f5fb1 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -7148,6 +7148,7 @@ static target_ulong disas_insn(DisasContext *s, 
> CPUState *cpu)
>  l1 = gen_new_label();
>  l2 = gen_new_label();
>  l3 = gen_new_label();
> +gen_update_cc_op(s);
>  b &= 3;
>  switch(b) {
>  case 0: /* loopnz */
> 

Re: [PATCH 2/2] tests: Exclude 'boot_linux.py' from fetch-acceptance rule

[Wainer dos Santos Moschetta]

Hi Philippe,

On 7/24/20 4:35 AM, Philippe Mathieu-Daudé wrote:

The boot_linux.py file triggers an exception:

   $ make fetch-acceptance
   AVOCADO tests/acceptance
   Fetching assets from tests/acceptance/empty_cpu_model.py.
   Fetching assets from tests/acceptance/vnc.py.
   Fetching assets from tests/acceptance/boot_linux_console.py.
   Fetching assets from tests/acceptance/boot_linux.py.
   Traceback (most recent call last):
 File 
"/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/__main__.py",
 line 11, in 
   sys.exit(main.run())
 File 
"/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/core/app.py",
 line 91, in run
   return method(self.parser.config)
 File 
"/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/plugins/assets.py",
 line 291, in run
   success, fail = fetch_assets(test_file)
 File 
"/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/plugins/assets.py",
 line 200, in fetch_assets
   handler = FetchAssetHandler(test_file, klass, method)
 File 
"/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/plugins/assets.py",
 line 65, in __init__
   self.visit(self.tree)
 File "/usr/lib64/python3.7/ast.py", line 271, in visit
   return visitor(node)
 File "/usr/lib64/python3.7/ast.py", line 279, in generic_visit
   self.visit(item)
 File "/usr/lib64/python3.7/ast.py", line 271, in visit
   return visitor(node)
 File 
"/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/plugins/assets.py",
 line 139, in visit_ClassDef
   self.generic_visit(node)
 File "/usr/lib64/python3.7/ast.py", line 279, in generic_visit
   self.visit(item)
 File "/usr/lib64/python3.7/ast.py", line 271, in visit
   return visitor(node)
 File 
"/var/tmp/qemu-builddir/tests/venv/lib64/python3.7/site-packages/avocado/plugins/assets.py",
 line 171, in visit_Assign
   self.asgmts[cur_klass][cur_method][name] = node.value.s
   KeyError: 'launch_and_wait'
   make: *** [tests/Makefile.include:949: fetch-acceptance] Error 1


Currently the acceptance tests use Avocado 7.6. I bumped to 80.0 (latest 
released) here and that error is gone. Could you double check?


Regards,

Wainer



Exclude it for now. We will revert this commit once the script is
fixed.

Signed-off-by: Philippe Mathieu-Daudé 
---
  tests/Makefile.include | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 238974d8da..7c9cf7a818 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -950,7 +950,7 @@ fetch-acceptance: check-venv
  $(TESTS_VENV_DIR)/bin/python -m avocado \
  $(if $(V),--show=$(AVOCADO_SHOW)) \
  assets fetch \
-$(wildcard tests/acceptance/*.py), \
+$(filter-out tests/acceptance/boot_linux.py,$(wildcard 
tests/acceptance/*.py)), \
  "AVOCADO", "tests/acceptance")
  
  check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images

Re: [PATCH v3 13/16] linux-user,ppc: fix clock_nanosleep() for linux-user-ppc

[Richard Henderson]

On 7/23/20 11:45 PM, Alex Bennée wrote:
> From: Laurent Vivier 
> 
> Our safe_clock_nanosleep() returns -1 and updates errno.
> 
> We don't need to update the CRF bit in syscall.c because it will
> be updated in ppc/cpu_loop.c as the return value is negative.
> 
> Signed-off-by: Laurent Vivier 
> Signed-off-by: Alex Bennée 
> Message-Id: <20200722174612.2917566-3-laur...@vivier.eu>
> ---
>  linux-user/syscall.c | 7 ---
>  1 file changed, 7 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v3 12/16] linux-user: fix clock_nanosleep()

[Richard Henderson]

On 7/23/20 11:45 PM, Alex Bennée wrote:
> From: Laurent Vivier 
> 
> If the call is interrupted by a signal handler, it fails with error EINTR
> and if "remain" is not NULL and "flags" is not TIMER_ABSTIME, it returns
> the remaining unslept time in "remain".
> 
> Update linux-user to not overwrite the "remain" structure if there is no
> error.
> 
> Found with "make check-tcg", linux-test fails on nanosleep test:
> 
>   TESTlinux-test on x86_64
> .../tests/tcg/multiarch/linux-test.c:242: nanosleep
> 
> Reported-by: Philippe Mathieu-Daudé 
> Signed-off-by: Laurent Vivier 
> Signed-off-by: Alex Bennée 
> Message-Id: <20200722174612.2917566-2-laur...@vivier.eu>
> ---
>  linux-user/syscall.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v3 07/16] target/i386: floatx80: avoid compound literals in static initializers

[Richard Henderson]

On 7/23/20 11:45 PM, Alex Bennée wrote:
> From: Laszlo Ersek 
> 
> Quoting ISO C99 6.7.8p4, "All the expressions in an initializer for an
> object that has static storage duration shall be constant expressions or
> string literals".
> 
> The compound literal produced by the make_floatx80() macro is not such a
> constant expression, per 6.6p7-9. (An implementation may accept it,
> according to 6.6p10, but is not required to.)
> 
> Therefore using "floatx80_zero" and make_floatx80() for initializing
> "f2xm1_table" and "fpatan_table" is not portable. And gcc-4.8 in RHEL-7.6
> actually chokes on them:
> 
>> target/i386/fpu_helper.c:871:5: error: initializer element is not constant
>>  { make_floatx80(0xbfff, 0x8000ULL),
>>  ^
> We've had the make_floatx80_init() macro for this purpose since commit
> 3bf7e40ab914 ("softfloat: fix for C99", 2012-03-17), so let's use that
> macro again.
> 
> Fixes: eca30647fc07
> Fixes: ff57bb7b6326
> Signed-off-by: Laszlo Ersek 
> Signed-off-by: Alex Bennée 
> Reviewed-by: Alex Bennée 
> Reviewed-by: Philippe Mathieu-Daudé 
> Cc: Aurelien Jarno 
> Cc: Eduardo Habkost 
> Cc: Joseph Myers 
> Cc: Paolo Bonzini 
> Cc: Peter Maydell 
> Cc: Richard Henderson 
> Link: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06566.html
> Link: https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04714.html
> Message-Id: <20200716144251.23004-1-ler...@redhat.com>
> Message-Id: <20200722062902.24509-8-alex.ben...@linaro.org>
> ---
>  include/fpu/softfloat.h  |   1 +
>  target/i386/fpu_helper.c | 426 +++
>  2 files changed, 214 insertions(+), 213 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v3 06/16] accel/tcg: better handle memory constrained systems

[Richard Henderson]

On 7/23/20 11:44 PM, Alex Bennée wrote:
> It turns out there are some 64 bit systems that have relatively low
> amounts of physical memory available to them (typically CI system).
> Even with swapping available a 1GB translation buffer that fills up
> can put the machine under increased memory pressure. Detect these low
> memory situations and reduce tb_size appropriately.
> 
> Fixes: 600e17b2615 ("accel/tcg: increase default code gen buffer size for 64 
> bit")
> Signed-off-by: Alex Bennée 
> Cc: BALATON Zoltan 
> Cc: Christian Ehrhardt 

Reviewed-by: Richard Henderson 

r~

[PATCH] linux-user: Fix syscall rt_sigtimedwait() implementation

[Filip Bozuta]

Implementation of 'rt_sigtimedwait()' in 'syscall.c' uses the
function 'target_to_host_timespec()' to transfer the value of
'struct timespec' from target to host. However, the implementation
doesn't check whether this conversion succeeds and thus can cause
an unaproppriate error instead of the 'EFAULT (Bad address)' which
is supposed to be set if the conversion from target to host fails.

This was confirmed with the LTP test for rt_sigtimedwait:
"/testcases/kernel/syscalls/rt_sigtimedwait/rt_sigtimedwait01.c"
which causes an unapropriate error in test case "test_bad_adress3"
which is run with a bad adress for the 'struct timespec' argument:

FAIL: test_bad_address3 (349): Unexpected failure: EAGAIN/EWOULDBLOCK (11)

The test fails with an unexptected errno 'EAGAIN/EWOULDBLOCK' instead
of the expected EFAULT.

After the changes from this patch, the test case is executed successfully
along with the other LTP test cases for 'rt_sigtimedwait()':

PASS: test_bad_address3 (349): Test passed

Signed-off-by: Filip Bozuta 
---
 linux-user/syscall.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1211e759c2..72735682cb 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8868,7 +8868,9 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 unlock_user(p, arg1, 0);
 if (arg3) {
 puts = 
-target_to_host_timespec(puts, arg3);
+if (target_to_host_timespec(puts, arg3)) {
+return -TARGET_EFAULT;
+}
 } else {
 puts = NULL;
 }
-- 
2.25.1

Re: [PATCH v3 17/21] migration/savevm: don't worry if bitmap migration postcopy failed

[Eric Blake]

On 7/24/20 3:43 AM, Vladimir Sementsov-Ogievskiy wrote:

First, if only bitmaps postcopy enabled (not ram postcopy)


is enabled (and not ram postcopy),


postcopy_pause_incoming crashes on assertion assert(mis->to_src_file).


on an



And anyway, bitmaps postcopy is not prepared to be somehow recovered.
The original idea instead is that if bitmaps postcopy failed, we just
loss some bitmaps, which is not critical. So, on failure we just need


lose


to remove unfinished bitmaps and guest should continue execution on
destination.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Andrey Shinkevich 
---
  migration/savevm.c | 37 -
  1 file changed, 32 insertions(+), 5 deletions(-)



Definitely a bug fix, but I'd like David's opinion on whether this is 
still 5.1 material (because it is limited to just bitmaps migration, 
which is opt-in) or too risky (because we've already had several 
releases where it was broken, what's one more?).


I'm less familiar with the code, so this is weak, but I did read through 
it and nothing jumped out at me, so:


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v7 40/47] block: Inline bdrv_co_block_status_from_*()

[Andrey Shinkevich]

On 25.06.2020 18:22, Max Reitz wrote:

With bdrv_filter_bs(), we can easily handle this default filter behavior
in bdrv_co_block_status().

blkdebug wants to have an additional assertion, so it keeps its own
implementation, except bdrv_co_block_status_from_file() needs to be
inlined there.

Suggested-by: Eric Blake 
Signed-off-by: Max Reitz 
---
  include/block/block_int.h | 23 --
  block/backup-top.c|  2 --
  block/blkdebug.c  |  7 --
  block/blklogwrites.c  |  1 -
  block/commit.c|  1 -
  block/copy-on-read.c  |  2 --
  block/filter-compress.c   |  2 --
  block/io.c| 51 +--
  block/mirror.c|  1 -
  block/throttle.c  |  1 -
  10 files changed, 22 insertions(+), 69 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 6e09e15ed4..e5a328c389 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1291,29 +1291,6 @@ void bdrv_default_perms(BlockDriverState *bs, BdrvChild 
*c,
  uint64_t perm, uint64_t shared,
  uint64_t *nperm, uint64_t *nshared);
  
-/*

- * Default implementation for drivers to pass bdrv_co_block_status() to
- * their file.
- */
-int coroutine_fn bdrv_co_block_status_from_file(BlockDriverState *bs,
-bool want_zero,
-int64_t offset,
-int64_t bytes,
-int64_t *pnum,
-int64_t *map,
-BlockDriverState **file);
-/*
- * Default implementation for drivers to pass bdrv_co_block_status() to
- * their backing file.
- */
-int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs,
-   bool want_zero,
-   int64_t offset,
-   int64_t bytes,
-   int64_t *pnum,
-   int64_t *map,
-   BlockDriverState **file);
-
  int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs);
  int64_t bdrv_primary_allocated_file_size(BlockDriverState *bs);
  int64_t bdrv_notsup_allocated_file_size(BlockDriverState *bs);
diff --git a/block/backup-top.c b/block/backup-top.c
index 89bd3937d0..bf5fc22fc7 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -185,8 +185,6 @@ BlockDriver bdrv_backup_top_filter = {
  .bdrv_co_pwritev_compressed = backup_top_co_pwritev_compressed,
  .bdrv_co_flush  = backup_top_co_flush,
  
-.bdrv_co_block_status   = bdrv_co_block_status_from_backing,

-
  .bdrv_refresh_filename  = backup_top_refresh_filename,
  
  .bdrv_child_perm= backup_top_child_perm,

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 7194bc7f06..cf78d8809e 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -757,8 +757,11 @@ static int coroutine_fn 
blkdebug_co_block_status(BlockDriverState *bs,
  return err;
  }
  
-return bdrv_co_block_status_from_file(bs, want_zero, offset, bytes,

-  pnum, map, file);
+assert(bs->file && bs->file->bs);
+*pnum = bytes;
+*map = offset;
+*file = bs->file->bs;
+return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID;
  }
  
  static void blkdebug_close(BlockDriverState *bs)

diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index 6753bd9a3e..c6b2711fe5 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -517,7 +517,6 @@ static BlockDriver bdrv_blk_log_writes = {
  .bdrv_co_pwrite_zeroes  = blk_log_writes_co_pwrite_zeroes,
  .bdrv_co_flush_to_disk  = blk_log_writes_co_flush_to_disk,
  .bdrv_co_pdiscard   = blk_log_writes_co_pdiscard,
-.bdrv_co_block_status   = bdrv_co_block_status_from_file,
  
  .is_filter  = true,

  .strong_runtime_opts= blk_log_writes_strong_runtime_opts,
diff --git a/block/commit.c b/block/commit.c
index 4122b6736d..ea9282daea 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -238,7 +238,6 @@ static void bdrv_commit_top_child_perm(BlockDriverState 
*bs, BdrvChild *c,
  static BlockDriver bdrv_commit_top = {
  .format_name= "commit_top",
  .bdrv_co_preadv = bdrv_commit_top_preadv,
-.bdrv_co_block_status   = bdrv_co_block_status_from_backing,
  .bdrv_refresh_filename  = bdrv_commit_top_refresh_filename,
  .bdrv_child_perm= bdrv_commit_top_child_perm,
  
diff --git a/block/copy-on-read.c b/block/copy-on-read.c

index a6a864f147..2816e61afe 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -146,8 

Re: [PATCH v5 6/6] target/ppc: add vmsumudm vmsumcud instructions

[Richard Henderson]

On 7/23/20 9:58 PM, Lijun Pan wrote:
> vmsumudm (Power ISA 3.0) - Vector Multiply-Sum Unsigned Doubleword Modulo
> VA-form.
> vmsumcud (Power ISA 3.1) - Vector Multiply-Sum & write Carry-out Unsigned
> Doubleword VA-form.
> 
> Signed-off-by: Lijun Pan 
> ---
> v5: update instruction flag for vmsumcud.
> integrate into this isa3.1 patch series
> v3: implement vmsumudm/vmsumcud through int128 functions,
> suggested by Richard Henderson.
> 
>  disas/ppc.c |  2 ++
>  target/ppc/helper.h |  4 ++-
>  target/ppc/int_helper.c | 49 -
>  target/ppc/translate.c  |  1 -
>  target/ppc/translate/vmx-impl.inc.c | 39 ---
>  target/ppc/translate/vmx-ops.inc.c  |  2 ++
>  6 files changed, 76 insertions(+), 21 deletions(-)
> 
> diff --git a/disas/ppc.c b/disas/ppc.c
> index 63e97cfe1d..bd76fae4c4 100644
> --- a/disas/ppc.c
> +++ b/disas/ppc.c
> @@ -2261,7 +2261,9 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>  { "vmsumshs",  VXA(4,  41), VXA_MASK,PPCVEC, { VD, VA, VB, 
> VC } },
>  { "vmsumubm",  VXA(4,  36), VXA_MASK,   PPCVEC,  { VD, VA, VB, 
> VC } },
>  { "vmsumuhm",  VXA(4,  38), VXA_MASK,   PPCVEC,  { VD, VA, VB, 
> VC } },
> +{ "vmsumudm",  VXA(4,  35), VXA_MASK,   PPCVEC, { VD, VA, VB, VC } },
>  { "vmsumuhs",  VXA(4,  39), VXA_MASK,   PPCVEC,  { VD, VA, VB, 
> VC } },
> +{ "vmsumcud",  VXA(4,  23), VXA_MASK,   PPCVEC, { VD, VA, VB, VC } },
>  { "vmulesb",   VX(4,  776), VX_MASK, PPCVEC, { VD, VA, VB } },
>  { "vmulesh",   VX(4,  840), VX_MASK, PPCVEC, { VD, VA, VB } },
>  { "vmuleub",   VX(4,  520), VX_MASK, PPCVEC, { VD, VA, VB } },
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 70a14029ca..00a31d64bc 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -274,10 +274,12 @@ DEF_HELPER_3(vpkpx, void, avr, avr, avr)
>  DEF_HELPER_5(vmhaddshs, void, env, avr, avr, avr, avr)
>  DEF_HELPER_5(vmhraddshs, void, env, avr, avr, avr, avr)
>  DEF_HELPER_5(vmsumuhm, void, env, avr, avr, avr, avr)
> +DEF_HELPER_5(vmsumudm, void, env, avr, avr, avr, avr)
>  DEF_HELPER_5(vmsumuhs, void, env, avr, avr, avr, avr)
>  DEF_HELPER_5(vmsumshm, void, env, avr, avr, avr, avr)
>  DEF_HELPER_5(vmsumshs, void, env, avr, avr, avr, avr)
> -DEF_HELPER_4(vmladduhm, void, avr, avr, avr, avr)
> +DEF_HELPER_5(vmsumcud, void, env, avr, avr, avr, avr)
> +DEF_HELPER_5(vmladduhm, void, env, avr, avr, avr, avr)
>  DEF_HELPER_FLAGS_2(mtvscr, TCG_CALL_NO_RWG, void, env, i32)
>  DEF_HELPER_FLAGS_1(mfvscr, TCG_CALL_NO_RWG, i32, env)
>  DEF_HELPER_3(lvebx, void, env, avr, tl)
> diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
> index 62b93b4568..2e919a7b8e 100644
> --- a/target/ppc/int_helper.c
> +++ b/target/ppc/int_helper.c
> @@ -913,7 +913,8 @@ void helper_vmhraddshs(CPUPPCState *env, ppc_avr_t *r, 
> ppc_avr_t *a,
>  }
>  }
>  
> -void helper_vmladduhm(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)
> +void helper_vmladduhm(CPUPPCState *env, ppc_avr_t *r, ppc_avr_t *a,
> +  ppc_avr_t *b, ppc_avr_t *c)

Why?

> +void helper_vmsumudm(CPUPPCState *env, ppc_avr_t *r,
> + ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)
> +{

...

> +void helper_vmsumcud(CPUPPCState *env, ppc_avr_t *r,
> + ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)

You don't actually use env in either helper, so you shouldn't pass it in.


r~

Re: [PATCH v5 3/6] target/ppc: add vmulh{su}w instructions

[Richard Henderson]

On 7/23/20 9:58 PM, Lijun Pan wrote:
> vmulhsw: Vector Multiply High Signed Word
> vmulhuw: Vector Multiply High Unsigned Word
> 
> Signed-off-by: Lijun Pan 
> ---
> v4/v5: no change
> Reviewed-by: Richard Henderson 
> v3: inline the helper_vmulh{su}w multiply directly instead of using macro
> v2: fix coding style
> use Power ISA 3.1 flag

The Reviewed-by tag goes above the "---" marker so that it is included when the
patch is applied.


r~

Re: [PATCH v5 2/6] target/ppc: add vmulld to INDEX_op_mul_vec case

[Richard Henderson]

On 7/23/20 9:58 PM, Lijun Pan wrote:
> Group vmuluwm and vmulld. Make vmulld-specific
> changes since it belongs to new ISA 3.1.
> 
> Signed-off-by: Lijun Pan 
> ---
> v5: no change
> v4: add missing changes, and split to 5/11, 6/11, 7/11
> v3: use tcg_gen_gvec_mul()
> v2: fix coding style
> use Power ISA 3.1 flag
> 
>  tcg/ppc/tcg-target.h |  2 ++
>  tcg/ppc/tcg-target.inc.c | 12 ++--
>  2 files changed, 12 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v5 1/6] Update PowerPC AT_HWCAP2 definition

[Richard Henderson]

On 7/23/20 9:58 PM, Lijun Pan wrote:
> Add PPC2_FEATURE2_ARCH_3_10 to the PowerPC AT_HWCAP2 definitions.
> 
> Signed-off-by: Lijun Pan 
> ---

Reviewed-by: Richard Henderson 

We should add the rest of the bits as well at some point.


r~

Re: [PATCH v3 15/21] migration/block-dirty-bitmap: relax error handling in incoming part

[Eric Blake]

On 7/24/20 3:43 AM, Vladimir Sementsov-Ogievskiy wrote:

Bitmaps data is not critical, and we should not fail the migration (or
use postcopy recovering) because of dirty-bitmaps migration failure.
Instead we should just lose unfinished bitmaps.

Still we have to report io stream violation errors, as they affect the
whole migration stream.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  migration/block-dirty-bitmap.c | 152 +
  1 file changed, 117 insertions(+), 35 deletions(-)




@@ -650,15 +695,32 @@ static int dirty_bitmap_load_bits(QEMUFile *f, 
DBMLoadState *s)
  
  if (s->flags & DIRTY_BITMAP_MIG_FLAG_ZEROES) {

  trace_dirty_bitmap_load_bits_zeroes();
-bdrv_dirty_bitmap_deserialize_zeroes(s->bitmap, first_byte, nr_bytes,
- false);
+if (!s->cancelled) {
+bdrv_dirty_bitmap_deserialize_zeroes(s->bitmap, first_byte,
+ nr_bytes, false);
+}
  } else {
  size_t ret;
  uint8_t *buf;
  uint64_t buf_size = qemu_get_be64(f);


Pre-existing, but if I understand, we are reading a value from the 
migration stream...



-uint64_t needed_size =
-bdrv_dirty_bitmap_serialization_size(s->bitmap,
- first_byte, nr_bytes);
+uint64_t needed_size;
+
+buf = g_malloc(buf_size);
+ret = qemu_get_buffer(f, buf, buf_size);


...and using it to malloc memory.  Is that a potential risk of a 
malicious stream causing us to allocate too much memory in relation to 
the guest's normal size?  If so, fixing that should be done separately.


I'm not a migration expert, but the patch looks reasonable to me.

Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: roms/seabios-hppa can't be built with gcc-10: -fno-ipa-sra

[Helge Deller]

On 22.07.20 21:11, Michael Tokarev wrote:
> Switching to gcc-10 makes seabios-hppa unbuildable.
> It fails at the final linking step with a lot of
> missing references to memcpy & memcmp all over the
> places.
>
> The notable difference between gcc-10 and previous
> gcc is that ccode32flat.o does _not_ have the text
> for these two functions but have two .isra.0:
>
> $ hppa-linux-gnu-nm ccode32flat.o | grep mem[sc]
> 03e0 t memcmp
>  U memcpy
> 2f38 t memcpy.isra.0
>  U memset
> 3a84 t memset.isra.0
>
>
> while previous version of the compiler did have them:
>
> $ hppa-linux-gnu-nm ccode32flat.o | grep mem[sc]
> 02fc t memcmp
> 370c t memcpy
> 036c t memset

I believe this is a compiler bug in gcc-10.
Adding other flags like -fno-builtin or similiar doesn't fix the issue.

> After adding -fno-ipa-sra to the gcc flags, the firmware
> is built successfully.
>
> I don't know what to make out of this. Previous versions
> of gcc apparently accepts -fno-ipa-sra too, for quite some
> time.  So maybe add this to the flags unconditionally?

I think this is currently the best way forward.
Do you want to send a patch, or should I just add this upstream
to seabios-hppa?

Helge

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Christian Schoenebeck]

On Freitag, 24. Juli 2020 18:50:47 CEST Peter Maydell wrote:
> On Fri, 24 Jul 2020 at 17:46, Philippe Mathieu-Daudé  
> wrote:
> > I guess we were expecting the distrib to update the pkg.
> 
> Apple's view is that you shouldn't be using the sasl header
> at all but instead their proprietary crypto library APIs, so
> I wouldn't expect them to ever ship something without the
> deprecation warnings.

AFAIK Apple's reason for this is similar to no longer providing headers for
OpenSSL [1] (or now actually BoringSSL): they cannot guarantee binary
compatibility of these libs beyond individual macOS releases (i.e. without
breaking old clients) and bad things happened [2] in the past for apps which
expected it would.

[1] https://lists.apple.com/archives/macnetworkprog/2015/Jun/msg00025.html
[2] https://lists.andrew.cmu.edu/pipermail/cyrus-sasl/2007-November/001254.html

The common recommendation is: "Ship your macOS app bundled with the preferred
version of these libs."

Best regards,
Christian Schoenebeck

Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from CpuInstanceProperties

[Igor Mammedov]

On Mon, 13 Jul 2020 14:30:29 -0500
Babu Moger  wrote:

> > -Original Message-
> > From: Igor Mammedov 
> > Sent: Monday, July 13, 2020 12:32 PM
> > To: Moger, Babu 
> > Cc: pbonz...@redhat.com; r...@twiddle.net; ehabk...@redhat.com; qemu-
> > de...@nongnu.org
> > Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> > CpuInstanceProperties
> > 
> > On Mon, 13 Jul 2020 11:43:33 -0500
> > Babu Moger  wrote:
> >   
> > > On 7/13/20 11:17 AM, Igor Mammedov wrote:  
> > > > On Mon, 13 Jul 2020 10:02:22 -0500
> > > > Babu Moger  wrote:
> > > >  
> > > >>> -Original Message-
> > > >>> From: Igor Mammedov 
> > > >>> Sent: Monday, July 13, 2020 4:08 AM
> > > >>> To: Moger, Babu 
> > > >>> Cc: pbonz...@redhat.com; r...@twiddle.net; ehabk...@redhat.com;
> > > >>> qemu- de...@nongnu.org
> > > >>> Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> > > >>> CpuInstanceProperties  
> > > > [...]  
> > >  +
> > >  +/*
> > >  + * Initialize topo_ids from CpuInstanceProperties
> > >  + * node_id in CpuInstanceProperties(or in CPU device) is a 
> > >  sequential
> > >  + * number, but while building the topology  
> > > >>>  
> > >  we need to separate it for
> > >  + * each socket(mod nodes_per_pkg).  
> > > >>> could you clarify a bit more on why this is necessary?  
> > > >>
> > > >> If you have two sockets and 4 numa nodes, node_id in
> > > >> CpuInstanceProperties will be number sequentially as 0, 1, 2, 3.
> > > >> But in EPYC topology, it will be  0, 1, 0, 1( Basically mod % number 
> > > >> of nodes  
> > per socket).  
> > > >
> > > > I'm confused, let's suppose we have 2 EPYC sockets with 2 nodes per
> > > > socket so APIC id woulbe be composed like:
> > > >
> > > >  1st socket
> > > >pkg_id(0) | node_id(0)
> > > >pkg_id(0) | node_id(1)
> > > >
> > > >  2nd socket
> > > >pkg_id(1) | node_id(0)
> > > >pkg_id(1) | node_id(1)
> > > >
> > > > if that's the case, then EPYC's node_id here doesn't look like a
> > > > NUMA node in the sense it's usually used (above config would have 4
> > > > different memory controllers => 4 conventional NUMA nodes).  
> > >
> > > EPIC model uses combination of socket id and node id to identify the
> > > numa nodes. So, it internally uses all the information.  
> > 
> > well with above values, EPYC's node_id doesn't look like it's specifying a
> > machine numa node, but rather a node index within single socket. In which 
> > case,
> > it doesn't make much sense calling it NUMA node_id, it's rather some index
> > within a socket. (it starts looking like terminology is all mixed up)
> > 
> > If you have access to a milti-socket EPYC machine, can you dump and post 
> > here
> > its apic ids, pls?  
> 
> Here is the output from my EPYC machine with 2 sockets and totally 8
> nodes(SMT disabled). The cpus 0-31 are in socket 0 and  cpus 32-63 in
> socket 1.
> 
> # lscpu
> Architecture:x86_64
> CPU op-mode(s):  32-bit, 64-bit
> Byte Order:  Little Endian
> CPU(s):  64
> On-line CPU(s) list: 0-63
> Thread(s) per core:  1
> Core(s) per socket:  32
> Socket(s):   2
> NUMA node(s):8
> Vendor ID:   AuthenticAMD
> CPU family:  23
> Model:   1
> Model name:  AMD Eng Sample: 1S1901A4VIHF5_30/19_N
> Stepping:2
> CPU MHz: 2379.233
> CPU max MHz: 1900.
> CPU min MHz: 1200.
> BogoMIPS:3792.81
> Virtualization:  AMD-V
> L1d cache:   32K
> L1i cache:   64K
> L2 cache:512K
> L3 cache:8192K
> NUMA node0 CPU(s):   0-7
> NUMA node1 CPU(s):   8-15
> NUMA node2 CPU(s):   16-23
> NUMA node3 CPU(s):   24-31
> NUMA node4 CPU(s):   32-39
> NUMA node5 CPU(s):   40-47
> NUMA node6 CPU(s):   48-55
> NUMA node7 CPU(s):   56-63
> 
> Here is the output of #cpuid  -l 0x801e  -r


(1)
> You may want to refer
> https://www.amd.com/system/files/TechDocs/54945_3.03_ppr_ZP_B2_pub.zip
> (section 2.1.12.2.1.3 ApicId Enumeration Requirements).
> Note that this is a general guideline. We tried to generalize in qemu as
> much as possible. It is bit complex.


 
> CPU 0:
>0x801e 0x00: eax=0x ebx=0x0100 ecx=0x0300
> edx=0x
[...]
> CPU 63:
>0x801e 0x00: eax=0x007e ebx=0x011f ecx=0x0307
> edx=0x
> 
> >   
> > >  
> > > >
> > > > I wonder if linux guest actually uses node_id encoded in apic id for
> > > > configuring/checking numa structures, or it just uses whatever ACPI
> > > > SRAT table provided.
> > > >  
> > >  + */
> > >  +static inline void x86_init_topo_ids(X86CPUTopoInfo *topo_info,
> > >  + CpuInstanceProperties props,
> > >  + X86CPUTopoIDs *topo_ids) {
> > >  +topo_ids->smt_id = props.has_thread_id ? props.thread_id : 0;
> > >  +topo_ids->core_id = props.has_core_id ? props.core_id : 0;
> > > 

Re: [PATCH v2 3/4] build: Don't make object files for dtrace on macOS

[Roman Bolshakov]

On Fri, Jul 17, 2020 at 12:35:16PM +0300, Roman Bolshakov wrote:
> dtrace on macOS uses unresolved symbols with a special prefix to define
> probes [1], only headers should be generated for USDT (dtrace(1)). But
> it doesn't support backwards compatible no-op -G flag [2] and implicit
> build rules fail.
> 
> 1. https://markmail.org/message/6grq2ygr5nwdwsnb
> 2. https://markmail.org/message/5xrxt2w5m42nojkz
> 
> Reviewed-by: Daniel P. Berrangé 
> Cc: Cameron Esfahani 
> Signed-off-by: Roman Bolshakov 
> ---
>  Makefile.objs | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index d22b3b45d7..982f15ba30 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -211,5 +211,7 @@ trace-events-files = $(SRC_PATH)/trace-events 
> $(trace-events-subdirs:%=$(SRC_PAT
>  trace-obj-y = trace-root.o
>  trace-obj-y += $(trace-events-subdirs:%=%/trace.o)
>  trace-obj-$(CONFIG_TRACE_UST) += trace-ust-all.o
> +ifneq ($(CONFIG_DARWIN),y)
>  trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace-root.o
>  trace-obj-$(CONFIG_TRACE_DTRACE) += 
> $(trace-events-subdirs:%=%/trace-dtrace.o)
> +endif
> -- 
> 2.26.1
> 

An article about DTrace [1] mentions that FreeBSD also doesn't need that:
"On FreeBSD/Mac OS X, you do not have to generate a separate probe
object file for linking. This makes the compilation process much more
straightforward [...]"

I don't know for sure but perhaps "-G" makes dummy object files there.

1. https://www.ibm.com/developerworks/aix/library/au-dtraceprobes.html

Thanks,
Roman

Re: [PATCH 3/3] scripts/qmp/qom-fuse: Fix getattr(), read() for files in /

[John Snow]

On 7/23/20 10:27 AM, Markus Armbruster wrote:

path, prop = "type".rsplit('/', 1) sets path to "", which doesn't
work.  Correct to "/".



BOTD. If it works for you, that's good news.

Reviewed-by: John Snow 


Signed-off-by: Markus Armbruster 
---
  scripts/qmp/qom-fuse | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index 405e6ebd67..7c7cff8edf 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -45,8 +45,10 @@ class QOMFS(Operations):
  return False
  
  def is_property(self, path):

+path, prop = path.rsplit('/', 1)
+if path == '':
+path = '/'
  try:
-path, prop = path.rsplit('/', 1)
  for item in self.qmp.command('qom-list', path=path):
  if item['name'] == prop:
  return True
@@ -55,8 +57,10 @@ class QOMFS(Operations):
  return False
  
  def is_link(self, path):

+path, prop = path.rsplit('/', 1)
+if path == '':
+path = '/'
  try:
-path, prop = path.rsplit('/', 1)
  for item in self.qmp.command('qom-list', path=path):
  if item['name'] == prop:
  if item['type'].startswith('link<'):
@@ -71,6 +75,8 @@ class QOMFS(Operations):
  return -ENOENT
  
  path, prop = path.rsplit('/', 1)

+if path == '':
+path = '/'
  try:
  data = self.qmp.command('qom-get', path=path, property=prop)
  data += '\n' # make values shell friendly

Re: [PATCH v7 12/21] multi-process: Connect Proxy Object with device in the remote process

[Jag Raman]

> On Jul 1, 2020, at 5:20 AM, Stefan Hajnoczi  wrote:
> 
> On Sat, Jun 27, 2020 at 10:09:34AM -0700, elena.ufimts...@oracle.com wrote:
>> From: Jagannathan Raman 
>> 
>> Send a message to the remote process to connect PCI device with the
>> corresponding Proxy object in QEMU
> 
> I thought the protocol was simplified to a 1:1 device:socket model, but
> this patch seems to implement an N:1 model?

Hi Stefan,

Thanks for your feedback on all the patches. We were able to address most
of your feedback in this series, and we are looking forward to sending the next
version out for review.

We wanted to double check with you regarding this patch alone.

The protocol is still a 1:1 device:socket model. It’s not possible for a proxy 
device
object to send messages to arbitrary devices in the remote.

> 
> In a 1:1 model the CONNECT_DEV message is not necessary because each
> socket is already associated with a specific remote device (e.g. qemu -M
> remote -object mplink,dev=lsi-scsi-1,sockpath=/tmp/lsi-scsi-1.sock).
> Connecting to the socket already indicates which device we are talking
> to.
> 
> The N:1 model will work but it's more complex. There is a main socket
> that is used for CONNECT_DEV (anything else?) and we need to worry about
> the lifecycle of the per-device sockets that are passed over the main
> socket.

The main socket is only used for CONNECT_DEV. The CONNECT_DEV
message sticks a fd to the remote device.

We are using the following command-line in the remote process:
qemu-system-x86_64 -machine remote,fd=4 -device lsi53c895a,id=lsi1 ...

The alternative approach would be to let the orchestrator to assign fds for
each remote device. In this approach, we would specify an ‘fd’ for each
device object like below:
qemu-system-x86_64 -machine remote -device lsi53c895a,id=lsi1,fd=4 …

The alternative approach would entail changes to the DeviceState struct
and qdev_device_add() function, which we thought is not preferable. Could
you please share your thoughts on this?

Thanks!
—
Jag

> 
>> @@ -50,3 +58,34 @@ gboolean mpqemu_process_msg(QIOChannel *ioc, GIOCondition 
>> cond,
>> 
>> return TRUE;
>> }
>> +
>> +static void process_connect_dev_msg(MPQemuMsg *msg, QIOChannel *com,
>> +Error **errp)
>> +{
>> +char *devid = (char *)msg->data2;
>> +QIOChannel *dioc = NULL;
>> +DeviceState *dev = NULL;
>> +MPQemuMsg ret = { 0 };
>> +int rc = 0;
>> +
>> +g_assert(devid && (devid[msg->size - 1] == '\0'));
> 
> Asserts are not suitable for external input validation since a failure
> aborts the program and lets the client cause a denial-of-service. When
> there are multiple clients, one misbehaved client shouldn't be able to
> kill the server. Please validate devid using an if statement and set
> errp on failure.
> 
> Can msg->size be 0? If yes, this code accesses before the beginning of
> the buffer.
> 
>> +
>> +dev = qdev_find_recursive(sysbus_get_default(), devid);
>> +if (!dev || !object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>> +rc = 0xff;
>> +goto exit;
>> +}
>> +
>> +dioc = qio_channel_new_fd(msg->fds[0], errp);
> 
> Missing error handling if qio_channel_new_fd() fails. We need to
> close(msg->fds[0]) ourselves in this case.
> 
>> +
>> +qio_channel_add_watch(dioc, G_IO_IN | G_IO_HUP, mpqemu_process_msg,
>> +  (void *)dev, NULL);
>> +
>> +exit:
>> +ret.cmd = RET_MSG;
>> +ret.bytestream = 0;
>> +ret.data1.u64 = rc;
>> +ret.size = sizeof(ret.data1);
>> +
>> +mpqemu_msg_send(, com);
>> +}
>> diff --git a/hw/pci/proxy.c b/hw/pci/proxy.c
>> index 6d62399c52..16649ed0ec 100644
>> --- a/hw/pci/proxy.c
>> +++ b/hw/pci/proxy.c
>> @@ -15,10 +15,38 @@
>> #include "io/channel-util.h"
>> #include "hw/qdev-properties.h"
>> #include "monitor/monitor.h"
>> +#include "io/mpqemu-link.h"
>> 
>> static void proxy_set_socket(PCIProxyDev *pdev, int fd, Error **errp)
>> {
>> +DeviceState *dev = DEVICE(pdev);
>> +MPQemuMsg msg = { 0 };
>> +int fds[2];
>> +Error *local_err = NULL;
>> +
>> pdev->com = qio_channel_new_fd(fd, errp);
>> +
>> +if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds)) {
>> +error_setg(errp, "Failed to create proxy channel with fd %d", fd);
>> +return;
> 
> pdev->com needs to be cleaned up.
> 
>> diff --git a/io/mpqemu-link.c b/io/mpqemu-link.c
>> index 5887c8c6c0..54df3b254e 100644
>> --- a/io/mpqemu-link.c
>> +++ b/io/mpqemu-link.c
>> @@ -234,6 +234,14 @@ bool mpqemu_msg_valid(MPQemuMsg *msg)
>> return false;
>> }
>> break;
>> +case CONNECT_DEV:
>> +if ((msg->num_fds != 1) ||
>> +(msg->fds[0] == -1) ||
>> +(msg->fds[0] == -1) ||
> 
> This line is duplicated.

Re: [PATCH 2/3] scripts/qmp/qom-fuse: Port to current Python module fuse

[John Snow]

On 7/23/20 10:27 AM, Markus Armbruster wrote:

Signed-off-by: Markus Armbruster 


Honestly, benefit of the doubt on this one. The Python looks fine, but I 
don't know much about the FUSE module. Still, it was broken before, so 
if you claim it now works for you, that's more useful than it used to be.


Reviewed-by: John Snow 


---
  scripts/qmp/qom-fuse | 93 ++--
  1 file changed, 47 insertions(+), 46 deletions(-)

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index b7dabe8d65..405e6ebd67 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -3,16 +3,18 @@
  # QEMU Object Model test tools
  #
  # Copyright IBM, Corp. 2012
+# Copyright (C) 2020 Red Hat, Inc.
  #
  # Authors:
  #  Anthony Liguori   
+#  Markus Armbruster 
  #
  # This work is licensed under the terms of the GNU GPL, version 2 or later.  
See
  # the COPYING file in the top-level directory.
  ##
  
  import fuse, stat

-from fuse import Fuse
+from fuse import FUSE, FuseOSError, Operations
  import os, posix, sys
  from errno import *
  
@@ -21,9 +23,8 @@ from qemu.qmp import QEMUMonitorProtocol
  
  fuse.fuse_python_api = (0, 2)
  
-class QOMFS(Fuse):

-def __init__(self, qmp, *args, **kwds):
-Fuse.__init__(self, *args, **kwds)
+class QOMFS(Operations):
+def __init__(self, qmp):
  self.qmp = qmp
  self.qmp.connect()
  self.ino_map = {}
@@ -65,21 +66,21 @@ class QOMFS(Fuse):
  except:
  return False
  
-def read(self, path, length, offset):

+def read(self, path, length, offset, fh):
  if not self.is_property(path):
  return -ENOENT
  
  path, prop = path.rsplit('/', 1)

  try:
-data = str(self.qmp.command('qom-get', path=path, property=prop))
+data = self.qmp.command('qom-get', path=path, property=prop)
  data += '\n' # make values shell friendly
  except:
-return -EPERM
+raise FuseOSError(EPERM)
  
  if offset > len(data):

  return ''
  
-return str(data[offset:][:length])

+return bytes(data[offset:][:length], encoding='utf-8')
  
  def readlink(self, path):

  if not self.is_link(path):
@@ -89,52 +90,52 @@ class QOMFS(Fuse):
  return prefix + str(self.qmp.command('qom-get', path=path,
   property=prop))
  
-def getattr(self, path):

+def getattr(self, path, fh=None):
  if self.is_link(path):
-value = posix.stat_result((0o755 | stat.S_IFLNK,
-   self.get_ino(path),
-   0,
-   2,
-   1000,
-   1000,
-   4096,
-   0,
-   0,
-   0))
+value = { 'st_mode': 0o755 | stat.S_IFLNK,
+  'st_ino': self.get_ino(path),
+  'st_dev': 0,
+  'st_nlink': 2,
+  'st_uid': 1000,
+  'st_gid': 1000,
+  'st_size': 4096,
+  'st_atime': 0,
+  'st_mtime': 0,
+  'st_ctime': 0 }
  elif self.is_object(path):
-value = posix.stat_result((0o755 | stat.S_IFDIR,
-   self.get_ino(path),
-   0,
-   2,
-   1000,
-   1000,
-   4096,
-   0,
-   0,
-   0))
+value = { 'st_mode': 0o755 | stat.S_IFDIR,
+  'st_ino': self.get_ino(path),
+  'st_dev': 0,
+  'st_nlink': 2,
+  'st_uid': 1000,
+  'st_gid': 1000,
+  'st_size': 4096,
+  'st_atime': 0,
+  'st_mtime': 0,
+  'st_ctime': 0 }
  elif self.is_property(path):
-value = posix.stat_result((0o644 | stat.S_IFREG,
-   self.get_ino(path),
-   0,
-   1,
-   1000,
-   1000,
-   4096,
-   0,
-   0,
-   0))
+value = { 'st_mode': 0o644 | stat.S_IFREG,
+  'st_ino': self.get_ino(path),
+   

[PATCH-for-5.2] default-configs: Remove ACPI_CPU_HOTPLUG from MIPS machines

[Philippe Mathieu-Daudé]

No MIPS machine uses the ACPI cpu-hotplug feature
(QEMU implementation is X86 specific).

Fixes: 135a67a692 ("ACPI: split CONFIG_ACPI into 4 pieces")
Signed-off-by: Philippe Mathieu-Daudé 
---
 default-configs/mips-softmmu-common.mak | 1 -
 1 file changed, 1 deletion(-)

diff --git a/default-configs/mips-softmmu-common.mak 
b/default-configs/mips-softmmu-common.mak
index da29c6c0b2..e9c208da3d 100644
--- a/default-configs/mips-softmmu-common.mak
+++ b/default-configs/mips-softmmu-common.mak
@@ -21,7 +21,6 @@ CONFIG_ACPI=y
 CONFIG_ACPI_X86=y
 CONFIG_ACPI_MEMORY_HOTPLUG=y
 CONFIG_ACPI_NVDIMM=y
-CONFIG_ACPI_CPU_HOTPLUG=y
 CONFIG_APM=y
 CONFIG_I8257=y
 CONFIG_PIIX4=y
-- 
2.21.3

Re: [PATCH 0/3] scripts/qmp/qom-fuse: Scrape off the bitrot

[John Snow]

On 7/23/20 11:21 AM, Thomas Huth wrote:

On 23/07/2020 16.27, Markus Armbruster wrote:

Markus Armbruster (3):
   scripts/qmp/qom-fuse: Unbreak import of QEMUMonitorProtocol
   scripts/qmp/qom-fuse: Port to current Python module fuse
   scripts/qmp/qom-fuse: Fix getattr(), read() for files in /


Could it be added to a CI pipeline, so that it does not bitrot again?

  Thomas



Honestly, I'm working on it, but I could use some help getting the 
python directory into shape so I can do it.


I am trying to add pylint/mypy/flake8 tests to python/qemu to prevent 
that code from bitrot, but the review/discussion there didn't go anywhere.


Once there is a solid regime in place for python/qemu/ that is part of 
CI, I can work on moving more scripts and tooling there to start 
including those as part of the CI regime to prevent rot.


--js

Re: [PATCH 1/3] scripts/qmp/qom-fuse: Unbreak import of QEMUMonitorProtocol

[John Snow]

On 7/23/20 10:27 AM, Markus Armbruster wrote:

Commit c7b942d7f8 "scripts/qmp: Fix shebang and imports" messed with
it for reasons I don't quite understand.  I do understand how it fails
now: it neglects to import sys.  Fix that.



Apologies. These scripts didn't appear to work because they don't have 
any clue where the script they are trying to import lives. I was working 
on a series that refactored ./python/qemu into a python package.


The back half of that series hasn't landed upstream yet, so the import 
refuddling looks an awful lot more arbitrary at the moment, but the idea 
is that the scripts SHOULD work without needing to explicitly set your 
PYTHONPATH. For the moment, I think that's better.


My ultimate end-game is to get most python scripts under ./python/ and 
checked with pylint/mypy etc. as it will help detect breaking changes if 
library routines change. I want to institute a tree-wide regime for 
python code management that has a unified vision about how imports work 
and so on.


I would hope that this would reduce confusion in the future about how to 
execute scripts, how to write import statements, etc.


Most of what I am doing is baby steps towards that.


It now fails because it expects an old version of module fuse.  That's
next.



See also my commit message: "There's more wrong with these scripts; ..."


Fixes: c7b942d7f84ef54f266921bf7668d43f1f2c7c79
Signed-off-by: Markus Armbruster 


Thanks:

Reviewed-by: John Snow 


---
  scripts/qmp/qom-fuse | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index 5fa6b3bf64..b7dabe8d65 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -13,7 +13,7 @@
  
  import fuse, stat

  from fuse import Fuse
-import os, posix
+import os, posix, sys
  from errno import *
  
  sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))

@@ -134,7 +134,7 @@ class QOMFS(Fuse):
  yield fuse.Direntry(str(item['name']))
  
  if __name__ == '__main__':

-import sys, os
+import os
  
  fs = QOMFS(QEMUMonitorProtocol(os.environ['QMP_SOCKET']))

  fs.main(sys.argv)

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Peter Maydell]

On Fri, 24 Jul 2020 at 17:46, Philippe Mathieu-Daudé  wrote:
> I guess we were expecting the distrib to update the pkg.

Apple's view is that you shouldn't be using the sasl header
at all but instead their proprietary crypto library APIs, so
I wouldn't expect them to ever ship something without the
deprecation warnings.

thanks
-- PMM

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Daniel P . Berrangé]

On Fri, Jul 24, 2020 at 06:46:23PM +0200, Philippe Mathieu-Daudé wrote:
> On 7/24/20 4:46 PM, Daniel P. Berrangé wrote:
> > On Fri, Jul 24, 2020 at 04:32:19PM +0200, Thomas Huth wrote:
> >> Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
> >> since -Werror is only enabled for Linux and MinGW builds by default. So
> >> let's enable them here now, too.
> >> For macOS, that unfortunately means that we have to disable the vnc-sasl
> >> feature, since this is marked as deprecated in the macOS headers and thus
> >> generates a lot of deprecation warnings.
> > 
> > I wonder if its possible to add
> > 
> > #pragma GCC diagnostic push
> > #pragma GCC diagnostic ignored "-Wdeprecated"
> > 
> > ...
> > 
> > #pragma GCC diagnostic pop
> > 
> > to silence just one source file ?
> 
> 3 years ago Peter said:
> 
> "The awkward part is
>  that it has to  be in force at the point where the deprecated
>  function is used, not where it's declared. So you can't just wrap
>  the #include of the ssl header in pragmas, you'd have to either
>  do it at every callsite or else over the whole .c file."

Nearly all our sasl code is isolated in ui/vnc-auth-sasl.c, so we
can just do pragma push/pop around that entire file.

There's then just two remaining cases in ui/vnc.c which are
easy enough to deal with, or we can move the calls out of vnc.c
into vnc-auth-sasl.c to fully isolate the code

> 
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg459264.html
> 
> I guess we were expecting the distrib to update the pkg.
> 
> > 
> > 
> > Regards,
> > Daniel
> > 
> 

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

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Philippe Mathieu-Daudé]

On 7/24/20 4:46 PM, Daniel P. Berrangé wrote:
> On Fri, Jul 24, 2020 at 04:32:19PM +0200, Thomas Huth wrote:
>> Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
>> since -Werror is only enabled for Linux and MinGW builds by default. So
>> let's enable them here now, too.
>> For macOS, that unfortunately means that we have to disable the vnc-sasl
>> feature, since this is marked as deprecated in the macOS headers and thus
>> generates a lot of deprecation warnings.
> 
> I wonder if its possible to add
> 
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wdeprecated"
> 
> ...
> 
> #pragma GCC diagnostic pop
> 
> to silence just one source file ?

3 years ago Peter said:

"The awkward part is
 that it has to  be in force at the point where the deprecated
 function is used, not where it's declared. So you can't just wrap
 the #include of the ssl header in pragmas, you'd have to either
 do it at every callsite or else over the whole .c file."

https://www.mail-archive.com/qemu-devel@nongnu.org/msg459264.html

I guess we were expecting the distrib to update the pkg.

> 
> 
> Regards,
> Daniel
> 

[PULL 0/3] Fixes 20200724 patches

[Gerd Hoffmann]

The following changes since commit 09e0cd773723219d21655587954da2769f64ba01:

  Merge remote-tracking branch 
'remotes/alistair/tags/pull-riscv-to-apply-20200722-1' into staging (2020-07-23 
19:00:42 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/fixes-20200724-pull-request

for you to fetch changes up to 9b52b17ba5e96cec182537715e87308108b47117:

  configure: Allow to build tools without pixman (2020-07-24 17:36:03 +0200)


bugfixes: virtio-input, usb-dwc2, pixman.



Peter Maydell (1):
  hw/input/virtio-input-hid.c: Don't undef CONFIG_CURSES

Thomas Huth (2):
  hw: Only compile the usb-dwc2 controller if it is really needed
  configure: Allow to build tools without pixman

 configure   | 2 +-
 hw/input/virtio-input-hid.c | 1 -
 hw/arm/Kconfig  | 1 +
 hw/usb/Kconfig  | 1 -
 4 files changed, 2 insertions(+), 3 deletions(-)

-- 
2.18.4

[PULL 2/3] hw/input/virtio-input-hid.c: Don't undef CONFIG_CURSES

[Gerd Hoffmann]

From: Peter Maydell 

virtio-input-hid.c undefines CONFIG_CURSES before including
ui/console.h. However since commits e2f82e924d057935 and b0766612d16da18
that header does not have behaviour dependent on CONFIG_CURSES.
Remove the now-unneeded undef.

Signed-off-by: Peter Maydell 
Reviewed-by: Thomas Huth 
Acked-by: Michael S. Tsirkin 
Message-id: 20200723192457.28136-1-peter.mayd...@linaro.org
Signed-off-by: Gerd Hoffmann 
---
 hw/input/virtio-input-hid.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
index 09cf2609854f..a7a244a95dbb 100644
--- a/hw/input/virtio-input-hid.c
+++ b/hw/input/virtio-input-hid.c
@@ -12,7 +12,6 @@
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-input.h"
 
-#undef CONFIG_CURSES
 #include "ui/console.h"
 
 #include "standard-headers/linux/input.h"
-- 
2.18.4

[PULL 3/3] configure: Allow to build tools without pixman

[Gerd Hoffmann]

From: Thomas Huth 

If pixman is not installed, it is currently not possible to run:

 .../configure  --disable-system --enable-tools

Seems like there was a dependency from one of the required source
files to pixman in the past, but since commit 1ac0206b2ae1ffaeec56
("qemu-timer.c: Trim list of included headers"), this dependency
should be gone. Thus allow to compile the tools without pixman now.

Signed-off-by: Thomas Huth 
Message-id: 20200723141123.14765-1-th...@redhat.com
Signed-off-by: Gerd Hoffmann 
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 4bd80ed507aa..2acc4d1465f8 100755
--- a/configure
+++ b/configure
@@ -4065,7 +4065,7 @@ fi
 ##
 # pixman support probe
 
-if test "$want_tools" = "no" && test "$softmmu" = "no"; then
+if test "$softmmu" = "no"; then
   pixman_cflags=
   pixman_libs=
 elif $pkg_config --atleast-version=0.21.8 pixman-1 > /dev/null 2>&1; then
-- 
2.18.4

[PULL 1/3] hw: Only compile the usb-dwc2 controller if it is really needed

[Gerd Hoffmann]

From: Thomas Huth 

The USB_DWC2 switch is currently "default y", so it is included in all
qemu-system-* builds, even if it is not needed. Even worse, it does a
"select USB", so USB devices are now showing up as available on targets
that do not support USB at all. This sysbus device should only be
included by the boards that need it, i.e. by the Raspi machines.

Fixes: 153ef1662c ("dwc-hsotg (dwc2) USB host controller emulation")
Signed-off-by: Thomas Huth 
Reviewed-by: Paul Zimmerman 
Message-id: 20200722154719.10130-1-th...@redhat.com
Signed-off-by: Gerd Hoffmann 
---
 hw/arm/Kconfig | 1 +
 hw/usb/Kconfig | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 4a224a6351ab..bc3a423940b7 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -315,6 +315,7 @@ config RASPI
 select FRAMEBUFFER
 select PL011 # UART
 select SDHCI
+select USB_DWC2
 
 config STM32F205_SOC
 bool
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index d4d8c37c2897..5e63dc75f815 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -48,7 +48,6 @@ config USB_MUSB
 
 config USB_DWC2
 bool
-default y
 select USB
 
 config TUSB6010
-- 
2.18.4

[PATCH 2/3] hw/arm/boot: Fix MTE for EL3 direct kernel boot

[Richard Henderson]

When booting an EL3 cpu with -kernel, we set up EL3 and then
drop down to EL2.  We need to enable access to v8.5-MemTag
tag allocation at EL3 before doing so.

Reported-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 hw/arm/boot.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index c44fd3382d..3e9816af80 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -739,6 +739,9 @@ static void do_cpu_reset(void *opaque)
 if (cpu_isar_feature(aa64_pauth, cpu)) {
 env->cp15.scr_el3 |= SCR_API | SCR_APK;
 }
+if (cpu_isar_feature(aa64_mte, cpu)) {
+env->cp15.scr_el3 |= SCR_ATA;
+}
 /* AArch64 kernels never boot in secure mode */
 assert(!info->secure_boot);
 /* This hook is only supported for AArch32 currently:
-- 
2.25.1

[PATCH for-5.1 0/3] target/arm: mte+pauth fixes

[Richard Henderson]

A couple of last minute fixes for MTE:

 (1) Peter pointed out that EL3's SCR.ATA needs to be set when
 we're booting a kernel directly.  Similarly for API & APK.

 (2) Vincenzo pointed out that with RRND=1, we can't rely on
 RGSR having being initialized.

 I suppose the only follow-on question here is whether it is
 better to minimize the number of calls to qemu_guest_getrandom,
 or instead to name that our IMPDEF algorithm and use it for
 every call to IRG.  We already have other user-space available
 RNG instructions that can drain the entropy pool, so this is
 not really different.


r~


Richard Henderson (3):
  hw/arm/boot: Fix PAUTH for EL3 direct kernel boot
  hw/arm/boot: Fix MTE for EL3 direct kernel boot
  target/arm: Improve IMPDEF algorithm for IRG

 hw/arm/boot.c   |  6 ++
 target/arm/mte_helper.c | 37 ++---
 2 files changed, 36 insertions(+), 7 deletions(-)

-- 
2.25.1

Re: [PATCH v2] gitlab-ci: Fix Avocado cache usage

[Philippe Mathieu-Daudé]

On 7/24/20 5:52 PM, Wainer dos Santos Moschetta wrote:
> Hi Philippe,
> 
> On 7/24/20 4:42 AM, Philippe Mathieu-Daudé wrote:
>> In commit 6957fd98dc ("gitlab: add avocado asset caching") we
>> tried to save the Avocado cache (as in commit c1073e44b4 with
>> Travis-CI) however it doesn't work as expected. For some reason
>> Avocado uses /root/avocado_cache/ which we can not select later.
>>
>> Manually generate a Avocado config to force the use of the
>> current directory.
>>
>> We add a new 'build-acceptance-cache' job that runs first,
>> (during the 'build' stage) to create/update the cache.
>>
>> The cache content is then pulled (but not updated) during the
>> 'test' stage.
>>
>> See:
>> - https://docs.gitlab.com/ee/ci/caching/
>> -
>> https://avocado-framework.readthedocs.io/en/latest/guides/writer/chapters/writing.html#fetching-asset-files
>>
>>
>> Reported-by: Thomas Huth 
>> Fixes: 6957fd98dc ("gitlab: add avocado asset caching")
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>> Since v1:
>> - add a specific 'build-acceptance-cache' job
>>
>> Thomas prefers to use a different cache for each job.
>> Since I had this patch ready, I prefer to post it as
>> v2 and will work on a v3 using Thomas suggestion.
>>
>> Supersedes: <20200723200318.28214-1-f4...@amsat.org>
>> Based-on: <20200724073524.26589-1-f4...@amsat.org>
>>    "tests: Add 'fetch-acceptance' rule"
>> ---
>>   .gitlab-ci.yml | 61 ++
>>   1 file changed, 52 insertions(+), 9 deletions(-)
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index 362e5ee755..a8d8a7e849 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -8,11 +8,9 @@ stages:
>>     - build
>>     - test
>>   -# We assume GitLab has it's own caching set up for RPM/APT
>> repositories so we
>> -# just take care of avocado assets here.
>> -cache:
>> -  paths:
>> -    - $HOME/avocado/data/cache
>> +# We assume GitLab has it's own caching set up for RPM/APT repositories
>> +cache: _cache
>> +  policy: pull
>>     include:
>>     - local: '/.gitlab-ci.d/edk2.yml'
>> @@ -47,11 +45,52 @@ include:
>>   - find . -type f -exec touch {} +
>>   - make $MAKE_CHECK_ARGS
>>   -.post_acceptance_template: _acceptance
>> +.acceptance_template: _definition
> 
> What if you:
> 
> - Keep the post_acceptance section which defines the common after_script
> only.
> 
> - Create the acceptance_definition as you did, with before_script only.
> This way it doesn't need to repeat the logic in build-acceptance-cache
> job definition.

See below [*].

> 
> 
>> +  cache:
>> +    # inherit all global cache settings
>> +    <<: *global_cache
>> +    key: acceptance_cache
>> +    paths:
>> +  - $CI_PROJECT_DIR/avocado_cache
>> +    policy: pull
> 
> Isn't this policy inherited from global settings already?

Uh, bug! I had it right and messed when cleaning before posting...
This one is "pull-push" (while global_cache is pull).

> 
>> +  before_script:
>> +    - JOBS=$(expr $(nproc) + 1)
>> +    - mkdir -p ~/.config/avocado
>> +    - echo "[datadir.paths]" > ~/.config/avocado/avocado.conf
>> +    - echo "cache_dirs = ['${CI_PROJECT_DIR}/avocado_cache']" >>
>> ~/.config/avocado/avocado.conf
>>     after_script:
>>   - cd build
>>   - python3 -c 'import json; r =
>> json.load(open("tests/results/latest/results.json"));
>> [print(t["logfile"]) for t in r["tests"] if t["status"] not in
>> ("PASS", "SKIP")]' | xargs cat
>> -    - du -chs $HOME/avocado/data/cache
>> +    - du -chs $CI_PROJECT_DIR/avocado_cache
>> +
>> +build-acceptance-cache:
>> +  stage: build
>> +  cache:
>> +    # inherit all global cache settings
>> +    <<: *global_cache
>> +    key: acceptance_cache
>> +    paths:
>> +  - $CI_PROJECT_DIR/avocado_cache
>> +    policy: pull-push
>> +  variables:
>> +    # any image should work
>> +    IMAGE: ubuntu2004
>> +    CONFIGURE_ARGS: --disable-user --disable-system
>> +  --disable-docs --disable-tools
>> +    MAKE_CHECK_ARGS: fetch-acceptance
>> +  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
>> +  before_script:
>> +    - mkdir -p ~/.config/avocado
>> +    - echo "[datadir.paths]" > ~/.config/avocado/avocado.conf
>> +    - echo "cache_dirs = ['${CI_PROJECT_DIR}/avocado_cache']" >>
>> ~/.config/avocado/avocado.conf
>> +  script:
>> +    - mkdir build
>> +    - cd build
>> +    - ../configure --disable-user --disable-system --disable-docs
>> --disable-tools
> Use the CONFIGURE_ARGS variable here, or not define it.
>> +    # ignore "asset fetched or already on cache" error
>> +    - make fetch-acceptance || true
> 
> Likewise for MAKE_CHECK_ARGS.

[*] The point here is to not call 'make -j"$JOBS"'. Using
variables for the same script seems over complicated IMO.

> 
> Regards,
> 
> Wainer
> 
>> +  after_script:
>> +    - du -chs $CI_PROJECT_DIR/avocado_cache
>>     build-system-ubuntu-main:
>>     <<: *native_build_job_definition
>> @@ -76,13 +115,15 @@ check-system-ubuntu-main:
>>     acceptance-system-ubuntu-main:

[PATCH 3/3] target/arm: Improve IMPDEF algorithm for IRG

[Richard Henderson]

When GCR_EL1.RRND==1, the choosing of the random value is IMPDEF,
and the kernel is not expected to have set RGSR_EL1.  Force a
non-zero value into SEED, so that we do not continually return
the same tag.

Reported-by: Vincenzo Frascino 
Signed-off-by: Richard Henderson 
---
 target/arm/mte_helper.c | 37 ++---
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 5ea57d487a..104752041f 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -24,6 +24,8 @@
 #include "exec/ram_addr.h"
 #include "exec/cpu_ldst.h"
 #include "exec/helper-proto.h"
+#include "qapi/error.h"
+#include "qemu/guest-random.h"
 
 
 static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude)
@@ -211,16 +213,37 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int 
ptr_mmu_idx,
 
 uint64_t HELPER(irg)(CPUARMState *env, uint64_t rn, uint64_t rm)
 {
-int rtag;
-
-/*
- * Our IMPDEF choice for GCR_EL1.RRND==1 is to behave as if
- * GCR_EL1.RRND==0, always producing deterministic results.
- */
 uint16_t exclude = extract32(rm | env->cp15.gcr_el1, 0, 16);
+int rrnd = extract32(env->cp15.gcr_el1, 16, 1);
 int start = extract32(env->cp15.rgsr_el1, 0, 4);
 int seed = extract32(env->cp15.rgsr_el1, 8, 16);
-int offset, i;
+int offset, i, rtag;
+
+/*
+ * Our IMPDEF choice for GCR_EL1.RRND==1 is to continue to use the
+ * deterministic algorithm.  Except that with RRND==1 the kernel is
+ * not required to have set RGSR_EL1.SEED != 0, which is required for
+ * the deterministic algorithm to function.  So we force a non-zero
+ * SEED for that case.
+ */
+if (unlikely(seed == 0) && rrnd) {
+do {
+Error *err = NULL;
+uint16_t two;
+
+if (qemu_guest_getrandom(, sizeof(two), ) < 0) {
+/*
+ * Failed, for unknown reasons in the crypto subsystem.
+ * Best we can do is log the reason and use a constant seed.
+ */
+qemu_log_mask(LOG_UNIMP, "IRG: Crypto failure: %s\n",
+  error_get_pretty(err));
+error_free(err);
+two = 1;
+}
+seed = two;
+} while (seed == 0);
+}
 
 /* RandomTag */
 for (i = offset = 0; i < 4; ++i) {
-- 
2.25.1

[PATCH 1/3] hw/arm/boot: Fix PAUTH for EL3 direct kernel boot

[Richard Henderson]

When booting an EL3 cpu with -kernel, we set up EL3 and then
drop down to EL2.  We need to enable access to v8.3-PAuth
keys and instructions at EL3 before doing so.

Signed-off-by: Richard Henderson 
---
 hw/arm/boot.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index fef4072db1..c44fd3382d 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -736,6 +736,9 @@ static void do_cpu_reset(void *opaque)
 } else {
 env->pstate = PSTATE_MODE_EL1h;
 }
+if (cpu_isar_feature(aa64_pauth, cpu)) {
+env->cp15.scr_el3 |= SCR_API | SCR_APK;
+}
 /* AArch64 kernels never boot in secure mode */
 assert(!info->secure_boot);
 /* This hook is only supported for AArch32 currently:
-- 
2.25.1

Re: [PATCH 1/1] python/machine: Change default timeout to 30 seconds

[John Snow]

On 7/20/20 4:20 PM, Eduardo Habkost wrote:

On Mon, Jul 20, 2020 at 12:02:52PM -0400, John Snow wrote:

3 seconds is too short for some tests running inside busy VMs. Build it out to
a rather generous 30 seconds to find out conclusively if there are more severe
problems in the merge/CI tests.

Signed-off-by: John Snow 


It's weird how the hard shutdown method has a more graceful
timeout than graceful shutdown (60 seconds vs 3 seconds).

I would make both have the same timeout, but it's better to try
this only after 5.1.0.

Reviewed-by: Eduardo Habkost 




Peter, do you want to take this directly to see if it starts to fix the 
merge tests for you?


It extends the shutdown timeout from 3 to 30 seconds. If it still hangs 
at 30 seconds, I think there's clearly something much worse going on 
that will need to be investigated.


--js

Re: [PATCH v0 0/4] background snapshot

[Peter Xu]

On Fri, Jul 24, 2020 at 11:06:17AM +0300, Denis Plotnikov wrote:
> 
> 
> On 23.07.2020 20:39, Peter Xu wrote:
> > On Thu, Jul 23, 2020 at 11:03:55AM +0300, Denis Plotnikov wrote:
> > > 
> > > On 22.07.2020 19:30, Peter Xu wrote:
> > > > On Wed, Jul 22, 2020 at 06:47:44PM +0300, Denis Plotnikov wrote:
> > > > > On 22.07.2020 18:42, Denis Plotnikov wrote:
> > > > > > On 22.07.2020 17:50, Peter Xu wrote:
> > > > > > > Hi, Denis,
> > > > > > Hi, Peter
> > > > > > > ...
> > > > > > > > How to use:
> > > > > > > > 1. enable background snapshot capability
> > > > > > > >       virsh qemu-monitor-command vm --hmp migrate_set_capability
> > > > > > > > background-snapshot on
> > > > > > > > 
> > > > > > > > 2. stop the vm
> > > > > > > >       virsh qemu-monitor-command vm --hmp stop
> > > > > > > > 
> > > > > > > > 3. Start the external migration to a file
> > > > > > > >       virsh qemu-monitor-command cent78-bs --hmp migrate 
> > > > > > > > exec:'cat
> > > > > > > > > ./vm_state'
> > > > > > > > 4. Wait for the migration finish and check that the migration
> > > > > > > > has completed state.
> > > > > > > Thanks for continued working on this project! I have two high 
> > > > > > > level
> > > > > > > questions
> > > > > > > before dig into the patches.
> > > > > > > 
> > > > > > > Firstly, is step 2 required?  Can we use a single QMP command to
> > > > > > > take snapshots
> > > > > > > (which can still be a "migrate" command)?
> > > > > > With this series it is required, but steps 2 and 3 should be merged 
> > > > > > into
> > > > > > a single one.
> > > > I'm not sure whether you're talking about the disk snapshot operations, 
> > > > anyway
> > > > yeah it'll be definitely good if we merge them into one in the next 
> > > > version.
> > > After thinking for a while, I remembered why I split these two steps.
> > > The vm snapshot consists of two parts: disk(s) snapshot(s) and vmstate.
> > > With migrate command we save the vmstate only. So, the steps to save
> > > the whole vm snapshot is the following:
> > > 
> > > 2. stop the vm
> > >      virsh qemu-monitor-command vm --hmp stop
> > > 
> > > 2.1. Make a disk snapshot, something like
> > >  virsh qemu-monitor-command vm --hmp snapshot_blkdev 
> > > drive-scsi0-0-0-0 ./new_data
> > > 3. Start the external migration to a file
> > >      virsh qemu-monitor-command vm --hmp migrate exec:'cat ./vm_state'
> > > 
> > > In this example, vm snapshot consists of two files: vm_state and the disk 
> > > file. new_data will contain all new disk data written since [2.1.] 
> > > executing.
> > But that's slightly different to the current interface of savevm and loadvm
> > which only requires a snapshot name, am I right?
> 
> Yes
> > Now we need both a snapshot
> > name (of the vmstate) and the name of the new snapshot image.
> 
> Yes
> > 
> > I'm not familiar with qemu image snapshots... my understanding is that 
> > current
> > snapshot (save_snapshot) used internal image snapshots, while in this 
> > proposal
> > you want the live snapshot to use extrenal snapshots.
> Correct, I want to add ability to make a external live snapshot. (live =
> asyn ram writing)
> >Is there any criteria on
> > making this decision/change?
> Internal snapshot is supported by qcow2 and sheepdog (I never heard of
> someone using the later).
> Because of qcow2 internal snapshot design, it's quite complex to implement
> "background" snapshot there.
> More details here:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg705116.html
> So, I decided to start with external snapshot to implement and approve the
> memory access intercepting part firstly.
> Once it's done for external snapshot we can start to approach the internal
> snapshots.

Fair enough.  Let's start with external snapshot then.  Thanks,

-- 
Peter Xu

QEMU | Pipeline #170508500 has failed for master | 7adfbea8

[GitLab via]

Your pipeline has failed.

Project: QEMU ( https://gitlab.com/qemu-project/qemu )
Branch: master ( https://gitlab.com/qemu-project/qemu/-/commits/master )

Commit: 7adfbea8 ( 
https://gitlab.com/qemu-project/qemu/-/commit/7adfbea8fd1efce36019a0c2f198ca73be9d3f18
 )
Commit Message: Merge remote-tracking branch 'remotes/ehabkost/...
Commit Author: Peter Maydell ( https://gitlab.com/pm215 )

Pipeline #170508500 ( 
https://gitlab.com/qemu-project/qemu/-/pipelines/170508500 ) triggered by Alex 
Bennée ( https://gitlab.com/stsquad )
had 1 failed build.

Job #655232072 ( https://gitlab.com/qemu-project/qemu/-/jobs/655232072/raw )

Stage: test
Name: acceptance-system-ubuntu-main
Trace: 16:25:52 ERROR| 
16:25:52 ERROR| Reproduced traceback from: 
/builds/qemu-project/qemu/build/tests/venv/lib/python3.8/site-packages/avocado/core/test.py:846
16:25:52 ERROR| Traceback (most recent call last):
16:25:52 ERROR|   File 
"/builds/qemu-project/qemu/build/tests/acceptance/avocado_qemu/__init__.py", 
line 172, in setUp
16:25:52 ERROR| self.cancel("No QEMU binary defined or found in the build 
tree")
16:25:52 ERROR|   File 
"/builds/qemu-project/qemu/build/tests/venv/lib/python3.8/site-packages/avocado/core/test.py",
 line 1081, in cancel
16:25:52 ERROR| raise exceptions.TestCancel(message)
16:25:52 ERROR| avocado.core.exceptions.TestCancel: No QEMU binary defined or 
found in the build tree
16:25:52 ERROR| 
16:25:52 ERROR| CANCEL 
38-tests/acceptance/vnc.py:Vnc.test_change_password_requires_a_password -> 
TestCancel: No QEMU binary defined or found in the build tree
16:25:52 INFO | 
16:25:52 DEBUG| PARAMS (key=arch, path=*, default=None) => None
16:25:52 DEBUG| PARAMS (key=machine, path=*, default=None) => None
16:25:52 DEBUG| PARAMS (key=qemu_bin, path=*, default=None) => None
16:25:52 ERROR| 
16:25:52 ERROR| Reproduced traceback from: 
/builds/qemu-project/qemu/build/tests/venv/lib/python3.8/site-packages/avocado/core/test.py:846
16:25:52 ERROR| Traceback (most recent call last):
16:25:52 ERROR|   File 
"/builds/qemu-project/qemu/build/tests/acceptance/avocado_qemu/__init__.py", 
line 172, in setUp
16:25:52 ERROR| self.cancel("No QEMU binary defined or found in the build 
tree")
16:25:52 ERROR|   File 
"/builds/qemu-project/qemu/build/tests/venv/lib/python3.8/site-packages/avocado/core/test.py",
 line 1081, in cancel
16:25:52 ERROR| raise exceptions.TestCancel(message)
16:25:52 ERROR| avocado.core.exceptions.TestCancel: No QEMU binary defined or 
found in the build tree
16:25:52 ERROR| 
16:25:52 ERROR| CANCEL 39-tests/acceptance/vnc.py:Vnc.test_change_password -> 
TestCancel: No QEMU binary defined or found in the build tree
16:25:52 INFO | 
$ du -chs $HOME/avocado/data/cache
du: cannot access '/root/avocado/data/cache': No such file or directory
0   total
section_end:1595607956:after_script
ERROR: Job failed: exit code 1



-- 
You're receiving this email because of your account on gitlab.com.

Re: [ipxe-devel] https booting

[Michael Brown]

On 22/07/2020 15:13, Daniel P. Berrangé wrote:

We could easily define etc/ipxe/https/{ciphers,cacerts} paths in a
different format if better suited for iPXE. Libvirt can set the right
path depending on whether its booting a VM with EDK2 vs legacy BIOS


The most useful for iPXE would probably be to expose the fw_cfg 
mechanism as a URI scheme.  This would give a general mechanism allowing 
for use cases such as running a script provided by the host via e.g.


  chain fw_cfg:///opt/org.example/script.ipxe

The ${crosscert} setting could then be pointed at a base URL within the 
fw_cfg space, e.g.


  #define CROSSCERT "fw_cfg:///etc/ipxe/crosscert/auto"

This would then work in the same way under either BIOS or UEFI (or other 
custom firmware), would provide a feature with applicability broader 
than just obtaining certificates, and would avoid any potential problems 
from allocating enough RAM to parse every root certificate from iPXE's 
fixed 512kB internal heap.


What do you think?

Michael

Re: [PATCH v3 13/21] migration/block-dirty-bitmap: simplify dirty_bitmap_load_complete

[Eric Blake]

On 7/24/20 3:43 AM, Vladimir Sementsov-Ogievskiy wrote:

bdrv_enable_dirty_bitmap_locked() call does nothing, as if we are in
postcopy, bitmap successor must be enabled, and reclaim operation will
enable the bitmap.

So, actually we need just call _reclaim_ in both if branches, and
making differences only to add an assertion seems not really good. The
logic becomes simple: on load complete we do reclaim and that's all.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Andrey Shinkevich 
---
  migration/block-dirty-bitmap.c | 25 -
  1 file changed, 4 insertions(+), 21 deletions(-)


Looks like 8-13 are all cleanups with no semantic change.  As it makes 
the later bug fix easier, I'm fine including them in 5.1 if the bug fix 
is also 5.1 material.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[Bug 1888601] Re: QEMU v5.1.0-rc0/rc1 hang with nested virtualization

[Simon Kaegi]

It hangs (still guessing here) immediately -- before anything is logged.
I'll try to get you a calltrace but have to figure out how to do that
first ;) Any pointers appreciated.

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

Title:
  QEMU v5.1.0-rc0/rc1 hang with nested virtualization

Status in QEMU:
  New

Bug description:
  We're running Kata Containers using QEMU and with v5.1.0rc0 and rc1
  have noticed a problem at startup where QEMu appears to hang. We are
  not seeing this problem on our bare metal nodes and only on a VSI that
  supports nested virtualization.

  We unfortunately see nothing at all in the QEMU logs to help
  understand the problem and a hung process is just a guess at this
  point.

  Using git bisect we first see the problem with...

  ---

  f19bcdfedd53ee93412d535a842a89fa27cae7f2 is the first bad commit
  commit f19bcdfedd53ee93412d535a842a89fa27cae7f2
  Author: Jason Wang 
  Date:   Wed Jul 1 22:55:28 2020 +0800

  virtio-pci: implement queue_enabled method

  With version 1, we can detect whether a queue is enabled via
  queue_enabled.

  Signed-off-by: Jason Wang 
  Signed-off-by: Cindy Lu 
  Message-Id: <20200701145538.22333-5-l...@redhat.com>
  Reviewed-by: Michael S. Tsirkin 
  Signed-off-by: Michael S. Tsirkin 
  Acked-by: Jason Wang 

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

  ---

  Reverting this commit (on top of 5.1.0-rc1) seems to work and prevent
  the hanging.

  ---

  Here's how kata ends up launching qemu in our environment --
  /opt/kata/bin/qemu-system-x86_64 -name 
sandbox-849df14c6065931adedb9d18bc9260a6d896f1814a8c5cfa239865772f1b7a5f -uuid 
6bec458e-1da7-4847-a5d7-5ab31d4d2465 -machine pc,accel=kvm,kernel_irqchip -cpu 
host,pmu=off -qmp 
unix:/run/vc/vm/849df14c6065931adedb9d18bc9260a6d896f1814a8c5cfa239865772f1b7a5f/qmp.sock,server,nowait
 -m 4096M,slots=10,maxmem=30978M -device 
pci-bridge,bus=pci.0,id=pci-bridge-0,chassis_nr=1,shpc=on,addr=2,romfile= 
-device virtio-serial-pci,disable-modern=true,id=serial0,romfile= -device 
virtconsole,chardev=charconsole0,id=console0 -chardev 
socket,id=charconsole0,path=/run/vc/vm/849df14c6065931adedb9d18bc9260a6d896f1814a8c5cfa239865772f1b7a5f/console.sock,server,nowait
 -device virtio-scsi-pci,id=scsi0,disable-modern=true,romfile= -object 
rng-random,id=rng0,filename=/dev/urandom -device 
virtio-rng-pci,rng=rng0,romfile= -device 
virtserialport,chardev=charch0,id=channel0,name=agent.channel.0 -chardev 
socket,id=charch0,path=/run/vc/vm/849df14c6065931adedb9d18bc9260a6d896f1814a8c5cfa239865772f1b7a5f/kata.sock,server,nowait
 -chardev 
socket,id=char-396c5c3e19e29353,path=/run/vc/vm/849df14c6065931adedb9d18bc9260a6d896f1814a8c5cfa239865772f1b7a5f/vhost-fs.sock
 -device 
vhost-user-fs-pci,chardev=char-396c5c3e19e29353,tag=kataShared,romfile= -netdev 
tap,id=network-0,vhost=on,vhostfds=3:4,fds=5:6 -device 
driver=virtio-net-pci,netdev=network-0,mac=52:ac:2d:02:1f:6f,disable-modern=true,mq=on,vectors=6,romfile=
 -global kvm-pit.lost_tick_policy=discard -vga none -no-user-config -nodefaults 
-nographic -daemonize -object 
memory-backend-file,id=dimm1,size=4096M,mem-path=/dev/shm,share=on -numa 
node,memdev=dimm1 -kernel /opt/kata/share/kata-containers/vmlinuz-5.7.9-74 
-initrd 
/opt/kata/share/kata-containers/kata-containers-initrd_alpine_1.11.2-6_agent.initrd
 -append tsc=reliable no_timer_check rcupdate.rcu_expedited=1 i8042.direct=1 
i8042.dumbkbd=1 i8042.nopnp=1 i8042.noaux=1 noreplace-smp reboot=k console=hvc0 
console=hvc1 iommu=off cryptomgr.notests net.ifnames=0 pci=lastbus=0 debug 
panic=1 nr_cpus=4 agent.use_vsock=false scsi_mod.scan=none 
init=/usr/bin/kata-agent -pidfile 
/run/vc/vm/849df14c6065931adedb9d18bc9260a6d896f1814a8c5cfa239865772f1b7a5f/pid 
-D 
/run/vc/vm/849df14c6065931adedb9d18bc9260a6d896f1814a8c5cfa239865772f1b7a5f/qemu.log
 -smp 2,cores=1,threads=1,sockets=4,maxcpus=4

  ---

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

Re: [PATCH v7 38/47] block: Drop backing_bs()

[Andrey Shinkevich]

On 25.06.2020 18:22, Max Reitz wrote:

We want to make it explicit where bs->backing is used, and we have done
so.  The old role of backing_bs() is now effectively taken by
bdrv_cow_bs().

Signed-off-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
  include/block/block_int.h | 5 -
  1 file changed, 5 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index c963ee9f28..6e09e15ed4 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -991,11 +991,6 @@ typedef enum BlockMirrorBackingMode {
  MIRROR_LEAVE_BACKING_CHAIN,
  } BlockMirrorBackingMode;
  
-static inline BlockDriverState *backing_bs(BlockDriverState *bs)

-{
-return bs->backing ? bs->backing->bs : NULL;
-}
-
  
  /* Essential block drivers which must always be statically linked into qemu, and

   * which therefore can be accessed without using bdrv_find_format() */



Reviewed-by: Andrey Shinkevich 

Re: [PATCH v2] gitlab-ci: Fix Avocado cache usage

[Wainer dos Santos Moschetta]

Hi Philippe,

On 7/24/20 4:42 AM, Philippe Mathieu-Daudé wrote:

In commit 6957fd98dc ("gitlab: add avocado asset caching") we
tried to save the Avocado cache (as in commit c1073e44b4 with
Travis-CI) however it doesn't work as expected. For some reason
Avocado uses /root/avocado_cache/ which we can not select later.

Manually generate a Avocado config to force the use of the
current directory.

We add a new 'build-acceptance-cache' job that runs first,
(during the 'build' stage) to create/update the cache.

The cache content is then pulled (but not updated) during the
'test' stage.

See:
- https://docs.gitlab.com/ee/ci/caching/
- 
https://avocado-framework.readthedocs.io/en/latest/guides/writer/chapters/writing.html#fetching-asset-files

Reported-by: Thomas Huth 
Fixes: 6957fd98dc ("gitlab: add avocado asset caching")
Signed-off-by: Philippe Mathieu-Daudé 
---
Since v1:
- add a specific 'build-acceptance-cache' job

Thomas prefers to use a different cache for each job.
Since I had this patch ready, I prefer to post it as
v2 and will work on a v3 using Thomas suggestion.

Supersedes: <20200723200318.28214-1-f4...@amsat.org>
Based-on: <20200724073524.26589-1-f4...@amsat.org>
   "tests: Add 'fetch-acceptance' rule"
---
  .gitlab-ci.yml | 61 ++
  1 file changed, 52 insertions(+), 9 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 362e5ee755..a8d8a7e849 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -8,11 +8,9 @@ stages:
- build
- test
  
-# We assume GitLab has it's own caching set up for RPM/APT repositories so we

-# just take care of avocado assets here.
-cache:
-  paths:
-- $HOME/avocado/data/cache
+# We assume GitLab has it's own caching set up for RPM/APT repositories
+cache: _cache
+  policy: pull
  
  include:

- local: '/.gitlab-ci.d/edk2.yml'
@@ -47,11 +45,52 @@ include:
  - find . -type f -exec touch {} +
  - make $MAKE_CHECK_ARGS
  
-.post_acceptance_template: _acceptance

+.acceptance_template: _definition


What if you:

- Keep the post_acceptance section which defines the common after_script 
only.


- Create the acceptance_definition as you did, with before_script only. 
This way it doesn't need to repeat the logic in build-acceptance-cache 
job definition.




+  cache:
+# inherit all global cache settings
+<<: *global_cache
+key: acceptance_cache
+paths:
+  - $CI_PROJECT_DIR/avocado_cache
+policy: pull


Isn't this policy inherited from global settings already?


+  before_script:
+- JOBS=$(expr $(nproc) + 1)
+- mkdir -p ~/.config/avocado
+- echo "[datadir.paths]" > ~/.config/avocado/avocado.conf
+- echo "cache_dirs = ['${CI_PROJECT_DIR}/avocado_cache']" >> 
~/.config/avocado/avocado.conf
after_script:
  - cd build
  - python3 -c 'import json; r = json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) for t 
in r["tests"] if t["status"] not in ("PASS", "SKIP")]' | xargs cat
-- du -chs $HOME/avocado/data/cache
+- du -chs $CI_PROJECT_DIR/avocado_cache
+
+build-acceptance-cache:
+  stage: build
+  cache:
+# inherit all global cache settings
+<<: *global_cache
+key: acceptance_cache
+paths:
+  - $CI_PROJECT_DIR/avocado_cache
+policy: pull-push
+  variables:
+# any image should work
+IMAGE: ubuntu2004
+CONFIGURE_ARGS: --disable-user --disable-system
+  --disable-docs --disable-tools
+MAKE_CHECK_ARGS: fetch-acceptance
+  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
+  before_script:
+- mkdir -p ~/.config/avocado
+- echo "[datadir.paths]" > ~/.config/avocado/avocado.conf
+- echo "cache_dirs = ['${CI_PROJECT_DIR}/avocado_cache']" >> 
~/.config/avocado/avocado.conf
+  script:
+- mkdir build
+- cd build
+- ../configure --disable-user --disable-system --disable-docs 
--disable-tools

Use the CONFIGURE_ARGS variable here, or not define it.

+# ignore "asset fetched or already on cache" error
+- make fetch-acceptance || true


Likewise for MAKE_CHECK_ARGS.

Regards,

Wainer


+  after_script:
+- du -chs $CI_PROJECT_DIR/avocado_cache
  
  build-system-ubuntu-main:

<<: *native_build_job_definition
@@ -76,13 +115,15 @@ check-system-ubuntu-main:
  
  acceptance-system-ubuntu-main:

<<: *native_test_job_definition
+  <<: *acceptance_definition
needs:
  - job: build-system-ubuntu-main
artifacts: true
+- job: build-acceptance-cache
+  artifacts: false
variables:
  IMAGE: ubuntu2004
  MAKE_CHECK_ARGS: check-acceptance
-  <<: *post_acceptance
  
  build-system-fedora-alt:

<<: *native_build_job_definition
@@ -107,13 +148,15 @@ check-system-fedora-alt:
  
  acceptance-system-fedora-alt:

<<: *native_test_job_definition
+  <<: *acceptance_definition
needs:
  - job: build-system-fedora-alt
artifacts: true
+- job: build-acceptance-cache
+  artifacts: false
variables:
  IMAGE: 

Re: [PATCH v7 37/47] qemu-img: Use child access functions

[Andrey Shinkevich]

On 25.06.2020 18:22, Max Reitz wrote:

This changes iotest 204's output, because blkdebug on top of a COW node
used to make qemu-img map disregard the rest of the backing chain (the
backing chain was broken by the filter).  With this patch, the
allocation in the base image is reported correctly.

Signed-off-by: Max Reitz 
---
  qemu-img.c | 36 ++--
  tests/qemu-iotests/204.out |  1 +
  2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 381271a74e..947be6ffac 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1089,7 +1089,7 @@ static int img_commit(int argc, char **argv)
  /* This is different from QMP, which by default uses the deepest file 
in
   * the backing chain (i.e., the very base); however, the traditional
   * behavior of qemu-img commit is using the immediate backing file. */
-base_bs = backing_bs(bs);
+base_bs = bdrv_backing_chain_next(bs);
  if (!base_bs) {
  error_setg(_err, "Image does not have a backing file");
  goto done;
@@ -1737,18 +1737,20 @@ static int convert_iteration_sectors(ImgConvertState 
*s, int64_t sector_num)
  if (s->sector_next_status <= sector_num) {
  uint64_t offset = (sector_num - src_cur_offset) * BDRV_SECTOR_SIZE;
  int64_t count;
+BlockDriverState *src_bs = blk_bs(s->src[src_cur]);
+BlockDriverState *base;
+
+if (s->target_has_backing) {
+base = bdrv_cow_bs(bdrv_skip_filters(src_bs));
+} else {
+base = NULL;
+}
  
  do {

  count = n * BDRV_SECTOR_SIZE;
  
-if (s->target_has_backing) {

-ret = bdrv_block_status(blk_bs(s->src[src_cur]), offset,
-count, , NULL, NULL);
-} else {
-ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
-  offset, count, , NULL,
-  NULL);
-}
+ret = bdrv_block_status_above(src_bs, base, offset, count, ,
+  NULL, NULL);
  
  if (ret < 0) {

  if (s->salvage) {
@@ -2673,7 +2675,8 @@ static int img_convert(int argc, char **argv)
   * s.target_backing_sectors has to be negative, which it will
   * be automatically).  The backing file length is used only
   * for optimizations, so such a case is not fatal. */
-s.target_backing_sectors = bdrv_nb_sectors(out_bs->backing->bs);
+s.target_backing_sectors =
+bdrv_nb_sectors(bdrv_backing_chain_next(out_bs));
  } else {
  s.target_backing_sectors = -1;
  }
@@ -3044,6 +3047,7 @@ static int get_block_status(BlockDriverState *bs, int64_t 
offset,
  
  depth = 0;

  for (;;) {
+bs = bdrv_skip_filters(bs);
  ret = bdrv_block_status(bs, offset, bytes, , , );
  if (ret < 0) {
  return ret;
@@ -3052,7 +3056,7 @@ static int get_block_status(BlockDriverState *bs, int64_t 
offset,
  if (ret & (BDRV_BLOCK_ZERO|BDRV_BLOCK_DATA)) {
  break;
  }
-bs = backing_bs(bs);
+bs = bdrv_cow_bs(bs);
  if (bs == NULL) {
  ret = 0;
  break;
@@ -3437,6 +3441,7 @@ static int img_rebase(int argc, char **argv)
  uint8_t *buf_old = NULL;
  uint8_t *buf_new = NULL;
  BlockDriverState *bs = NULL, *prefix_chain_bs = NULL;
+BlockDriverState *unfiltered_bs;
  char *filename;
  const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
  int c, flags, src_flags, ret;
@@ -3571,6 +3576,8 @@ static int img_rebase(int argc, char **argv)
  }
  bs = blk_bs(blk);
  
+unfiltered_bs = bdrv_skip_filters(bs);

+
  if (out_basefmt != NULL) {
  if (bdrv_find_format(out_basefmt) == NULL) {
  error_report("Invalid format name: '%s'", out_basefmt);
@@ -3582,7 +3589,7 @@ static int img_rebase(int argc, char **argv)
  /* For safe rebasing we need to compare old and new backing file */
  if (!unsafe) {
  QDict *options = NULL;
-BlockDriverState *base_bs = backing_bs(bs);
+BlockDriverState *base_bs = bdrv_cow_bs(unfiltered_bs);
  
  if (base_bs) {

  blk_old_backing = blk_new(qemu_get_aio_context(),
@@ -3738,8 +3745,9 @@ static int img_rebase(int argc, char **argv)
   * If cluster wasn't changed since prefix_chain, we don't need
   * to take action
   */
-ret = bdrv_is_allocated_above(backing_bs(bs), prefix_chain_bs,
-  false, offset, n, );
+ret = bdrv_is_allocated_above(bdrv_cow_bs(unfiltered_bs),
+  prefix_chain_bs, false,
+

Re: [PATCH v3 07/21] migration/block-dirty-bitmap: fix dirty_bitmap_mig_before_vm_start

[Eric Blake]

On 7/24/20 3:43 AM, Vladimir Sementsov-Ogievskiy wrote:

No reason to use _locked version of bdrv_enable_dirty_bitmap, as we
don't lock this mutex before. Moreover, the adjacent
bdrv_dirty_bitmap_enable_successor do lock the mutex.


Grammar suggestion:

Using the _locked version of bdrv_enable_dirty_bitmap to bypass locking 
is wrong as we do not already own the mutex.  Moreover, the adjacent 
call to bdrv_dirty_bitmap_enable_successor grabs the mutex.




Fixes: 58f72b965e9e1q
Cc: qemu-sta...@nongnu.org # v3.0
Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Andrey Shinkevich 
---
  migration/block-dirty-bitmap.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Eric Blake 

I'm comfortable taking this in 5.1.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH V2] tpm_emulator: Report an error if chardev is missing

[Marc-André Lureau]

Hi

On Fri, Jul 24, 2020 at 6:16 PM Stefan Berger 
wrote:

> This patch fixes the odd error reporting when trying to send a file
> descriptor to the TPM emulator if one has not passed a valid chardev.
>
> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Failed to send
> CMD_SET_DATAFD: Success
> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Could not
> cleanly shutdown the TPM: Success
>
> This is the new error report:
>
> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: parameter
> 'chardev' is missing
>
> This change does not hide the display of supported TPM types if a
> non-existent type is passed:
>
> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev nonexistent,id=tpm0
> qemu-system-x86_64: -tpmdev nonexistent,id=tpm0: Parameter 'type' expects
> a TPM backend type
> Supported TPM types (choose only one):
>  passthrough   Passthrough TPM backend driver
> emulator   TPM emulator backend driver
>
> Signed-off-by: Stefan Berger 
>

Reviewed-by: Marc-André Lureau 

---
>  backends/tpm/tpm_emulator.c | 38 ++---
>  1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index 9605339f93..a9b0f55e67 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -549,27 +549,30 @@ err_exit:
>  static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, QemuOpts
> *opts)
>  {
>  const char *value;
> +Error *err = NULL;
> +Chardev *dev;
>
>  value = qemu_opt_get(opts, "chardev");
> -if (value) {
> -Error *err = NULL;
> -Chardev *dev = qemu_chr_find(value);
> -
> -if (!dev) {
> -error_report("tpm-emulator: tpm chardev '%s' not found.",
> value);
> -goto err;
> -}
> +if (!value) {
> +error_report("tpm-emulator: parameter 'chardev' is missing");
> +goto err;
> +}
>
> -if (!qemu_chr_fe_init(_emu->ctrl_chr, dev, )) {
> -error_prepend(, "tpm-emulator: No valid chardev found at
> '%s':",
> -  value);
> -error_report_err(err);
> -goto err;
> -}
> +dev = qemu_chr_find(value);
> +if (!dev) {
> +error_report("tpm-emulator: tpm chardev '%s' not found", value);
> +goto err;
> +}
>
> -tpm_emu->options->chardev = g_strdup(value);
> +if (!qemu_chr_fe_init(_emu->ctrl_chr, dev, )) {
> +error_prepend(, "tpm-emulator: No valid chardev found at
> '%s':",
> +  value);
> +error_report_err(err);
> +goto err;
>  }
>
> +tpm_emu->options->chardev = g_strdup(value);
> +
>  if (tpm_emulator_prepare_data_fd(tpm_emu) < 0) {
>  goto err;
>  }
> @@ -925,6 +928,11 @@ static void tpm_emulator_shutdown(TPMEmulator
> *tpm_emu)
>  {
>  ptm_res res;
>
> +if (!tpm_emu->options->chardev) {
> +/* was never properly initialized */
> +return;
> +}
> +
>  if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SHUTDOWN, , 0, sizeof(res))
> < 0) {
>  error_report("tpm-emulator: Could not cleanly shutdown the TPM:
> %s",
>   strerror(errno));
> --
> 2.24.1
>
>
>

-- 
Marc-André Lureau

Re: Possible regression with VGA and resolutions in Windows 10?

[Gerd Hoffmann]

On Fri, Jul 24, 2020 at 03:31:23PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 24, 2020 at 04:10:32PM +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > The behavior is similar when setting a custom resolution with the xres
> > > and yres parameters. Setting it the first time works fine and it is
> > > shown along with the short list. Setting it to something different on
> > > the next boot will not be recognized unless the display adapter is
> > > uninstalled and the VM rebooted.
> > 
> > Interesting.  Seems Windows caches the list of resolutions (or the edid
> > blob) somewhere in the registry instead of loading it on every boot.
> > I've seen simliar behavior with usb device info.
> > 
> > [ something for the 5.1 release notes I think, thanks for testing this ]
> 
> Do we need to be disabling edid in the old machine types to prevent this
> change in guest ABI due to the changed BIOS ?
> 
> eg existing VMs using a versioned machine type shouldn't suddenly get edid
> enabled where previously it was not present. Only VMs using the new 5.1 or
> unversioned machine types should see the change in behaviour.

Well, the *device* feature actually is versioned, but it is present in
qemu for quite a while already.  Now the *vgabios* update makes the edid
available via bios interface.  We have two independent changes, so it
isn't that simple ...

take care,
  Gerd

Re: [BUG] vhost-vdpa: qemu-system-s390x crashes with second virtio-net-ccw device

[Cornelia Huck]

On Fri, 24 Jul 2020 11:17:57 -0400
"Michael S. Tsirkin"  wrote:

> On Fri, Jul 24, 2020 at 04:56:27PM +0200, Cornelia Huck wrote:
> > On Fri, 24 Jul 2020 09:30:58 -0400
> > "Michael S. Tsirkin"  wrote:
> >   
> > > On Fri, Jul 24, 2020 at 03:27:18PM +0200, Cornelia Huck wrote:  
> > > > When I start qemu with a second virtio-net-ccw device (i.e. adding
> > > > -device virtio-net-ccw in addition to the autogenerated device), I get
> > > > a segfault. gdb points to
> > > > 
> > > > #0  0x55d6ab52681d in virtio_net_get_config (vdev=, 
> > > > config=0x55d6ad9e3f80 "RT") at 
> > > > /home/cohuck/git/qemu/hw/net/virtio-net.c:146
> > > > 146 if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > > > 
> > > > (backtrace doesn't go further)  
> > 
> > The core was incomplete, but running under gdb directly shows that it
> > is just a bog-standard config space access (first for that device).
> > 
> > The cause of the crash is that nc->peer is not set... no idea how that
> > can happen, not that familiar with that part of QEMU. (Should the code
> > check, or is that really something that should not happen?)
> > 
> > What I don't understand is why it is set correctly for the first,
> > autogenerated virtio-net-ccw device, but not for the second one, and
> > why virtio-net-pci doesn't show these problems. The only difference
> > between -ccw and -pci that comes to my mind here is that config space
> > accesses for ccw are done via an asynchronous operation, so timing
> > might be different.  
> 
> Hopefully Jason has an idea. Could you post a full command line
> please? Do you need a working guest to trigger this? Does this trigger
> on an x86 host?

Yes, it does trigger with tcg-on-x86 as well. I've been using

s390x-softmmu/qemu-system-s390x -M s390-ccw-virtio,accel=tcg -cpu qemu,zpci=on 
-m 1024 -nographic -device virtio-scsi-ccw,id=scsi0,devno=fe.0.0001 
-drive file=/path/to/image,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
 
-device virtio-net-ccw

It seems it needs the guest actually doing something with the nics; I
cannot reproduce the crash if I use the old advent calendar moon buggy
image and just add a virtio-net-ccw device.

(I don't think it's a problem with my local build, as I see the problem
both on my laptop and on an LPAR.)

> 
> > > > 
> > > > Starting qemu with no additional "-device virtio-net-ccw" (i.e., only
> > > > the autogenerated virtio-net-ccw device is present) works. Specifying
> > > > several "-device virtio-net-pci" works as well.
> > > > 
> > > > Things break with 1e0a84ea49b6 ("vhost-vdpa: introduce vhost-vdpa net
> > > > client"), 38140cc4d971 ("vhost_net: introduce set_config & get_config")
> > > > works (in-between state does not compile).
> > > 
> > > Ouch. I didn't test all in-between states :(
> > > But I wish we had a 0-day instrastructure like kernel has,
> > > that catches things like that.  
> > 
> > Yep, that would be useful... so patchew only builds the complete series?
> >   
> > >   
> > > > This is reproducible with tcg as well. Same problem both with
> > > > --enable-vhost-vdpa and --disable-vhost-vdpa.
> > > > 
> > > > Have not yet tried to figure out what might be special with
> > > > virtio-ccw... anyone have an idea?
> > > > 
> > > > [This should probably be considered a blocker?]
> > 
> > I think so, as it makes s390x unusable with more that one
> > virtio-net-ccw device, and I don't even see a workaround.  
> 

[Bug 1888728] Re: Bare chroot in linux-user fails with pgb_reserved_va: Assertion `guest_base != 0' failed.

[John Paul Adrian Glaubitz]

Here you go: https://people.debian.org/~glaubitz/sid-m68k-sbuild.tgz

Thanks for looking into it!

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

Title:
  Bare chroot in linux-user fails with pgb_reserved_va: Assertion
  `guest_base != 0' failed.

Status in QEMU:
  New

Bug description:
  Trying to run a bare chroot with no additional bind mounts fails on
  git master (8ffa52c20d5693d454f65f2024a1494edfea65d4) with:

  root@nofan:~/qemu> chroot /local_scratch/sid-m68k-sbuild/
  qemu-m68k-static: /root/qemu/linux-user/elfload.c:2315: pgb_reserved_va: 
Assertion `guest_base != 0' failed.
  Aborted
  root@nofan:~/qemu>

  The problem can be worked around by bind-mounting /proc from the host
  system into the target chroot:

  root@nofan:~/qemu> mount -o bind /proc/ /local_scratch/sid-m68k-sbuild/proc/
  root@nofan:~/qemu> chroot /local_scratch/sid-m68k-sbuild/
  bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
  (sid-m68k-sbuild)root@nofan:/#

  Host system is an up-to-date Debian unstable (2020-07-23).

  I have not been able to bisect the issue yet since there is another
  annoying linux-user bug (virtual memory exhaustion) that was somewhere
  introduced and fixed between v5.0.0 and HEAD and overshadows the
  original Assertion failure bug.

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

Re: [BUG] vhost-vdpa: qemu-system-s390x crashes with second virtio-net-ccw device

[Michael S. Tsirkin]

On Fri, Jul 24, 2020 at 04:56:27PM +0200, Cornelia Huck wrote:
> On Fri, 24 Jul 2020 09:30:58 -0400
> "Michael S. Tsirkin"  wrote:
> 
> > On Fri, Jul 24, 2020 at 03:27:18PM +0200, Cornelia Huck wrote:
> > > When I start qemu with a second virtio-net-ccw device (i.e. adding
> > > -device virtio-net-ccw in addition to the autogenerated device), I get
> > > a segfault. gdb points to
> > > 
> > > #0  0x55d6ab52681d in virtio_net_get_config (vdev=, 
> > > config=0x55d6ad9e3f80 "RT") at 
> > > /home/cohuck/git/qemu/hw/net/virtio-net.c:146
> > > 146   if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > > 
> > > (backtrace doesn't go further)
> 
> The core was incomplete, but running under gdb directly shows that it
> is just a bog-standard config space access (first for that device).
> 
> The cause of the crash is that nc->peer is not set... no idea how that
> can happen, not that familiar with that part of QEMU. (Should the code
> check, or is that really something that should not happen?)
> 
> What I don't understand is why it is set correctly for the first,
> autogenerated virtio-net-ccw device, but not for the second one, and
> why virtio-net-pci doesn't show these problems. The only difference
> between -ccw and -pci that comes to my mind here is that config space
> accesses for ccw are done via an asynchronous operation, so timing
> might be different.

Hopefully Jason has an idea. Could you post a full command line
please? Do you need a working guest to trigger this? Does this trigger
on an x86 host?

> > > 
> > > Starting qemu with no additional "-device virtio-net-ccw" (i.e., only
> > > the autogenerated virtio-net-ccw device is present) works. Specifying
> > > several "-device virtio-net-pci" works as well.
> > > 
> > > Things break with 1e0a84ea49b6 ("vhost-vdpa: introduce vhost-vdpa net
> > > client"), 38140cc4d971 ("vhost_net: introduce set_config & get_config")
> > > works (in-between state does not compile).  
> > 
> > Ouch. I didn't test all in-between states :(
> > But I wish we had a 0-day instrastructure like kernel has,
> > that catches things like that.
> 
> Yep, that would be useful... so patchew only builds the complete series?
> 
> > 
> > > This is reproducible with tcg as well. Same problem both with
> > > --enable-vhost-vdpa and --disable-vhost-vdpa.
> > > 
> > > Have not yet tried to figure out what might be special with
> > > virtio-ccw... anyone have an idea?
> > > 
> > > [This should probably be considered a blocker?]  
> 
> I think so, as it makes s390x unusable with more that one
> virtio-net-ccw device, and I don't even see a workaround.

Re: [PULL 0/1] x86 bug fix for -rc2

[Peter Maydell]

On Thu, 23 Jul 2020 at 20:14, Eduardo Habkost  wrote:
>
> The following changes since commit 8ffa52c20d5693d454f65f2024a1494edfea65d4:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
> (2020-07-23 13:38:21 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/ehabkost/qemu.git tags/x86-next-for-5.1-pull-request
>
> for you to fetch changes up to 0baa4b445e28f37243e5dc72e7efe32f0c9d7801:
>
>   KVM: fix CPU reset wrt HF2_GIF_MASK (2020-07-23 15:03:54 -0400)
>
> 
> x86 bug fix for -rc2
>
> A fix from Vitaly Kuznetsov for a CPU reset bug
> reported by Jan Kiszka.
>
> 


Applied, thanks.

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

-- PMM

Re: [PULL 4/4] qapi/error: Check format string argument in error_*prepend()

[Philippe Mathieu-Daudé]

On 7/24/20 4:08 PM, Daniel P. Berrangé wrote:
> On Fri, Jul 24, 2020 at 03:47:04PM +0200, Markus Armbruster wrote:
>> From: Philippe Mathieu-Daudé 
>>
>> error_propagate_prepend() "behaves like error_prepend()", and
>> error_prepend() uses "formatting @fmt, ... like printf()".
>> error_prepend() checks its format string argument, but
>> error_propagate_prepend() does not. Fix by addint the format
>> attribute to error_propagate_prepend() and error_vprepend().
>>
>> This would have caught the bug fixed in the previous commit.
>>
>> Missed in commit 4b5766488f "error: Fix use of error_prepend() with
>> _fatal, _abort".
> 
> FWIW, I'd suggest a followup patch that adds -Wsuggest-attribute=format
> to CFLAGS, as after a quick hack to try a build, I think all the things
> it reports are valid cases needing the format attribute.
> 
> qemu/util/error.c:62:5: warning: function ‘error_setv’ might be a candidate 
> for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> qemu/util/error.c:133:5: warning: function ‘error_vprepend’ might be a 
> candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> qemu/util/qemu-error.c:236:5: warning: function ‘vreport’ might be a 
> candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> qemu/contrib/libvhost-user/libvhost-user.c:161:5: warning: function 
> ‘vu_panic’ might be a candidate for ‘gnu_printf’ format attribute 
> [-Wsuggest-attribute=format]
> qemu/tools/virtiofsd/fuse_log.c:20:5: warning: function ‘default_log_func’ 
> might be a candidate for ‘gnu_printf’ format attribute 
> [-Wsuggest-attribute=format]
> qemu/tools/virtiofsd/passthrough_ll.c:2752:9: warning: function ‘log_func’ 
> might be a candidate for ‘gnu_printf’ format attribute 
> [-Wsuggest-attribute=format]
> qemu/tools/virtiofsd/passthrough_ll.c:2754:9: warning: function ‘log_func’ 
> might be a candidate for ‘gnu_printf’ format attribute 
> [-Wsuggest-attribute=format]
> qemu/hw/xen/xen-bus-helper.c:124:9: warning: function ‘xs_node_vscanf’ might 
> be a candidate for ‘gnu_scanf’ format attribute [-Wsuggest-attribute=format]
> qemu/disas.c:497:5: warning: function ‘plugin_printf’ might be a candidate 
> for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]

I have the printf() ones ready but am waiting to be closer to 5.2.

> 
> 
> Regards,
> Daniel
> 

Re: [PATCH v4 3/8] s390/sclp: rework sclp boundary and length checks

[Collin Walling]

On 7/23/20 2:26 AM, Cornelia Huck wrote:
> On Tue, 21 Jul 2020 14:40:14 -0400
> Collin Walling  wrote:
> 
>> On 7/21/20 4:41 AM, David Hildenbrand wrote:
> 
>>> The options I would support are
>>>
>>> 1. "sccb_boundary_is_valid" which returns "true" if valid
>>> 2. "sccb_boundary_is_invalid" which returns "true" if invalid
>>> 3. "sccb_boundary_validate" which returns "0" if valid and -EINVAL if not.
>>>
>>> Which makes reading this code a bit easier.
>>>   
> 
> Of these, I like option 1 best.
> 
>>
>> Sounds good. I'll takes this into consideration for the next round. (I
>> may wait just a little longer for that to allow more reviews to come in
>> from whoever has the time, if that's okay.)
> 
> We have to wait for (a) QEMU to do a release and (b) the Linux changes
> to merge upstream anyway, so we're not in a hurry :)
> 
> As said before, it already looked good from my side, but the suggested
> changes are fine with me as well.
> 
> 

Okay, thanks for the info.

I do want to send out a v5 of these patches. While working with someone
who is implementing the kernel support for the extended-length SCCB, we
found some snags. I'll highlight these changes/fixes in the respective
patches on the next version.

Thanks!

-- 
Regards,
Collin

Stay safe and stay healthy

Re: [PATCH v3 01/21] qemu-iotests/199: fix style

[Eric Blake]

On 7/24/20 3:43 AM, Vladimir Sementsov-Ogievskiy wrote:

Mostly, satisfy pep8 complains.


complaints

I can touch that up while staging.



Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Andrey Shinkevich 
Tested-by: Eric Blake 
---
  tests/qemu-iotests/199 | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)




--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 1/3] configure: Fix atomic64 test for --enable-werror on macOS

[Christian Schoenebeck]

On Freitag, 24. Juli 2020 16:32:18 CEST Thomas Huth wrote:
> When using --enable-werror for the macOS builders in the Cirrus-CI,
> the atomic64 test is currently failing, and config.log shows a bunch
> of error messages like this:
> 
>  config-temp/qemu-conf.c:6:7: error: implicit declaration of function
>  '__atomic_load_8' is invalid in C99
> [-Werror,-Wimplicit-function-declaration] y = __atomic_load_8(, 0);
>   ^
>  config-temp/qemu-conf.c:6:7: error: this function declaration is not a
>  prototype [-Werror,-Wstrict-prototypes]
> 
> Seems like these __atomic_*_8 functions are available in one of the
> libraries there, so that the test links and passes there when not
> using --enable-werror. But there does not seem to be a valid prototype
> for them in any of the header files, so that the test fails when using
> --enable-werror.
> 
> Fix it by using the "official" built-in functions instead (see e.g.
> https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html).
> We are not using the *_8 variants in QEMU anyway.
> 
> Suggested-by: Christian Schoenebeck 
> Signed-off-by: Thomas Huth 
> ---
>  configure | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/configure b/configure
> index 4bd80ed507..9eaf501f50 100755
> --- a/configure
> +++ b/configure
> @@ -5919,11 +5919,11 @@ int main(void)
>  {
>uint64_t x = 0, y = 0;
>  #ifdef __ATOMIC_RELAXED
> -  y = __atomic_load_8(, 0);
> -  __atomic_store_8(, y, 0);
> -  __atomic_compare_exchange_8(, , x, 0, 0, 0);
> -  __atomic_exchange_8(, y, 0);
> -  __atomic_fetch_add_8(, y, 0);
> +  y = __atomic_load_n(, __ATOMIC_RELAXED);
> +  __atomic_store_n(, y, __ATOMIC_RELAXED);
> +  __atomic_compare_exchange_n(, , x, 0, __ATOMIC_RELAXED,
> __ATOMIC_RELAXED); +  __atomic_exchange_n(, y, __ATOMIC_RELAXED);

Ah right, there is also the __atomic_*_n() variant of these functions. I 
actually had the more generic variants in mind.

But LGTM and yes, it resolves the warnings on macOS, so ...

Reviewed-by: Christian Schoenebeck 

> +  __atomic_fetch_add(, y, __ATOMIC_RELAXED);
>  #else
>typedef char is_host64[sizeof(void *) >= sizeof(uint64_t) ? 1 : -1];
>__sync_lock_test_and_set(, y);

Best regards,
Christian Schoenebeck

signature.asc
Description: This is a digitally signed message part.

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Peter Maydell]

On Fri, 24 Jul 2020 at 15:32, Thomas Huth  wrote:
>
> Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
> since -Werror is only enabled for Linux and MinGW builds by default. So
> let's enable them here now, too.
> For macOS, that unfortunately means that we have to disable the vnc-sasl
> feature, since this is marked as deprecated in the macOS headers and thus
> generates a lot of deprecation warnings.

For my local OSX builds I use
--extra-cflags='-Werror -Wno-error=deprecated-declarations'
to work around the SASL thing.

thanks
-- PMM

Re: [BUG] vhost-vdpa: qemu-system-s390x crashes with second virtio-net-ccw device

[Cornelia Huck]

On Fri, 24 Jul 2020 09:30:58 -0400
"Michael S. Tsirkin"  wrote:

> On Fri, Jul 24, 2020 at 03:27:18PM +0200, Cornelia Huck wrote:
> > When I start qemu with a second virtio-net-ccw device (i.e. adding
> > -device virtio-net-ccw in addition to the autogenerated device), I get
> > a segfault. gdb points to
> > 
> > #0  0x55d6ab52681d in virtio_net_get_config (vdev=, 
> > config=0x55d6ad9e3f80 "RT") at 
> > /home/cohuck/git/qemu/hw/net/virtio-net.c:146
> > 146 if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > 
> > (backtrace doesn't go further)

The core was incomplete, but running under gdb directly shows that it
is just a bog-standard config space access (first for that device).

The cause of the crash is that nc->peer is not set... no idea how that
can happen, not that familiar with that part of QEMU. (Should the code
check, or is that really something that should not happen?)

What I don't understand is why it is set correctly for the first,
autogenerated virtio-net-ccw device, but not for the second one, and
why virtio-net-pci doesn't show these problems. The only difference
between -ccw and -pci that comes to my mind here is that config space
accesses for ccw are done via an asynchronous operation, so timing
might be different.

> > 
> > Starting qemu with no additional "-device virtio-net-ccw" (i.e., only
> > the autogenerated virtio-net-ccw device is present) works. Specifying
> > several "-device virtio-net-pci" works as well.
> > 
> > Things break with 1e0a84ea49b6 ("vhost-vdpa: introduce vhost-vdpa net
> > client"), 38140cc4d971 ("vhost_net: introduce set_config & get_config")
> > works (in-between state does not compile).  
> 
> Ouch. I didn't test all in-between states :(
> But I wish we had a 0-day instrastructure like kernel has,
> that catches things like that.

Yep, that would be useful... so patchew only builds the complete series?

> 
> > This is reproducible with tcg as well. Same problem both with
> > --enable-vhost-vdpa and --disable-vhost-vdpa.
> > 
> > Have not yet tried to figure out what might be special with
> > virtio-ccw... anyone have an idea?
> > 
> > [This should probably be considered a blocker?]  

I think so, as it makes s390x unusable with more that one
virtio-net-ccw device, and I don't even see a workaround.