[Qemu-devel] [PATCH 27/47] MAINTAINERS: add missing OpenRISC entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a4df19d304..35a7253e22 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -207,6 +207,7 @@ S: Odd Fixes
 F: target/openrisc/
 F: hw/openrisc/
 F: tests/tcg/openrisc/
+F: default-configs/or1k-softmmu.mak
 
 PowerPC
 M: David Gibson 
-- 
2.13.3

[Qemu-devel] [PATCH 24/47] MAINTAINERS: add missing MicroBlaze entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1424788a9d..fa6b0d189a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -165,6 +165,7 @@ S: Maintained
 F: target/microblaze/
 F: hw/microblaze/
 F: disas/microblaze.c
+F: default-configs/microblaze*-softmmu.mak
 
 MIPS
 M: Aurelien Jarno 
-- 
2.13.3

[Qemu-devel] [PATCH 31/47] MAINTAINERS: add missing TriCore entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index e363a18631..7997c90be8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -277,6 +277,7 @@ S: Maintained
 F: target/tricore/
 F: hw/tricore/
 F: include/hw/tricore/
+F: default-configs/tricore-softmmu.mak
 
 Guest CPU Cores (KVM):
 --
-- 
2.13.3

[Qemu-devel] [PATCH 28/47] MAINTAINERS: add missing PowerPC entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 35a7253e22..9a0da13bcd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -218,6 +218,7 @@ F: target/ppc/
 F: hw/ppc/
 F: include/hw/ppc/
 F: disas/ppc.c
+F: default-configs/ppc*-softmmu.mak
 
 S390
 M: Richard Henderson 
-- 
2.13.3

[Qemu-devel] [PATCH 23/47] MAINTAINERS: add missing LM32 entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 06b092bf85..1424788a9d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -150,6 +150,7 @@ F: hw/*/milkymist-*
 F: include/hw/char/lm32_juart.h
 F: include/hw/lm32/
 F: tests/tcg/lm32/
+F: default-configs/lm32-softmmu.mak
 
 M68K
 M: Laurent Vivier 
-- 
2.13.3

[Qemu-devel] [PATCH 30/47] MAINTAINERS: add missing SPARC entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bb1cb1fc1c..e363a18631 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -245,6 +245,7 @@ F: target/sparc/
 F: hw/sparc/
 F: hw/sparc64/
 F: disas/sparc.c
+F: default-configs/sparc*-softmmu.mak
 
 UniCore32
 M: Guan Xuetao 
-- 
2.13.3

[Qemu-devel] [PATCH 13/47] MAINTAINERS: add missing entry for virtio/rng

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a9f950248a..842c66c325 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1103,6 +1103,7 @@ F: include/hw/virtio/virtio-rng.h
 F: include/sysemu/rng*.h
 F: backends/rng*.c
 F: tests/virtio-rng-test.c
+F: include/standard-headers/linux/virtio_rng.h
 
 virtio-crypto
 M: Gonglei 
-- 
2.13.3

[Qemu-devel] [PATCH 26/47] MAINTAINERS: add missing NiosII entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7354afb9f4..a4df19d304 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -199,6 +199,7 @@ S: Maintained
 F: target/nios2/
 F: hw/nios2/
 F: disas/nios2.c
+F: default-configs/nios2-softmmu.mak
 
 OpenRISC
 M: Stafford Horne 
-- 
2.13.3

[Qemu-devel] [PATCH 18/47] MAINTAINERS: add missing TCG entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8cb94af6c5..6b83dac812 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1633,6 +1633,7 @@ Common code
 M: Richard Henderson 
 S: Maintained
 F: tcg/
+F: disas.c
 
 AArch64 target
 M: Claudio Fontana 
-- 
2.13.3

[Qemu-devel] [PATCH 25/47] MAINTAINERS: add missing MIPS entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fa6b0d189a..7354afb9f4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -182,6 +182,7 @@ F: include/hw/intc/mips_gic.h
 F: include/hw/timer/mips_gictimer.h
 F: tests/tcg/mips/
 F: disas/mips.c
+F: default-configs/mips*-softmmu.mak
 
 Moxie
 M: Anthony Green 
-- 
2.13.3

[Qemu-devel] [PATCH 12/47] MAINTAINERS: add missing entry for virtio/input

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bfdb2f8928..a9f950248a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1084,6 +1084,7 @@ M: Gerd Hoffmann 
 S: Maintained
 F: hw/input/virtio-input*.c
 F: include/hw/virtio/virtio-input.h
+F: include/standard-headers/linux/virtio_input.h
 
 virtio-serial
 M: Amit Shah 
-- 
2.13.3

[Qemu-devel] [PATCH 21/47] MAINTAINERS: add missing CRIS entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 286ff7e509..14109f4513 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -131,6 +131,7 @@ F: hw/cris/
 F: include/hw/cris/
 F: tests/tcg/cris/
 F: disas/cris.c
+F: default-configs/cris-softmmu.mak
 
 HPPA (PA-RISC)
 M: Richard Henderson 
-- 
2.13.3

[Qemu-devel] [PATCH 16/47] MAINTAINERS: add missing entry for gdb

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9670a3901a..653c2a1db1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1320,6 +1320,7 @@ L: qemu-devel@nongnu.org
 S: Odd Fixes
 F: gdbstub*
 F: gdb-xml/
+F: scripts/qemugdb/
 
 Memory API
 M: Paolo Bonzini 
-- 
2.13.3

[Qemu-devel] [PATCH 22/47] MAINTAINERS: add missing M68k entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 14109f4513..06b092bf85 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -156,6 +156,7 @@ M: Laurent Vivier 
 S: Maintained
 F: target/m68k/
 F: disas/m68k.c
+F: default-configs/m68k-softmmu.mak
 
 MicroBlaze
 M: Edgar E. Iglesias 
-- 
2.13.3

[Qemu-devel] [PATCH 08/47] MAINTAINERS: add missing entry for virtio

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 69987b5e5b..6da3264d4e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1046,6 +1046,7 @@ F: hw/virtio/Makefile.objs
 F: hw/virtio/trace-events
 F: net/vhost-user.c
 F: include/hw/virtio/
+F: include/standard-headers/linux/virtio_*
 F: tests/virtio-balloon-test.c
 
 virtio-9p
-- 
2.13.3

[Qemu-devel] [PATCH 19/47] MAINTAINERS: add missing qcow2 entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6b83dac812..2dcdda4cd5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1856,6 +1856,7 @@ M: Max Reitz 
 L: qemu-bl...@nongnu.org
 S: Supported
 F: block/qcow2*
+F: docs/interop/qcow2.txt
 
 qcow
 M: Kevin Wolf 
-- 
2.13.3

[Qemu-devel] [RFC PATCH 15/47] MAINTAINERS: add missing VMWare entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---

RFC because I'm not sure about tests/

 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d8aef164ca..9670a3901a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1140,6 +1140,7 @@ M: Dmitry Fleytman 
 S: Maintained
 F: hw/net/vmxnet*
 F: hw/scsi/vmw_pvscsi*
+F: tests/vmxnet3-test.c
 
 Rocker
 M: Jiri Pirko 
-- 
2.13.3

[Qemu-devel] [PATCH 14/47] MAINTAINERS: add missing entry for virtio/crypto

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 842c66c325..d8aef164ca 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -,6 +,7 @@ S: Supported
 F: hw/virtio/virtio-crypto.c
 F: hw/virtio/virtio-crypto-pci.c
 F: include/hw/virtio/virtio-crypto.h
+F: include/standard-headers/linux/virtio_crypto.h
 
 nvme
 M: Keith Busch 
-- 
2.13.3

[Qemu-devel] [PATCH 06/47] MAINTAINERS: add missing entry for vhost

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ece02522be..ddf6f3f6d8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1035,6 +1035,7 @@ vhost
 M: Michael S. Tsirkin 
 S: Supported
 F: hw/*/*vhost*
+F: docs/interop/vhost-user.txt
 
 virtio
 M: Michael S. Tsirkin 
-- 
2.13.3

[Qemu-devel] [PATCH 20/47] MAINTAINERS: add missing PCI entries

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2dcdda4cd5..286ff7e509 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -933,6 +933,9 @@ F: include/hw/pci/*
 F: hw/misc/pci-testdev.c
 F: hw/pci/*
 F: hw/pci-bridge/*
+F: default-configs/pci.mak
+F: docs/pci*
+F: docs/specs/*pci*
 
 ACPI/SMBIOS
 M: Michael S. Tsirkin 
-- 
2.13.3

[Qemu-devel] [PATCH 10/47] MAINTAINERS: add missing entry for virtio/blk

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4f9ce6e686..2958dd2479 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1067,6 +1067,7 @@ S: Supported
 F: hw/block/virtio-blk.c
 F: hw/block/dataplane/*
 F: tests/virtio-blk-test.c
+F: include/standard-headers/linux/virtio_blk.h
 T: git git://github.com/stefanha/qemu.git block
 
 virtio-ccw
-- 
2.13.3

[Qemu-devel] [PATCH 04/47] MAINTAINERS: add missing USB entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f10252c292..3b472d7a09 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1008,6 +1008,7 @@ F: docs/usb2.txt
 F: docs/usb-storage.txt
 F: include/hw/usb.h
 F: include/hw/usb/
+F: default-configs/usb.mak
 
 USB (serial adapter)
 M: Gerd Hoffmann 
-- 
2.13.3

[Qemu-devel] [PATCH 17/47] MAINTAINERS: add missing Guest Agent entries

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 4 
 1 file changed, 4 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 653c2a1db1..8cb94af6c5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1448,6 +1448,10 @@ QEMU Guest Agent
 M: Michael Roth 
 S: Maintained
 F: qga/
+F: qemu-ga.texi
+F: scripts/qemu-guest-agent/
+F: tests/test-qga.c
+F: docs/interop/qemu-ga-ref.texi
 T: git git://github.com/mdroth/qemu.git qga
 
 QOM
-- 
2.13.3

[Qemu-devel] [PATCH 09/47] MAINTAINERS: add missing entry for virtio/9p

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6da3264d4e..4f9ce6e686 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1056,6 +1056,7 @@ S: Supported
 F: hw/9pfs/
 F: fsdev/
 F: tests/virtio-9p-test.c
+F: include/standard-headers/linux/virtio_9p.h
 T: git git://github.com/kvaneesh/QEMU.git
 T: git git://github.com/gkurz/qemu.git 9p-next
 
-- 
2.13.3

[Qemu-devel] [PATCH 11/47] MAINTAINERS: add missing entry for virtio/ccw

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2958dd2479..bfdb2f8928 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1075,6 +1075,7 @@ M: Cornelia Huck 
 M: Christian Borntraeger 
 S: Supported
 F: hw/s390x/virtio-ccw.[hc]
+F: include/standard-headers/asm-s390/virtio-ccw.h
 T: git git://github.com/cohuck/qemu.git s390-next
 T: git git://github.com/borntraeger/qemu.git s390-next
 
-- 
2.13.3

[Qemu-devel] [PATCH 07/47] MAINTAINERS: add missing entry for vfio/ccw

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ddf6f3f6d8..69987b5e5b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1029,6 +1029,7 @@ S: Supported
 F: hw/vfio/ccw.c
 F: hw/s390x/s390-ccw.c
 F: include/hw/s390x/s390-ccw.h
+F: linux-headers/linux/vfio_ccw.h
 T: git git://github.com/cohuck/qemu.git s390-next
 
 vhost
-- 
2.13.3

[Qemu-devel] [PATCH 03/47] MAINTAINERS: add missing STM32 entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 795f89f709..f10252c292 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -556,6 +556,7 @@ F: hw/char/stm32f2xx_usart.c
 F: hw/timer/stm32f2xx_timer.c
 F: hw/adc/*
 F: hw/ssi/stm32f2xx_spi.c
+F: include/hw/*/stm32*.h
 
 Netduino 2
 M: Alistair Francis 
-- 
2.13.3

[Qemu-devel] [PATCH 00/47] add missing entries in MAINTAINERS

[Philippe Mathieu-Daudé]

Hi,

I first prepared this series thinking about 2.10 but then realized if someone
is calling ./scripts/get_maintainer.pl he better is using an updated git
clone :)

Distribs do provide docs/ files in /usr/share/doc/qemu* but not the
MAINTAINERS file so no hurry for this series.

I provided the script I used in the last patch, I plan to improve it so
patchew/travis can start to use it: if a new file is added to the repository
without being covered in MAINTAINERS the script remember the submitter about
it and eventually he'll resend his patch, I think this is a good practice and
help the next contributors (anyway this is a valuable experience to look at a
file history to see who has been hacking around when there is no MAINTAINERS
entry).

I don't want to overburden current maintainers, so the last patches add
Orphans entries but I tried to select relevant commiters and include them in
each patch.

The criteria I used is "If I change this file, who should I notify? Who would
be interested?"

Regards,

Phil.

Philippe Mathieu-Daudé (47):
  MAINTAINERS: add missing entry for documentation
  MAINTAINERS: add missing ARM entries
  MAINTAINERS: add missing STM32 entry
  MAINTAINERS: add missing USB entry
  MAINTAINERS: add missing KVM entry
  MAINTAINERS: add missing entry for vhost
  MAINTAINERS: add missing entry for vfio/ccw
  MAINTAINERS: add missing entry for virtio
  MAINTAINERS: add missing entry for virtio/9p
  MAINTAINERS: add missing entry for virtio/blk
  MAINTAINERS: add missing entry for virtio/ccw
  MAINTAINERS: add missing entry for virtio/input
  MAINTAINERS: add missing entry for virtio/rng
  MAINTAINERS: add missing entry for virtio/crypto
  MAINTAINERS: add missing VMWare entry
  MAINTAINERS: add missing entry for gdb
  MAINTAINERS: add missing guest-agent entries
  MAINTAINERS: add missing TCG entry
  MAINTAINERS: add missing qcow2 entry
  MAINTAINERS: add missing PCI entries
  MAINTAINERS: add missing CRIS entry
  MAINTAINERS: add missing M68k entry
  MAINTAINERS: add missing LM32 entry
  MAINTAINERS: add missing MicroBlaze entry
  MAINTAINERS: add missing MIPS entry
  MAINTAINERS: add missing NiosII entry
  MAINTAINERS: add missing OpenRISC entry
  MAINTAINERS: add missing PowerPC entry
  MAINTAINERS: add missing SH-4 entry
  MAINTAINERS: add missing SPARC entry
  MAINTAINERS: add missing TriCore entry
  MAINTAINERS: add missing UniCore32 entry
  MAINTAINERS: add missing Xtensa entry
  MAINTAINERS: add missing X86 entries
  MAINTAINERS: add missing entries for throttling infra
  MAINTAINERS: add missing entry test for megasas
  MAINTAINERS: update docs/devel/ entries
  MAINTAINERS: update docs/interop/ entries
  RFC MAINTAINERS: add missing SSI entries
  RFC MAINTAINERS: add missing entry for AIO
  RFC MAINTAINERS: add missing I2C entries
  RFC MAINTAINERS: add missing Bluetooth entries
  RFC MAINTAINERS: add missing TPM entries
  RFC MAINTAINERS: add missing entry for tilegx
  RFC MAINTAINERS: add missing entries for loader
  RFC MAINTAINERS: add missing entries for Coccinelle scripts
  MAINTAINERS: script to find outdated entry

 MAINTAINERS | 102 
 scripts/check_maintainer.sh |  21 +
 2 files changed, 114 insertions(+), 9 deletions(-)
 create mode 100755 scripts/check_maintainer.sh

-- 
2.13.3

[Qemu-devel] [PATCH 05/47] MAINTAINERS: add missing KVM entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3b472d7a09..ece02522be 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -278,6 +278,7 @@ S: Supported
 F: */kvm.*
 F: accel/kvm/
 F: include/sysemu/kvm*.h
+F: linux-headers/asm-*/kvm*.h
 
 ARM
 M: Peter Maydell 
-- 
2.13.3

[Qemu-devel] [PATCH 02/47] MAINTAINERS: add missing ARM entries

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 972118e70b..795f89f709 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -120,6 +120,8 @@ F: include/hw/cpu/a*mpcore.h
 F: disas/arm.c
 F: disas/arm-a64.cc
 F: disas/libvixl/
+F: default-configs/arm-softmmu.mak
+F: default-configs/aarch64-softmmu.mak
 
 CRIS
 M: Edgar E. Iglesias 
@@ -380,6 +382,7 @@ M: Peter Maydell 
 L: qemu-...@nongnu.org
 S: Maintained
 F: hw/char/pl011.c
+F: include/hw/char/pl011.h
 F: hw/display/pl110*
 F: hw/dma/pl080.c
 F: hw/dma/pl330.c
@@ -402,14 +405,19 @@ F: hw/intc/arm*
 F: hw/intc/gic_internal.h
 F: hw/misc/a9scu.c
 F: hw/misc/arm11scu.c
+F: hw/misc/arm_sysctl.c
 F: hw/timer/a9gtimer*
 F: hw/timer/arm_*
+F: hw/timer/armv7m_systick.c
 F: include/hw/arm/arm.h
+F: include/hw/arm/armv7m*.h
 F: include/hw/intc/arm*
 F: include/hw/misc/a9scu.h
 F: include/hw/misc/arm11scu.h
 F: include/hw/timer/a9gtimer.h
 F: include/hw/timer/arm_mptimer.h
+F: include/hw/timer/armv7m_systick.h
+F: tests/test-arm-mptimer.c
 
 Exynos
 M: Igor Mitsyanko 
-- 
2.13.3

[Qemu-devel] [RFC PATCH 01/47] MAINTAINERS: add missing entry for documentation

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---

because having only a "Build system architecture" entry in Documentation seems
odd to me, so RFC.

 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5ea273f899..972118e70b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1886,6 +1886,9 @@ W: http://patchew.org/QEMU/
 
 Documentation
 -
+Overall
+F: docs/
+
 Build system architecture
 M: Daniel P. Berrange 
 S: Odd Fixes
-- 
2.13.3

Re: [Qemu-devel] [Qemu-ppc] [for-2.11 PATCH 00/26] spapr: add support for PHB hotplug

[Cédric Le Goater]

On 07/28/2017 05:40 AM, David Gibson wrote:
> On Fri, Jul 28, 2017 at 01:27:05PM +1000, Alexey Kardashevskiy wrote:
>> On 28/07/17 02:39, Greg Kurz wrote:
>>> On Wed, 26 Jul 2017 17:31:17 -0300
>>> Daniel Henrique Barboza  wrote:
>>>
 I've tested the patch set using Greg's Github branch. It worked fine in 
 my tests
 using a Fedora 26 and an Ubuntu 17.04 guests. I have two observations
 though:

 1 - This is not related to this patch set per se because it is 
 reproducible on master, but
 I think it is interfering with this new feature.  There is a 
 warning/error message in
 the kernel right after SLOF that goes:

 (...)
   -> smp_release_cpus()  
 spinning_secondaries = 0
   <- smp_release_cpus()
 Linux ppc64le
 #1 SMP Mon Jul 1[0.030450] pci :00:02.0: of_irq_parse_pci: 
 failed with rc=-22
 [0.030552] pci :00:0f.0: of_irq_parse_pci: failed with rc=-22
 [  OK  ] Started Security Auditing Service.
 (...)

>>>
>>> This is a regression in QEMU master introduced by this commit:
>>>
>>> commit b87680427e8a3ff682f66514e99a8344e7437247
>>> Author: Cédric Le Goater 
>>> Date:   Wed Jul 5 19:13:15 2017 +0200
>>>
>>> spapr: populate device tree depending on XIVE_EXPLOIT option
>>> 
>>> When XIVE is supported, the device tree should be populated
>>> accordingly and the XIVE memory regions mapped to activate MMIOs.
>>> 
>>> Depending on the design we choose, we could also allocate different
>>> ICS and ICP objects, or switch between objects. This needs to be
>>> discussed.
>>> 
>>> Signed-off-by: Cédric Le Goater 
>>> Signed-off-by: David Gibson 
>>>
>>> It is very similar to the issue that motivated the new 
>>> KVMPPC_H_UPDATE_PHANDLE
>>> hcall (see patch 24 and 26 in this series):
>>>
>>> - QEMU creates an "interrupt-controller" node with a phandle property
>>>   with the value 0x
>>> - QEMU passes the FDT to SLOF
>>> - SLOF converts all references to the phandle to an SLOF internal value
>>>
>>> => from now on (ie, until the next machine reset), the guest only knows
>>>the OF phandle.
>>>
>>> - during CAS, if we go XICS, we send back an updated FDT with the
>>>   phandle of the "interrupt-controller" node reverted to 0x
>>>
>>> => the guest complains because all cold-plugged devices nodes refer
>>>to the OF phandle, not 0x
>>>
>>> A solution is to use the value set by KVMPPC_H_UPDATE_PHANDLE during CAS
>>> instead of 0x. I could verify it makes the guest warning disappear.
>>>
>>> I'll send a dedicated patchset to fix this in 2.10.
>>
>>
>> The SLOF I pushed for 2.10 does not have it though. And the rest of XIVE is
>> not targeted for 2.10 anyway. So imho the solution is reverting "spapr:
>> populate device tree depending on XIVE_EXPLOIT option" for 2.10.
> 
> I agree, I've applied the revert to ppc-for-2.10 now.

Sure. 

Greg, have you found a solution to get around this problem or do we need
to populate the device tree with the interrupt controller node for SLOF ? 

Thanks,

C.  

Re: [Qemu-devel] [PATCH] vfio/pci-quirks: Set non-zero GMS memory size for IGD

[Dmitry Fleytman]

> On 28 Jul 2017, at 07:51 AM, Zhang, Xiong Y  wrote:
> 
>>> On 26 Jul 2017, at 08:22 AM, Zhang, Xiong Y 
>> wrote:
>>> 
>>> Sorry, we indeed found Intel windows guest graphic driver couldn't be bind
>> when GMS memory size is zero. And we have fixed it and the next intel
>> windows driver release will contain this fix.
>>> So currently please use x-igd-gms in legacy mode to work around this.
>>> 
>>> BTW, how did you know window driver allocate extra ~4G memory when
>> GMS size set to 0 ?
>> 
>> We noticed that with new IGD driver memory usage on VMs raised by around
>> 4G.
>> 
>> Then we examined memory allocations with poolmon
>> (https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/poolm
>> on)
>> and saw that around 4G of memory is allocated with IGD driver pool tag.
>> Additionally we noticed that on VMs with less than 5G of memory IGD driver
>> does not load and system does fallback to standard VGA driver.
>> 
> [Zhang, Xiong Y]  I tried our internal driver and didn't found the above two 
> issues, please wait for the next intel graphic driver release.

Thanks!

> 
> thanks

Re: [Qemu-devel] [PATCH] build-sys: add --disable-vhost-user

[Philippe Mathieu-Daudé]

On 07/26/2017 02:52 PM, Michael S. Tsirkin wrote:

On Tue, Jul 18, 2017 at 06:34:37PM +0200, Marc-André Lureau wrote:

...

In the future, we may want to hide vhost-user from QAPI/introspection
with conditional compilation, although the design of this hasn't been
fully fleshed out yet and shouldn't prevent this patch from being
applied.

Signed-off-by: Marc-André Lureau 


After an offline discussion, looks like the idea is to be able to ship a
cut down binary with less features (and e.g. smaller attack surface?).


I think it deserves it own thread, there will be many ideas about it.

I liked this idea a first because it speeds up compilation time (so the 
CI build system can run more tests, cover more of less code...) but then 
we have this unreadable unmanageable ifdefery very hard to maintain, you 
end up not compiling this piece of code, then move things around, 
refactor a bit, change some API and zombie code survives around until 
someone who has this lib try to compile the obsolete code.


This is also why I now prefer to build via docker images (the 
qemu:debian-amd64 try to come with as many dev packages as possible to 
increase code coverage).


There is some dream about having one monolithic QEMU binary able to run 
all cpus/machines/devices. I think this way do reduce attack surface by 
exposing all the surface. This is a bit what is happening during release 
candidate testing when /master is closed, people start to use QEMU 
[differently than while adding features], more surface is stressed, then 
many issues appear and get resolved.


Thomas Huth did some effort adding more poisoned macros and move common 
objects to common-obj, we can do more here, we may need to split more 
sources to move more common code.



Another experience is I'm trying to model a 32-bit big endian ARM board 
then got bugged with 64-bit code this board will never use, or issue 
with PCI and AHCI, so I started to comment those objects using 
obj-$(CONFIG_X) but then CONFIG_X also depends of CONFIG_Y and so, then 
it become hardly scalable, see [1]:


+CONFIG_LAN9118=$(or 
$(CONFIG_REALVIEW),$(CONFIG_EXYNOS4),$(CONFIG_FSL_IMX31))
+CONFIG_SMC91C111=$(or 
$(CONFIG_PXA2XX),$(CONFIG_MAINSTONE),$(CONFIG_REALVIEW))
+CONFIG_ARM_MPTIMER=$(or 
$(CONFIG_A9MPCORE),$(CONFIG_ARM11MPCORE),$(CONFIG_A15MPCORE))

+CONFIG_HIGHBANK=$(and $(CONFIG_A9MPCORE),$(CONFIG_A15MPCORE))

we already have:

crypto-obj-$(if $(CONFIG_NETTLE),n,$(if $(CONFIG_GCRYPT_HMAC),n,y)) += 
hmac-glib.o
crypto/Makefile.objs:23:crypto-obj-$(if $(CONFIG_GCRYPT),n,$(if 
$(CONFIG_GNUTLS_RND),n,y)) += random-platform.o


I ended dropping this branch, I was thinking about using kconfig to 
manage dependencies... The obj-$(CONFIG_XYZ) is useful when kept tiny, 
at most 1 bitwise operation.


I wanted to share my experience and thoughts about it, I hope it's still 
understandable because it's very late :S


Regards,

Phil.

> Besides failing build on freebsd, this patch has too much ifdefery for
> my taste. How about we patch just net/net.c?

[1] 
https://github.com/philmd/qemu/commit/2c1c515345d626f47d0968821980f9389d4588b2#diff-6006132695caf8d409e781fbf3d4401f

Re: [Qemu-devel] [PATCH] vfio/pci-quirks: Set non-zero GMS memory size for IGD

[Zhang, Xiong Y]

> > On 26 Jul 2017, at 08:22 AM, Zhang, Xiong Y 
> wrote:
> >
> > Sorry, we indeed found Intel windows guest graphic driver couldn't be bind
> when GMS memory size is zero. And we have fixed it and the next intel
> windows driver release will contain this fix.
> > So currently please use x-igd-gms in legacy mode to work around this.
> >
> > BTW, how did you know window driver allocate extra ~4G memory when
> GMS size set to 0 ?
> 
> We noticed that with new IGD driver memory usage on VMs raised by around
> 4G.
> 
> Then we examined memory allocations with poolmon
> (https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/poolm
> on)
> and saw that around 4G of memory is allocated with IGD driver pool tag.
> Additionally we noticed that on VMs with less than 5G of memory IGD driver
> does not load and system does fallback to standard VGA driver.
> 
[Zhang, Xiong Y]  I tried our internal driver and didn't found the above two 
issues, please wait for the next intel graphic driver release.

thanks

Re: [Qemu-devel] [PULL for-2.10 00/14] A set of s390x patches

[Fam Zheng]

On Tue, 07/25 12:19, Cornelia Huck wrote:
> > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> > fatal: unable to access 'https://github.com/patchew-project/qemu/': OpenSSL 
> > SSL_connect: SSL_ERROR_SYSCALL in connection to github.com:443 
> 
> Hm... connection issues?

Yes, patchew still doesn't have the ability to distinguish env/network issue
from real failures. This one should be easy to add because it happens in the
preparation phase, maybe it's a good idea for the next thing to work on.

Fam

Re: [Qemu-devel] [for-2.11 PATCH 22/26] spapr_pci: provide node start offset via spapr_populate_pci_dt()

[David Gibson]

On Tue, Jul 25, 2017 at 08:02:41PM +0200, Greg Kurz wrote:
> From: Michael Roth 
> 
> PHB hotplug re-uses PHB device tree generation code and passes
> it to a guest via RTAS. Doing this requires knowledge of where
> exactly in the device tree the node describing the PHB begins.
> 
> Provide this via a new optional pointer that can be used to
> store the PHB node's start offset.
> 
> Signed-off-by: Michael Roth 
> Reviewed-by: David Gibson 
> Signed-off-by: Greg Kurz 

Blech.  The patch is correct and you can't do much better at the
moment.  I really hope in the next cycle I get a chance to do a bunch
of the DT construction cleanups which should avoid this messy passing
of dt offsets around.

> ---
> Changes since RFC:
> - rebased against ppc-for-2.10
> ---
>  hw/ppc/spapr.c  |2 +-
>  hw/ppc/spapr_pci.c  |6 +-
>  include/hw/pci-host/spapr.h |3 ++-
>  3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 632040f35ecc..1a6cd4efeb97 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1098,7 +1098,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>  }
>  
>  QLIST_FOREACH(phb, >phbs, list) {
> -ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt);
> +ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt, NULL);
>  if (ret < 0) {
>  error_report("couldn't setup PCI devices in fdt");
>  exit(1);
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index b73e099e0285..79f10ff453d0 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -2109,7 +2109,8 @@ static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
>  
>  int spapr_populate_pci_dt(sPAPRPHBState *phb,
>uint32_t xics_phandle,
> -  void *fdt)
> +  void *fdt,
> +  int *node_offset)
>  {
>  int bus_off, i, j, ret;
>  char nodename[FDT_NAME_MAX];
> @@ -2166,6 +2167,9 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>  if (bus_off < 0) {
>  return bus_off;
>  }
> +if (node_offset) {
> +*node_offset = bus_off;
> +}
>  
>  /* Write PHB properties */
>  _FDT(fdt_setprop_string(fdt, bus_off, "device_type", "pci"));
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 31bae68167f2..7837fb0b1110 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -115,7 +115,8 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct 
> sPAPRPHBState *phb, int pin)
>  
>  int spapr_populate_pci_dt(sPAPRPHBState *phb,
>uint32_t xics_phandle,
> -  void *fdt);
> +  void *fdt,
> +  int *node_offset);
>  
>  void spapr_pci_rtas_init(void);
>  
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v8 3/3] migration: add bitmap for received page

[Peter Xu]

On Thu, Jul 27, 2017 at 10:27:41AM +0300, Alexey Perevalov wrote:
> On 07/27/2017 05:35 AM, Peter Xu wrote:
> >On Wed, Jul 26, 2017 at 06:24:11PM +0300, Alexey Perevalov wrote:
> >>On 07/26/2017 11:43 AM, Peter Xu wrote:
> >>>On Wed, Jul 26, 2017 at 11:07:17AM +0300, Alexey Perevalov wrote:
> On 07/26/2017 04:49 AM, Peter Xu wrote:
> >On Thu, Jul 20, 2017 at 09:52:34AM +0300, Alexey Perevalov wrote:
> >>This patch adds ability to track down already received
> >>pages, it's necessary for calculation vCPU block time in
> >>postcopy migration feature, maybe for restore after
> >>postcopy migration failure.
> >>Also it's necessary to solve shared memory issue in
> >>postcopy livemigration. Information about received pages
> >>will be transferred to the software virtual bridge
> >>(e.g. OVS-VSWITCHD), to avoid fallocate (unmap) for
> >>already received pages. fallocate syscall is required for
> >>remmaped shared memory, due to remmaping itself blocks
> >>ioctl(UFFDIO_COPY, ioctl in this case will end with EEXIT
> >>error (struct page is exists after remmap).
> >>
> >>Bitmap is placed into RAMBlock as another postcopy/precopy
> >>related bitmaps.
> >>
> >>Reviewed-by: Peter Xu 
> >>Signed-off-by: Alexey Perevalov 
> >>---
> >[...]
> >
> >>  static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
> >>-void *from_addr, uint64_t pagesize)
> >>+   void *from_addr, uint64_t pagesize, 
> >>RAMBlock *rb)
> >>  {
> >>+int ret;
> >>  if (from_addr) {
> >>  struct uffdio_copy copy_struct;
> >>  copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
> >>  copy_struct.src = (uint64_t)(uintptr_t)from_addr;
> >>  copy_struct.len = pagesize;
> >>  copy_struct.mode = 0;
> >>-return ioctl(userfault_fd, UFFDIO_COPY, _struct);
> >>+ret = ioctl(userfault_fd, UFFDIO_COPY, _struct);
> >>  } else {
> >>  struct uffdio_zeropage zero_struct;
> >>  zero_struct.range.start = (uint64_t)(uintptr_t)host_addr;
> >>  zero_struct.range.len = pagesize;
> >>  zero_struct.mode = 0;
> >>-return ioctl(userfault_fd, UFFDIO_ZEROPAGE, _struct);
> >>+ret = ioctl(userfault_fd, UFFDIO_ZEROPAGE, _struct);
> >>+}
> >>+if (!ret) {
> >>+ramblock_recv_bitmap_set(host_addr, rb);
> >Wait...
> >
> >Now we are using 4k-page/bit bitmap, do we need to take care of the
> >huge pages here?  Looks like we are only setting the first bit of it
> >if it is a huge page?
> First version was per ramblock page size, IOW bitmap was smaller in
> case of hugepages.
> >>>Yes, but this is not the first version any more. :)
> >>>
> >>>This patch is using:
> >>>
> >>>   bitmap_new(rb->max_length >> TARGET_PAGE_BITS);
> >>>
> >>>to allocate bitmap, so it is using small pages always for bitmap,
> >>>right? (I should not really say "4k" pages, here I think the size is
> >>>host page size, which is the thing returned from getpagesize()).
> >>>
> You mentioned that TARGET_PAGE_SIZE is reasonable for precopy case,
> in "Re: [Qemu-devel] [PATCH v1 2/2] migration: add bitmap for copied page"
> I though TARGET_PAGE_SIZE as transmition unit, is using in precopy even
> hugepage case.
> But it's not so logically, page being marked as dirty, should be sent as a
> whole page.
> >>>Sorry if I misunderstood, but I didn't see anything wrong - we are
> >>>sending pages in small pages, but when postcopy is there, we do
> >>>UFFDIO_COPY in huge page, so everything is fine?
> >>I think yes, we chose TARGET_PAGE_SIZE because of wider
> >>use case ranges.
> >So... are you going to post another version? IIUC we just need to use
> >a bitmap_set() to replace the ramblock_recv_bitmap_set(), while set
> >the size with "pagesize / TARGET_PAGE_SIZE"?
> From my point of view TARGET_PAGE_SIZE/TARGET_PAGE_BITS it's a platform
> specific
> 
> and it used in ram_load to copy to buffer so it's more preferred for bitmap 
> size
> and I'm not going to replace ramblock_recv_bitmap_set helper - it calculates 
> offset.
> 
> >
> >(I think I was wrong when saying getpagesize() above: the small page
> >  should be target page size, while the huge page should be the host's)
> I think we should forget about huge page case in "received bitmap"
> concept, maybe in "uffd_copied bitmap" it was reasonable ;)

Again, I am not sure I got the whole idea of the reply...

However, I do think when we UFFDIO_COPY a huge page, then we should do
bitmap_set() on the received bitmap for the whole range that the huge
page covers.

IMHO, the bitmap is defined as "one bit per small page", and the small
page size is TARGET_PAGE_SIZE. We cannot just assume that "as long as
the first bit of the huge 

Re: [Qemu-devel] [for-2.11 PATCH 26/26] spapr: add hotplug hooks for PHB hotplug

[David Gibson]

On Thu, Jul 27, 2017 at 07:09:55PM +0200, Greg Kurz wrote:
> On Thu, 27 Jul 2017 14:41:31 +1000
> Alexey Kardashevskiy  wrote:
> 
> > On 26/07/17 18:40, Greg Kurz wrote:
> > > Hotplugging PHBs is a machine-level operation, but PHBs reside on the
> > > main system bus, so we register spapr machine as the handler for the
> > > main system bus.
> > > 
> > > Signed-off-by: Michael Roth 
> > > Signed-off-by: Greg Kurz 
> > > ---
> > > - rebased against ppc-for-2.10
> > > - converted to unplug_request
> > > - handle drc_id at pre-plug
> > > - reset hotplugged PHB at plug
> > > - compatibility with older machine types
> > > ---
> > >  hw/ppc/spapr.c  |  114 
> > > +++
> > >  hw/ppc/spapr_drc.c  |1 
> > >  hw/ppc/spapr_pci.c  |2 -
> > >  include/hw/pci-host/spapr.h |2 +
> > >  include/hw/ppc/spapr.h  |1 
> > >  5 files changed, 118 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 90485054c2e7..589f76ef9fb8 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -2540,6 +2540,10 @@ static void ppc_spapr_init(MachineState *machine)
> > >  register_savevm_live(NULL, "spapr/htab", -1, 1,
> > >   _htab_handlers, spapr);
> > >  
> > > +if (spapr->dr_phb_enabled) {
> > > +qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine), 
> > > NULL);
> > > +}
> > > +
> > >  qemu_register_boot_set(spapr_boot_set, spapr);
> > >  
> > >  if (kvm_enabled()) {
> > > @@ -3238,6 +3242,103 @@ out:
> > >  error_propagate(errp, local_err);
> > >  }
> > >  
> > > +static void spapr_phb_pre_plug(HotplugHandler *hotplug_dev, DeviceState 
> > > *dev,
> > > +   Error **errp)
> > > +{
> > > +sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> > > +
> > > +if (sphb->drc_id == (uint32_t)-1) {
> > > +sphb->drc_id = sphb->index;
> > > +}
> > > +
> > > +if (sphb->drc_id >= SPAPR_DRC_MAX_PHB) {
> > > +error_setg(errp, "PHB id %d out of range", sphb->drc_id);
> > > +}  
> > 
> > 
> > sphb->index in considered 16bits in the existing code (even though it is
> > defined as 32bit) and SPAPR_DRC_MAX_PHB is just 256. I'd suggest using the
> > same limit for both, either 256 or 65536 is fine for me.
> > 
> > It is actually a bit weird - it is possible to completely configure few
> > PHBs in the command line so they will have index==-1 but PCI hotplug code -
> > spapr_phb_get_pci_func_drc() and spapr_phb_realize() - does not check for
> > this and just does (sphb->index << 16).
> 
> You're right and this looks like a bug... I'll try to come up with a fix.
> 
> > May be just ditch drc_id, enforce index not to be -1 and use it as drc_id?
> > 
> 
> This was how Mike did it in the original patchset but David suggested
> to introduce drc_id (to preserve existing setups I guess):
> 
> http://patchwork.ozlabs.org/patch/466262/

Huh.  So I did.  But.. sorry, I've changed my mind.

The fact that needing a DRC forces us to have a reasonable small id
for each PHB seems like a good excuse to make index mandatory - I'm
not convinced anyone was actually creating PHBs without index, and
this does allow us to simplify a bunch of things.

I'd like to see that done as a preliminary cleanup patch, though.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [for-2.11 PATCH 21/26] qdev: pass an Object * to qbus_set_hotplug_handler()

[David Gibson]

On Tue, Jul 25, 2017 at 08:02:28PM +0200, Greg Kurz wrote:
> From: Michael Roth 
> 
> Certain devices types, like memory/CPU, are now being handled using a
> hotplug interface provided by a top-level MachineClass. Hotpluggable
> host bridges are another such device where it makes sense to use a
> machine-level hotplug handler. However, unlike those devices,
> host-bridges have a parent bus (the main system bus), and devices with
> a parent bus use a different mechanism for registering their hotplug
> handlers: qbus_set_hotplug_handler(). This interface currently expects
> a handler to be a subclass of DeviceClass, but this is not the case
> for MachineClass, which derives directly from ObjectClass.
> 
> Internally, the interface only requires an ObjectClass, so expose that
> in qbus_set_hotplug_handler().
> 
> Cc: Michael S. Tsirkin 
> Cc: Eduardo Habkost 
> Signed-off-by: Michael Roth 
> Signed-off-by: Greg Kurz 

Reviewed-by: David Gibson 

> ---
> Changes since RFC:
> - rebased against ppc-for-2.10
> - changed qbus_set_hotplug_handler() declaration and dropped
>   qbus_set_hotplug_handler_internal() (Paolo)
> - updated title and changelog accordingly
> ---
>  hw/acpi/piix4.c   |2 +-
>  hw/char/virtio-serial-bus.c   |2 +-
>  hw/core/bus.c |   11 ++-
>  hw/pci/pcie.c |2 +-
>  hw/pci/shpc.c |2 +-
>  hw/ppc/spapr_pci.c|2 +-
>  hw/s390x/css-bridge.c |2 +-
>  hw/s390x/s390-pci-bus.c   |6 +++---
>  hw/scsi/virtio-scsi.c |2 +-
>  hw/scsi/vmw_pvscsi.c  |2 +-
>  hw/usb/dev-smartcard-reader.c |2 +-
>  include/hw/qdev-core.h|3 +--
>  12 files changed, 15 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index f276967365c4..f99c7438235e 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -447,7 +447,7 @@ static void piix4_update_bus_hotplug(PCIBus *pci_bus, 
> void *opaque)
>  
>  /* pci_bus cannot outlive PIIX4PMState, because /machine keeps it alive
>   * and it's not hot-unpluggable */
> -qbus_set_hotplug_handler(BUS(pci_bus), DEVICE(s), _abort);
> +qbus_set_hotplug_handler(BUS(pci_bus), OBJECT(s), _abort);
>  }
>  
>  static void piix4_pm_machine_ready(Notifier *n, void *opaque)
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index f5bc173844e4..4880236f4258 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -1042,7 +1042,7 @@ static void virtio_serial_device_realize(DeviceState 
> *dev, Error **errp)
>  /* Spawn a new virtio-serial bus on which the ports will ride as devices 
> */
>  qbus_create_inplace(>bus, sizeof(vser->bus), 
> TYPE_VIRTIO_SERIAL_BUS,
>  dev, vdev->bus_name);
> -qbus_set_hotplug_handler(BUS(>bus), DEVICE(vser), errp);
> +qbus_set_hotplug_handler(BUS(>bus), OBJECT(vser), errp);
>  vser->bus.vser = vser;
>  QTAILQ_INIT(>ports);
>  
> diff --git a/hw/core/bus.c b/hw/core/bus.c
> index 4651f244864c..e09843f6abea 100644
> --- a/hw/core/bus.c
> +++ b/hw/core/bus.c
> @@ -22,22 +22,15 @@
>  #include "hw/qdev.h"
>  #include "qapi/error.h"
>  
> -static void qbus_set_hotplug_handler_internal(BusState *bus, Object *handler,
> -  Error **errp)
> +void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
>  {
> -
>  object_property_set_link(OBJECT(bus), OBJECT(handler),
>   QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
>  }
>  
> -void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, Error 
> **errp)
> -{
> -qbus_set_hotplug_handler_internal(bus, OBJECT(handler), errp);
> -}
> -
>  void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
>  {
> -qbus_set_hotplug_handler_internal(bus, OBJECT(bus), errp);
> +qbus_set_hotplug_handler(bus, OBJECT(bus), errp);
>  }
>  
>  int qbus_walk_children(BusState *bus,
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 32191f2a55f7..52d9ff947f7c 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -443,7 +443,7 @@ void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot)
>  dev->exp.hpev_notified = false;
>  
>  qbus_set_hotplug_handler(BUS(pci_bridge_get_sec_bus(PCI_BRIDGE(dev))),
> - DEVICE(dev), NULL);
> + OBJECT(dev), NULL);
>  }
>  
>  void pcie_cap_slot_reset(PCIDevice *dev)
> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> index 69fc14b218d8..feed8a45e535 100644
> --- a/hw/pci/shpc.c
> +++ b/hw/pci/shpc.c
> @@ -648,7 +648,7 @@ int shpc_init(PCIDevice *d, PCIBus *sec_bus, MemoryRegion 
> *bar,
>  shpc_cap_update_dword(d);
>  memory_region_add_subregion(bar, offset, >mmio);
>  
> -qbus_set_hotplug_handler(BUS(sec_bus), 

Re: [Qemu-devel] [for-2.11 PATCH 25/26] spapr_pci: drop abusive sanity check when migrating the LSI table

[David Gibson]

On Tue, Jul 25, 2017 at 08:03:21PM +0200, Greg Kurz wrote:
> The guest can allocate blocks of IRQs when calling the ibm,change-msi
> RTAS call. This has an impact on the IRQ numbers returned by subsequent
> calls to spapr_ics_alloc_block().
> 
> It doesn't cause any problem right now because PHB are cold plugged and
> the LSI table have the same numbers at the destination and the source.
> 
> But this won't be the case anymore when we support PHB hotplug. In this
> case the IRQ numbers in the LSI table are state that we should migrate.
> 
> This patch hence changes the destination to always accept the LSI table
> from the migration stream, like it is already done for MSIs. No effort
> is made to preserve the current behavior of failing migration when LSI
> numbers are different with older machine types, for simplicity.
> 
> Signed-off-by: Greg Kurz 

Urgh.  So, yes, there's a real problem here.  But fundamentally it's
because we're dynamically allocating the LSIs from a pool, rather than
having a fixed mapping (mea culpa, I did this in a bunch of places
before I knew better and it's come to bite us several times since).

Migrating the value like this will fix the problem, but I'm going to
have to think if it's the best way.  If we really do need an allocator
or something like it, then we'll have to migrate something - although
the fact that the LSIs will depend on the order you hotplug things is
kinda nasty still.

Making the LSIs fixed will address several other oddities in this
area, but will obviously been a wider ranging rework of the irq
assignment in the machine (and some machine version compatibility
cruft, of course).

> ---
>  hw/ppc/spapr_pci.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 58406a1b7e93..157867af8178 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1887,8 +1887,7 @@ static const VMStateDescription vmstate_spapr_pci_lsi = 
> {
>  .version_id = 1,
>  .minimum_version_id = 1,
>  .fields = (VMStateField[]) {
> -VMSTATE_UINT32_EQUAL(irq, struct spapr_pci_lsi, NULL),
> -
> +VMSTATE_UINT32(irq, struct spapr_pci_lsi),
>  VMSTATE_END_OF_LIST()
>  },
>  };
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-ppc] [for-2.11 PATCH 00/26] spapr: add support for PHB hotplug

[David Gibson]

On Fri, Jul 28, 2017 at 01:27:05PM +1000, Alexey Kardashevskiy wrote:
> On 28/07/17 02:39, Greg Kurz wrote:
> > On Wed, 26 Jul 2017 17:31:17 -0300
> > Daniel Henrique Barboza  wrote:
> > 
> >> I've tested the patch set using Greg's Github branch. It worked fine in 
> >> my tests
> >> using a Fedora 26 and an Ubuntu 17.04 guests. I have two observations
> >> though:
> >>
> >> 1 - This is not related to this patch set per se because it is 
> >> reproducible on master, but
> >> I think it is interfering with this new feature.  There is a 
> >> warning/error message in
> >> the kernel right after SLOF that goes:
> >>
> >> (...)
> >>   -> smp_release_cpus()  
> >> spinning_secondaries = 0
> >>   <- smp_release_cpus()
> >> Linux ppc64le
> >> #1 SMP Mon Jul 1[0.030450] pci :00:02.0: of_irq_parse_pci: 
> >> failed with rc=-22
> >> [0.030552] pci :00:0f.0: of_irq_parse_pci: failed with rc=-22
> >> [  OK  ] Started Security Auditing Service.
> >> (...)
> >>
> > 
> > This is a regression in QEMU master introduced by this commit:
> > 
> > commit b87680427e8a3ff682f66514e99a8344e7437247
> > Author: Cédric Le Goater 
> > Date:   Wed Jul 5 19:13:15 2017 +0200
> > 
> > spapr: populate device tree depending on XIVE_EXPLOIT option
> > 
> > When XIVE is supported, the device tree should be populated
> > accordingly and the XIVE memory regions mapped to activate MMIOs.
> > 
> > Depending on the design we choose, we could also allocate different
> > ICS and ICP objects, or switch between objects. This needs to be
> > discussed.
> > 
> > Signed-off-by: Cédric Le Goater 
> > Signed-off-by: David Gibson 
> > 
> > It is very similar to the issue that motivated the new 
> > KVMPPC_H_UPDATE_PHANDLE
> > hcall (see patch 24 and 26 in this series):
> > 
> > - QEMU creates an "interrupt-controller" node with a phandle property
> >   with the value 0x
> > - QEMU passes the FDT to SLOF
> > - SLOF converts all references to the phandle to an SLOF internal value
> > 
> > => from now on (ie, until the next machine reset), the guest only knows
> >the OF phandle.
> > 
> > - during CAS, if we go XICS, we send back an updated FDT with the
> >   phandle of the "interrupt-controller" node reverted to 0x
> > 
> > => the guest complains because all cold-plugged devices nodes refer
> >to the OF phandle, not 0x
> > 
> > A solution is to use the value set by KVMPPC_H_UPDATE_PHANDLE during CAS
> > instead of 0x. I could verify it makes the guest warning disappear.
> > 
> > I'll send a dedicated patchset to fix this in 2.10.
> 
> 
> The SLOF I pushed for 2.10 does not have it though. And the rest of XIVE is
> not targeted for 2.10 anyway. So imho the solution is reverting "spapr:
> populate device tree depending on XIVE_EXPLOIT option" for 2.10.

I agree, I've applied the revert to ppc-for-2.10 now.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [for-2.11 PATCH 18/26] spapr: create DR connectors for PHBs

[David Gibson]

On Tue, Jul 25, 2017 at 08:01:50PM +0200, Greg Kurz wrote:
> From: Michael Roth 
> 
> Signed-off-by: Michael Roth 
> Reviewed-by: David Gibson 
> Signed-off-by: Greg Kurz 



> ---
> Changes since RFC:
> - rebased against ppc-for-2.10 (reset hooks registering already merged)
> - added new DRC type for PHB
> ---
>  hw/ppc/spapr.c |   15 +++
>  hw/ppc/spapr_drc.c |   17 +
>  include/hw/ppc/spapr_drc.h |8 
>  3 files changed, 40 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8dc505343c0f..5950c009ab7e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -98,6 +98,9 @@
>  
>  #define PHANDLE_XICP0x
>  
> +/* maximum number of hotpluggable PHBs */
> +#define SPAPR_DRC_MAX_PHB   256

I wonder if we should actually make this a machine property.

>  static ICSState *spapr_ics_create(sPAPRMachineState *spapr,
>const char *type_ics,
>int nr_irqs, Error **errp)
> @@ -2384,6 +2387,18 @@ static void ppc_spapr_init(MachineState *machine)
>  
>  spapr->dr_phb_enabled = smc->dr_phb_enabled;
>  
> +/* Setup hotplug / dynamic-reconfiguration connectors. top-level
> + * connectors (described in root DT node's "ibm,drc-types" property)
> + * are pre-initialized here. additional child connectors (such as
> + * connectors for a PHBs PCI slots) are added as needed during their
> + * parent's realization.
> + */
> +if (spapr->dr_phb_enabled) {
> +for (i = 0; i < SPAPR_DRC_MAX_PHB; i++) {
> +spapr_dr_connector_new(OBJECT(machine), TYPE_SPAPR_DRC_PHB, i);
> +}
> +}
> +
>  /* Set up PCI */
>  spapr_pci_rtas_init();
>  
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index eb8024d37c54..2e1049ce61c7 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -697,6 +697,15 @@ static void spapr_drc_lmb_class_init(ObjectClass *k, 
> void *data)
>  drck->release = spapr_lmb_release;
>  }
>  
> +static void spapr_drc_phb_class_init(ObjectClass *k, void *data)
> +{
> +sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
> +
> +drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PHB;
> +drck->typename = "PHB";
> +drck->drc_name_prefix = "PHB ";
> +}
> +
>  static const TypeInfo spapr_dr_connector_info = {
>  .name  = TYPE_SPAPR_DR_CONNECTOR,
>  .parent= TYPE_DEVICE,
> @@ -740,6 +749,13 @@ static const TypeInfo spapr_drc_lmb_info = {
>  .class_init= spapr_drc_lmb_class_init,
>  };
>  
> +static const TypeInfo spapr_drc_phb_info = {
> +.name  = TYPE_SPAPR_DRC_PHB,
> +.parent= TYPE_SPAPR_DRC_LOGICAL,

I thought PHB DRCs were physical..

> +.instance_size = sizeof(sPAPRDRConnector),
> +.class_init= spapr_drc_phb_class_init,
> +};
> +
>  /* helper functions for external users */
>  
>  sPAPRDRConnector *spapr_drc_by_index(uint32_t index)
> @@ -1179,6 +1195,7 @@ static void spapr_drc_register_types(void)
>  type_register_static(_drc_cpu_info);
>  type_register_static(_drc_pci_info);
>  type_register_static(_drc_lmb_info);
> +type_register_static(_drc_phb_info);
>  
>  spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
>  rtas_set_indicator);
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index a7958d0a8d14..535fc61b98a8 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -69,6 +69,14 @@
>  #define SPAPR_DRC_LMB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
>  TYPE_SPAPR_DRC_LMB)
>  
> +#define TYPE_SPAPR_DRC_PHB "spapr-drc-phb"
> +#define SPAPR_DRC_PHB_GET_CLASS(obj) \
> +OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PHB)
> +#define SPAPR_DRC_PHB_CLASS(klass) \
> +OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_PHB)
> +#define SPAPR_DRC_PHB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> +TYPE_SPAPR_DRC_PHB)
> +
>  /*
>   * Various hotplug types managed by sPAPRDRConnector
>   *
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [for-2.11 PATCH 17/26] spapr_pci: introduce drc_id property

[David Gibson]

On Tue, Jul 25, 2017 at 08:01:38PM +0200, Greg Kurz wrote:
> With the addition of PHB hotplug, we have a static number of DRCs
> that can be used to handle hotplug/unplug operations on our PHBs,
> and need a consistent way to map PHBs to these connectors, and
> assign a unique identifiers for the connectors.
> 
> This patch adds a drc_id property for that purpose.
> 
> Signed-off-by: Greg Kurz 

I'd prefer to see this folded into the patch that actually uses this
property.

> ---
>  hw/ppc/spapr_pci.c  |1 +
>  include/hw/pci-host/spapr.h |2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 994d2f36105f..54533d8a3841 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1878,6 +1878,7 @@ static Property spapr_phb_properties[] = {
>   pre_2_8_migration, false),
>  DEFINE_PROP_BOOL("pcie-extended-configuration-space", sPAPRPHBState,
>   pcie_ecs, true),
> +DEFINE_PROP_UINT32("drc_id", sPAPRPHBState, drc_id, -1),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 5a4e9686d562..31bae68167f2 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -82,6 +82,8 @@ struct sPAPRPHBState {
>  
>  bool pcie_ecs; /* Allow access to PCIe extended config space? */
>  
> +uint32_t drc_id;
> +
>  /* Fields for migration compatibility hacks */
>  bool pre_2_8_migration;
>  uint32_t mig_liobn;
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [for-2.11 PATCH 24/26] spapr: allow guest to update the XICS phandle

[David Gibson]

On Tue, Jul 25, 2017 at 08:03:06PM +0200, Greg Kurz wrote:
> The "phandle" property of the XICS node is referenced by the "interrupt-map"
> property of each PHB node. This is used by the guest OS to setup IRQs for
> all PCI devices.
> 
> QEMU uses an arbitrary value (0x) for this phandle, but SLOF converts
> this value to a SLOF specific one, which is then presented to the guest OS.
> 
> This patches introduces the new KVMPPC_H_UPDATE_PHANDLE hcall, which is used
> by SLOF to communicate the patched phandle value back to QEMU. This value
> is then cached and preserved accross migration until machine reset.
> 
> This is required to be able to support PHB hotplug.
> 
> Note, that SLOF already has some code to call KVMPPC_H_RTAS_UPDATE, so we
> have to introduce its number even if QEMU currently doesn't implement it.
> 
> Suggested-by: Thomas Huth 
> Signed-off-by: Greg Kurz 

Ugh.  I really, really hope we can avoid this, though I don't
immediately see how.  Having to have two way communication between
qemu and SLOF about the device tree contents just seems like opening
the door to endless complexities.

This is basically a consequence of the fact that both qemu and partly
responsible for constructing the device tree for the guest, and that's
not easy to avoid.

Hrm.. Thomas, I know it's not really the OF way, but would it be
feasible to change SLOF to use the phandles as supplied by qemu rather
than creating its own?

> ---
>  hw/ppc/spapr.c |   25 +++--
>  hw/ppc/spapr_hcall.c   |   20 
>  include/hw/ppc/spapr.h |   11 ++-
>  3 files changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 1a6cd4efeb97..90485054c2e7 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -96,8 +96,6 @@
>  
>  #define MIN_RMA_SLOF128UL
>  
> -#define PHANDLE_XICP0x
> -
>  /* maximum number of hotpluggable PHBs */
>  #define SPAPR_DRC_MAX_PHB   256
>  
> @@ -1454,6 +1452,7 @@ static void ppc_spapr_reset(void)
>  first_ppc_cpu->env.nip = SPAPR_ENTRY_POINT;
>  
>  spapr->cas_reboot = false;
> +spapr->xics_phandle = UINT32_MAX;

Uh, is this defaulting to the phandle being (u32)(-1)?  That's one of
the two explicitly forbidden values for a phandle, so that's probably
not a good idea.

>  }
>  
>  static void spapr_create_nvram(sPAPRMachineState *spapr)
> @@ -1652,6 +1651,26 @@ static const VMStateDescription 
> vmstate_spapr_patb_entry = {
>  },
>  };
>  
> +static bool spapr_xics_phandle_needed(void *opaque)
> +{
> +sPAPRMachineState *spapr = opaque;
> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(MACHINE(spapr));
> +
> +/* Don't break older machine types that don't support PHB hotplug. */
> +return smc->dr_phb_enabled && spapr->xics_phandle != UINT32_MAX;
> +}
> +
> +static const VMStateDescription vmstate_spapr_xics_phandle = {
> +.name = "spapr_xics_phandle",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = spapr_xics_phandle_needed,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT32(xics_phandle, sPAPRMachineState),
> +VMSTATE_END_OF_LIST()
> +},
> +};
> +
>  static const VMStateDescription vmstate_spapr = {
>  .name = "spapr",
>  .version_id = 3,
> @@ -1671,6 +1690,7 @@ static const VMStateDescription vmstate_spapr = {
>  _spapr_ov5_cas,
>  _spapr_patb_entry,
>  _spapr_pending_events,
> +_spapr_xics_phandle,
>  NULL
>  }
>  };
> @@ -2702,6 +2722,7 @@ static void spapr_machine_initfn(Object *obj)
>  
>  spapr->htab_fd = -1;
>  spapr->use_hotplug_event_source = true;
> +spapr->xics_phandle = UINT32_MAX;
>  object_property_add_str(obj, "kvm-type",
>  spapr_get_kvm_type, spapr_set_kvm_type, NULL);
>  object_property_set_description(obj, "kvm-type",
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 72ea5a8247bf..ce8a9eb66b23 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1623,6 +1623,25 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>  return H_SUCCESS;
>  }
>  
> +static target_ulong h_update_phandle(PowerPCCPU *cpu, sPAPRMachineState 
> *spapr,
> + target_ulong opcode, target_ulong *args)
> +{
> +target_ulong old_phandle = args[0];
> +target_ulong new_phandle = args[1];
> +
> +if (new_phandle >= UINT32_MAX) {
> +return H_PARAMETER;
> +}
> +
> +/* We only have a "phandle" property in the XICS node at the moment. */
> +if (old_phandle != (uint32_t) PHANDLE_XICP) {
> +return H_PARAMETER;
> +}
> +
> +spapr->xics_phandle = (uint32_t) new_phandle;
> +return H_SUCCESS;
> +}
> +
>  static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1];
>  static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - 
> 

Re: [Qemu-devel] [PATCH v6 0/4] Remove dead error handling and convert to realize

[Jason Wang]

On 2017年07月26日 16:13, Mao Zhongyi wrote:

This series mainly implements the conversion of rocker to realize,
also remove the dead error handling and rename the unusual macro
name.

v6:

I posted another patch(commit 4cee3cf3) after patch1 of this series but
before patch1 be merged, they changed the same function caused this
series not to be applied cleanly on HEAD. so resend v6 to fix it.

Because there is no change on the patch content, so still add the R-B.

Cc: jasow...@redhat.com
Cc: j...@resnulli.us
Cc: arm...@redhat.com
Cc: f4...@amsat.org

Mao Zhongyi (4):
   net/rocker: Remove the dead error handling
   net/rocker: Plug memory leak in pci_rocker_init()
   net/rocker: Convert to realize()
   net/rocker: Fix the unusual macro name

  hw/net/rocker/rocker.c| 94 +++
  hw/net/rocker/rocker_desc.c   | 10 -
  hw/net/rocker/rocker_fp.c |  4 --
  hw/net/rocker/rocker_of_dpa.c | 20 -
  hw/net/rocker/rocker_world.c  | 12 +++---
  5 files changed, 28 insertions(+), 112 deletions(-)



Applied.

Thanks

Re: [Qemu-devel] [Qemu-ppc] [for-2.11 PATCH 00/26] spapr: add support for PHB hotplug

[Alexey Kardashevskiy]

On 28/07/17 02:39, Greg Kurz wrote:
> On Wed, 26 Jul 2017 17:31:17 -0300
> Daniel Henrique Barboza  wrote:
> 
>> I've tested the patch set using Greg's Github branch. It worked fine in 
>> my tests
>> using a Fedora 26 and an Ubuntu 17.04 guests. I have two observations
>> though:
>>
>> 1 - This is not related to this patch set per se because it is 
>> reproducible on master, but
>> I think it is interfering with this new feature.  There is a 
>> warning/error message in
>> the kernel right after SLOF that goes:
>>
>> (...)
>>   -> smp_release_cpus()  
>> spinning_secondaries = 0
>>   <- smp_release_cpus()
>> Linux ppc64le
>> #1 SMP Mon Jul 1[0.030450] pci :00:02.0: of_irq_parse_pci: 
>> failed with rc=-22
>> [0.030552] pci :00:0f.0: of_irq_parse_pci: failed with rc=-22
>> [  OK  ] Started Security Auditing Service.
>> (...)
>>
> 
> This is a regression in QEMU master introduced by this commit:
> 
> commit b87680427e8a3ff682f66514e99a8344e7437247
> Author: Cédric Le Goater 
> Date:   Wed Jul 5 19:13:15 2017 +0200
> 
> spapr: populate device tree depending on XIVE_EXPLOIT option
> 
> When XIVE is supported, the device tree should be populated
> accordingly and the XIVE memory regions mapped to activate MMIOs.
> 
> Depending on the design we choose, we could also allocate different
> ICS and ICP objects, or switch between objects. This needs to be
> discussed.
> 
> Signed-off-by: Cédric Le Goater 
> Signed-off-by: David Gibson 
> 
> It is very similar to the issue that motivated the new KVMPPC_H_UPDATE_PHANDLE
> hcall (see patch 24 and 26 in this series):
> 
> - QEMU creates an "interrupt-controller" node with a phandle property
>   with the value 0x
> - QEMU passes the FDT to SLOF
> - SLOF converts all references to the phandle to an SLOF internal value
> 
> => from now on (ie, until the next machine reset), the guest only knows
>the OF phandle.
> 
> - during CAS, if we go XICS, we send back an updated FDT with the
>   phandle of the "interrupt-controller" node reverted to 0x
> 
> => the guest complains because all cold-plugged devices nodes refer
>to the OF phandle, not 0x
> 
> A solution is to use the value set by KVMPPC_H_UPDATE_PHANDLE during CAS
> instead of 0x. I could verify it makes the guest warning disappear.
> 
> I'll send a dedicated patchset to fix this in 2.10.


The SLOF I pushed for 2.10 does not have it though. And the rest of XIVE is
not targeted for 2.10 anyway. So imho the solution is reverting "spapr:
populate device tree depending on XIVE_EXPLOIT option" for 2.10.


-- 
Alexey



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] build-sys: add --disable-vhost-user

[Michael S. Tsirkin]

On Fri, Jul 28, 2017 at 01:31:31AM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jul 26, 2017 at 7:53 PM Michael S. Tsirkin  wrote:
> >
> > On Tue, Jul 18, 2017 at 06:34:37PM +0200, Marc-André Lureau wrote:
> > > Learn to compile out vhost-user. Keep it enabled by default on
> > > non-mingw, that is assumed to be on POSIX.
> > >
> > > When trying to make a vhost-user netdev, it gives the following error:
> > >
> > > -netdev vhost-user,id=foo,chardev=chr-test: Parameter 'type' expects a 
> > > netdev backend type
> > >
> > > And similar error with the HMP/QMP monitors.
> > >
> > > In the future, we may want to hide vhost-user from QAPI/introspection
> > > with conditional compilation, although the design of this hasn't been
> > > fully fleshed out yet and shouldn't prevent this patch from being
> > > applied.
> > >
> > > Signed-off-by: Marc-André Lureau 
> >
> > After an offline discussion, looks like the idea is to be able to ship a
> > cut down binary with less features (and e.g. smaller attack surface?).
> >
> > Besides failing build on freebsd, this patch has too much ifdefery for
> > my taste. How about we patch just net/net.c?
> >
> 
> The point with --disable-vhost-user was to remove all vhost-user code.

Does not seem to be worth the cost of all these ifdefs.

> For netdev vhost-user only,

Wouldn't we want to disable scsi as well?

> it would be as simple as
> removing/compiling out the line:
> 
>[NET_CLIENT_DRIVER_VHOST_USER] = net_init_vhost_user,

And I guess the help and tests as well?

> Not sure it's worth a configure option in this case.

Seems like a reasonable thing to do but up to you.

> >
> >
> > > ---
> > >  hw/net/vhost_net.c| 12 ++--
> > >  hw/net/virtio-net.c   |  4 
> > >  hw/virtio/virtio-pci.c|  4 ++--
> > >  net/hub.c |  2 ++
> > >  net/net.c |  4 +++-
> > >  configure | 22 +-
> > >  default-configs/pci.mak   |  2 +-
> > >  default-configs/s390x-softmmu.mak |  2 +-
> > >  net/Makefile.objs |  2 +-
> > >  qemu-options.hx   |  4 
> > >  tests/Makefile.include|  6 +++---
> > >  11 files changed, 52 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index e037db63a3..565a1cc99d 100644
> > > --- a/hw/net/vhost_net.c
> > > +++ b/hw/net/vhost_net.c
> > > @@ -56,6 +56,7 @@ static const int kernel_feature_bits[] = {
> > >  VHOST_INVALID_FEATURE_BIT
> > >  };
> > >
> > > +#ifdef CONFIG_VHOST_USER
> > >  /* Features supported by others. */
> > >  static const int user_feature_bits[] = {
> > >  VIRTIO_F_NOTIFY_ON_EMPTY,
> > > @@ -86,6 +87,7 @@ static const int user_feature_bits[] = {
> > >
> > >  VHOST_INVALID_FEATURE_BIT
> > >  };
> > > +#endif
> > >
> > >  static const int *vhost_net_get_feature_bits(struct vhost_net *net)
> > >  {
> > > @@ -95,9 +97,11 @@ static const int *vhost_net_get_feature_bits(struct 
> > > vhost_net *net)
> > >  case NET_CLIENT_DRIVER_TAP:
> > >  feature_bits = kernel_feature_bits;
> > >  break;
> > > +#ifdef CONFIG_VHOST_USER
> > >  case NET_CLIENT_DRIVER_VHOST_USER:
> > >  feature_bits = user_feature_bits;
> > >  break;
> > > +#endif
> > >  default:
> > >  error_report("Feature bits not defined for this type: %d",
> > >  net->nc->info->type);
> > > @@ -193,6 +197,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions 
> > > *options)
> > >  }
> > >  }
> > >
> > > +#ifdef CONFIG_VHOST_USER
> > >  /* Set sane init value. Override when guest acks. */
> > >  if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> > >  features = vhost_user_get_acked_features(net->nc);
> > > @@ -203,7 +208,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions 
> > > *options)
> > >  goto fail;
> > >  }
> > >  }
> > > -
> > > +#endif
> > >  vhost_net_ack_features(net, features);
> > >
> > >  return net;
> > > @@ -309,6 +314,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState 
> > > *ncs,
> > >  net = get_vhost_net(ncs[i].peer);
> > >  vhost_net_set_vq_index(net, i * 2);
> > >
> > > +#ifdef CONFIG_VHOST_USER
> > >  /* Suppress the masking guest notifiers on vhost user
> > >   * because vhost user doesn't interrupt masking/unmasking
> > >   * properly.
> > > @@ -316,8 +322,8 @@ int vhost_net_start(VirtIODevice *dev, NetClientState 
> > > *ncs,
> > >  if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> > >  dev->use_guest_notifier_mask = false;
> > >  }
> > > +#endif
> > >   }
> > > -
> > >  r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
> > >  if (r < 0) {
> > >  error_report("Error binding guest notifier: %d", -r);
> > 

Re: [Qemu-devel] [PATCH for-2.10 0/3] qdev/vfio: defer DEVICE_DEL to avoid races with libvirt

[David Gibson]

On Thu, Jul 27, 2017 at 08:47:33AM -0600, Alex Williamson wrote:
> On Thu, 27 Jul 2017 20:53:48 +1000
> David Gibson  wrote:
> 
> > On Thu, Jul 27, 2017 at 10:11:48AM +0100, Peter Maydell wrote:
> > > On 27 July 2017 at 02:30, Michael Roth  wrote: 
> > >  
> > > > In particular, Mellanox CX4 adapters on PowerNV hosts might not be fully
> > > > quiesced by vfio-pci's finalize() routine until up to 6s after the
> > > > DEVICE_DELETED was emitted, leading to detach-device on the libvirt 
> > > > side pretty
> > > > much always crashing the host.  
> > > 
> > > My initial naive thought is that if the host kernel can crash then
> > > this is a host kernel bug... shouldn't the host kernel refuse
> > > the subsequent libvirt rebind if it would cause a crash ?  
> > 
> > I think so too, but I haven't been able to convince Alex.  Nor
> > find time to fix it in the kernel myself.
> 
> It's not me you need to convince, it's GregKH[1].  That interpretation
> is that the user bind request is a mandate and we'll fall over
> ourselves to try to do as they ask.  I think the best I might be able
> to do is to kill the QEMU process to avoid compromising the kernel
> rather than killing the kernel after the isolation compromise has
> occurred.  Messing with driver binding is a privileged operation, and
> the kernel believes you get to keep all the pieces when it fails.
> Sorry.  Thanks,

Ah, my mistake.  Well, I guess I'll whinge at GregKH at some point.

Anyway, the basic point remains - I still think we should address this
in the kernel, but that's not going to happen soon.  So we're left
with addressing it in qemu and/or libvirt.  And as others have pointed
out, there are reasons to do that even if the kernel does get changed
to protect itself better here.

> 
> Alex
> 
> [1] https://lkml.org/lkml/2017/7/10/728
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [for-2.11 PATCH 13/26] qdev: store DeviceState's canonical path to use when unparenting

[David Gibson]

On Thu, Jul 27, 2017 at 06:50:42PM +0200, Greg Kurz wrote:
> On Wed, 26 Jul 2017 15:24:43 +1000
> David Gibson  wrote:
> 
> > On Tue, Jul 25, 2017 at 08:00:47PM +0200, Greg Kurz wrote:
> > > From: Michael Roth 
> > > 
> > > device_unparent(dev, ...) is called when a device is unparented,
> > > either directly, or as a result of a parent device being
> > > finalized, and handles some final cleanup for the device. Part
> > > of this includes emiting a DEVICE_DELETED QMP event to notify
> > > management, which includes the device's path in the composition
> > > tree as provided by object_get_canonical_path().
> > > 
> > > object_get_canonical_path() assumes the device is still connected
> > > to the machine/root container, and will assert otherwise, but
> > > in some situations this isn't the case:
> > > 
> > > If the parent is finalized as a result of object_unparent(), it
> > > will still be attached to the composition tree at the time any
> > > children are unparented as a result of that same call to
> > > object_unparent(). However, in some cases, object_unparent()
> > > will complete without finalizing the parent device, due to
> > > lingering references that won't be released till some time later.
> > > One such example is if the parent has MemoryRegion children (which
> > > take a ref on their parent), who in turn have AddressSpace's (which
> > > take a ref on their regions), since those AddressSpaces get cleaned
> > > up asynchronously by the RCU thread.
> > > 
> > > In this case qdev:device_unparent() may be called for a child Device
> > > that no longer has a path to the root/machine container, causing
> > > object_get_canonical_path() to assert.
> > > 
> > > Fix this by storing the canonical path during realize() so the
> > > information will still be available for device_unparent() in such
> > > cases.  
> > 
> > Hm.  I'm no expert on the QOM model, but I'm not sure this is the
> > right approach.
> > 
> > I would have thought the right time to emit the DEVICE_DELETED message
> > would be when the device leaves the main composition tree, even if it
> > could be finalized later.
> > 
> > If we made that the case, does this problem go away?
> > 
> 
> I'm no expert either and I confess I took this patch simply because it was
> in Michael's original patchset. :)
> 
> But according to Michael's answer, it seems that the issue has a broader
> scope than just PHB hotplug...

Ok.  I see Michael has posted this and a couple of other things
separately.  Let's hope that can get resolved upstream, and rebase
this series on top of the result.

> 
> > > Cc: Michael S. Tsirkin 
> > > Cc: Paolo Bonzini 
> > > Signed-off-by: Michael Roth 
> > > Signed-off-by: Greg Kurz 
> > > ---
> > > Changes since RFC:
> > > - rebased against ppc-for-2.10
> > > ---
> > >  hw/core/qdev.c |   15 ---
> > >  include/hw/qdev-core.h |1 +
> > >  2 files changed, 13 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > > index 606ab53c42cd..a64b35c16251 100644
> > > --- a/hw/core/qdev.c
> > > +++ b/hw/core/qdev.c
> > > @@ -928,6 +928,12 @@ static void device_set_realized(Object *obj, bool 
> > > value, Error **errp)
> > >  goto post_realize_fail;
> > >  }
> > >  
> > > +/* always re-initialize since we clean up in device_unparent() 
> > > instead
> > > + * of unrealize()
> > > + */
> > > +g_free(dev->canonical_path);
> > > +dev->canonical_path = object_get_canonical_path(OBJECT(dev));
> > > +
> > >  if (qdev_get_vmsd(dev)) {
> > >  if (vmstate_register_with_alias_id(dev, -1, 
> > > qdev_get_vmsd(dev), dev,
> > > dev->instance_id_alias,
> > > @@ -984,6 +990,7 @@ child_realize_fail:
> > >  }
> > >  
> > >  post_realize_fail:
> > > +g_free(dev->canonical_path);
> > >  if (dc->unrealize) {
> > >  dc->unrealize(dev, NULL);
> > >  }
> > > @@ -1102,10 +1109,12 @@ static void device_unparent(Object *obj)
> > >  
> > >  /* Only send event if the device had been completely realized */
> > >  if (dev->pending_deleted_event) {
> > > -gchar *path = object_get_canonical_path(OBJECT(dev));
> > > +g_assert(dev->canonical_path);
> > >  
> > > -qapi_event_send_device_deleted(!!dev->id, dev->id, path, 
> > > _abort);
> > > -g_free(path);
> > > +qapi_event_send_device_deleted(!!dev->id, dev->id, 
> > > dev->canonical_path,
> > > +   _abort);
> > > +g_free(dev->canonical_path);
> > > +dev->canonical_path = NULL;
> > >  }
> > >  
> > >  qemu_opts_del(dev->opts);
> > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> > > index ae317286a480..9237b6849ff3 100644
> > > --- a/include/hw/qdev-core.h

Re: [Qemu-devel] [PATCH 1/2] Add more function keys to QEMU

[Programmingkid]

> On Jul 27, 2017, at 10:55 AM, Eric Blake  wrote:
> 
> On 07/27/2017 09:51 AM, Programmingkid wrote:
> 
> You forgot in-reply-to: and references: headers, meaning this was not
> threaded with your 0/2 patch.

Git send-email doesn't work for me. It complains about missing Perl modules. 

> 
>> There are now keyboards that have 19 function keys. This patch extends QEMU 
>> so these function keys can be used. 
>> 
>> Signed-off-by: John Arbuckle 
>> ---
>> qapi-schema.json  | 6 +-
>> ui/input-keymap.c | 4 
>> 2 files changed, 9 insertions(+), 1 deletion(-)
> 
> Conflicts with Gerd's pending UI pull; so you'll need to rebase.
> Furthermore, while Gerd added keys after softfreeze (necessary to fix a
> regression), your additions seem to be a new feature rather than a bug
> fix, and may therefore be more appropriate for 2.11.

Good idea. I will change the 'since' values to 2.11 in the next patch.

Re: [Qemu-devel] [PATCH 1/2] Add more function keys to QEMU

[Programmingkid]

> On Jul 27, 2017, at 10:54 AM, Daniel P. Berrange  wrote:
> 
> On Thu, Jul 27, 2017 at 10:51:33AM -0400, Programmingkid wrote:
>> There are now keyboards that have 19 function keys. This patch extends
>> QEMU so these function keys can be used. 
>> 
>> Signed-off-by: John Arbuckle 
>> ---
>> qapi-schema.json  | 6 +-
>> ui/input-keymap.c | 4 
>> 2 files changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 9c6c3e1..a051820 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -4842,6 +4842,10 @@
>> # @hiragana: since 2.9
>> # @henkan: since 2.9
>> # @yen: since 2.9
>> +# @f16: since 2.10
>> +# @f17: since 2.10
>> +# @f18: since 2.10
>> +# @f19: since 2.10
>> #
>> # Since: 1.3.0
>> #
>> @@ -4864,7 +4868,7 @@
>> 'props', 'undo', 'front', 'copy', 'open', 'paste', 'find', 'cut',
>> 'lf', 'help', 'meta_l', 'meta_r', 'compose', 'pause',
>> 'ro', 'hiragana', 'henkan', 'yen',
>> -'kp_comma', 'kp_equals', 'power' ] }
>> +'kp_comma', 'kp_equals', 'power', 'f16', 'f17', 'f18', 'f19'] }
> 
> Linux and AT set 1 go all the way to F24, and OS-X goes to F20, so don't
> arbitrarily stop short at F19
> 
> Regards,
> Daniel

I think I will future-proof this patch by going all the way to F30. 

[Qemu-devel] [PATCH v3] vhost-user: fix watcher need be removed when vhost-user hotplug

[w00273186]

From: Yunjian Wang 

"nc" is freed after hotplug vhost-user, but the watcher is not removed.
The QEMU crash when the watcher access the "nc" when socket disconnects.

Program received signal SIGSEGV, Segmentation fault.
#0  object_get_class (obj=obj@entry=0x2) at qom/object.c:750
#1  0x7f9bb4180da1 in qemu_chr_fe_disconnect (be=) at 
chardev/char-fe.c:372
#2  0x7f9bb40d1100 in net_vhost_user_watch (chan=, 
cond=, opaque=) at net/vhost-user.c:188
#3  0x7f9baf97f99a in g_main_context_dispatch () from 
/usr/lib64/libglib-2.0.so.0
#4  0x7f9bb41d7ebc in glib_pollfds_poll () at util/main-loop.c:213
#5  os_host_main_loop_wait (timeout=) at util/main-loop.c:261
#6  main_loop_wait (nonblocking=nonblocking@entry=0) at util/main-loop.c:515
#7  0x7f9bb3e266a7 in main_loop () at vl.c:1917
#8  main (argc=, argv=, envp=) 
at vl.c:4786

Signed-off-by: Yunjian Wang 
---
v3:
 -fix conflicts with current master.

v2:
 -move the chunk before deinit.

ps:
 reproduce steps:
1. virsh attach-device vm0 vhost-user.xml
2. virsh detach-device vm0 vhost-user.xml
3. virsh attach-device vm0 vhost-user.xml
4. service openvswitch restart
5. repeat step 2~4

 the vhost-user xml:

  
  
  
  

---
 net/vhost-user.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 36f32a2..c23927c 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -151,6 +151,10 @@ static void vhost_user_cleanup(NetClientState *nc)
 s->vhost_net = NULL;
 }
 if (nc->queue_index == 0) {
+if (s->watch) {
+g_source_remove(s->watch);
+s->watch = 0;
+}
 qemu_chr_fe_deinit(>chr, true);
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for 2.10 0/8] docs: fix broken paths

[Philippe Mathieu-Daudé]

On 07/27/2017 10:25 PM, Philippe Mathieu-Daudé wrote:

Hi, following Cleber Rosa example I cleaned more invalid references.

Eric said this can wait 2.11, however these patches don't change any code
generated, I think the 2.10 users deserve an up-to-date doc :p


Well if someone is looking at that doc he'll rapidly end up cloning the 
repository... so this can wait 2.11 :)

[Qemu-devel] [PATCH for 2.10 8/8] docs: fix broken paths to docs/spin/

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 docs/spin/aio_notify.promela| 6 +++---
 docs/spin/aio_notify_accept.promela | 4 ++--
 docs/spin/aio_notify_bug.promela| 4 ++--
 docs/spin/tcg-exclusive.promela | 2 +-
 include/block/aio.h | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/docs/spin/aio_notify.promela b/docs/spin/aio_notify.promela
index fccc7ee1c3..a8f032560d 100644
--- a/docs/spin/aio_notify.promela
+++ b/docs/spin/aio_notify.promela
@@ -8,15 +8,15 @@
  * the WTFPL will do.
  *
  * To simulate it:
- * spin -p docs/aio_notify.promela
+ * spin -p docs/spin/aio_notify.promela
  *
  * To verify it:
- * spin -a docs/aio_notify.promela
+ * spin -a docs/spin/aio_notify.promela
  * gcc -O2 pan.c
  * ./a.out -a
  *
  * To verify it (with a bug planted in the model):
- * spin -a -DBUG docs/aio_notify.promela
+ * spin -a -DBUG docs/spin/aio_notify.promela
  * gcc -O2 pan.c
  * ./a.out -a
  */
diff --git a/docs/spin/aio_notify_accept.promela 
b/docs/spin/aio_notify_accept.promela
index 9cef2c955d..491f36a59c 100644
--- a/docs/spin/aio_notify_accept.promela
+++ b/docs/spin/aio_notify_accept.promela
@@ -8,13 +8,13 @@
  * the WTFPL will do.
  *
  * To verify the buggy version:
- * spin -a -DBUG1 docs/aio_notify_bug.promela
+ * spin -a -DBUG1 docs/spin/aio_notify_bug.promela
  * gcc -O2 pan.c
  * ./a.out -a -f
  * (or -DBUG2)
  *
  * To verify the fixed version:
- * spin -a docs/aio_notify_bug.promela
+ * spin -a docs/spin/aio_notify_bug.promela
  * gcc -O2 pan.c
  * ./a.out -a -f
  *
diff --git a/docs/spin/aio_notify_bug.promela b/docs/spin/aio_notify_bug.promela
index b3bfca1ca4..49c69cee3d 100644
--- a/docs/spin/aio_notify_bug.promela
+++ b/docs/spin/aio_notify_bug.promela
@@ -8,12 +8,12 @@
  * the WTFPL will do.
  *
  * To verify the buggy version:
- * spin -a -DBUG docs/aio_notify_bug.promela
+ * spin -a -DBUG docs/spin/aio_notify_bug.promela
  * gcc -O2 pan.c
  * ./a.out -a -f
  *
  * To verify the fixed version:
- * spin -a docs/aio_notify_bug.promela
+ * spin -a docs/spin/aio_notify_bug.promela
  * gcc -O2 pan.c
  * ./a.out -a -f
  *
diff --git a/docs/spin/tcg-exclusive.promela b/docs/spin/tcg-exclusive.promela
index c91cfca9f7..50a084c5c4 100644
--- a/docs/spin/tcg-exclusive.promela
+++ b/docs/spin/tcg-exclusive.promela
@@ -9,7 +9,7 @@
  * the WTFPL will do.
  *
  * To verify it:
- * spin -a docs/tcg-exclusive.promela
+ * spin -a docs/spin/tcg-exclusive.promela
  * gcc pan.c -O2
  * ./a.out -a
  *
diff --git a/include/block/aio.h b/include/block/aio.h
index e9aeeaec94..386d7f24dc 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -104,7 +104,7 @@ struct AioContext {
  *
  * Note that event_notifier_set *cannot* be optimized the same way.  For
  * more information on the problem that would result, see "#ifdef BUG2"
- * in the docs/aio_notify_accept.promela formal model.
+ * in the docs/spin/aio_notify_accept.promela formal model.
  */
 bool notified;
 EventNotifier notifier;
-- 
2.13.3

[Qemu-devel] [PATCH for 2.10 7/8] docs: fix broken paths to docs/specs/ivshmem-spec.txt

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 docs/specs/pci-ids.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
index 95adee07d6..bb99a0257e 100644
--- a/docs/specs/pci-ids.txt
+++ b/docs/specs/pci-ids.txt
@@ -40,7 +40,7 @@ maintained as part of the virtio specification.
 1af4:1100  Used as PCI Subsystem ID for existing hardware devices emulated
by qemu.
 
-1af4:1110  ivshmem device (shared memory, docs/specs/ivshmem_device_spec.txt)
+1af4:1110  ivshmem device (shared memory, docs/specs/ivshmem-spec.txt)
 
 All other device IDs are reserved.
 
-- 
2.13.3

[Qemu-devel] [PATCH for 2.10 3/8] docs: fix broken paths to docs/devel/qapi-code-gen.txt

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 docs/devel/writing-qmp-commands.txt | 2 +-
 include/qapi/visitor.h  | 2 +-
 qapi/introspect.json| 2 +-
 qapi/qapi-util.c| 2 +-
 scripts/qapi2texi.py| 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/docs/devel/writing-qmp-commands.txt 
b/docs/devel/writing-qmp-commands.txt
index 69793e320e..4f5b24c0c4 100644
--- a/docs/devel/writing-qmp-commands.txt
+++ b/docs/devel/writing-qmp-commands.txt
@@ -7,7 +7,7 @@ This document doesn't discuss QMP protocol level details, nor 
does it dive
 into the QAPI framework implementation.
 
 For an in-depth introduction to the QAPI framework, please refer to
-docs/qapi-code-gen.txt. For documentation about the QMP protocol,
+docs/devel/qapi-code-gen.txt. For documentation about the QMP protocol,
 start with docs/interop/qmp-intro.txt.
 
 == Overview ==
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index fe9faf469f..0f3b8cb459 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -36,7 +36,7 @@
  * QemuOpts, and clone visitors have some implementation limitations;
  * see the documentation for each visitor for more details on what it
  * supports.  Also, see visitor-impl.h for the callback contracts
- * implemented by each visitor, and docs/qapi-code-gen.txt for more
+ * implemented by each visitor, and docs/devel/qapi-code-gen.txt for more
  * about the QAPI code generator.
  *
  * All of the visitors are created via:
diff --git a/qapi/introspect.json b/qapi/introspect.json
index 1dbaef56eb..cf77ff0669 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -226,7 +226,7 @@
 #
 # @members: the alternate type's members, in no particular order.
 #   The members' wire encoding is distinct, see
-#   docs/qapi-code-gen.txt section Alternate types.
+#   docs/devel/qapi-code-gen.txt section Alternate types.
 #
 # On the wire, this can be any of the members.
 #
diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
index e28dbd0ac3..46eda7d196 100644
--- a/qapi/qapi-util.c
+++ b/qapi/qapi-util.c
@@ -40,7 +40,7 @@ int qapi_enum_parse(const char * const lookup[], const char 
*buf,
  * It may be prefixed by __RFQDN_ (downstream extension), where RFQDN
  * may contain only letters, digits, hyphen and period.
  * The special exception for enumeration names is not implemented.
- * See docs/qapi-code-gen.txt for more on QAPI naming rules.
+ * See docs/devel/qapi-code-gen.txt for more on QAPI naming rules.
  * Keep this consistent with scripts/qapi.py!
  * If @complete, the parse fails unless it consumes @str completely.
  * Return its length on success, -1 on failure.
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 9e015002ef..a317526e51 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -91,7 +91,7 @@ def texi_format(doc):
 # doesn't.
 #
 # Make sure to update section "Documentation markup" in
-# docs/qapi-code-gen.txt when fixing this.
+# docs/devel/qapi-code-gen.txt when fixing this.
 if line.startswith('| '):
 line = EXAMPLE_FMT(code=line[2:])
 elif line.startswith('= '):
-- 
2.13.3

[Qemu-devel] [PATCH for 2.10 5/8] docs: fix broken paths to docs/devel/tracing.txt

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 audio/trace-events  | 2 +-
 block/trace-events  | 2 +-
 chardev/trace-events| 2 +-
 crypto/trace-events | 2 +-
 hw/9pfs/trace-events| 2 +-
 hw/acpi/trace-events| 2 +-
 hw/alpha/trace-events   | 2 +-
 hw/arm/trace-events | 2 +-
 hw/audio/trace-events   | 2 +-
 hw/block/dataplane/trace-events | 2 +-
 hw/block/trace-events   | 2 +-
 hw/char/trace-events| 2 +-
 hw/display/trace-events | 2 +-
 hw/dma/trace-events | 2 +-
 hw/i386/trace-events| 2 +-
 hw/input/trace-events   | 2 +-
 hw/intc/trace-events| 2 +-
 hw/isa/trace-events | 2 +-
 hw/mem/trace-events | 2 +-
 hw/misc/trace-events| 2 +-
 hw/net/trace-events | 2 +-
 hw/nvram/trace-events   | 2 +-
 hw/pci/trace-events | 2 +-
 hw/ppc/trace-events | 2 +-
 hw/s390x/trace-events   | 2 +-
 hw/scsi/trace-events| 2 +-
 hw/sd/trace-events  | 2 +-
 hw/sparc/trace-events   | 2 +-
 hw/timer/trace-events   | 2 +-
 hw/usb/trace-events | 2 +-
 hw/vfio/trace-events| 2 +-
 hw/virtio/trace-events  | 2 +-
 hw/xen/trace-events | 2 +-
 io/trace-events | 2 +-
 linux-user/trace-events | 2 +-
 migration/trace-events  | 2 +-
 net/trace-events| 2 +-
 qom/trace-events| 2 +-
 scripts/simpletrace.py  | 2 +-
 target/arm/trace-events | 2 +-
 target/i386/trace-events| 2 +-
 target/mips/trace-events| 2 +-
 target/ppc/trace-events | 2 +-
 target/s390x/trace-events   | 2 +-
 target/sparc/trace-events   | 2 +-
 ui/trace-events | 2 +-
 util/trace-events   | 2 +-
 47 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/audio/trace-events b/audio/trace-events
index 517359039e..122604287f 100644
--- a/audio/trace-events
+++ b/audio/trace-events
@@ -1,4 +1,4 @@
-# See docs/tracing.txt for syntax documentation.
+# See docs/devel/tracing.txt for syntax documentation.
 
 # audio/alsaaudio.c
 alsa_revents(int revents) "revents = %d"
diff --git a/block/trace-events b/block/trace-events
index 4a4df25323..8d10a82941 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -1,4 +1,4 @@
-# See docs/tracing.txt for syntax documentation.
+# See docs/devel/tracing.txt for syntax documentation.
 
 # block.c
 bdrv_open_common(void *bs, const char *filename, int flags, const char 
*format_name) "bs %p filename \"%s\" flags %#x format_name \"%s\""
diff --git a/chardev/trace-events b/chardev/trace-events
index 822dde668b..d0e5f3bbc1 100644
--- a/chardev/trace-events
+++ b/chardev/trace-events
@@ -1,4 +1,4 @@
-# See docs/tracing.txt for syntax documentation.
+# See docs/devel/tracing.txt for syntax documentation.
 
 # chardev/wctablet.c
 wct_init(void) ""
diff --git a/crypto/trace-events b/crypto/trace-events
index dc6ddd30d6..e589990359 100644
--- a/crypto/trace-events
+++ b/crypto/trace-events
@@ -1,4 +1,4 @@
-# See docs/tracing.txt for syntax documentation.
+# See docs/devel/tracing.txt for syntax documentation.
 
 # crypto/tlscreds.c
 qcrypto_tls_creds_load_dh(void *creds, const char *filename) "TLS creds load 
DH creds=%p filename=%s"
diff --git a/hw/9pfs/trace-events b/hw/9pfs/trace-events
index fb4de3d465..08a4abf22e 100644
--- a/hw/9pfs/trace-events
+++ b/hw/9pfs/trace-events
@@ -1,4 +1,4 @@
-# See docs/tracing.txt for syntax documentation.
+# See docs/devel/tracing.txt for syntax documentation.
 
 # hw/9pfs/virtio-9p.c
 v9fs_rerror(uint16_t tag, uint8_t id, int err) "tag %d id %d err %d"
diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
index c379607a3e..e3b41e9df4 100644
--- a/hw/acpi/trace-events
+++ b/hw/acpi/trace-events
@@ -1,4 +1,4 @@
-# See docs/tracing.txt for syntax documentation.
+# See docs/devel/tracing.txt for syntax documentation.
 
 # hw/acpi/memory_hotplug.c
 mhp_acpi_invalid_slot_selected(uint32_t slot) "0x%"PRIx32
diff --git a/hw/alpha/trace-events b/hw/alpha/trace-events
index e44ff01a09..46024cca0b 100644
--- a/hw/alpha/trace-events
+++ b/hw/alpha/trace-events
@@ -1,4 +1,4 @@
-# See docs/tracing.txt for syntax documentation.
+# See docs/devel/tracing.txt for syntax documentation.
 
 # hw/alpha/pci.c
 alpha_pci_iack_write(void) ""
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index d5f33a2a03..193063ed99 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -1,4 +1,4 @@
-# See docs/tracing.txt for syntax documentation.
+# See docs/devel/tracing.txt for syntax documentation.
 
 # hw/arm/virt-acpi-build.c
 virt_acpi_setup(void) "No fw cfg or ACPI disabled. Bailing out."
diff --git a/hw/audio/trace-events b/hw/audio/trace-events
index 3210386e86..47e2ed53d7 100644
--- a/hw/audio/trace-events
+++ b/hw/audio/trace-events
@@ 

[Qemu-devel] [PATCH for 2.10 1/8] docs: fix broken paths to docs/interop dir

[Philippe Mathieu-Daudé]

From: Cleber Rosa 

With the move of some docs to docs/interop on d59157e, a couple of
references where not updated.

Signed-off-by: Cleber Rosa 
[PMD: fixed another reference in docs/interop/qmp-spec.txt]
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Blake 
---
 docs/devel/writing-qmp-commands.txt | 2 +-
 qapi-schema.json| 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/devel/writing-qmp-commands.txt 
b/docs/devel/writing-qmp-commands.txt
index 1e6375495b..69793e320e 100644
--- a/docs/devel/writing-qmp-commands.txt
+++ b/docs/devel/writing-qmp-commands.txt
@@ -8,7 +8,7 @@ into the QAPI framework implementation.
 
 For an in-depth introduction to the QAPI framework, please refer to
 docs/qapi-code-gen.txt. For documentation about the QMP protocol,
-start with docs/qmp-intro.txt.
+start with docs/interop/qmp-intro.txt.
 
 == Overview ==
 
diff --git a/qapi-schema.json b/qapi-schema.json
index dcc12c83a9..5fda5fa4d2 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -23,7 +23,7 @@
 # | -> data issued by the Client
 # | <- Server data response
 #
-# Please, refer to the QMP specification (docs/qmp-spec.txt) for
+# Please, refer to the QMP specification (docs/interop/qmp-spec.txt) for
 # detailed information on the Server command and response formats.
 #
 # = Stability Considerations
@@ -108,7 +108,7 @@
 #
 # Notes: This command is valid exactly when first connecting: it must be
 # issued before any other command will be accepted, and will fail once the
-# monitor is accepting other commands. (see qemu docs/qmp-spec.txt)
+# monitor is accepting other commands. (see qemu docs/interop/qmp-spec.txt)
 #
 # Since: 0.13
 #
-- 
2.13.3

[Qemu-devel] [PATCH for 2.10 6/8] docs: fix broken paths to docs/config/ich9-ehci-uhci.cfg

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 docs/usb2.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/usb2.txt b/docs/usb2.txt
index b9e7548073..09df45b5b1 100644
--- a/docs/usb2.txt
+++ b/docs/usb2.txt
@@ -50,7 +50,7 @@ companion controllers with two ports each.
 There is a config file in docs which will do all this for you, just
 try ...
 
-qemu -readconfig docs/ich9-ehci-uhci.cfg
+qemu -readconfig docs/config/ich9-ehci-uhci.cfg
 
 ... then use "bus=ehci.0" to assign your usb devices to that bus.
 
-- 
2.13.3

[Qemu-devel] [PATCH for 2.10 4/8] docs: fix broken paths to docs/devel/atomics.txt

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 docs/devel/lockcnt.txt | 2 +-
 include/qemu/atomic.h  | 4 ++--
 tcg/README | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/devel/lockcnt.txt b/docs/devel/lockcnt.txt
index 2a79b3205b..7c099bc6c8 100644
--- a/docs/devel/lockcnt.txt
+++ b/docs/devel/lockcnt.txt
@@ -145,7 +145,7 @@ can also be more efficient in two ways:
 - on some platforms, one can implement QemuLockCnt to hold the lock
   and the mutex in a single word, making the fast path no more expensive
   than simply managing a counter using atomic operations (see
-  docs/atomics.txt).  This can be very helpful if concurrent access to
+  docs/devel/atomics.txt).  This can be very helpful if concurrent access to
   the data structure is expected to be rare.
 
 
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index e07c7972ab..b6b62fb771 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -8,7 +8,7 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
  *
- * See docs/atomics.txt for discussion about the guarantees each
+ * See docs/devel/atomics.txt for discussion about the guarantees each
  * atomic primitive is meant to provide.
  */
 
@@ -427,7 +427,7 @@
  * sequentially consistent operations.
  *
  * As long as they are used as paired operations they are safe to
- * use. See docs/atomic.txt for more discussion.
+ * use. See docs/devel/atomics.txt for more discussion.
  */
 
 #ifndef atomic_mb_read
diff --git a/tcg/README b/tcg/README
index bf49e8242b..03bfb6acd4 100644
--- a/tcg/README
+++ b/tcg/README
@@ -446,7 +446,7 @@ when MTTCG is enabled.
 The guest translators should generate this opcode for all guest instructions
 which have ordering side effects.
 
-Please see docs/atomics.txt for more information on memory barriers.
+Please see docs/devel/atomics.txt for more information on memory barriers.
 
 * 64-bit guest on 32-bit host support
 
-- 
2.13.3

[Qemu-devel] [PATCH for 2.10 2/8] docs: fix broken paths to docs/interop/qcow2.txt

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 docs/qcow2-cache.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt
index 1fdd6f9ce7..b0571de4b8 100644
--- a/docs/qcow2-cache.txt
+++ b/docs/qcow2-cache.txt
@@ -15,7 +15,7 @@ not a straightforward operation.
 This document attempts to give an overview of the L2 and refcount
 caches, and how to configure them.
 
-Please refer to the docs/specs/qcow2.txt file for an in-depth
+Please refer to the docs/interop/qcow2.txt file for an in-depth
 technical description of the qcow2 file format.
 
 
-- 
2.13.3

[Qemu-devel] [PATCH for 2.10 0/8] docs: fix broken paths

[Philippe Mathieu-Daudé]

Hi, following Cleber Rosa example I cleaned more invalid references.

Eric said this can wait 2.11, however these patches don't change any code
generated, I think the 2.10 users deserve an up-to-date doc :p

I used the following command (and consider include it in some CI test job):

$ git grep docs/ \
  | sed -ne "s/.* \(docs[^ :)}\"\']*\).*/\1/p" \
  | sed -e 's/\(.*\)\.$/\1/p' | sort -u | while read p;do
ls -ld $p 1>/dev/null
  done

Regards,

Phil.

Cleber Rosa (1):
  docs: fix broken paths to docs/interop dir

Philippe Mathieu-Daudé (7):
  docs: fix broken paths to docs/interop/qcow2.txt
  docs: fix broken paths to docs/devel/qapi-code-gen.txt
  docs: fix broken paths to docs/devel/atomics.txt
  docs: fix broken paths to docs/devel/tracing.txt
  docs: fix broken paths to docs/config/ich9-ehci-uhci.cfg
  docs: fix broken paths to docs/specs/ivshmem-spec.txt
  docs: fix broken paths to docs/spin/

 audio/trace-events  | 2 +-
 block/trace-events  | 2 +-
 chardev/trace-events| 2 +-
 crypto/trace-events | 2 +-
 docs/devel/lockcnt.txt  | 2 +-
 docs/devel/writing-qmp-commands.txt | 4 ++--
 docs/qcow2-cache.txt| 2 +-
 docs/specs/pci-ids.txt  | 2 +-
 docs/spin/aio_notify.promela| 6 +++---
 docs/spin/aio_notify_accept.promela | 4 ++--
 docs/spin/aio_notify_bug.promela| 4 ++--
 docs/spin/tcg-exclusive.promela | 2 +-
 docs/usb2.txt   | 2 +-
 hw/9pfs/trace-events| 2 +-
 hw/acpi/trace-events| 2 +-
 hw/alpha/trace-events   | 2 +-
 hw/arm/trace-events | 2 +-
 hw/audio/trace-events   | 2 +-
 hw/block/dataplane/trace-events | 2 +-
 hw/block/trace-events   | 2 +-
 hw/char/trace-events| 2 +-
 hw/display/trace-events | 2 +-
 hw/dma/trace-events | 2 +-
 hw/i386/trace-events| 2 +-
 hw/input/trace-events   | 2 +-
 hw/intc/trace-events| 2 +-
 hw/isa/trace-events | 2 +-
 hw/mem/trace-events | 2 +-
 hw/misc/trace-events| 2 +-
 hw/net/trace-events | 2 +-
 hw/nvram/trace-events   | 2 +-
 hw/pci/trace-events | 2 +-
 hw/ppc/trace-events | 2 +-
 hw/s390x/trace-events   | 2 +-
 hw/scsi/trace-events| 2 +-
 hw/sd/trace-events  | 2 +-
 hw/sparc/trace-events   | 2 +-
 hw/timer/trace-events   | 2 +-
 hw/usb/trace-events | 2 +-
 hw/vfio/trace-events| 2 +-
 hw/virtio/trace-events  | 2 +-
 hw/xen/trace-events | 2 +-
 include/block/aio.h | 2 +-
 include/qapi/visitor.h  | 2 +-
 include/qemu/atomic.h   | 4 ++--
 io/trace-events | 2 +-
 linux-user/trace-events | 2 +-
 migration/trace-events  | 2 +-
 net/trace-events| 2 +-
 qapi-schema.json| 4 ++--
 qapi/introspect.json| 2 +-
 qapi/qapi-util.c| 2 +-
 qom/trace-events| 2 +-
 scripts/qapi2texi.py| 2 +-
 scripts/simpletrace.py  | 2 +-
 target/arm/trace-events | 2 +-
 target/i386/trace-events| 2 +-
 target/mips/trace-events| 2 +-
 target/ppc/trace-events | 2 +-
 target/s390x/trace-events   | 2 +-
 target/sparc/trace-events   | 2 +-
 tcg/README  | 2 +-
 ui/trace-events | 2 +-
 util/trace-events   | 2 +-
 64 files changed, 71 insertions(+), 71 deletions(-)

-- 
2.13.3

Re: [Qemu-devel] [PATCHv2 02/04] colo-compare: Processpactkets in the IOThreadofthe primary

[Fam Zheng]

On Thu, 07/27 15:47, Zhang Chen wrote:
> CC. Fam and David.
> 
> Any idea about it?

Is it possible to use g_main_context_{push,pop}_thread_default to "move" chardev
GSources to IOThread's context, then use g_main_context_query like main thread
and poll the fds in aio_poll?

Fam

> 
> 
> Thanks
> 
> Zhang Chen
> 
> 
> On 07/25/2017 09:02 AM, wang.yong...@zte.com.cn wrote:
> > 
> > >On 24/07/2017 12:38, wang.yong...@zte.com.cn wrote:
> > 
> > >> finally use g_main_loop_run to replace aio_poll  in the
> > 
> > >> iothread_run function.
> > 
> > >
> > 
> > >That would make the performance of virtio-blk with iothreads worse,
> > 
> > >unfortunately.  aio_poll is much more optimized than g_main_loop_run.
> > 
> > Hi Paolo,
> > 
> > Any other good idea to achieve this?
> > 
> > 
> > Thanks
> > 
> > 
> > >
> > 
> > >Paolo
> > 
> > >
> > 
> > >> After that IOThread
> > 
> > >> runs the GMainContext event loop,chardev and IOThread can work
> > together.
> > 
> > >>
> > 
> > >> How about it? If feasible, I will try to submit
> > 
> > 
> > 
> > 
> > 原始邮件
> > *发件人：*;
> > *收件人：*王勇10170530;;;;
> > *抄送人：*;;;;王广10165992;
> > *日 期 ：*2017年07月24日 19:59
> > *主 题 ：**Re: [PATCHv2 02/04] colo-compare: Processpactkets in the
> > IOThreadofthe primary*
> > 
> > 
> > On 24/07/2017 12:38, wang.yong...@zte.com.cn wrote:
> > > finally use g_main_loop_run to replace aio_poll  in the
> > > iothread_run function.
> > 
> > That would make the performance of virtio-blk with iothreads worse,
> > unfortunately.  aio_poll is much more optimized than g_main_loop_run.
> > 
> > Paolo
> > 
> > > After that IOThread
> > > runs the GMainContext event loop,chardev and IOThread can work together.
> > >
> > > How about it? If feasible, I will try to submit a patch.
> > 
> > 
> > 
> 
> -- 
> Thanks
> Zhang Chen
> 
> 
> 
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.10 4/5] target/arm: Move PMSAv7 reset into arm_cpu_reset() so M profile MPUs get reset

[Philippe Mathieu-Daudé]

On 07/27/2017 07:59 AM, Peter Maydell wrote:

When the PMSAv7 implementation was originally added it was for R profile
CPUs only, and reset was handled using the cpreg .resetfn hooks.
Unfortunately for M profile cores this doesn't work, because they do
not register any cpregs. Move the reset handling into arm_cpu_reset(),
where it will work for both R profile and M profile cores.

Signed-off-by: Peter Maydell 


Reviewed-by: Philippe Mathieu-Daudé 


---
  target/arm/cpu.c| 14 ++
  target/arm/helper.c | 28 
  2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 96d1f84..05c038b 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -232,6 +232,20 @@ static void arm_cpu_reset(CPUState *s)
  
  env->vfp.xregs[ARM_VFP_FPEXC] = 0;

  #endif
+
+if (arm_feature(env, ARM_FEATURE_PMSA) &&
+arm_feature(env, ARM_FEATURE_V7)) {
+if (cpu->pmsav7_dregion > 0) {
+memset(env->pmsav7.drbar, 0,
+   sizeof(*env->pmsav7.drbar) * cpu->pmsav7_dregion);
+memset(env->pmsav7.drsr, 0,
+   sizeof(*env->pmsav7.drsr) * cpu->pmsav7_dregion);
+memset(env->pmsav7.dracr, 0,
+   sizeof(*env->pmsav7.dracr) * cpu->pmsav7_dregion);
+}
+env->pmsav7.rnr = 0;
+}
+
  set_flush_to_zero(1, >vfp.standard_fp_status);
  set_flush_inputs_to_zero(1, >vfp.standard_fp_status);
  set_default_nan_mode(1, >vfp.standard_fp_status);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 63187de..a9247e9 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2404,18 +2404,6 @@ static void pmsav7_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
  *u32p = value;
  }
  
-static void pmsav7_reset(CPUARMState *env, const ARMCPRegInfo *ri)

-{
-ARMCPU *cpu = arm_env_get_cpu(env);
-uint32_t *u32p = *(uint32_t **)raw_ptr(env, ri);
-
-if (!u32p) {
-return;
-}
-
-memset(u32p, 0, sizeof(*u32p) * cpu->pmsav7_dregion);
-}
-
  static void pmsav7_rgnr_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
  {
@@ -2433,22 +2421,30 @@ static void pmsav7_rgnr_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
  }
  
  static const ARMCPRegInfo pmsav7_cp_reginfo[] = {

+/* Reset for all these registers is handled in arm_cpu_reset(),
+ * because the PMSAv7 is also used by M-profile CPUs, which do
+ * not register cpregs but still need the state to be reset.
+ */
  { .name = "DRBAR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 0,
.access = PL1_RW, .type = ARM_CP_NO_RAW,
.fieldoffset = offsetof(CPUARMState, pmsav7.drbar),
-  .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset 
},
+  .readfn = pmsav7_read, .writefn = pmsav7_write,
+  .resetfn = arm_cp_reset_ignore },
  { .name = "DRSR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 2,
.access = PL1_RW, .type = ARM_CP_NO_RAW,
.fieldoffset = offsetof(CPUARMState, pmsav7.drsr),
-  .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset 
},
+  .readfn = pmsav7_read, .writefn = pmsav7_write,
+  .resetfn = arm_cp_reset_ignore },
  { .name = "DRACR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 4,
.access = PL1_RW, .type = ARM_CP_NO_RAW,
.fieldoffset = offsetof(CPUARMState, pmsav7.dracr),
-  .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset 
},
+  .readfn = pmsav7_read, .writefn = pmsav7_write,
+  .resetfn = arm_cp_reset_ignore },
  { .name = "RGNR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 2, .opc2 = 0,
.access = PL1_RW,
.fieldoffset = offsetof(CPUARMState, pmsav7.rnr),
-  .writefn = pmsav7_rgnr_write },
+  .writefn = pmsav7_rgnr_write,
+  .resetfn = arm_cp_reset_ignore },
  REGINFO_SENTINEL
  };
  

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.10 2/5] target/arm: Don't allow guest to make System space executable for M profile

[Philippe Mathieu-Daudé]

Hi Peter,

On 07/27/2017 07:59 AM, Peter Maydell wrote:

For an M profile v7PMSA, the system space (0xe000 - 0x) can
never be executable, even if the guest tries to set the MPU registers
up that way. Enforce this restriction.

Signed-off-by: Peter Maydell 
---
  target/arm/helper.c | 16 +++-
  1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index ceef225..169c361 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8251,6 +8251,14 @@ static inline bool is_ppb_region(CPUARMState *env, 
uint32_t address)
  extract32(address, 20, 12) == 0xe00;
  }
  


I wonder if these should renamed pmsav7_is_ppb_region() and 
pmsav7_is_system_region().



+static inline bool is_system_region(CPUARMState *env, uint32_t address)
+{
+/* True if address is in the M profile system region
+ * 0xe000 - 0x
+ */
+return arm_feature(env, ARM_FEATURE_M) && extract32(address, 29, 3) == 0x7;
+}
+
  static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
   int access_type, ARMMMUIdx mmu_idx,
   hwaddr *phys_ptr, int *prot, uint32_t *fsr)
@@ -8354,6 +8362,12 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, 
uint32_t address,
  get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);
  } else { /* a MPU hit! */
  uint32_t ap = extract32(env->pmsav7.dracr[n], 8, 3);


Maybe names access_perms/execute_never are easier to read..


+uint32_t xn = extract32(env->pmsav7.dracr[n], 12, 1);
+


clear MemManage exceptions:

   *fsr &= ~0xff;


+if (is_system_region(env, address)) {
+/* System space is always execute never */
+xn = 1;


   } else {
   xn = extract32(env->pmsav7.dracr[n], 12, 1);


+}
  
  if (is_user) { /* User mode AP bit decoding */

  switch (ap) {
@@ -8394,7 +8408,7 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, 
uint32_t address,
  }
  
  /* execute never */

-if (env->pmsav7.dracr[n] & (1 << 12)) {
+if (xn) {
  *prot &= ~PAGE_EXEC;


and here we now can set eXecuteNever violation:

*fsr |= R_V7M_CFSR_IACCVIOL_MASK;


  }
  }


}
*fsr = 0x00d; /* Permission fault */

I don't understand this mask, I don't have bit [2] defined in my 
datashit, maybe it was expected to turn on exception Entry/Return which 
I have defined as bits 4 and 3 respectively, so I'd rather see here:


*fsr |= R_V7M_CFSR_MUNSTKERR_MASK | R_V7M_CFSR_MSTKERR_MASK;

What do you think?

Regards,

Phil.

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.10 1/5] target/arm: Don't do MPU lookups for addresses in M profile PPB region

[Philippe Mathieu-Daudé]

On 07/27/2017 07:59 AM, Peter Maydell wrote:

The M profile PMSAv7 specification says that if the address being looked
up is in the PPB region (0xe000 - 0xe00f) then we do not use
the MPU regions but always use the default memory map. Implement this
(we were previously behaving like an R profile PMSAv7, which does not
special case this).

Signed-off-by: Peter Maydell 
---
  target/arm/helper.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 4ed32c5..ceef225 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8244,6 +8244,13 @@ static bool pmsav7_use_background_region(ARMCPU *cpu,
  }
  }
  
+static inline bool is_ppb_region(CPUARMState *env, uint32_t address)

+{
+/* True if address is in the M profile PPB region 0xe000 - 0xe00f 
*/
+return arm_feature(env, ARM_FEATURE_M) &&
+extract32(address, 20, 12) == 0xe00;
+}
+
  static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
   int access_type, ARMMMUIdx mmu_idx,
   hwaddr *phys_ptr, int *prot, uint32_t *fsr)
@@ -8255,7 +8262,15 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, 
uint32_t address,
  *phys_ptr = address;
  *prot = 0;
  
-if (regime_translation_disabled(env, mmu_idx)) { /* MPU disabled */

+if (regime_translation_disabled(env, mmu_idx) ||
+is_ppb_region(env, address)) {
+/* MPU disabled or M profile PPB access: use default memory map.
+ * The other case which uses the default memory map in the
+ * v7M ARM ARM pseudocode is exception vector reads from the vector
+ * table. In QEMU those accesses are done in arm_v7m_load_vector(),
+ * which always does a direct read using address_space_ldl(), rather
+ * than going via this function, so we don't need to check that here.
+ */


Reviewed-by: Philippe Mathieu-Daudé 


  get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);
  } else { /* MPU enabled */
  for (n = (int)cpu->pmsav7_dregion - 1; n >= 0; n--) {

Re: [Qemu-devel] [PATCH] build-sys: add --disable-vhost-user

[Marc-André Lureau]

Hi

On Wed, Jul 26, 2017 at 7:53 PM Michael S. Tsirkin  wrote:
>
> On Tue, Jul 18, 2017 at 06:34:37PM +0200, Marc-André Lureau wrote:
> > Learn to compile out vhost-user. Keep it enabled by default on
> > non-mingw, that is assumed to be on POSIX.
> >
> > When trying to make a vhost-user netdev, it gives the following error:
> >
> > -netdev vhost-user,id=foo,chardev=chr-test: Parameter 'type' expects a 
> > netdev backend type
> >
> > And similar error with the HMP/QMP monitors.
> >
> > In the future, we may want to hide vhost-user from QAPI/introspection
> > with conditional compilation, although the design of this hasn't been
> > fully fleshed out yet and shouldn't prevent this patch from being
> > applied.
> >
> > Signed-off-by: Marc-André Lureau 
>
> After an offline discussion, looks like the idea is to be able to ship a
> cut down binary with less features (and e.g. smaller attack surface?).
>
> Besides failing build on freebsd, this patch has too much ifdefery for
> my taste. How about we patch just net/net.c?
>

The point with --disable-vhost-user was to remove all vhost-user code.

For netdev vhost-user only, it would be as simple as
removing/compiling out the line:

   [NET_CLIENT_DRIVER_VHOST_USER] = net_init_vhost_user,

Not sure it's worth a configure option in this case.

>
>
> > ---
> >  hw/net/vhost_net.c| 12 ++--
> >  hw/net/virtio-net.c   |  4 
> >  hw/virtio/virtio-pci.c|  4 ++--
> >  net/hub.c |  2 ++
> >  net/net.c |  4 +++-
> >  configure | 22 +-
> >  default-configs/pci.mak   |  2 +-
> >  default-configs/s390x-softmmu.mak |  2 +-
> >  net/Makefile.objs |  2 +-
> >  qemu-options.hx   |  4 
> >  tests/Makefile.include|  6 +++---
> >  11 files changed, 52 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index e037db63a3..565a1cc99d 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -56,6 +56,7 @@ static const int kernel_feature_bits[] = {
> >  VHOST_INVALID_FEATURE_BIT
> >  };
> >
> > +#ifdef CONFIG_VHOST_USER
> >  /* Features supported by others. */
> >  static const int user_feature_bits[] = {
> >  VIRTIO_F_NOTIFY_ON_EMPTY,
> > @@ -86,6 +87,7 @@ static const int user_feature_bits[] = {
> >
> >  VHOST_INVALID_FEATURE_BIT
> >  };
> > +#endif
> >
> >  static const int *vhost_net_get_feature_bits(struct vhost_net *net)
> >  {
> > @@ -95,9 +97,11 @@ static const int *vhost_net_get_feature_bits(struct 
> > vhost_net *net)
> >  case NET_CLIENT_DRIVER_TAP:
> >  feature_bits = kernel_feature_bits;
> >  break;
> > +#ifdef CONFIG_VHOST_USER
> >  case NET_CLIENT_DRIVER_VHOST_USER:
> >  feature_bits = user_feature_bits;
> >  break;
> > +#endif
> >  default:
> >  error_report("Feature bits not defined for this type: %d",
> >  net->nc->info->type);
> > @@ -193,6 +197,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions 
> > *options)
> >  }
> >  }
> >
> > +#ifdef CONFIG_VHOST_USER
> >  /* Set sane init value. Override when guest acks. */
> >  if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> >  features = vhost_user_get_acked_features(net->nc);
> > @@ -203,7 +208,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions 
> > *options)
> >  goto fail;
> >  }
> >  }
> > -
> > +#endif
> >  vhost_net_ack_features(net, features);
> >
> >  return net;
> > @@ -309,6 +314,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState 
> > *ncs,
> >  net = get_vhost_net(ncs[i].peer);
> >  vhost_net_set_vq_index(net, i * 2);
> >
> > +#ifdef CONFIG_VHOST_USER
> >  /* Suppress the masking guest notifiers on vhost user
> >   * because vhost user doesn't interrupt masking/unmasking
> >   * properly.
> > @@ -316,8 +322,8 @@ int vhost_net_start(VirtIODevice *dev, NetClientState 
> > *ncs,
> >  if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> >  dev->use_guest_notifier_mask = false;
> >  }
> > +#endif
> >   }
> > -
> >  r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
> >  if (r < 0) {
> >  error_report("Error binding guest notifier: %d", -r);
> > @@ -414,10 +420,12 @@ VHostNetState *get_vhost_net(NetClientState *nc)
> >  case NET_CLIENT_DRIVER_TAP:
> >  vhost_net = tap_get_vhost_net(nc);
> >  break;
> > +#ifdef CONFIG_VHOST_USER
> >  case NET_CLIENT_DRIVER_VHOST_USER:
> >  vhost_net = vhost_user_get_vhost_net(nc);
> >  assert(vhost_net);
> >  break;
> > +#endif
> >  default:
> >  break;
> >  }
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 148071a396..9bda4ab4f8 

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.10 3/5] target/arm: Rename cp15.c6_rgnr to pmsav7.rnr

[Philippe Mathieu-Daudé]

On 07/27/2017 07:43 PM, Philippe Mathieu-Daudé wrote:

On 07/27/2017 07:59 AM, Peter Maydell wrote:

[...]

-u32p += env->cp15.c6_rgnr;
+u32p += env->pmsav7.rnr;
  tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */
  *u32p = value;
  }
@@ -2447,7 +2447,7 @@ static const ARMCPRegInfo pmsav7_cp_reginfo[] = {
.readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = 
pmsav7_reset },
  { .name = "RGNR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 2, .opc2 
= 0,


"RGNR" -> "RNR"


Ah "RGNR" for -R and "RNR" for -M hmmm... still better keep the name 
matching the field, "rnr".





.access = PL1_RW,
-  .fieldoffset = offsetof(CPUARMState, cp15.c6_rgnr),
+  .fieldoffset = offsetof(CPUARMState, pmsav7.rnr),
.writefn = pmsav7_rgnr_write },


pmsav7_rnr_write


  REGINFO_SENTINEL
  };
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 1a40469..93c1a78 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -151,7 +151,7 @@ static bool pmsav7_rgnr_vmstate_validate(void 
*opaque, int version_id)


pmsav7_rnr_vmstate_validate()


  {
  ARMCPU *cpu = opaque;
-return cpu->env.cp15.c6_rgnr < cpu->pmsav7_dregion;
+return cpu->env.pmsav7.rnr < cpu->pmsav7_dregion;
  }
  static const VMStateDescription vmstate_pmsav7 = {


[...]
 VMSTATE_VALIDATE("rgnr is valid", pmsav7_rgnr_vmstate_validate),

also this one "rnr is valid" ^^^

once fixed:
Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.10 5/5] target/arm: Migrate MPU_RNR register state for M profile cores

[Philippe Mathieu-Daudé]

On 07/27/2017 07:59 AM, Peter Maydell wrote:

The PMSAv7 region number register is migrated for R profile
cores using the cpreg scheme, but M profile doesn't use
cpregs, and so we weren't migrating the MPU_RNR register state
at all. Fix that by adding a migration subsection for the
M profile case.

Signed-off-by: Peter Maydell 


Reviewed-by: Philippe Mathieu-Daudé 


---
  target/arm/machine.c | 28 
  1 file changed, 28 insertions(+)

diff --git a/target/arm/machine.c b/target/arm/machine.c
index 93c1a78..1f66da4 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -171,6 +171,29 @@ static const VMStateDescription vmstate_pmsav7 = {
  }
  };
  
+static bool pmsav7_rnr_needed(void *opaque)

+{
+ARMCPU *cpu = opaque;
+CPUARMState *env = >env;
+
+/* For R profile cores pmsav7.rnr is migrated via the cpreg
+ * "RGNR" definition in helper.h. For M profile we have to
+ * migrate it separately.
+ */
+return arm_feature(env, ARM_FEATURE_M);
+}
+
+static const VMStateDescription vmstate_pmsav7_rnr = {
+.name = "cpu/pmsav7-rnr",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = pmsav7_rnr_needed,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(env.pmsav7.rnr, ARMCPU),
+VMSTATE_END_OF_LIST()
+}
+};
+
  static int get_cpsr(QEMUFile *f, void *opaque, size_t size,
  VMStateField *field)
  {
@@ -377,6 +400,11 @@ const VMStateDescription vmstate_arm_cpu = {
  _iwmmxt,
  _m,
  _thumb2ee,
+/* pmsav7_rnr must come before pmsav7 so that we have the
+ * region number before we test it in the VMSTATE_VALIDATE
+ * in vmstate_pmsav7.
+ */
+_pmsav7_rnr,
  _pmsav7,
  NULL
  }

Re: [Qemu-devel] [PATCH for 2.10 3/4] fdt: probe for v1.4.2 using fdt_setprop_inplace_namelen_partial()

[Michael Roth]

Quoting Michael Roth (2017-07-27 16:43:36)
> Quoting Philippe Mathieu-Daudé (2017-07-26 16:40:09)
> > instead of fdt_first_subnode() which is v1.4.0
> > 
> > Signed-off-by: Philippe Mathieu-Daudé 
> 
> Signed-off-by: Michael Roth 

Doh, please ignore that. I meant:

Reviewed-by: Michael Roth 

> 
> > ---
> >  configure | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/configure b/configure
> > index 0d5bdb3ae9..2d803d6a77 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3565,7 +3565,10 @@ if test "$fdt" != "no" ; then
> >cat > $TMPC << EOF
> >  #include 
> >  #include 
> > -int main(void) { fdt_first_subnode(0, 0); return 0; }
> > +int main(void) {
> > +fdt_setprop_inplace_namelen_partial(0, 0, 0, 0, 0, 0, 0);
> > +return 0;
> > +}
> >  EOF
> >if compile_prog "" "$fdt_libs" ; then
> >  # system DTC is good - use it
> > -- 
> > 2.13.3
> > 
> > 
> 
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.10 3/5] target/arm: Rename cp15.c6_rgnr to pmsav7.rnr

[Philippe Mathieu-Daudé]

Hi Peter,

you missed some rgnr -> rnr :|

On 07/27/2017 07:59 AM, Peter Maydell wrote:

Almost all of the PMSAv7 state is in the pmsav7 substruct of
the ARM CPU state structure. The exception is the region
number register, which is in cp15.c6_rgnr. This exception
is a bit odd for M profile, which otherwise generally does
not store state in the cp15 substruct.

Rename cp15.c6_rgnr to pmsav7.rnr accordingly.

Signed-off-by: Peter Maydell 
---
  hw/intc/armv7m_nvic.c | 14 +++---
  target/arm/cpu.h  |  3 +--
  target/arm/helper.c   |  6 +++---
  target/arm/machine.c  |  2 +-
  4 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 26a4b2d..323e2d4 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -536,13 +536,13 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
  case 0xd94: /* MPU_CTRL */
  return cpu->env.v7m.mpu_ctrl;
  case 0xd98: /* MPU_RNR */
-return cpu->env.cp15.c6_rgnr;
+return cpu->env.pmsav7.rnr;
  case 0xd9c: /* MPU_RBAR */
  case 0xda4: /* MPU_RBAR_A1 */
  case 0xdac: /* MPU_RBAR_A2 */
  case 0xdb4: /* MPU_RBAR_A3 */
  {
-int region = cpu->env.cp15.c6_rgnr;
+int region = cpu->env.pmsav7.rnr;
  
  if (region >= cpu->pmsav7_dregion) {

  return 0;
@@ -554,7 +554,7 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
  case 0xdb0: /* MPU_RASR_A2 */
  case 0xdb8: /* MPU_RASR_A3 */
  {
-int region = cpu->env.cp15.c6_rgnr;
+int region = cpu->env.pmsav7.rnr;
  
  if (region >= cpu->pmsav7_dregion) {

  return 0;
@@ -681,7 +681,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value)
PRIu32 "/%" PRIu32 "\n",
value, cpu->pmsav7_dregion);
  } else {
-cpu->env.cp15.c6_rgnr = value;
+cpu->env.pmsav7.rnr = value;
  }
  break;
  case 0xd9c: /* MPU_RBAR */
@@ -702,9 +702,9 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value)
region, cpu->pmsav7_dregion);
  return;
  }
-cpu->env.cp15.c6_rgnr = region;
+cpu->env.pmsav7.rnr = region;
  } else {
-region = cpu->env.cp15.c6_rgnr;
+region = cpu->env.pmsav7.rnr;
  }
  
  if (region >= cpu->pmsav7_dregion) {

@@ -720,7 +720,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value)
  case 0xdb0: /* MPU_RASR_A2 */
  case 0xdb8: /* MPU_RASR_A3 */
  {
-int region = cpu->env.cp15.c6_rgnr;
+int region = cpu->env.pmsav7.rnr;
  
  if (region >= cpu->pmsav7_dregion) {

  return;
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 102c58a..b39d64a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -305,8 +305,6 @@ typedef struct CPUARMState {
  uint64_t par_el[4];
  };
  
-uint32_t c6_rgnr;

-
  uint32_t c9_insn; /* Cache lockdown registers.  */
  uint32_t c9_data;
  uint64_t c9_pmcr; /* performance monitor control register */
@@ -519,6 +517,7 @@ typedef struct CPUARMState {
  uint32_t *drbar;
  uint32_t *drsr;
  uint32_t *dracr;
+uint32_t rnr;
  } pmsav7;
  
  void *nvic;

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 169c361..63187de 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2385,7 +2385,7 @@ static uint64_t pmsav7_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
  return 0;
  }
  
-u32p += env->cp15.c6_rgnr;

+u32p += env->pmsav7.rnr;
  return *u32p;
  }
  
@@ -2399,7 +2399,7 @@ static void pmsav7_write(CPUARMState *env, const ARMCPRegInfo *ri,

  return;
  }
  
-u32p += env->cp15.c6_rgnr;

+u32p += env->pmsav7.rnr;
  tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */
  *u32p = value;
  }
@@ -2447,7 +2447,7 @@ static const ARMCPRegInfo pmsav7_cp_reginfo[] = {
.readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset 
},
  { .name = "RGNR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 2, .opc2 = 0,


"RGNR" -> "RNR"


.access = PL1_RW,
-  .fieldoffset = offsetof(CPUARMState, cp15.c6_rgnr),
+  .fieldoffset = offsetof(CPUARMState, pmsav7.rnr),
.writefn = pmsav7_rgnr_write },


pmsav7_rnr_write


  REGINFO_SENTINEL
  };
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 1a40469..93c1a78 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -151,7 +151,7 @@ static bool pmsav7_rgnr_vmstate_validate(void *opaque, int 
version_id)


pmsav7_rnr_vmstate_validate()


  {
  ARMCPU *cpu = opaque;
  
-return cpu->env.cp15.c6_rgnr < cpu->pmsav7_dregion;

+return cpu->env.pmsav7.rnr < cpu->pmsav7_dregion;
  }

Re: [Qemu-devel] [RFC] block-insert-node and block-job-delete

[John Snow]

On 07/26/2017 02:23 PM, Manos Pitsidianakis wrote:

On Wed, Jul 26, 2017 at 04:12:21PM +0100, Stefan Hajnoczi wrote:

On Wed, Jul 26, 2017 at 05:19:24PM +0300, Manos Pitsidianakis wrote:
This proposal follows a discussion we had with Kevin and Stefan on 
filter

node management.

With block filter drivers arises a need to configure filter nodes on 
runtime
with QMP on live graphs. A problem with doing live graph 
modifications is
that some block jobs modify the graph when they are done and don't 
operate
under any synchronisation, resulting in a race condition if we try to 
insert

a filter node in the place of an existing edge.


But maybe you are thinking about higher level race conditions between
QMP commands.  Can you give an example of the race?


Exactly, synchronisation between the QMP/user actions. Here's an 
example, correct me if I'm wrong:


User issues a block-commit to this tree to commit A onto B:
BB -> A -> B

This inserts the dummy mirror node at the top since this is a mirror job 
(active commit)

BB -> dummy -> A -> B

User wants to add a filter node F below the BB. First we create the node:
BB -> dummy -> A -> B
   F /

Commit finishes before we do block-insert-node, dummy and A is removed 
from the BB's chain, mirror_exit calls bdrv_replace_node()


BB > B
F -> /

Now inserting F under BB will error since dummy does not exist any more.



Exactly how much of a problem is this? You, the user, have instructed 
QEMU to do two conflicting things:


(1) Squash A down into B, and
(2) Insert a filter above A

Shame on you for deleting the node you were trying to reference, no? And 
as an added bonus, QEMU will even error out on your command instead of 
trying to do something and getting it wrong.


Is this a problem?


The proposal doesn't guard against the following:
User issues a block-commit to this tree to commit B onto C:
BB -> A -> B -> C

The dummy node (commit's top bs is A):
BB -> A -> dummy -> B -> C

blockdev-add of a filter we wish to eventually be between A and C:
BB -> A -> dummy -> B -> C
 F/

if commit finishes before we issue block-insert-node (commit_complete()
calls bdrv_set_backing_hd() which only touches A)
BB -> A --> C
   F -> dummy -> B /
resulting in error when issuing block-insert-node,


Because "B" and "dummy" have already actually been deleted, I assume. 
(The diagram is confusing.)




else if we set the job to manual-delete and insert F:
BB -> A -> F -> dummy -> B -> C
deleting the job will result in F being lost since the job's top bs 
wasn't updated.




Seems like a fairly good reason not to pursue this avenue then, yes?

I think I agree with Stefan's concerns that this is more of a temporary 
workaround for the benefit of jobs, but that there may well be other 
reasons (especially in the future) where graph manipulations may occur 
invisibly to the user.


Do you have any use cases for failure here that don't involve the user 
themselves asking for two conflicting things? That is, QEMU doing some 
graph reorganization without the user's knowledge coupled with the user 
asking for a new filter node?


Are there any failures that result in anything more dangerous or bad 
than a "404: node not found" style error where we simply reject the 
premise of what the user was attempting to accomplish?

Re: [Qemu-devel] [PATCH for 2.10 04/35] ivshmem: fix incorrect error handling in ivshmem_recv_msg()

[Paolo Bonzini]

On 25/07/2017 10:18, Markus Armbruster wrote:
> Philippe Mathieu-Daudé  writes:
> 
>> If qemu_chr_fe_read_all() returns -EINTR the do {} statement continues and 
>> the
>> n accumulator used to complete reads upto sizeof(msg) is decremented by 4 
>> (the
>> value of EINTR on Linux).
>> To avoid that, use simpler if() statements and continue if EINTR occured.
>>
>> hw/misc/ivshmem.c:650:14: warning: Loss of sign in implicit conversion
>> } while (n < sizeof(msg));
>>  ^
>>
> 
> Let's add "Screwed up in commit 3a55fc0f, v2.6.0."
> 
>> Reported-by: Clang Static Analyzer
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>> get_maintainer.pl: No maintainers found!
>>
>>  hw/misc/ivshmem.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
>> index a58f9ee579..47a015f072 100644
>> --- a/hw/misc/ivshmem.c
>> +++ b/hw/misc/ivshmem.c
>> @@ -642,7 +642,10 @@ static int64_t ivshmem_recv_msg(IVShmemState *s, int 
>> *pfd, Error **errp)
>>  do {
>>  ret = qemu_chr_fe_read_all(>server_chr, (uint8_t *) + n,
>> sizeof(msg) - n);
>> -if (ret < 0 && ret != -EINTR) {
>> +if (ret < 0) {
>> +if (ret == -EINTR) {
>> +continue;
>> +}
>>  error_setg_errno(errp, -ret, "read from server failed");
>>  return INT64_MIN;
>>  }
> 
> Reviewed-by: Markus Armbruster 
> 
> Paolo, you taking this through your miscellaneous queue would save me
> (and possibly Peter) a bit of work.  Only if you have something queued
> already.  Let me know.

Fair enough, I'll pick this up.

Paolo

Re: [Qemu-devel] [PATCH for 2.10 3/4] fdt: probe for v1.4.2 using fdt_setprop_inplace_namelen_partial()

[Michael Roth]

Quoting Philippe Mathieu-Daudé (2017-07-26 16:40:09)
> instead of fdt_first_subnode() which is v1.4.0
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Signed-off-by: Michael Roth 

> ---
>  configure | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 0d5bdb3ae9..2d803d6a77 100755
> --- a/configure
> +++ b/configure
> @@ -3565,7 +3565,10 @@ if test "$fdt" != "no" ; then
>cat > $TMPC << EOF
>  #include 
>  #include 
> -int main(void) { fdt_first_subnode(0, 0); return 0; }
> +int main(void) {
> +fdt_setprop_inplace_namelen_partial(0, 0, 0, 0, 0, 0, 0);
> +return 0;
> +}
>  EOF
>if compile_prog "" "$fdt_libs" ; then
>  # system DTC is good - use it
> -- 
> 2.13.3
> 
> 

Re: [Qemu-devel] [PATCH for 2.10 2/4] fdt: check fdt_required condition can be satisfied _after_ testing libfdt

[Michael Roth]

Quoting Philippe Mathieu-Daudé (2017-07-26 16:40:08)
> Signed-off-by: Philippe Mathieu-Daudé 

Previously we failed if fdt_required and the user had built with
--disable-fdt.  For anything other than --disable-fdt, we'd force
the compile check afterward, and then fail if it's not available.

With this patch, we now probe libfdt if --disable-fdt isn't
specified, regardless of whether or not it's required by the
build. It still seems to behave correctly, but I'm not sure
what bug this re-arrangement fixes. Was there some other case
that was missed prior to this patch?

Also, with patch 4 applied on top of this, if they happen to have
a dtc submodule checked out that doesn't build, they will get a
configure error even if they are building a target where where
fdt_required="no".

> ---
>  configure | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/configure b/configure
> index 063de32773..0d5bdb3ae9 100755
> --- a/configure
> +++ b/configure
> @@ -3558,15 +3558,6 @@ for target in $target_list; do
>esac
>  done
> 
> -if test "$fdt_required" = "yes"; then
> -  if test "$fdt" = "no"; then
> -error_exit "fdt disabled but some requested targets require it." \
> -  "You can turn off fdt only if you also disable all the system 
> emulation" \
> -  "targets which need it (by specifying a cut down --target-list)."
> -  fi
> -  fdt=yes
> -fi
> -
>  if test "$fdt" != "no" ; then
>fdt_libs="-lfdt"
># explicitly check for libfdt_env.h as it is missing in some stable 
> installs
> @@ -3603,6 +3594,15 @@ EOF
>fi
>  fi
> 
> +if test "$fdt_required" = "yes"; then
> +  if test "$fdt" = "no"; then
> +error_exit "fdt disabled but some requested targets require it." \
> +  "You can turn off fdt only if you also disable all the system 
> emulation" \
> +  "targets which need it (by specifying a cut down --target-list)."
> +  fi
> +  fdt=yes
> +fi
> +
>  libs_softmmu="$libs_softmmu $fdt_libs"
> 
>  ##
> -- 
> 2.13.3
> 
> 

Re: [Qemu-devel] [Qemu-block] [PATCH v3 1/3] block: add bdrv_get_format_alloc_stat format interface

[John Snow]

On 07/12/2017 11:18 AM, Vladimir Sementsov-Ogievskiy wrote:

30.06.2017 03:27, John Snow wrote:


On 06/06/2017 12:26 PM, Vladimir Sementsov-Ogievskiy wrote:

The function should collect statistics, about used/unused by top-level
format driver space (in its .file) and allocation status
(data/zero/discarded/after-eof) of corresponding areas in this .file.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block.c   | 16 ++
  include/block/block.h |  3 +++
  include/block/block_int.h |  2 ++
  qapi/block-core.json  | 55 
+++

  4 files changed, 76 insertions(+)

diff --git a/block.c b/block.c
index 50ba264143..7d720ae0c2 100644
--- a/block.c
+++ b/block.c
@@ -3407,6 +3407,22 @@ int64_t 
bdrv_get_allocated_file_size(BlockDriverState *bs)

  }
  /**
+ * Collect format allocation info. See BlockFormatAllocInfo 
definition in

+ * qapi/block-core.json.
+ */
+int bdrv_get_format_alloc_stat(BlockDriverState *bs, 
BlockFormatAllocInfo *bfai)

+{
+BlockDriver *drv = bs->drv;
+if (!drv) {
+return -ENOMEDIUM;
+}
+if (drv->bdrv_get_format_alloc_stat) {
+return drv->bdrv_get_format_alloc_stat(bs, bfai);
+}
+return -ENOTSUP;
+}
+
+/**
   * Return number of sectors on success, -errno on error.
   */
  int64_t bdrv_nb_sectors(BlockDriverState *bs)
diff --git a/include/block/block.h b/include/block/block.h
index 9b355e92d8..646376a772 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -335,6 +335,9 @@ typedef enum {
  int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, 
BdrvCheckMode fix);

+int bdrv_get_format_alloc_stat(BlockDriverState *bs,
+   BlockFormatAllocInfo *bfai);
+
  /* The units of offset and total_work_size may be chosen 
arbitrarily by the
   * block driver; total_work_size may change during the course of 
the amendment

   * operation */
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8d3724cce6..458c715e99 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -208,6 +208,8 @@ struct BlockDriver {
  int64_t (*bdrv_getlength)(BlockDriverState *bs);
  bool has_variable_length;
  int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
+int (*bdrv_get_format_alloc_stat)(BlockDriverState *bs,
+  BlockFormatAllocInfo *bfai);
  int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState 
*bs,

  uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ea0b3e8b13..fd7b52bd69 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -139,6 +139,61 @@
 '*format-specific': 'ImageInfoSpecific' } }
  ##
+# @BlockFormatAllocInfo:
+#
+#
+# Allocation relations between format file and underlying protocol 
file.

+# All fields are in bytes.
+#

I guess this is a relation in the sense that the format differentiates
between used-unused and the protocol differentiates between
data-zero-trim which gives us the 2D matrix, showing a relation between
"two" files.

I find the use of the word "file" here a bit of a misdirection, though,
as qcow2 (or any other format) in this sense is not a file but rather a
schema used for interpreting data, and the file itself belongs solely to
the protocol.

I might suggest phrasing this as ...

"Allocation information of an underlying protocol file, as partitioned
by a format driver's utilization of said allocations."

Maybe that's too wordy. Eric?

+# There are two types of the format file portions: 'used' and 
'unused'. It's up
+# to the format how to interpret these types. For now the only 
format supporting

"format file" seems misleading for similar reasons to my objection
above, which is that the format doesn't have a file, exactly. It's an
abstract schema.

"It's up to the format how to interpret these types" is also a bit too
vague to help inform readers what the types mean, IMO.

Here's an attempt:

"Allocations may be considered either used or unused by the format
driver interpreting those allocations. It is at the discretion of the
format driver (e.g. qcow2) which regions of its backing storage are
considered in-use or not."


So we are saying about "allocations". But unallocated data may be 
used/unused too, so,

can we call unallocated areas "allocations"?



You're right, "allocation" may imply something that is extant, but these 
are more like regions ...


Uh;

"Regions" may be considered either used or unused?

or

"Regions of the underlying protocol file may be considered used or 
unused by the format driver interpreting these regions."


Something along these lines, perhaps?

Re: [Qemu-devel] [PATCH] block: Make bdrv_img_create() size selection easier to read

[John Snow]

On 07/18/2017 12:00 PM, Eric Blake wrote:

All callers of bdrv_img_create() pass in a size, or -1 to read the
size from the backing file.  We then set that size as the QemuOpt
default, which means we will reuse that default rather than the
final parameter to qemu_opt_get_size() several lines later.  But
it is rather confusing to read subsequent checks of 'size == -1'
when it looks (without seeing the full context) like size defaults
to 0; it also doesn't help that a size of 0 is valid (for some
formats).

Rework the logic to make things more legible.

Signed-off-by: Eric Blake 


Reviewed-by: John Snow 


---
  block.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 2dd9262cd0..57195ec40f 100644
--- a/block.c
+++ b/block.c
@@ -4398,7 +4398,7 @@ void bdrv_img_create(const char *filename, const char 
*fmt,

  /* The size for the image must always be specified, unless we have a 
backing
   * file and we have not been forbidden from opening it. */
-size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
+size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, img_size);
  if (backing_file && !(flags & BDRV_O_NO_BACKING)) {
  BlockDriverState *bs;
  char *full_backing = g_new0(char, PATH_MAX);

Re: [Qemu-devel] [PATCH] build-sys: there is no qemu-ga.c

[Michael Roth]

Quoting Marc-André Lureau (2017-07-27 10:45:05)
> It got moved in qga/main.c from commit 2870dc3456c9c.
> 
> Signed-off-by: Marc-André Lureau 

Reviewed-by: Michael Roth 

> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index ef721480eb..97a58a0f4e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -443,7 +443,7 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-introspect.py 
> $(qapi-py)
> "GEN","$@")
>  
>  QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h 
> qga-qapi-visit.h qga-qmp-commands.h)
> -$(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
> +$(qga-obj-y): $(QGALIB_GEN)
>  
>  qemu-ga$(EXESUF): $(qga-obj-y) $(COMMON_LDADDS)
> $(call LINK, $^)
> -- 
> 2.14.0.rc0.1.g40ca67566
> 
> 

Re: [Qemu-devel] [for-2.10 PATCH] spapr_drc: fix realize and unrealize

[Michael Roth]

Quoting Greg Kurz (2017-07-27 08:45:47)
> If object_property_add_alias() returns an error in realize(), we should
> propagate it to the caller and certainly not unref the DRC.

Indeed. I think that was the result of this part of the code
originally living in spapr_dr_connector_new() during development,
where it had previously made sense to free the object if there was a
failure and then return NULL. We definitely shouldn't be during it
now...

> 
> Same thing goes for unrealize(). Since object_property_del() is the last
> call, we can even get rid of the intermediate Error *.
> 
> And finally, unrealize() should undo all registrations performed by
> realize().
> 
> Signed-off-by: Greg Kurz 
> ---
> 
> David,
> 
> As agreed on the list, here's the version of the fix for 2.10. I'll respin
> my PHB hotplug series on top of it.
> 
> Cheers,
> 
> --
> Greg
> 
>  hw/ppc/spapr_drc.c |   15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 15bae5c216a9..47d94e782ac2 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -506,11 +506,11 @@ static void realize(DeviceState *d, Error **errp)
>  trace_spapr_drc_realize_child(spapr_drc_index(drc), child_name);
>  object_property_add_alias(root_container, link_name,
>drc->owner, child_name, );
> +g_free(child_name);
>  if (err) {
> -error_report_err(err);
> -object_unref(OBJECT(drc));
> +error_propagate(errp, err);
> +return;


Since spapr_dr_connector_new() calls realize() with errp = NULL, we
basically lose the error now. I think maybe we should at least report it
still, but even better would be to have _new() call object_property_* with
error_abort, since callers of spapr_dr_connector_new() don't all check
for the return value and even if they did the appropriate action would
always be to report+abort() anyway.

Looks good otherwise.

>  }
> -g_free(child_name);
>  vmstate_register(DEVICE(drc), spapr_drc_index(drc), _spapr_drc,
>   drc);
>  qemu_register_reset(drc_reset, drc);
> @@ -522,16 +522,13 @@ static void unrealize(DeviceState *d, Error **errp)
>  sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d);
>  Object *root_container;
>  char name[256];
> -Error *err = NULL;
> 
>  trace_spapr_drc_unrealize(spapr_drc_index(drc));
> +qemu_unregister_reset(drc_reset, drc);
> +vmstate_unregister(DEVICE(drc), _spapr_drc, drc);
>  root_container = container_get(object_get_root(), DRC_CONTAINER_PATH);
>  snprintf(name, sizeof(name), "%x", spapr_drc_index(drc));
> -object_property_del(root_container, name, );
> -if (err) {
> -error_report_err(err);
> -object_unref(OBJECT(drc));
> -}
> +object_property_del(root_container, name, errp);
>  }
> 
>  sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
> 
> 

Re: [Qemu-devel] [qemu PATCH for 2.10] i386: acpi: provide an XSDT instead of an RSDT

[Kevin O'Connor]

On Wed, Jul 26, 2017 at 11:31:36AM +0200, Paolo Bonzini wrote:
> The tables that QEMU provides are not ACPI 1.0 compatible since commit
> 77af8a2b95 ("hw/i386: Use Rev3 FADT (ACPI 2.0) instead of Rev1 to improve
> guest OS support.", 2017-05-03).  This is visible with Windows 2000,
> which refuses to parse the rev3 FADT and fails to boot.
> 
> The recommended solution in this case is to build two FADTs, v1 being
> pointed to by the RSDT and v3 by the XSDT.  However, we leave this task
> to the firmware.  This patch simply switches the RSDT to the XSDT, which
> is valid for all ACPI 2.0-friendly operating systems and also leaves
> SeaBIOS the freedom to build an RSDT that points to the compatibility
> FADT.

Another possible solution to this issue would be for QEMU to instruct
the firmware to build both rev1 and rev3 FADTs, but be clear which
links are for legacy purposes only.  This could be done with a new
ADD_LEGACY_POINTER linker loader command.  Existing firmwares should
ignore the new ADD_LEGACY_POINTER command and new versions of SeaBIOS
could be extended to honor it.

I proto-typed it (but haven't done significant testing).  Admittedly,
it is a pretty ugly hack.

-Kevin


== SeaBIOS patch ===

--- a/src/fw/romfile_loader.c
+++ b/src/fw/romfile_loader.c
@@ -234,6 +234,7 @@ int romfile_loader_execute(const char *name)
 case ROMFILE_LOADER_COMMAND_ALLOCATE:
 romfile_loader_allocate(entry, files);
 break;
+case ROMFILE_LOADER_COMMAND_ADD_LEGACY_POINTER:
 case ROMFILE_LOADER_COMMAND_ADD_POINTER:
 romfile_loader_add_pointer(entry, files);
 break;
diff --git a/src/fw/romfile_loader.h b/src/fw/romfile_loader.h
index fcd4ab2..4e266e8 100644
--- a/src/fw/romfile_loader.h
+++ b/src/fw/romfile_loader.h
@@ -77,6 +77,7 @@ enum {
 ROMFILE_LOADER_COMMAND_ADD_POINTER= 0x2,
 ROMFILE_LOADER_COMMAND_ADD_CHECKSUM   = 0x3,
 ROMFILE_LOADER_COMMAND_WRITE_POINTER  = 0x4,
+ROMFILE_LOADER_COMMAND_ADD_LEGACY_POINTER = 0x5,
 };
 
 enum {


== QEMU patch ===

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 36a6cc4..eed1a2c 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1576,7 +1576,7 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, 
bool mfre)
 /* Build rsdt table */
 void
 build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
-   const char *oem_id, const char *oem_table_id)
+   const char *oem_id, const char *oem_table_id, unsigned fadt)
 {
 int i;
 unsigned rsdt_entries_offset;
@@ -1587,12 +1587,15 @@ build_rsdt(GArray *table_data, BIOSLinker *linker, 
GArray *table_offsets,
 
 rsdt = acpi_data_push(table_data, rsdt_len);
 rsdt_entries_offset = (char *)rsdt->table_offset_entry - table_data->data;
-for (i = 0; i < table_offsets->len; ++i) {
+bios_linker_loader_add_legacy_pointer(linker,
+ACPI_BUILD_TABLE_FILE, rsdt_entries_offset, rsdt_entry_size,
+ACPI_BUILD_TABLE_FILE, fadt);
+for (i = 1; i < table_offsets->len; ++i) {
 uint32_t ref_tbl_offset = g_array_index(table_offsets, uint32_t, i);
 uint32_t rsdt_entry_offset = rsdt_entries_offset + rsdt_entry_size * i;
 
 /* rsdt->table_offset_entry to be filled by Guest linker */
-bios_linker_loader_add_pointer(linker,
+bios_linker_loader_add_legacy_pointer(linker,
 ACPI_BUILD_TABLE_FILE, rsdt_entry_offset, rsdt_entry_size,
 ACPI_BUILD_TABLE_FILE, ref_tbl_offset);
 }
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index 046183a..3be6ac4 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -100,10 +100,11 @@ struct BiosLinkerLoaderEntry {
 typedef struct BiosLinkerLoaderEntry BiosLinkerLoaderEntry;
 
 enum {
-BIOS_LINKER_LOADER_COMMAND_ALLOCATE  = 0x1,
-BIOS_LINKER_LOADER_COMMAND_ADD_POINTER   = 0x2,
-BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM  = 0x3,
-BIOS_LINKER_LOADER_COMMAND_WRITE_POINTER = 0x4,
+BIOS_LINKER_LOADER_COMMAND_ALLOCATE   = 0x1,
+BIOS_LINKER_LOADER_COMMAND_ADD_POINTER= 0x2,
+BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM   = 0x3,
+BIOS_LINKER_LOADER_COMMAND_WRITE_POINTER  = 0x4,
+BIOS_LINKER_LOADER_COMMAND_ADD_LEGACY_POINTER = 0x5,
 };
 
 enum {
@@ -243,28 +244,13 @@ void bios_linker_loader_add_checksum(BIOSLinker *linker, 
const char *file_name,
 g_array_append_vals(linker->cmd_blob, , sizeof entry);
 }
 
-/*
- * bios_linker_loader_add_pointer: ask guest to patch address in
- * destination file with a pointer to source file
- *
- * @linker: linker object instance
- * @dest_file: destination file that must be changed
- * @dst_patched_offset: location within destination file blob to be patched
- *  with the pointer to @src_file+@src_offset (i.e. source
- *   

[Qemu-devel] [PATCH] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Using C structs makes the code more readable and prevents type conversion
errors.

Borrow multiboot1 header from GRUB project.

Signed-off-by: Anatol Pomozov 
---
 hw/i386/multiboot.c| 124 +-
 hw/i386/multiboot_header.h | 254 +
 2 files changed, 304 insertions(+), 74 deletions(-)
 create mode 100644 hw/i386/multiboot_header.h

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index f13e23139b..19113c0fce 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -28,6 +28,7 @@
 #include "hw/hw.h"
 #include "hw/nvram/fw_cfg.h"
 #include "multiboot.h"
+#include "multiboot_header.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "sysemu/sysemu.h"
@@ -47,39 +48,9 @@
 #error multiboot struct needs to fit in 16 bit real mode
 #endif
 
-enum {
-/* Multiboot info */
-MBI_FLAGS   = 0,
-MBI_MEM_LOWER   = 4,
-MBI_MEM_UPPER   = 8,
-MBI_BOOT_DEVICE = 12,
-MBI_CMDLINE = 16,
-MBI_MODS_COUNT  = 20,
-MBI_MODS_ADDR   = 24,
-MBI_MMAP_ADDR   = 48,
-MBI_BOOTLOADER  = 64,
-
-MBI_SIZE= 88,
-
-/* Multiboot modules */
-MB_MOD_START= 0,
-MB_MOD_END  = 4,
-MB_MOD_CMDLINE  = 8,
-
-MB_MOD_SIZE = 16,
-
-/* Region offsets */
-ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
-ADDR_MBI  = ADDR_E820_MAP + 0x500,
-
-/* Multiboot flags */
-MULTIBOOT_FLAGS_MEMORY  = 1 << 0,
-MULTIBOOT_FLAGS_BOOT_DEVICE = 1 << 1,
-MULTIBOOT_FLAGS_CMDLINE = 1 << 2,
-MULTIBOOT_FLAGS_MODULES = 1 << 3,
-MULTIBOOT_FLAGS_MMAP= 1 << 6,
-MULTIBOOT_FLAGS_BOOTLOADER  = 1 << 9,
-};
+/* Region offsets */
+#define ADDR_E820_MAP MULTIBOOT_STRUCT_ADDR
+#define ADDR_MBI  (ADDR_E820_MAP + 0x500)
 
 typedef struct {
 /* buffer holding kernel, cmdlines and mb_infos */
@@ -128,14 +99,15 @@ static void mb_add_mod(MultibootState *s,
hwaddr start, hwaddr end,
hwaddr cmdline_phys)
 {
-char *p;
+multiboot_module_t *mod;
 assert(s->mb_mods_count < s->mb_mods_avail);
 
-p = (char *)s->mb_buf + s->offset_mbinfo + MB_MOD_SIZE * s->mb_mods_count;
+mod = s->mb_buf + s->offset_mbinfo +
+  sizeof(multiboot_module_t) * s->mb_mods_count;
 
-stl_p(p + MB_MOD_START,   start);
-stl_p(p + MB_MOD_END, end);
-stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+stl_p(>mod_start, start);
+stl_p(>mod_end,   end);
+stl_p(>cmdline,   cmdline_phys);
 
 mb_debug("mod%02d: "TARGET_FMT_plx" - "TARGET_FMT_plx"\n",
  s->mb_mods_count, start, end);
@@ -151,26 +123,29 @@ int load_multiboot(FWCfgState *fw_cfg,
int kernel_file_size,
uint8_t *header)
 {
-int i, is_multiboot = 0;
+int i;
+bool is_multiboot = false;
 uint32_t flags = 0;
 uint32_t mh_entry_addr;
 uint32_t mh_load_addr;
 uint32_t mb_kernel_size;
 MultibootState mbs;
-uint8_t bootinfo[MBI_SIZE];
+multiboot_info_t bootinfo;
 uint8_t *mb_bootinfo_data;
 uint32_t cmdline_len;
+struct multiboot_header *multiboot_header;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
 for (i = 0; i < (8192 - 48); i += 4) {
-if (ldl_p(header+i) == 0x1BADB002) {
-uint32_t checksum = ldl_p(header+i+8);
-flags = ldl_p(header+i+4);
+multiboot_header = (struct multiboot_header *)(header + i);
+if (ldl_p(_header->magic) == MULTIBOOT_HEADER_MAGIC) {
+uint32_t checksum = ldl_p(_header->checksum);
+flags = ldl_p(_header->flags);
 checksum += flags;
-checksum += (uint32_t)0x1BADB002;
+checksum += (uint32_t)MULTIBOOT_HEADER_MAGIC;
 if (!checksum) {
-is_multiboot = 1;
+is_multiboot = true;
 break;
 }
 }
@@ -180,13 +155,13 @@ int load_multiboot(FWCfgState *fw_cfg,
 return 0; /* no multiboot */
 
 mb_debug("qemu: I believe we found a multiboot image!\n");
-memset(bootinfo, 0, sizeof(bootinfo));
+memset(, 0, sizeof(bootinfo));
 memset(, 0, sizeof(mbs));
 
-if (flags & 0x0004) { /* MULTIBOOT_HEADER_HAS_VBE */
+if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & 0x0001)) { /* MULTIBOOT_HEADER_HAS_ADDR */
+if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
 uint64_t elf_entry;
 uint64_t elf_low, elf_high;
 int kernel_size;
@@ -217,14 +192,14 @@ int load_multiboot(FWCfgState *fw_cfg,
 mb_debug("qemu: loading multiboot-elf kernel (%#x bytes) with entry 
%#zx\n",
   mb_kernel_size, (size_t)mh_entry_addr);
 } else {
-/* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_ADDR. */
-uint32_t 

Re: [Qemu-devel] [RFC v5 1/8] hw/arm/smmu-common: smmu base class

[Auger Eric]

Hi Tomasz,

On 25/07/2017 14:12, Tomasz Nowicki wrote:
> Hi Eric,
> 
> I found out what is going on regarding vhost-net outgoing packet's
> payload corruption. My packets were corrupted because of inconsistent
> IOVA to HVA translation in IOTLB. Please see below.
> 
> On 09.07.2017 22:51, Eric Auger wrote:
>> Introduces the base device and class for the ARM smmu.
>> Implements VMSAv8-64 table lookup and translation. VMSAv8-32
>> is not implemented.
>>
>> Signed-off-by: Eric Auger 
>> Signed-off-by: Prem Mallappa 
>>
>> ---
> 
> [...]
> 
>> +
>> +/**
>> + * smmu_page_walk_level_64 - Walk an IOVA range from a specific level
>> + * @baseaddr: table base address corresponding to @level
>> + * @level: level
>> + * @cfg: translation config
>> + * @start: end of the IOVA range
>> + * @end: end of the IOVA range
>> + * @hook_fn: the hook that to be called for each detected area
>> + * @private: private data for the hook function
>> + * @read: whether parent level has read permission
>> + * @write: whether parent level has write permission
>> + * @nofail: indicates whether each iova of the range
>> + *  must be translated or whether failure is allowed
>> + * @notify_unmap: whether we should notify invalid entries
>> + *
>> + * Return 0 on success, < 0 on errors not related to translation
>> + * process, > 1 on errors related to translation process (only
>> + * if nofail is set)
>> + */
>> +static int
>> +smmu_page_walk_level_64(dma_addr_t baseaddr, int level,
>> +SMMUTransCfg *cfg, uint64_t start, uint64_t end,
>> +smmu_page_walk_hook hook_fn, void *private,
>> +bool read, bool write, bool nofail,
>> +bool notify_unmap)
>> +{
>> +uint64_t subpage_size, subpage_mask, pte, iova = start;
>> +bool read_cur, write_cur, entry_valid;
>> +int ret, granule_sz, stage;
>> +IOMMUTLBEntry entry;
>> +
>> +granule_sz = cfg->granule_sz;
>> +stage = cfg->stage;
>> +subpage_size = 1ULL << level_shift(level, granule_sz);
>> +subpage_mask = level_page_mask(level, granule_sz);
>> +
>> +trace_smmu_page_walk_level_in(level, baseaddr, granule_sz,
>> +  start, end, subpage_size);
>> +
>> +while (iova < end) {
>> +dma_addr_t next_table_baseaddr;
>> +uint64_t iova_next, pte_addr;
>> +uint32_t offset;
>> +
>> +iova_next = (iova & subpage_mask) + subpage_size;
>> +offset = iova_level_offset(iova, level, granule_sz);
>> +pte_addr = baseaddr + offset * sizeof(pte);
>> +pte = get_pte(baseaddr, offset);
>> +
>> +trace_smmu_page_walk_level(level, iova, subpage_size,
>> +   baseaddr, offset, pte);
>> +
>> +if (pte == (uint64_t)-1) {
>> +if (nofail) {
>> +return SMMU_TRANS_ERR_WALK_EXT_ABRT;
>> +}
>> +goto next;
>> +}
>> +if (is_invalid_pte(pte) || is_reserved_pte(pte, level)) {
>> +trace_smmu_page_walk_level_res_invalid_pte(stage, level,
>> baseaddr,
>> +   pte_addr,
>> offset, pte);
>> +if (nofail) {
>> +return SMMU_TRANS_ERR_WALK_EXT_ABRT;
>> +}
>> +goto next;
>> +}
> 
> 
> vhost maintains its IOTLB cache and when there is no IOVA->HVA
> translation, it asks QEMU for help. However, IOTLB entries invalidations
> are guest initiative so for any DMA unmap at guest side we trap to SMMU
> emulation code and call:
> smmu_notify_all -> smmuv3_replay_single -> smmu_page_walk_64 ->
> smmu_page_walk_level_64 -> smmuv3_replay_hook -> vhost_iommu_unmap_notify
> 
> The thing is that smmuv3_replay_hook() is never called because guest
> zeros PTE before we trap to QEMU so that smmu_page_walk_level_64() fails
> on ^^^ is_invalid_pte(pte) check. This way we keep old IOTLB entry in
> vhost and subsequent translations may be broken.

Thank you for the time you spent on this. I will work on this vhost use
case asap and will let you know.

Thanks

Eric
> 
>> +
>> +read_cur = read; /* TODO */
>> +write_cur = write; /* TODO */
>> +entry_valid = read_cur | write_cur; /* TODO */
>> +
>> +if (is_page_pte(pte, level)) {
>> +uint64_t gpa = get_page_pte_address(pte, granule_sz);
>> +int perm = IOMMU_ACCESS_FLAG(read_cur, write_cur);
>> +
>> +trace_smmu_page_walk_level_page_pte(stage, level,
>> entry.iova,
>> +baseaddr, pte_addr,
>> pte, gpa);
>> +if (!entry_valid && !notify_unmap) {
>> +printf("%s entry_valid=%d notify_unmap=%d\n", __func__,
>> +   entry_valid, notify_unmap);
>> +goto next;
>> +}
>> +ret = call_entry_hook(iova, subpage_mask, gpa, perm,
>> +  

Re: [Qemu-devel] [RFC v5 2/8] hw/arm/smmuv3: smmuv3 emulation model

[Auger Eric]

Hi Tomasz,

On 13/07/2017 14:00, Tomasz Nowicki wrote:
> Hi Eric,
> 
> On 09.07.2017 22:51, Eric Auger wrote:
>> From: Prem Mallappa 
>>
>> Introduces the SMMUv3 derived model. This is based on
>> System MMUv3 specification (v17).
>>
>> Signed-off-by: Prem Mallappa 
>> Signed-off-by: Eric Auger 
>>
>> ---
>> v4 -> v5:
>> - change smmuv3_translate proto (IOMMUAccessFlags flag)
>> - has_stagex replaced by is_ste_stagex
>> - smmu_cfg_populate removed
>> - added smmuv3_decode_config and reworked error management
>> - remwork the naming of IOMMU mrs
>> - fix SMMU_CMDQ_CONS offset
>>
> 
> [...]
> 
>> +
>> +/*
>> + *  Register Access Primitives
>> + */
>> +
>> +static inline void smmu_write64_reg(SMMUV3State *s, uint32_t addr,
>> uint64_t val)
>> +{
>> +addr >>= 2;
>> +s->regs[addr] = val & 0xULL;
>> +s->regs[addr + 1] = val & ~0xULL;
>> +}
>> +
>> +static inline void smmu_write_reg(SMMUV3State *s, uint32_t addr,
>> uint64_t val)
>> +{
>> +s->regs[addr >> 2] = val;
>> +}
>> +
>> +static inline uint32_t smmu_read_reg(SMMUV3State *s, uint32_t addr)
>> +{
>> +return s->regs[addr >> 2];
>> +}
>> +
>> +static inline uint64_t smmu_read64_reg(SMMUV3State *s, uint32_t addr)
>> +{
>> +addr >>= 2;
>> +return s->regs[addr] | (s->regs[addr + 1] << 32);
> 
> To be consistent with smmu_write64_reg() we should not shift here second
> half of register, instead simply:
> 
> return s->regs[addr] | s->regs[addr + 1];

Thanks for spotting this. I think regs should be uint32_t instead and
extract64() could be used on write64 and shift would stay on read64().

Regards

Eric
> 
> Thanks,
> Tomasz

Re: [Qemu-devel] [RFC v5 2/8] hw/arm/smmuv3: smmuv3 emulation model

[Auger Eric]

Hi Tomasz,

On 13/07/2017 14:57, Tomasz Nowicki wrote:
> Hi Eric,
> 
> On 09.07.2017 22:51, Eric Auger wrote:
>> From: Prem Mallappa 
>>
>> Introduces the SMMUv3 derived model. This is based on
>> System MMUv3 specification (v17).
>>
>> Signed-off-by: Prem Mallappa 
>> Signed-off-by: Eric Auger 
>>
>> ---
>> v4 -> v5:
>> - change smmuv3_translate proto (IOMMUAccessFlags flag)
>> - has_stagex replaced by is_ste_stagex
>> - smmu_cfg_populate removed
>> - added smmuv3_decode_config and reworked error management
>> - remwork the naming of IOMMU mrs
>> - fix SMMU_CMDQ_CONS offset
>>
> 
> [...]
> 
>> +
>> +static void smmu_update_qreg(SMMUV3State *s, SMMUQueue *q, hwaddr reg,
>> + uint32_t off, uint64_t val, unsigned size)
>> +{
>> +   if (size == 8 && off == 0) {
>> +smmu_write64_reg(s, reg, val);
> 
> Based on my observation we never get here.
> 
> If I read the code correctly,
> memory_region_dispatch_{write|read}()->memory_region_{write|read}_accessor()
> will cut all 8-bytes accesses into 4-bytes slices. However, this makes
> my SMMUv3 register handling happy:
> 
>  static const MemoryRegionOps smmu_mem_ops = {
>  .read = smmu_read_mmio,
>  .write = smmu_write_mmio,
>  .endianness = DEVICE_LITTLE_ENDIAN,
>  .valid = {
>  .min_access_size = 4,
>  .max_access_size = 8,
>  },
> +.impl = {
> +.min_access_size = 4,
> +.max_access_size = 8,
> +},
Yes indeed: without the .impl setting, only 4byte accesses are performed
by access_with_adjusted_size() which sets access_size_max to 4.

Thanks a lot.

Best regards

Eric
>  };
> 
> Thanks,
> Tomasz

Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation

[Lluís Vilanova]

Daniel P Berrange writes:

> On Thu, Jul 27, 2017 at 11:54:29AM +0100, Peter Maydell wrote:
>> On 27 July 2017 at 11:43, Daniel P. Berrange  wrote:
>> > Maybe I'm missing something, but aren't all these things
>> > already possible via either the statically defined tracepoints
>> > QEMU exposes, or by placing dynamic tracepoints on arbitrary
>> > code locations using dtrace/systemtap/lttng-ust.
>> 
>> Last time I looked we didn't have tracepoints on most of
>> the events you'd be interested in.
>> 
>> That said, yes, I was going to ask if we could do this via
>> leveraging the tracepoint infrastructure and whatever scripting
>> facilities it provides. Are there any good worked examples of
>> this sort of thing? Can you do it as an ordinary non-root user?

> Do you have a particular thing you'd like to see an example of ?

> To dynamically probe a function which doesn't have a tracepoint
> defined you can do:

> probe process("/usr/bin/qemu-x86_64").function("helper_syscall") {
>   printf("syscall stasrt\n")
> }

> but getting access to the function args is not as easy as with
> pre-defined tracepoints.

> You can't typically run this as root, however, I don't think that's a
> huge issue, because most QEMU deployments are not running as your own
> user account anyway, so you can't directly interact with them no
> matter what.

> If the goal is to be easy to instrument without havig to rebuild
> QEMU, then I think using one of the existing trace backends is
> the best viable option, as those are already enabled by distros.
> I find it very unlikely that Fedora/RHEL would ever enable a
> trace backend that lets you load arbitrary code into the QEMU
> process, so you'd be back to having to rebuild QEMU again even
> with that approach.

My idea is to use the guest code tracing events to perform some complex analysis
of the executed guest code. E.g., tracing executed instructions to a file to
calculate SimPoints [1], analysing guest instructions and memory accesses to
track information flow or feeding that info to a processor simulator. In fact,
I've used my non-upstream version of QEMU+instrumentation to do the first two
and a few others.

My problem with dtrace is efficiency of the instrumentation code, and the lack
of APIs to perform a few more complex analysis like peek/poke on guest memory,
registers or even control event tracing states. I'm not proposing to have such
APIs on QEMU upstream (unless there's interest), but they are easy enough for me
to maintain for my particular needs if we're talking about the instrumentation
support in this series.

So Stefan, could you elaborate on your proposal of having to build the
instrumentation code into QEMU? I would prefer a dynamic library if we can craft
it in a way that ensures the proper license restrictions, but a "static library"
could work just as well for me (I initially had a branch with both solutions,
but it got too tedious to rebase).


Thanks,
  Lluis

Re: [Qemu-devel] [PATCH] tcg/README: fix a description error.

[John Snow]

On 07/26/2017 10:36 PM, Jiang Biao wrote:

The atomics.txt is not in the docs directory but in docs/devel/
instead.

Signed-off-by: Jiang Biao 
---
  tcg/README | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/README b/tcg/README
index bf49e82..03bfb6a 100644
--- a/tcg/README
+++ b/tcg/README
@@ -446,7 +446,7 @@ when MTTCG is enabled.
  The guest translators should generate this opcode for all guest instructions
  which have ordering side effects.
  
-Please see docs/atomics.txt for more information on memory barriers.

+Please see docs/devel/atomics.txt for more information on memory barriers.
  
  * 64-bit guest on 32-bit host support
  



Reviewed-by: John Snow 

CC qemu-trivial

Re: [Qemu-devel] [Qemu-trivial] [PATCH for 2.10 v2 18/20] 9pfs: avoid sign conversion error simplifying the code

[Philippe Mathieu-Daudé]

On 07/27/2017 08:40 AM, Greg Kurz wrote:

On Wed, 26 Jul 2017 23:42:22 -0300
Philippe Mathieu-Daudé  wrote:
Reviewed-by: Greg Kurz 

Now, I'm not sure this can be merged during hard freeze since it is
more code cleanup than actual bug fixing...


Hmm the commit message is probably not enough.
The problem is this code can send broken packets, see inlined:




  hw/9pfs/9p.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 333dbb6f8e..0a37c8bd13 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -945,7 +945,6 @@ static void coroutine_fn v9fs_version(void *opaque)
  v9fs_string_init();
  err = pdu_unmarshal(pdu, offset, "ds", >msize, );
  if (err < 0) {


if err == -1


-offset = err;


here this sets offset = (size_t)(-1) = SIZE_MAX


  goto out;
  }
  trace_v9fs_version(pdu->tag, pdu->id, s->msize, version.data);
@@ -962,13 +961,12 @@ static void coroutine_fn v9fs_version(void *opaque)
  
  err = pdu_marshal(pdu, offset, "ds", s->msize, );

  if (err < 0) {
-offset = err;
  goto out;
  }
-offset += err;


here offset += SIZE_MAX which wraps, so this equivs to offset -= 1


+err += offset;
  trace_v9fs_version_return(pdu->tag, pdu->id, s->msize, version.data);
  out:
-pdu_complete(pdu, offset);


now we have offset = 7 - 1 = 6, since 6 > 0 pdu_complete() does not 
marshal the error code but 6 bytes of crap from pdu?


Maybe I missed something.


+pdu_complete(pdu, err);
  v9fs_string_free();
  }
  


Regards,

Phil.

Re: [Qemu-devel] [PATCH] multiboot: Change multiboot_info from array of bytes to a C struct

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH] multiboot: Change multiboot_info from array of 
bytes to a C struct
Message-id: 20170727191013.53517-1-anatol.pomo...@gmail.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/20170727191013.53517-1-anatol.pomo...@gmail.com -> 
patchew/20170727191013.53517-1-anatol.pomo...@gmail.com
Switched to a new branch 'test'
202968a multiboot: Change multiboot_info from array of bytes to a C struct

=== OUTPUT BEGIN ===
Checking PATCH 1/1: multiboot: Change multiboot_info from array of bytes to a C 
struct...
WARNING: line over 80 characters
#77: FILE: hw/i386/multiboot.c:105:
+mod = s->mb_buf + s->offset_mbinfo + sizeof(multiboot_module_t) * 
s->mb_mods_count;

ERROR: line over 90 characters
#193: FILE: hw/i386/multiboot.c:260:
+mbs.offset_cmdlines   = mbs.offset_mbinfo + mbs.mb_mods_avail * 
sizeof(multiboot_module_t);

WARNING: line over 80 characters
#263: FILE: hw/i386/multiboot_header.h:17:
+ *  DEVELOPER OR DISTRIBUTOR BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER 
LIABILITY,

WARNING: line over 80 characters
#265: FILE: hw/i386/multiboot_header.h:19:
+ *  IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE 
SOFTWARE.

ERROR: code indent should never use tabs
#272: FILE: hw/i386/multiboot_header.h:26:
+#define MULTIBOOT_SEARCH^I^I^I8192$

ERROR: code indent should never use tabs
#273: FILE: hw/i386/multiboot_header.h:27:
+#define MULTIBOOT_HEADER_ALIGN^I^I^I4$

ERROR: code indent should never use tabs
#276: FILE: hw/i386/multiboot_header.h:30:
+#define MULTIBOOT_HEADER_MAGIC^I^I^I0x1BADB002$

ERROR: code indent should never use tabs
#279: FILE: hw/i386/multiboot_header.h:33:
+#define MULTIBOOT_BOOTLOADER_MAGIC^I^I0x2BADB002$

ERROR: code indent should never use tabs
#282: FILE: hw/i386/multiboot_header.h:36:
+#define MULTIBOOT_MOD_ALIGN^I^I^I0x1000$

ERROR: code indent should never use tabs
#285: FILE: hw/i386/multiboot_header.h:39:
+#define MULTIBOOT_INFO_ALIGN^I^I^I0x0004$

ERROR: code indent should never use tabs
#290: FILE: hw/i386/multiboot_header.h:44:
+#define MULTIBOOT_PAGE_ALIGN^I^I^I0x0001$

ERROR: code indent should never use tabs
#293: FILE: hw/i386/multiboot_header.h:47:
+#define MULTIBOOT_MEMORY_INFO^I^I^I0x0002$

ERROR: code indent should never use tabs
#296: FILE: hw/i386/multiboot_header.h:50:
+#define MULTIBOOT_VIDEO_MODE^I^I^I0x0004$

ERROR: code indent should never use tabs
#299: FILE: hw/i386/multiboot_header.h:53:
+#define MULTIBOOT_AOUT_KLUDGE^I^I^I0x0001$

ERROR: code indent should never use tabs
#304: FILE: hw/i386/multiboot_header.h:58:
+#define MULTIBOOT_INFO_MEMORY^I^I^I0x0001$

ERROR: code indent should never use tabs
#306: FILE: hw/i386/multiboot_header.h:60:
+#define MULTIBOOT_INFO_BOOTDEV^I^I^I0x0002$

ERROR: code indent should never use tabs
#308: FILE: hw/i386/multiboot_header.h:62:
+#define MULTIBOOT_INFO_CMDLINE^I^I^I0x0004$

ERROR: code indent should never use tabs
#310: FILE: hw/i386/multiboot_header.h:64:
+#define MULTIBOOT_INFO_MODS^I^I^I0x0008$

ERROR: code indent should never use tabs
#315: FILE: hw/i386/multiboot_header.h:69:
+#define MULTIBOOT_INFO_AOUT_SYMS^I^I0x0010$

ERROR: code indent should never use tabs
#317: FILE: hw/i386/multiboot_header.h:71:
+#define MULTIBOOT_INFO_ELF_SHDR^I^I^I0X0020$

ERROR: code indent should never use tabs
#320: FILE: hw/i386/multiboot_header.h:74:
+#define MULTIBOOT_INFO_MEM_MAP^I^I^I0x0040$

ERROR: code indent should never use tabs
#323: FILE: hw/i386/multiboot_header.h:77:
+#define MULTIBOOT_INFO_DRIVE_INFO^I^I0x0080$

ERROR: code indent should never use tabs
#326: FILE: hw/i386/multiboot_header.h:80:
+#define MULTIBOOT_INFO_CONFIG_TABLE^I^I0x0100$

ERROR: code indent should never use tabs
#329: FILE: hw/i386/multiboot_header.h:83:
+#define MULTIBOOT_INFO_BOOT_LOADER_NAME^I^I0x0200$

ERROR: code indent should never use tabs
#332: FILE: hw/i386/multiboot_header.h:86:
+#define MULTIBOOT_INFO_APM_TABLE^I^I0x0400$

ERROR: code indent should never use tabs
#335: FILE: hw/i386/multiboot_header.h:89:
+#define MULTIBOOT_INFO_VBE_INFO^I^I0x0800$

ERROR: code indent should never use tabs
#336: FILE: hw/i386/multiboot_header.h:90:
+#define MULTIBOOT_INFO_FRAMEBUFFER_INFO^I0x1000$

ERROR: open brace '{' following struct go 

Re: [Qemu-devel] [Xen-devel] [PULL 3/3] xen-disk: add support for multi-page shared rings

[Stefano Stabellini]

On Thu, 27 Jul 2017, Olaf Hering wrote:
> On Tue, Jun 27, Stefano Stabellini wrote:
> 
> > From: Paul Durrant 
> > The blkif protocol has had provision for negotiation of multi-page shared
> > rings for some time now and many guest OS have support in their frontend
> > drivers.
> 
> > +++ b/hw/block/xen_disk.c
> 
> > +domids = g_malloc0_n(blkdev->nr_ring_ref, sizeof(uint32_t));
> 
> According to [1] g_malloc0_n requires at least glib-2.24. As a result
> compilation of qemu-2.10 fails in SLE11, which has just glib-2.22.
> 
> Olaf
> 
> [1] https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html

It looks like QEMU supports glib 2.22, so this ought to work. In fact
the only user of g_malloc0_n is xen_disk.c. I would be happy to take a
patch that replaces g_malloc0_n with g_malloc0.

Re: [Qemu-devel] [PULL v1 0/7] MMIO Exec pull request

[Juan Quintela]

Peter Maydell  wrote:
> On 21 July 2017 at 10:13, Dr. David Alan Gilbert  wrote:
>> I don't fully understand the way memory_region_do_invalidate_mmio_ptr
>> works; I see it dropping the memory region; if that's also dropping
>> the RAMBlock then it will upset migration.   Even if the CPU is stopped
>> I dont think that stops the migration thread walking through the list of
>> RAMBlocks.
>
> memory_region_do_invalidate_mmio_ptr() calls memory_region_unref(),
> which will eventually result in memory_region_finalize() being
> called, which will call the MR destructor, which in this case is
> memory_region_destructor_ram(), which calls qemu_ram_free() on
> the RAMBlock, which removes the RAMBlock from the list (after
> taking the ramlist lock).
>
>> Even then, the problem is migration keeps a 'dirty_pages' count which is
>> calculated at the start of migration and updated as we dirty and send
>> pages; if we add/remove a RAMBlock then that dirty_pages count is wrong
>> and we either never finish migration (since dirty_pages never reaches
>> zero) or finish early with some unsent data.
>> And then there's the 'received' bitmap currently being added for
>> postcopy which tracks each page that's been received (that's not in yet
>> though).
>
> It sounds like we really need to make migration robust against
> RAMBlock changes -- in the hotplug case it's certainly possible
> for RAMBlocks to be newly created or destroyed while migration
> is in progress.

There is code to disable hotplug while we are migrating.  For 2.10 we
disabled *all* hotplug/unplug.  If there are things that are safe, we
can add them as we do them.

The problem with ramblocks is that we do the equivalent of:

foreach ramblock
  for each page in this ramblock
 if page is dirty, send page

But we could take a lot of time/rounds sending a single ramblock,
because we go back/forth with top level migration functions/loops.

Later, Juan.

[Qemu-devel] [PATCH] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Using C structs makes the code more readable and prevents type conversion
errors.

Borrow multiboot1 header from GRUB project.
---
 hw/i386/multiboot.c| 122 -
 hw/i386/multiboot_header.h | 265 +
 2 files changed, 313 insertions(+), 74 deletions(-)
 create mode 100644 hw/i386/multiboot_header.h

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index f13e23139b..596c7435d5 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -28,6 +28,7 @@
 #include "hw/hw.h"
 #include "hw/nvram/fw_cfg.h"
 #include "multiboot.h"
+#include "multiboot_header.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "sysemu/sysemu.h"
@@ -47,39 +48,9 @@
 #error multiboot struct needs to fit in 16 bit real mode
 #endif
 
-enum {
-/* Multiboot info */
-MBI_FLAGS   = 0,
-MBI_MEM_LOWER   = 4,
-MBI_MEM_UPPER   = 8,
-MBI_BOOT_DEVICE = 12,
-MBI_CMDLINE = 16,
-MBI_MODS_COUNT  = 20,
-MBI_MODS_ADDR   = 24,
-MBI_MMAP_ADDR   = 48,
-MBI_BOOTLOADER  = 64,
-
-MBI_SIZE= 88,
-
-/* Multiboot modules */
-MB_MOD_START= 0,
-MB_MOD_END  = 4,
-MB_MOD_CMDLINE  = 8,
-
-MB_MOD_SIZE = 16,
-
-/* Region offsets */
-ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
-ADDR_MBI  = ADDR_E820_MAP + 0x500,
-
-/* Multiboot flags */
-MULTIBOOT_FLAGS_MEMORY  = 1 << 0,
-MULTIBOOT_FLAGS_BOOT_DEVICE = 1 << 1,
-MULTIBOOT_FLAGS_CMDLINE = 1 << 2,
-MULTIBOOT_FLAGS_MODULES = 1 << 3,
-MULTIBOOT_FLAGS_MMAP= 1 << 6,
-MULTIBOOT_FLAGS_BOOTLOADER  = 1 << 9,
-};
+/* Region offsets */
+#define ADDR_E820_MAP MULTIBOOT_STRUCT_ADDR
+#define ADDR_MBI  (ADDR_E820_MAP + 0x500)
 
 typedef struct {
 /* buffer holding kernel, cmdlines and mb_infos */
@@ -128,14 +99,14 @@ static void mb_add_mod(MultibootState *s,
hwaddr start, hwaddr end,
hwaddr cmdline_phys)
 {
-char *p;
+multiboot_module_t *mod;
 assert(s->mb_mods_count < s->mb_mods_avail);
 
-p = (char *)s->mb_buf + s->offset_mbinfo + MB_MOD_SIZE * s->mb_mods_count;
+mod = s->mb_buf + s->offset_mbinfo + sizeof(multiboot_module_t) * 
s->mb_mods_count;
 
-stl_p(p + MB_MOD_START,   start);
-stl_p(p + MB_MOD_END, end);
-stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+stl_p(>mod_start, start);
+stl_p(>mod_end,   end);
+stl_p(>cmdline,   cmdline_phys);
 
 mb_debug("mod%02d: "TARGET_FMT_plx" - "TARGET_FMT_plx"\n",
  s->mb_mods_count, start, end);
@@ -151,26 +122,29 @@ int load_multiboot(FWCfgState *fw_cfg,
int kernel_file_size,
uint8_t *header)
 {
-int i, is_multiboot = 0;
+int i;
+bool is_multiboot = false;
 uint32_t flags = 0;
 uint32_t mh_entry_addr;
 uint32_t mh_load_addr;
 uint32_t mb_kernel_size;
 MultibootState mbs;
-uint8_t bootinfo[MBI_SIZE];
+multiboot_info_t bootinfo;
 uint8_t *mb_bootinfo_data;
 uint32_t cmdline_len;
+struct multiboot_header *multiboot_header;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
 for (i = 0; i < (8192 - 48); i += 4) {
-if (ldl_p(header+i) == 0x1BADB002) {
-uint32_t checksum = ldl_p(header+i+8);
-flags = ldl_p(header+i+4);
+multiboot_header = (struct multiboot_header *)(header + i);
+if (ldl_p(_header->magic) == MULTIBOOT_HEADER_MAGIC) {
+uint32_t checksum = ldl_p(_header->checksum);
+flags = ldl_p(_header->flags);
 checksum += flags;
-checksum += (uint32_t)0x1BADB002;
+checksum += (uint32_t)MULTIBOOT_HEADER_MAGIC;
 if (!checksum) {
-is_multiboot = 1;
+is_multiboot = true;
 break;
 }
 }
@@ -180,13 +154,13 @@ int load_multiboot(FWCfgState *fw_cfg,
 return 0; /* no multiboot */
 
 mb_debug("qemu: I believe we found a multiboot image!\n");
-memset(bootinfo, 0, sizeof(bootinfo));
+memset(, 0, sizeof(bootinfo));
 memset(, 0, sizeof(mbs));
 
-if (flags & 0x0004) { /* MULTIBOOT_HEADER_HAS_VBE */
+if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & 0x0001)) { /* MULTIBOOT_HEADER_HAS_ADDR */
+if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
 uint64_t elf_entry;
 uint64_t elf_low, elf_high;
 int kernel_size;
@@ -217,14 +191,14 @@ int load_multiboot(FWCfgState *fw_cfg,
 mb_debug("qemu: loading multiboot-elf kernel (%#x bytes) with entry 
%#zx\n",
   mb_kernel_size, (size_t)mh_entry_addr);
 } else {
-/* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_ADDR. */
-uint32_t mh_header_addr = ldl_p(header+i+12);
-uint32_t mh_load_end_addr = 

Re: [Qemu-devel] [PULL v1 0/7] MMIO Exec pull request

[Juan Quintela]

"Dr. David Alan Gilbert"  wrote:
> * KONRAD Frederic (frederic.kon...@adacore.com) wrote:
>> 
>> 

>> 
>> Let's imagine somebody (eg: u-boot guest) wants to execute code
>> from the LQSPI area.
>> 
>> memory_region_request_mmio_ptr is called (the guest is not
>> running yet) it will create a mmio-interface which create the
>> ram memory region with a pointer provided by the device.
>> Then the guest can execute code from that and this somehow is
>> breaking migration because this ram memory region is migrated.
>> 
>> Now something is writing to the device.. The cache is no longer
>> valid, we have to drop this mmio-interface. This is done in an
>> async_safe work so cpu are stopped at this moment. So I think we
>> are safe for that.
>
> I don't fully understand the way memory_region_do_invalidate_mmio_ptr
> works; I see it dropping the memory region; if that's also dropping
> the RAMBlock then it will upset migration.   Even if the CPU is stopped
> I dont think that stops the migration thread walking through the list of
> RAMBlocks.

I made sure that we don't allow Memory hot[un]plug during migration.
Basically we were already assuming that RAMBlocks are static during
migration,  to relax that, we need to change other things, and it is too
late to investigate that.


> Even then, the problem is migration keeps a 'dirty_pages' count which is
> calculated at the start of migration and updated as we dirty and send
> pages; if we add/remove a RAMBlock then that dirty_pages count is wrong
> and we either never finish migration (since dirty_pages never reaches
> zero) or finish early with some unsent data.

Yeap, that is trouble.

> And then there's the 'received' bitmap currently being added for
> postcopy which tracks each page that's been received (that's not in yet
> though).

Later, Juan.

Re: [Qemu-devel] [for-2.11 PATCH 26/26] spapr: add hotplug hooks for PHB hotplug

[Michael Roth]

Quoting Greg Kurz (2017-07-27 12:09:55)
> On Thu, 27 Jul 2017 14:41:31 +1000
> Alexey Kardashevskiy  wrote:
> 
> > On 26/07/17 18:40, Greg Kurz wrote:
> > > Hotplugging PHBs is a machine-level operation, but PHBs reside on the
> > > main system bus, so we register spapr machine as the handler for the
> > > main system bus.
> > > 
> > > Signed-off-by: Michael Roth 
> > > Signed-off-by: Greg Kurz 
> > > ---
> > > - rebased against ppc-for-2.10
> > > - converted to unplug_request
> > > - handle drc_id at pre-plug
> > > - reset hotplugged PHB at plug
> > > - compatibility with older machine types
> > > ---
> > >  hw/ppc/spapr.c  |  114 
> > > +++
> > >  hw/ppc/spapr_drc.c  |1 
> > >  hw/ppc/spapr_pci.c  |2 -
> > >  include/hw/pci-host/spapr.h |2 +
> > >  include/hw/ppc/spapr.h  |1 
> > >  5 files changed, 118 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 90485054c2e7..589f76ef9fb8 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -2540,6 +2540,10 @@ static void ppc_spapr_init(MachineState *machine)
> > >  register_savevm_live(NULL, "spapr/htab", -1, 1,
> > >   _htab_handlers, spapr);
> > >  
> > > +if (spapr->dr_phb_enabled) {
> > > +qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine), 
> > > NULL);
> > > +}
> > > +
> > >  qemu_register_boot_set(spapr_boot_set, spapr);
> > >  
> > >  if (kvm_enabled()) {
> > > @@ -3238,6 +3242,103 @@ out:
> > >  error_propagate(errp, local_err);
> > >  }
> > >  
> > > +static void spapr_phb_pre_plug(HotplugHandler *hotplug_dev, DeviceState 
> > > *dev,
> > > +   Error **errp)
> > > +{
> > > +sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> > > +
> > > +if (sphb->drc_id == (uint32_t)-1) {
> > > +sphb->drc_id = sphb->index;
> > > +}
> > > +
> > > +if (sphb->drc_id >= SPAPR_DRC_MAX_PHB) {
> > > +error_setg(errp, "PHB id %d out of range", sphb->drc_id);
> > > +}  
> > 
> > 
> > sphb->index in considered 16bits in the existing code (even though it is
> > defined as 32bit) and SPAPR_DRC_MAX_PHB is just 256. I'd suggest using the
> > same limit for both, either 256 or 65536 is fine for me.
> > 
> > It is actually a bit weird - it is possible to completely configure few
> > PHBs in the command line so they will have index==-1 but PCI hotplug code -
> > spapr_phb_get_pci_func_drc() and spapr_phb_realize() - does not check for
> > this and just does (sphb->index << 16).
> 
> You're right and this looks like a bug... I'll try to come up with a fix.

Yup, that's a bug, and we can trigger it currently by adding 2
additional PHBs that don't have an index specified. QOM catches
and reports them as "attempt to add duplicate property", but it's
just reported by spapr_dr_connector_new() and doesn't seem to be
treated as fatal (and probably should be).

Might also see this more in practice now with the multi-phb support
in libvirt, though I'd imagine those would tend to rely on phb->index
being set.

Now that phb->drc_id is available though we can just use that instead.
I agree it should be limited to 16-bit or smaller to avoid any
possibility of overlap.

> 
> > May be just ditch drc_id, enforce index not to be -1 and use it as drc_id?
> > 
> 
> This was how Mike did it in the original patchset but David suggested
> to introduce drc_id (to preserve existing setups I guess):
> 
> http://patchwork.ozlabs.org/patch/466262/

Althrough IIRC what David proposed was to handle it like the other
properties which are filled in automatically when 'index' is specified,
i.e. 'drc_id' would be set automatically by 'index' if index is
specified, or we can set it explicitly if 'index' not specified, but
if we try to set both 'index' and 'drc_id' we trigger the "Either "index"
or other parameters must be specified for PAPR PHB, not both" error.

And it looks like 'index' is limited to 30, so it fits within our
16-bit limit for drc_id.

> 
> > 
> > 
> > > +}
> > > +
> > > +static void spapr_phb_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> > > +   Error **errp)
> > > +{
> > > +sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
> > > +sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> > > +void *fdt = NULL;
> > > +int fdt_start_offset;
> > > +int fdt_size;
> > > +Error *local_err = NULL;
> > > +sPAPRDRConnector *drc;
> > > +uint32_t phandle;
> > > +int ret;
> > > +bool hotplugged = spapr_drc_hotplugged(dev);
> > > +
> > > +if (!spapr->dr_phb_enabled) {
> > > +return;
> > > +}
> > > +
> > > +drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, sphb->drc_id);
> > > +/* hotplug hooks should check it's enabled before getting this far */
> > > +g_assert(drc);
> > > +
> 

Re: [Qemu-devel] [RFC PATCH v2 0/4] Allow RedHat PCI bridges reserve more buses than necessary during init

[Marcel Apfelbaum]

On 26/07/2017 21:49, Michael S. Tsirkin wrote:

On Wed, Jul 26, 2017 at 07:22:42PM +0300, Marcel Apfelbaum wrote:

On 26/07/2017 18:20, Laszlo Ersek wrote:

On 07/26/17 08:48, Marcel Apfelbaum wrote:

On 25/07/2017 18:46, Laszlo Ersek wrote:


[snip]


(2) Bus range reservation, and hotplugging bridges. What's the
motivation? Our recommendations in "docs/pcie.txt" suggest flat
hierarchies.



It remains flat. You have one single PCIE-PCI bridge plugged
into a PCIe Root Port, no deep nesting.

The reason is to be able to support legacy PCI devices without
"committing" with a DMI-PCI bridge in advance. (Keep Q35 without)
legacy hw.

The only way to support PCI devices in Q35 is to have them cold-plugged
into the pcie.0 bus, which is good, but not enough for expanding the
Q35 usability in order to make it eventually the default
QEMU x86 machine (I know this is another discussion and I am in
minority, at least for now).

The plan is:
Start Q35 machine as usual, but one of the PCIe Root Ports includes
hints for firmware needed t support legacy PCI devices. (IO Ports range,
extra bus,...)

Once a pci device is needed you have 2 options:
1. Plug a PCIe-PCI bridge into a PCIe Root Port and the PCI device
 in the bridge.
2. Hotplug a PCIe-PCI bridge into a PCIe Root Port and then hotplug
 a PCI device into the bridge.




Hi Laszlo,


Thank you for the explanation, it makes the intent a lot clearer.

However, what does the hot-pluggability of the PCIe-PCI bridge buy us?
In other words, what does it buy us when we do not add the PCIe-PCI
bridge immediately at guest startup, as an integrated device?
  > Why is it a problem to "commit" in advance? I understand that we might
not like the DMI-PCI bridge (due to it being legacy), but what speaks
against cold-plugging the PCIe-PCI bridge either as an integrated device
in pcie.0 (assuming that is permitted), or cold-plugging the PCIe-PCI
bridge in a similarly cold-plugged PCIe root port?



We want to keep Q35 clean, and for most cases we don't want any
legacy PCI stuff if not especially required.


BTW, what are the PCI devices that we actually need?



Is not about what we need, if Q35 will become a "transition" machine,
any existing emulated PCI device is fair game, since we would
want to run on Q35 also pc configurations.

Thanks,
Marcel

Re: [Qemu-devel] [RFC PATCH v2 0/4] Allow RedHat PCI bridges reserve more buses than necessary during init

[Marcel Apfelbaum]

On 26/07/2017 21:31, Laszlo Ersek wrote:

On 07/26/17 18:22, Marcel Apfelbaum wrote:

On 26/07/2017 18:20, Laszlo Ersek wrote:


[snip]


However, what does the hot-pluggability of the PCIe-PCI bridge buy us?
In other words, what does it buy us when we do not add the PCIe-PCI
bridge immediately at guest startup, as an integrated device?
  > Why is it a problem to "commit" in advance? I understand that we might
not like the DMI-PCI bridge (due to it being legacy), but what speaks
against cold-plugging the PCIe-PCI bridge either as an integrated device
in pcie.0 (assuming that is permitted), or cold-plugging the PCIe-PCI
bridge in a similarly cold-plugged PCIe root port?



We want to keep Q35 clean, and for most cases we don't want any
legacy PCI stuff if not especially required.


I mean, in the cold-plugged case, you use up two bus numbers at the
most, one for the root port, and another for the PCIe-PCI bridge. In the
hot-plugged case, you have to start with the cold-plugged root port just
the same (so that you can communicate the bus number reservation *at
all*), and then reserve (= use up in advance) the bus number, the IO
space, and the MMIO space(s). I don't see the difference; hot-plugging
the PCIe-PCI bridge (= not committing in advance) doesn't seem to save
any resources.



Is not about resources, more about usage model.


I guess I would see a difference if we reserved more than one bus number
in the hotplug case, namely in order to support recursive hotplug under
the PCIe-PCI bridge. But, you confirmed that we intend to keep the flat
hierarchy (ie the exercise is only for enabling legacy PCI endpoints,
not for recursive hotplug).  The PCIe-PCI bridge isn't a device that
does anything at all on its own, so why not just coldplug it? Its
resources have to be reserved in advance anyway.



Even if we prefer flat hierarchies, we should allow a sane nested
bridges configuration, so we will some times reserve more than one.


So, thus far I would say "just cold-plug the PCIe-PCI bridge at startup,
possibly even make it an integrated device, and then you don't need to
reserve bus numbers (and other apertures)".

Where am I wrong?



Nothing wrong, I am just looking for feature parity Q35 vs PC.
Users may want to continue using [nested] PCI bridges, and
we want the Q35 machine to be used by more users in order
to make it reliable faster, while keeping it clean by default.

We had a discussion on this matter on last year KVM forum
and the hot-pluggable PCIe-PCI bridge was the general consensus.


OK. I don't want to question or go back on that consensus now; I'd just
like to point out that all that you describe (nested bridges, and
enabling legacy PCI with PCIe-PCI bridges, *on demand*) is still
possible with cold-plugging.

I.e., the default setup of Q35 does not need to include legacy PCI
bridges. It's just that the pre-launch configuration effort for a Q35
user to *reserve* resources for legacy PCI is the exact same as the
pre-launch configuration effort to *actually cold-plug* the bridge.

[snip]


The PI spec says,


[...] For all the root HPCs and the nonroot HPCs, call
EFI_PCI_HOT_PLUG_INIT_PROTOCOL.GetResourcePadding() to obtain the
amount of overallocation and add that amount to the requests from the
physical devices. Reprogram the bus numbers by taking into account the
bus resource padding information. [...]


However, according to my interpretation of the source code, PciBusDxe
does not consider bus number padding for non-root HPCs (which are "all"
HPCs on QEMU).



Theoretically speaking, it is possible to change the  behavior, right?


Not just theoretically; in the past I have changed PciBusDxe -- it
wouldn't identify QEMU's hotplug controllers (root port, downstream port
etc) appropriately, and I managed to get some patches in. It's just that
the less we understand the current code and the more intrusive/extensive
the change is, the harder it is to sell the *idea*. PciBusDxe is
platform-independent and shipped on many a physical system too.



Understood, but from your explanation it sounds like the existings
callback sites(hooks) are enough.


That's the problem: they don't appear to, if you consider bus number
reservations. The existing callback sites seem fine regarding IO and
MMIO, but the only callback site that honors bus number reservation is
limited to "root" (in the previously defined sense) hotplug controllers.

So this is something that will need investigation, and my most recent
queries into the "hotplug preparation" parts of PciBusDxe indicate that
those parts are quite... "forgotten". :) I guess this might be because
on physical systems the level of PCI(e) hotpluggery that we plan to do
is likely unheard of :)



I admit is possible that it looks a little "crazy" on bare-metal,
but as long as we "color inside the lines" we are allowed to push
it a little :)

Thanks,
Marcel


Thanks!
Laszlo

Re: [Qemu-devel] [SeaBIOS] Commit 77af8a2b95b79699de650965d5228772743efe84 breaks Windows 2000 support

[Laszlo Ersek]

On 07/27/17 16:59, Kevin O'Connor wrote:
> On Wed, Jul 26, 2017 at 04:21:23PM -0400, Paolo Bonzini wrote:

>>> C - We'd be introducing "shared ownership" of the acpi tables.  Some
>>> of the tables would be produced by QEMU and some of them by
>>> SeaBIOS.  Explaining when and why to future developers would be
>>> a challenge.
>>
>> The advantage is that the same shared ownership is already present in
>> OVMF.  The RSDP/RSDT/XSDT are entirely created by the firmware in
>> OVMF. (The rev1 FADT isn't but that's just missing code; the table
>> manager in general would be ready for that).  In any case this
>> doesn't seem like something that cannot be solved by code comments.
>
> I'd argue that the shared ownership in the EDK2 code was a poor design
> choice.

The reason we can't just exclude the reference implementation of
EFI_ACPI_TABLE_PROTOCOL from OVMF whole-sale, and reimplement the ACPI
linker/loader from scratch, is that some other (independent) edk2
modules will want to use EFI_ACPI_TABLE_PROTOCOL for installing their
own (one-off) tables, such as IBFT, BGRT and so on, *in addition to*
QEMU's. Given that these ACPI tables mostly do *not* describe hardware
(but software features and/or configuration), it's hard to claim that
they should also be generated by QEMU.

Therefore the dual origin for ACPI tables looks unavoidable in UEFI,
it's just that there should be a lot more flexible "connect" from QEMU's
linker/loader to the installed ACPI tables than EFI_ACPI_TABLE_PROTOCOL.

Basically this is a fight over ownership. Each of QEMU's ACPI
linker/loader and EFI_ACPI_TABLE_PROTOCOL thinks that it fully owns the
root of the table tree. :(

> Case in point - we're only having this conversation because of its
> limitations - SeaBIOS is capable of deploying the acpi tables in the
> proposed layout without any code changes today.

Yes.

But let's not forget that SeaBIOS is capable of delegating the full
low-level construction of the table tree to QEMU because no independent
/ 3rd party BIOS-level code wants to install its own tables (again,
IBFT, BGRT, ...) This is not true of UEFI, where the guiding principle
of the standardized interfaces is to enable cooperation between
independent, binary-only modules. (So, for example, if you shove a new
PCI add-on card in your motherboard, the UEFI driver in that oprom could
install a separate ACPI table, by looking up and calling
EFI_ACPI_TABLE_PROTOCOL.)

> I'm not against changing SeaBIOS, but it's a priority for me that we
> continue to make it possible to deploy future ACPI table changes (no
> matter how quirky) in a way that does not require future SeaBIOS
> releases.

It's a good goal.

I apologize for forgetting the context, but what exactly was the
argument against:

- splitting modern ACPI generation from ancient ACPI generation (so that
we can assign separate maintainers to ancient vs. modern),

- restricting ancient ACPI generation to old machine types?

Thanks,
Laszlo

[Qemu-devel] [PATCH] ui: correctly detect spice PAUSE scancode sequence

[Daniel P. Berrange]

The SPICE input code is currently detcting 0xe1 0x1d 0x45 as
the PAUSE key make sequence and 0xe1 0x9d 0xc5 as the break
sequence. This is incorrect, because all 6 scancodes together
are the make sequence, and there is no break sequence.

Signed-off-by: Daniel P. Berrange 
---
 ui/spice-input.c | 32 +---
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/ui/spice-input.c b/ui/spice-input.c
index cda9976469..3d41aa1831 100644
--- a/ui/spice-input.c
+++ b/ui/spice-input.c
@@ -50,6 +50,7 @@ static const SpiceKbdInterface kbd_interface = {
 
 static void kbd_push_key(SpiceKbdInstance *sin, uint8_t scancode)
 {
+static const uint8_t pauseseq[] = { 0xe1, 0x1d, 0x45, 0xe1, 0x9d, 0xc5 };
 QemuSpiceKbd *kbd = container_of(sin, QemuSpiceKbd, sin);
 int keycode;
 bool up;
@@ -58,32 +59,25 @@ static void kbd_push_key(SpiceKbdInstance *sin, uint8_t 
scancode)
 kbd->emul0 = true;
 return;
 }
-keycode = scancode & ~SCANCODE_UP;
-up = scancode & SCANCODE_UP;
-if (kbd->emul0) {
-kbd->emul0 = false;
-keycode |= SCANCODE_GREY;
-}
 
-if (scancode == SCANCODE_EMUL1) {
+if (scancode == pauseseq[kbd->pauseseq]) {
 kbd->pauseseq++;
-return;
-} else if (kbd->pauseseq == 1) {
-if (keycode == 0x1d) {
-kbd->pauseseq++;
-return;
-} else {
-kbd->pauseseq = 0;
-}
-} else if (kbd->pauseseq == 2) {
-if (keycode == 0x45) {
-qemu_input_event_send_key_qcode(NULL, Q_KEY_CODE_PAUSE, !up);
+if (kbd->pauseseq == G_N_ELEMENTS(pauseseq)) {
+qemu_input_event_send_key_qcode(NULL, Q_KEY_CODE_PAUSE, true);
 kbd->pauseseq = 0;
-return;
 }
+return;
+} else {
 kbd->pauseseq = 0;
 }
 
+keycode = scancode & ~SCANCODE_UP;
+up = scancode & SCANCODE_UP;
+if (kbd->emul0) {
+kbd->emul0 = false;
+keycode |= SCANCODE_GREY;
+}
+
 qemu_input_event_send_key_number(NULL, keycode, !up);
 }
 
-- 
2.13.3

Re: [Qemu-devel] [PULL 4/7] ui: add multimedia keys

[Daniel P. Berrange]

On Thu, Jul 27, 2017 at 04:00:22PM +0200, Gerd Hoffmann wrote:
> Add multimedia keys to QKeyCodes and to the keymaps.
> 
> Signed-off-by: Gerd Hoffmann 
> Reviewed-by: Eric Blake 
> Message-id: 20170726152918.11995-5-kra...@redhat.com
> ---
>  ui/input-keymap.c | 44 
>  qapi-schema.json  | 28 +++-
>  2 files changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/ui/input-keymap.c b/ui/input-keymap.c
> index 7461e1edde..ae781beae9 100644
> --- a/ui/input-keymap.c
> +++ b/ui/input-keymap.c
> @@ -116,6 +116,28 @@ static int linux_to_qcode[KEY_CNT] = {
>  [KEY_LEFTMETA]   = Q_KEY_CODE_META_L,
>  [KEY_RIGHTMETA]  = Q_KEY_CODE_META_R,
>  [KEY_MENU]   = Q_KEY_CODE_MENU,
> +
> +[KEY_SLEEP]  = Q_KEY_CODE_SLEEP,
> +[KEY_WAKEUP] = Q_KEY_CODE_WAKE,
> +[KEY_CALC]   = Q_KEY_CODE_CALCULATOR,
> +[KEY_MAIL]   = Q_KEY_CODE_MAIL,
> +[KEY_COMPUTER]   = Q_KEY_CODE_COMPUTER,
> +
> +[KEY_STOP]   = Q_KEY_CODE_AC_STOP,

We already have a Q_KEY_CODE_STOP that I think is the correct mapping
for KEY_STOP, so this new Q_KEY_CODE_AC_STOP looks like a dupe.

> +[KEY_BOOKMARKS]  = Q_KEY_CODE_AC_BOOKMARKS,
> +[KEY_BACK]   = Q_KEY_CODE_AC_BACK,
> +[KEY_FORWARD]= Q_KEY_CODE_AC_FORWARD,
> +[KEY_HOMEPAGE]   = Q_KEY_CODE_AC_HOME,
> +[KEY_REFRESH]= Q_KEY_CODE_AC_REFRESH,
> +[KEY_FIND]   = Q_KEY_CODE_AC_SEARCH,

Similarly we already have a Q_KEY_CODE_FIND that should map to
KEY_FIND, so this Q_KEY_CODE_AC_SEARCH is another dup AFAICT.

I'm curious what the 'AC_' prefix on all these is indicating ?
Do we actually need it ?

> +
> +[KEY_NEXTSONG]   = Q_KEY_CODE_AUDIONEXT,
> +[KEY_PREVIOUSSONG]   = Q_KEY_CODE_AUDIOPREV,
> +[KEY_STOPCD] = Q_KEY_CODE_AUDIOSTOP,
> +[KEY_PLAYCD] = Q_KEY_CODE_AUDIOPLAY,
> +[KEY_MUTE]   = Q_KEY_CODE_AUDIOMUTE,
> +[KEY_VOLUMEDOWN] = Q_KEY_CODE_VOLUMEDOWN,
> +[KEY_VOLUMEUP]   = Q_KEY_CODE_VOLUMEUP,

Missing Q_KEY_CODE_MEDIASELECT entry - presumably it was supposed
to map to  KEY_MEDIA

>  };
>  
>  static const int qcode_to_number[] = {
> @@ -252,6 +274,28 @@ static const int qcode_to_number[] = {
>  [Q_KEY_CODE_YEN] = 0x7d,
>  [Q_KEY_CODE_KP_COMMA] = 0x7e,
>  
> +[Q_KEY_CODE_SLEEP] = 0xdf,
> +[Q_KEY_CODE_WAKE] = 0xe3,
> +[Q_KEY_CODE_CALCULATOR] = 0xa1,
> +[Q_KEY_CODE_MAIL] = 0xec,
> +[Q_KEY_CODE_COMPUTER] = 0xeb,
> +
> +[Q_KEY_CODE_AC_STOP] = 0xe8,
> +[Q_KEY_CODE_AC_BOOKMARKS] = 0xe6,
> +[Q_KEY_CODE_AC_BACK] = 0xea,
> +[Q_KEY_CODE_AC_FORWARD] = 0xe9,
> +[Q_KEY_CODE_AC_HOME] = 0xb2,
> +[Q_KEY_CODE_AC_REFRESH] = 0xe7,
> +[Q_KEY_CODE_AC_SEARCH] = 0xe5,
> +
> +[Q_KEY_CODE_AUDIONEXT] = 0x99,
> +[Q_KEY_CODE_AUDIOPREV] = 0x90,
> +[Q_KEY_CODE_AUDIOSTOP] = 0xa4,
> +[Q_KEY_CODE_AUDIOPLAY] = 0xa2,
> +[Q_KEY_CODE_AUDIOMUTE] = 0xa0,
> +[Q_KEY_CODE_VOLUMEDOWN] = 0xae,
> +[Q_KEY_CODE_VOLUMEUP] = 0xb0,

Again no MEDIASELECT entry

> +
>  [Q_KEY_CODE__MAX] = 0,
>  };
>  
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 9c6c3e1a53..9cb15092a7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4843,6 +4843,27 @@
>  # @henkan: since 2.9
>  # @yen: since 2.9
>  #
> +# @sleep: since 2.10
> +# @wake: since 2.10
> +# @audionext: since 2.10
> +# @audioprev: since 2.10
> +# @audiostop: since 2.10
> +# @audioplay: since 2.10
> +# @audiomute: since 2.10
> +# @volumeup: since 2.10
> +# @volumedown: since 2.10
> +# @mediaselect: since 2.10
> +# @mail: since 2.10
> +# @calculator: since 2.10
> +# @computer: since 2.10
> +# @ac_search: since 2.10
> +# @ac_home: since 2.10
> +# @ac_back: since 2.10
> +# @ac_forward: since 2.10
> +# @ac_stop: since 2.10
> +# @ac_refresh: since 2.10
> +# @ac_bookmarks: since 2.10
> +#
>  # Since: 1.3.0
>  #
>  ##
> @@ -4864,7 +4885,12 @@
>  'props', 'undo', 'front', 'copy', 'open', 'paste', 'find', 'cut',
>  'lf', 'help', 'meta_l', 'meta_r', 'compose', 'pause',
>  'ro', 'hiragana', 'henkan', 'yen',
> -'kp_comma', 'kp_equals', 'power' ] }
> +'kp_comma', 'kp_equals', 'power', 'sleep', 'wake',
> +'audionext', 'audioprev', 'audiostop', 'audioplay', 'audiomute',
> +'volumeup', 'volumedown', 'mediaselect',
> +'mail', 'calculator', 'computer',
> +'ac_search', 'ac_home', 'ac_back', 'ac_forward', 'ac_stop',
> +'ac_refresh', 'ac_bookmarks' ] }
>  
>  ##
>  # @KeyValue:
> -- 
> 2.9.3
> 
> 

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: [Qemu-devel] [for-2.11 PATCH 26/26] spapr: add hotplug hooks for PHB hotplug

[Greg Kurz]

On Thu, 27 Jul 2017 14:41:31 +1000
Alexey Kardashevskiy  wrote:

> On 26/07/17 18:40, Greg Kurz wrote:
> > Hotplugging PHBs is a machine-level operation, but PHBs reside on the
> > main system bus, so we register spapr machine as the handler for the
> > main system bus.
> > 
> > Signed-off-by: Michael Roth 
> > Signed-off-by: Greg Kurz 
> > ---
> > - rebased against ppc-for-2.10
> > - converted to unplug_request
> > - handle drc_id at pre-plug
> > - reset hotplugged PHB at plug
> > - compatibility with older machine types
> > ---
> >  hw/ppc/spapr.c  |  114 
> > +++
> >  hw/ppc/spapr_drc.c  |1 
> >  hw/ppc/spapr_pci.c  |2 -
> >  include/hw/pci-host/spapr.h |2 +
> >  include/hw/ppc/spapr.h  |1 
> >  5 files changed, 118 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 90485054c2e7..589f76ef9fb8 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2540,6 +2540,10 @@ static void ppc_spapr_init(MachineState *machine)
> >  register_savevm_live(NULL, "spapr/htab", -1, 1,
> >   _htab_handlers, spapr);
> >  
> > +if (spapr->dr_phb_enabled) {
> > +qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine), 
> > NULL);
> > +}
> > +
> >  qemu_register_boot_set(spapr_boot_set, spapr);
> >  
> >  if (kvm_enabled()) {
> > @@ -3238,6 +3242,103 @@ out:
> >  error_propagate(errp, local_err);
> >  }
> >  
> > +static void spapr_phb_pre_plug(HotplugHandler *hotplug_dev, DeviceState 
> > *dev,
> > +   Error **errp)
> > +{
> > +sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> > +
> > +if (sphb->drc_id == (uint32_t)-1) {
> > +sphb->drc_id = sphb->index;
> > +}
> > +
> > +if (sphb->drc_id >= SPAPR_DRC_MAX_PHB) {
> > +error_setg(errp, "PHB id %d out of range", sphb->drc_id);
> > +}  
> 
> 
> sphb->index in considered 16bits in the existing code (even though it is
> defined as 32bit) and SPAPR_DRC_MAX_PHB is just 256. I'd suggest using the
> same limit for both, either 256 or 65536 is fine for me.
> 
> It is actually a bit weird - it is possible to completely configure few
> PHBs in the command line so they will have index==-1 but PCI hotplug code -
> spapr_phb_get_pci_func_drc() and spapr_phb_realize() - does not check for
> this and just does (sphb->index << 16).

You're right and this looks like a bug... I'll try to come up with a fix.

> May be just ditch drc_id, enforce index not to be -1 and use it as drc_id?
> 

This was how Mike did it in the original patchset but David suggested
to introduce drc_id (to preserve existing setups I guess):

http://patchwork.ozlabs.org/patch/466262/

> 
> 
> > +}
> > +
> > +static void spapr_phb_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> > +   Error **errp)
> > +{
> > +sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
> > +sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> > +void *fdt = NULL;
> > +int fdt_start_offset;
> > +int fdt_size;
> > +Error *local_err = NULL;
> > +sPAPRDRConnector *drc;
> > +uint32_t phandle;
> > +int ret;
> > +bool hotplugged = spapr_drc_hotplugged(dev);
> > +
> > +if (!spapr->dr_phb_enabled) {
> > +return;
> > +}
> > +
> > +drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, sphb->drc_id);
> > +/* hotplug hooks should check it's enabled before getting this far */
> > +g_assert(drc);
> > +
> > +if (hotplugged) {
> > +if (spapr->xics_phandle == UINT32_MAX) {
> > +error_setg(_err,
> > +   "SLOF didn't update the XICS phandle. PHB hotplug 
> > cancelled");
> > +goto out;
> > +}
> > +phandle = spapr->xics_phandle;
> > +
> > +spapr_phb_reset(dev);  
> 
> 
> It could be DEVICE_GET_CLASS(dev)->reset(dev) without exporting
> spapr_phb_reset. Not sure how this fits into the current QEMU coding style
> though.
> 
> 
> 
> > +} else {
> > +phandle = PHANDLE_XICP;
> > +}
> > +
> > +fdt = create_device_tree(_size);
> > +ret = spapr_populate_pci_dt(sphb, phandle, fdt, _start_offset);
> > +if (ret < 0) {
> > +error_setg(_err, "unable to create FDT for %sPHB",
> > +   dev->hotplugged ? "hotplugged " : "");
> > +goto out;
> > +}
> > +
> > +if (hotplugged) {
> > +/* generally SLOF creates these, for hotplug it's up to QEMU */
> > +_FDT(fdt_setprop_string(fdt, fdt_start_offset, "name", "pci"));
> > +}
> > +
> > +spapr_drc_attach(drc, DEVICE(dev), fdt, fdt_start_offset, _err);
> > +out:
> > +if (local_err) {
> > +error_propagate(errp, local_err);
> > +g_free(fdt);
> > +return;
> > +}
> > +
> > +if (hotplugged) {
> > +