Re: [Qemu-devel] [Qemu-ppc] [PATCH 14/15] target-ppc: Use tcg_gen_extract_*

[David Gibson]

On Mon, Oct 17, 2016 at 02:38:06PM +1100, David Gibson wrote:
> On Sat, Oct 15, 2016 at 08:37:49PM -0700, Richard Henderson wrote:
> > Use the new primitives for RDWINM and RLDICL.
> > 
> > Cc: qemu-...@nongnu.org
> > Signed-off-by: Richard Henderson 
> 
> Applied to ppc-for-2.8, thanks.

Uh.. wait.. no, wasn't paying attention to the fact that it needs the
whole series.

> > ---
> >  target-ppc/translate.c | 9 -
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> > index bfc1301..724d95c 100644
> > --- a/target-ppc/translate.c
> > +++ b/target-ppc/translate.c
> > @@ -1977,9 +1977,8 @@ static void gen_rlwinm(DisasContext *ctx)
> >  if (mb == 0 && me == (31 - sh)) {
> >  tcg_gen_shli_tl(t_ra, t_rs, sh);
> >  tcg_gen_ext32u_tl(t_ra, t_ra);
> > -} else if (sh != 0 && me == 31 && sh == (32 - mb)) {
> > -tcg_gen_ext32u_tl(t_ra, t_rs);
> > -tcg_gen_shri_tl(t_ra, t_ra, mb);
> > +} else if (me == 31 && (me - mb + 1) + sh <= 32) {
> > +tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
> >  } else {
> >  target_ulong mask;
> >  #if defined(TARGET_PPC64)
> > @@ -2094,8 +2093,8 @@ static void gen_rldinm(DisasContext *ctx, int mb, int 
> > me, int sh)
> >  
> >  if (sh != 0 && mb == 0 && me == (63 - sh)) {
> >  tcg_gen_shli_tl(t_ra, t_rs, sh);
> > -} else if (sh != 0 && me == 63 && sh == (64 - mb)) {
> > -tcg_gen_shri_tl(t_ra, t_rs, mb);
> > +} else if (me == 63 && (me - mb + 1) + sh <= 64) {
> > +tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
> >  } else {
> >  tcg_gen_rotli_tl(t_ra, t_rs, sh);
> >  tcg_gen_andi_tl(t_ra, t_ra, MASK(mb, me));
> 



-- 
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] Fix build for less common build directories names

[Stefan Weil]

Am 16.10.2016 um 21:04 schrieb Peter Maydell:

On 16 October 2016 at 15:31, Michael Tokarev  wrote:

13.10.2016 21:29, Stefan Weil wrote:

scripts/tracetool generates a C preprocessor macro from the name of the
build directory. Any characters which are possible in a directory name
but not allowed in a macro name must be substituted, otherwise builds
will fail.


Applied to -trivial, thank you!


Consensus on this and the other thread seemed to be
that we should just fix the bug where tracetool is
putting the build directory name into symbols
rather than working around that...

thanks
-- PMM



IMHO the patch can be applied nevertheless, as it uses a clearer
regular expression for characters allowed in macro names
(not one that is specific for the current QEMU code).

Stefan

Re: [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms
Type: series
Message-id: 1476678204-11511-1-git-send-email-da...@gibson.dropbear.id.au

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

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

# Useful git options
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 show --no-patch --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
Switched to a new branch 'test'
c2105ef Split ISA and sysbus versions of m48t59 device
d3ad698 Allow ISA bus to be configured out
2b39254 Split serial-isa into its own config option

=== OUTPUT BEGIN ===
Checking PATCH 1/3: Split serial-isa into its own config option...
Checking PATCH 2/3: Allow ISA bus to be configured out...
Checking PATCH 3/3: Split ISA and sysbus versions of m48t59 device...
ERROR: do not use C99 // comments
#65: FILE: hw/timer/m48t59-internal.h:28:
+//#define DEBUG_NVRAM

total: 1 errors, 0 warnings, 613 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

[Qemu-devel] [PATCH 2/3] Allow ISA bus to be configured out

[David Gibson]

Currently, the code to handle the legacy ISA bus is always included in
qemu.  However there are lots of platforms that don't include ISA legacy
devies, and quite a few that have never used ISA legacy devices at all.

This patch allows the ISA bus code to be disabled in the configuration for
platforms where it doesn't make sense.

For now, the default configs are adjusted to include ISA on all platforms
including PCI: anything with PCI can at least in principle add an i82378
PCI->ISA bridge.  Also, CONFIG_IDE_CORE which is already in pci.mak
requires ISA support.

We also explicitly enable ISA on some other non-PCI platforms which include
ISA devices: moxie, sparc and unicore32.  We may want to pare this down in
future.

The platforms that will lose ISA by default are: cris, lm32, microblazeel,
microblaze, or32, s390x, tricore, xtensaeb, xtensa.  As far as I can tell
none of these ever used ISA.

Signed-off-by: David Gibson 
Acked-by: Michael S. Tsirkin 
---
 default-configs/moxie-softmmu.mak | 1 +
 default-configs/pci.mak   | 2 ++
 default-configs/sparc-softmmu.mak | 1 +
 default-configs/unicore32-softmmu.mak | 1 +
 hw/isa/Makefile.objs  | 2 +-
 5 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/default-configs/moxie-softmmu.mak 
b/default-configs/moxie-softmmu.mak
index 7e22863..e00d099 100644
--- a/default-configs/moxie-softmmu.mak
+++ b/default-configs/moxie-softmmu.mak
@@ -1,5 +1,6 @@
 # Default configuration for moxie-softmmu
 
+CONFIG_ISA_BUS=y
 CONFIG_MC146818RTC=y
 CONFIG_SERIAL=y
 CONFIG_SERIAL_ISA=y
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index d8d6548..60dc651 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -1,4 +1,6 @@
 CONFIG_PCI=y
+# For now, CONFIG_IDE_CORE requires ISA, so we enable it here
+CONFIG_ISA_BUS=y
 CONFIG_VIRTIO_PCI=y
 CONFIG_VIRTIO=y
 CONFIG_USB_UHCI=y
diff --git a/default-configs/sparc-softmmu.mak 
b/default-configs/sparc-softmmu.mak
index ab796b3..004b0f4 100644
--- a/default-configs/sparc-softmmu.mak
+++ b/default-configs/sparc-softmmu.mak
@@ -1,5 +1,6 @@
 # Default configuration for sparc-softmmu
 
+CONFIG_ISA_BUS=y
 CONFIG_ECC=y
 CONFIG_ESP=y
 CONFIG_ESCC=y
diff --git a/default-configs/unicore32-softmmu.mak 
b/default-configs/unicore32-softmmu.mak
index de38577..5f6c4a8 100644
--- a/default-configs/unicore32-softmmu.mak
+++ b/default-configs/unicore32-softmmu.mak
@@ -1,4 +1,5 @@
 # Default configuration for unicore32-softmmu
+CONFIG_ISA_BUS=y
 CONFIG_PUV3=y
 CONFIG_PTIMER=y
 CONFIG_PCKBD=y
diff --git a/hw/isa/Makefile.objs b/hw/isa/Makefile.objs
index 9164556..fb37c55 100644
--- a/hw/isa/Makefile.objs
+++ b/hw/isa/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += isa-bus.o
+common-obj-$(CONFIG_ISA_BUS) += isa-bus.o
 common-obj-$(CONFIG_APM) += apm.o
 common-obj-$(CONFIG_I82378) += i82378.o
 common-obj-$(CONFIG_PC87312) += pc87312.o
-- 
2.7.4

[Qemu-devel] [PATCH 3/3] Split ISA and sysbus versions of m48t59 device

[David Gibson]

The m48t59 device supports both ISA and direct sysbus attached versions of
the device in the one .c file.  This can be awkward for some embedded
machine types which need the sysbus M48T59, but don't want to pull in the
ISA bus code and its other dependencies.

Therefore, this patch splits out the code for the ISA attached M48T59 into
its own C file.  It will be built when both CONFIG_M48T59 and
CONFIG_ISA_BUS are enabled.

Signed-off-by: David Gibson 
---
 hw/timer/Makefile.objs |   3 +
 hw/timer/m48t59-internal.h |  82 
 hw/timer/m48t59-isa.c  | 180 +++
 hw/timer/m48t59.c  | 228 -
 4 files changed, 283 insertions(+), 210 deletions(-)
 create mode 100644 hw/timer/m48t59-internal.h
 create mode 100644 hw/timer/m48t59-isa.c

diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 7ba8c23..bf3ea3c 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -6,6 +6,9 @@ common-obj-$(CONFIG_DS1338) += ds1338.o
 common-obj-$(CONFIG_HPET) += hpet.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_M48T59) += m48t59.o
+ifeq ($(CONFIG_ISA_BUS),y)
+common-obj-$(CONFIG_M48T59) += m48t59-isa.o
+endif
 common-obj-$(CONFIG_PL031) += pl031.o
 common-obj-$(CONFIG_PUV3) += puv3_ost.o
 common-obj-$(CONFIG_TWL92230) += twl92230.o
diff --git a/hw/timer/m48t59-internal.h b/hw/timer/m48t59-internal.h
new file mode 100644
index 000..c50f134
--- /dev/null
+++ b/hw/timer/m48t59-internal.h
@@ -0,0 +1,82 @@
+/*
+ * QEMU M48T59 and M48T08 NVRAM emulation (common header)
+ *
+ * Copyright (c) 2003-2005, 2007 Jocelyn Mayer
+ * Copyright (c) 2013 Hervé Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef HW_M48T59_H
+#define HW_M48T59_H 1
+
+//#define DEBUG_NVRAM
+
+#if defined(DEBUG_NVRAM)
+#define NVRAM_PRINTF(fmt, ...) do { printf(fmt , ## __VA_ARGS__); } while (0)
+#else
+#define NVRAM_PRINTF(fmt, ...) do { } while (0)
+#endif
+
+/*
+ * The M48T02, M48T08 and M48T59 chips are very similar. The newer '59 has
+ * alarm and a watchdog timer and related control registers. In the
+ * PPC platform there is also a nvram lock function.
+ */
+
+typedef struct M48txxInfo {
+const char *bus_name;
+uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
+uint32_t size;
+} M48txxInfo;
+
+typedef struct M48t59State {
+/* Hardware parameters */
+qemu_irq IRQ;
+MemoryRegion iomem;
+uint32_t size;
+int32_t base_year;
+/* RTC management */
+time_t   time_offset;
+time_t   stop_time;
+/* Alarm & watchdog */
+struct tm alarm;
+QEMUTimer *alrm_timer;
+QEMUTimer *wd_timer;
+/* NVRAM storage */
+uint8_t *buffer;
+/* Model parameters */
+uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
+/* NVRAM storage */
+uint16_t addr;
+uint8_t  lock;
+} M48t59State;
+
+uint32_t m48t59_read(M48t59State *NVRAM, uint32_t addr);
+void m48t59_write(M48t59State *NVRAM, uint32_t addr, uint32_t val);
+void m48t59_reset_common(M48t59State *NVRAM);
+void m48t59_realize_common(M48t59State *s, Error **errp);
+
+static inline void m48t59_toggle_lock(M48t59State *NVRAM, int lock)
+{
+NVRAM->lock ^= 1 << lock;
+}
+
+extern const MemoryRegionOps m48t59_io_ops;
+
+#endif /* HW_M48T59_H */
diff --git a/hw/timer/m48t59-isa.c b/hw/timer/m48t59-isa.c
new file mode 100644
index 000..3a521dc
--- /dev/null
+++ b/hw/timer/m48t59-isa.c
@@ -0,0 +1,180 @@
+/*
+ * QEMU M48T59 and M48T08 NVRAM emulation (ISA bus interface
+ *
+ * Copyright (c) 2003-2005, 2007 Jocelyn Mayer
+ * Copyright (c) 2013 Hervé Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to 

[Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms

[David Gibson]

This is a rebase/repost of a series I sent quite some time ago.  This
makes some cleanups that are a start on allowing ISA to be compiled
out for platforms which don't use it.

David Gibson (3):
  Split serial-isa into its own config option
  Allow ISA bus to be configured out
  Split ISA and sysbus versions of m48t59 device

 default-configs/alpha-softmmu.mak   |   1 +
 default-configs/arm-softmmu.mak |   1 +
 default-configs/i386-softmmu.mak|   1 +
 default-configs/mips-softmmu-common.mak |   1 +
 default-configs/moxie-softmmu.mak   |   2 +
 default-configs/pci.mak |   3 +
 default-configs/ppc-softmmu.mak |   1 +
 default-configs/ppc64-softmmu.mak   |   1 +
 default-configs/ppcemb-softmmu.mak  |   1 +
 default-configs/sh4-softmmu.mak |   1 +
 default-configs/sh4eb-softmmu.mak   |   1 +
 default-configs/sparc-softmmu.mak   |   1 +
 default-configs/sparc64-softmmu.mak |   1 +
 default-configs/unicore32-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak  |   1 +
 hw/char/Makefile.objs   |   3 +-
 hw/isa/Makefile.objs|   2 +-
 hw/timer/Makefile.objs  |   3 +
 hw/timer/m48t59-internal.h  |  82 
 hw/timer/m48t59-isa.c   | 180 +
 hw/timer/m48t59.c   | 228 +++-
 21 files changed, 304 insertions(+), 212 deletions(-)
 create mode 100644 hw/timer/m48t59-internal.h
 create mode 100644 hw/timer/m48t59-isa.c

-- 
2.7.4

[Qemu-devel] [PATCH 1/3] Split serial-isa into its own config option

[David Gibson]

At present, the core device model code for 8250-like serial ports
(serial.c) and the code for serial ports attached to ISA-style legacy IO
(serial-isa.c) are both controlled by the CONFIG_SERIAL variable.

There are lots and lots of embedded platforms that have 8250-like serial
ports but have never had anything resembling ISA legacy IO.  Therefore,
split serial-isa into its own CONFIG_SERIAL_ISA option so it can be
disabled for platforms where it's not appropriate.

For now, I enabled CONFIG_SERIAL_ISA in every default-config where
CONFIG_SERIAL is enabled, excepting microblaze, or32, and xtensa.  As best
as I can tell, those platforms never used legacy ISA, and also don't
include PCI support (which would allow connection of a PCI->ISA bridge
and/or a southbridge including legacy ISA serial ports).

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 
---
 default-configs/alpha-softmmu.mak   | 1 +
 default-configs/arm-softmmu.mak | 1 +
 default-configs/i386-softmmu.mak| 1 +
 default-configs/mips-softmmu-common.mak | 1 +
 default-configs/moxie-softmmu.mak   | 1 +
 default-configs/pci.mak | 1 +
 default-configs/ppc-softmmu.mak | 1 +
 default-configs/ppc64-softmmu.mak   | 1 +
 default-configs/ppcemb-softmmu.mak  | 1 +
 default-configs/sh4-softmmu.mak | 1 +
 default-configs/sh4eb-softmmu.mak   | 1 +
 default-configs/sparc64-softmmu.mak | 1 +
 default-configs/x86_64-softmmu.mak  | 1 +
 hw/char/Makefile.objs   | 3 ++-
 14 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/default-configs/alpha-softmmu.mak 
b/default-configs/alpha-softmmu.mak
index 7f6161e..e0d75e3 100644
--- a/default-configs/alpha-softmmu.mak
+++ b/default-configs/alpha-softmmu.mak
@@ -3,6 +3,7 @@
 include pci.mak
 include usb.mak
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_I8254=y
 CONFIG_PCKBD=y
 CONFIG_VGA_CIRRUS=y
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 6de3e16..dcbcea7 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -6,6 +6,7 @@ CONFIG_VGA=y
 CONFIG_NAND=y
 CONFIG_ECC=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PTIMER=y
 CONFIG_SD=y
 CONFIG_MAX7310=y
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 0b51360..3f2e820 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -15,6 +15,7 @@ CONFIG_IPMI_EXTERN=y
 CONFIG_ISA_IPMI_KCS=y
 CONFIG_ISA_IPMI_BT=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
diff --git a/default-configs/mips-softmmu-common.mak 
b/default-configs/mips-softmmu-common.mak
index 0394514..5b8b0c9 100644
--- a/default-configs/mips-softmmu-common.mak
+++ b/default-configs/mips-softmmu-common.mak
@@ -9,6 +9,7 @@ CONFIG_VGA_ISA_MM=y
 CONFIG_VGA_CIRRUS=y
 CONFIG_VMWARE_VGA=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
diff --git a/default-configs/moxie-softmmu.mak 
b/default-configs/moxie-softmmu.mak
index 1a95476..7e22863 100644
--- a/default-configs/moxie-softmmu.mak
+++ b/default-configs/moxie-softmmu.mak
@@ -2,4 +2,5 @@
 
 CONFIG_MC146818RTC=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_VGA=y
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index fff7ce3..d8d6548 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -27,6 +27,7 @@ CONFIG_AHCI=y
 CONFIG_ESP=y
 CONFIG_ESP_PCI=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_SERIAL_PCI=y
 CONFIG_IPACK=y
 CONFIG_WDT_IB6300ESB=y
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index d4d0f9b..13eb94f 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -45,5 +45,6 @@ CONFIG_PLATFORM_BUS=y
 CONFIG_ETSEC=y
 CONFIG_LIBDECNUMBER=y
 # For PReP
+CONFIG_SERIAL_ISA=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/ppc64-softmmu.mak 
b/default-configs/ppc64-softmmu.mak
index db5a4d6..7a3b10e 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -51,6 +51,7 @@ CONFIG_XICS=$(CONFIG_PSERIES)
 CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
 CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
 # For PReP
+CONFIG_SERIAL_ISA=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_MEM_HOTPLUG=y
diff --git a/default-configs/ppcemb-softmmu.mak 
b/default-configs/ppcemb-softmmu.mak
index 54acc4d..7f56004 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -5,6 +5,7 @@ include sound.mak
 include usb.mak
 CONFIG_M48T59=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_I8257=y
 CONFIG_OPENPIC=y
 CONFIG_PFLASH_CFI01=y
diff --git a/default-configs/sh4-softmmu.mak b/default-configs/sh4-softmmu.mak
index 8e00390..546d855 100644
--- a/default-configs/sh4-softmmu.mak
+++ b/default-configs/sh4-softmmu.mak
@@ -3,6 +3,7 @@
 include pci.mak
 

Re: [Qemu-devel] [PATCH v3 2/3] exec: rename cpu_exec_init() as cpu_exec_realizefn()

[David Gibson]

On Sat, Oct 15, 2016 at 12:52:48AM +0200, Laurent Vivier wrote:
> Modify all CPUs to call it from XXX_cpu_realizefn() function.
> 
> Remove all the cannot_destroy_with_object_finalize_yet as
> unsafe references have been moved to cpu_exec_realizefn().
> (tested with QOM command provided by commit 4c315c27)
> 
> for arm:
> 
> Setting of cpu->mp_affinity is moved from arm_cpu_initfn()
> to arm_cpu_realizefn() as setting of cpu_index is now done
> in cpu_exec_realizefn().
> 
> Signed-off-by: Laurent Vivier 

Reviewed-by: David Gibson 

> ---
>  exec.c  |  2 +-
>  include/exec/exec-all.h |  1 -
>  include/qom/cpu.h   |  1 +
>  target-alpha/cpu.c  | 15 +++
>  target-arm/cpu.c| 39 +--
>  target-cris/cpu.c   | 15 +++
>  target-i386/cpu.c   | 11 +--
>  target-lm32/cpu.c   | 15 +++
>  target-m68k/cpu.c   | 15 +++
>  target-microblaze/cpu.c | 14 +++---
>  target-mips/cpu.c   | 15 +++
>  target-moxie/cpu.c  | 15 +++
>  target-openrisc/cpu.c   | 15 +++
>  target-ppc/translate_init.c |  2 +-
>  target-s390x/cpu.c  |  8 +---
>  target-sh4/cpu.c| 15 +++
>  target-sparc/cpu.c  | 18 +-
>  target-tilegx/cpu.c | 15 +++
>  target-tricore/cpu.c| 15 +++
>  target-unicore32/cpu.c  | 18 +-
>  target-xtensa/cpu.c | 15 +++
>  21 files changed, 128 insertions(+), 151 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index d1e57c4..203eb52 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -634,7 +634,7 @@ void cpu_exec_initfn(CPUState *cpu)
>  #endif
>  }
>  
> -void cpu_exec_init(CPUState *cpu, Error **errp)
> +void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>  {
>  CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
>  
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 336a57c..9797d55 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -57,7 +57,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>uint32_t flags,
>int cflags);
>  
> -void cpu_exec_init(CPUState *cpu, Error **errp);
>  void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
>  void QEMU_NORETURN cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
>  
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index d7648a9..5520c6c 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -947,6 +947,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int 
> asidx);
>  void QEMU_NORETURN cpu_abort(CPUState *cpu, const char *fmt, ...)
>  GCC_FMT_ATTR(2, 3);
>  void cpu_exec_initfn(CPUState *cpu);
> +void cpu_exec_realizefn(CPUState *cpu, Error **errp);
>  void cpu_exec_exit(CPUState *cpu);
>  
>  #ifdef CONFIG_SOFTMMU
> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
> index 6d01d7f..30d77ce 100644
> --- a/target-alpha/cpu.c
> +++ b/target-alpha/cpu.c
> @@ -59,6 +59,13 @@ static void alpha_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>  {
>  CPUState *cs = CPU(dev);
>  AlphaCPUClass *acc = ALPHA_CPU_GET_CLASS(dev);
> +Error *local_err = NULL;
> +
> +cpu_exec_realizefn(cs, _err);
> +if (local_err != NULL) {
> +error_propagate(errp, local_err);
> +return;
> +}
>  
>  qemu_init_vcpu(cs);
>  
> @@ -266,7 +273,6 @@ static void alpha_cpu_initfn(Object *obj)
>  CPUAlphaState *env = >env;
>  
>  cs->env_ptr = env;
> -cpu_exec_init(cs, _abort);
>  tlb_flush(cs, 1);
>  
>  alpha_translate_init();
> @@ -309,13 +315,6 @@ static void alpha_cpu_class_init(ObjectClass *oc, void 
> *data)
>  cc->disas_set_info = alpha_cpu_disas_set_info;
>  
>  cc->gdb_num_core_regs = 67;
> -
> -/*
> - * Reason: alpha_cpu_initfn() calls cpu_exec_init(), which saves
> - * the object in cpus -> dangling pointer after final
> - * object_unref().
> - */
> -dc->cannot_destroy_with_object_finalize_yet = true;
>  }
>  
>  static const TypeInfo alpha_cpu_type_info = {
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 1b9540e..364a45d 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -441,22 +441,11 @@ static void arm_cpu_initfn(Object *obj)
>  CPUState *cs = CPU(obj);
>  ARMCPU *cpu = ARM_CPU(obj);
>  static bool inited;
> -uint32_t Aff1, Aff0;
>  
>  cs->env_ptr = >env;
> -cpu_exec_init(cs, _abort);
>  cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
>   g_free, g_free);
>  
> -/* This cpu-id-to-MPIDR affinity is used only for TCG; KVM will override 
> it.
> - * We don't support setting cluster ID ([16..23]) (known as Aff2
> - * in later ARM ARM versions), 

Re: [Qemu-devel] [PATCH v3 0/3] Split cpu_exec_init() into an init and a realize part

[David Gibson]

On Sat, Oct 15, 2016 at 12:52:46AM +0200, Laurent Vivier wrote:
> Since commit 42ecaba ("target-i386: Call cpu_exec_init() on realize"),
> , commit 6dd0f83 ("target-ppc: Move cpu_exec_init() call to realize 
> function"),
> and commit c6644fc ("s390x/cpu: Get rid of side effects when creating a 
> vcpu"),
> cpu_exec_init() has been moved to realize function for some architectures
> to implement CPU htoplug. This allows any failures from cpu_exec_init() to be
> handled appropriately.
> 
> This series tries to do the same work for all the other CPUs.
> 
> But as the ARM Virtual Machine ("virt") needs the "memory" property of the CPU
> in the machine init function (the "memory" property is created in
> cpu_exec_init() we want to move to the realize part), split cpu_exec_init() in
> two parts: a realize part (cpu_exec_realizefn(), adding the CPU in the
> environment) and an init part (cpu_exec_initfn(), initializing the CPU, like
> adding the "memory" property). To mirror the realize part, add an unrealize
> part, and remove the cpu_exec_exit() call from the finalize part.
> 
> This also allows to remove all the "cannot_destroy_with_object_finalize_yet"
> properties from the CPU device class.

This is looking good to me - the v3 re-org has made it quite a bit
easier to follow.

Whose tree should this go via?

> v3:
> - reorganize the series in 3 patches:
>   1- split cpu_exec_init() and call the init part from
>  cpu_common_initfn()
>   2- move all the cpu_exec_realize() calls to the
>  XXX_cpu_realizefn() functions
>   3- create a cpu_common_unrealizefn() function
>   and call the cpu_exec_unrealize() function from here,
>   except for ppc64 and i386 that have their own XX_cpu_unrealizefn()
>   functions
> - rename cpu_exec_init(), cpu_exec_realize(), cpu_exec_unrealize()
>   to cpu_exec_initfn(), cpu_exec_realizefn() and cpu_exec_unrealizefn(),
> - move cpu_exec_unrealizefn() to the device class unrealize
>   and declare it in qom/cpu.h instead of exec/exec-all.h
> 
> v2:
> - rename cpu_exec_exit() as cpu_exec_unrealize(),
>   as it un-does what cpu_exec_realize() does,
> - remove cpu_exec_exit() from ppc_cpu_unrealizefn() as
>   it is called from cpu_common_finalize(),
> - add a patch to move all cpu_exec_init() from
>   all XX_cpu_initfn() to cpu_common_initfn(),
> - arm: move setting of cpu->mp_affinity to
>   arm_cpu_realizefn() as the cpu_index is now set in
>   cpu_exec_realizefn().
> - update some commit messages
> 
> Laurent Vivier (3):
>   exec: split cpu_exec_init()
>   exec: rename cpu_exec_init() as cpu_exec_realizefn()
>   exec: call cpu_exec_exit() from a CPU unrealize common function
> 
>  exec.c  | 12 +++-
>  include/exec/exec-all.h |  1 -
>  include/qom/cpu.h   |  4 +++-
>  qom/cpu.c   | 10 +-
>  target-alpha/cpu.c  | 15 +++
>  target-arm/cpu.c| 39 +--
>  target-cris/cpu.c   | 15 +++
>  target-i386/cpu.c   | 13 +++--
>  target-lm32/cpu.c   | 15 +++
>  target-m68k/cpu.c   | 15 +++
>  target-microblaze/cpu.c | 14 +++---
>  target-mips/cpu.c   | 15 +++
>  target-moxie/cpu.c  | 15 +++
>  target-openrisc/cpu.c   | 15 +++
>  target-ppc/translate_init.c |  4 ++--
>  target-s390x/cpu.c  |  8 +---
>  target-sh4/cpu.c| 15 +++
>  target-sparc/cpu.c  | 18 +-
>  target-tilegx/cpu.c | 15 +++
>  target-tricore/cpu.c| 15 +++
>  target-unicore32/cpu.c  | 18 +-
>  target-xtensa/cpu.c | 15 +++
>  22 files changed, 148 insertions(+), 158 deletions(-)
> 

-- 
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 v3 1/3] exec: split cpu_exec_init()

[David Gibson]

On Sat, Oct 15, 2016 at 12:52:47AM +0200, Laurent Vivier wrote:
> Put in cpu_exec_initfn() what initializes the CPU,
> and let in cpu_exec_init() what adds it to the environment.
> 
> As cpu_exec_initfn() is called by all XX_cpu_initfn() call it
> directly in cpu_common_initfn().
> cpu_exec_init() is now a realize function, it will be renamed
> to cpu_exec_realizefn() and moved to the XX_cpu_realizefn()
> function in a following patch.
> 
> Signed-off-by: Laurent Vivier 

Reviewed-by: David Gibson 

> ---
>  exec.c| 10 ++
>  include/qom/cpu.h |  1 +
>  qom/cpu.c |  2 ++
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 374c364..d1e57c4 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -610,11 +610,8 @@ void cpu_exec_exit(CPUState *cpu)
>  }
>  }
>  
> -void cpu_exec_init(CPUState *cpu, Error **errp)
> +void cpu_exec_initfn(CPUState *cpu)
>  {
> -CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
> -Error *local_err ATTRIBUTE_UNUSED = NULL;
> -
>  cpu->as = NULL;
>  cpu->num_ases = 0;
>  
> @@ -635,6 +632,11 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
>  cpu->memory = system_memory;
>  object_ref(OBJECT(cpu->memory));
>  #endif
> +}
> +
> +void cpu_exec_init(CPUState *cpu, Error **errp)
> +{
> +CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
>  
>  cpu_list_add(cpu);
>  
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 6d481a1..d7648a9 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -946,6 +946,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int 
> asidx);
>  
>  void QEMU_NORETURN cpu_abort(CPUState *cpu, const char *fmt, ...)
>  GCC_FMT_ATTR(2, 3);
> +void cpu_exec_initfn(CPUState *cpu);
>  void cpu_exec_exit(CPUState *cpu);
>  
>  #ifdef CONFIG_SOFTMMU
> diff --git a/qom/cpu.c b/qom/cpu.c
> index c40f774..85f1132 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -362,6 +362,8 @@ static void cpu_common_initfn(Object *obj)
>  QTAILQ_INIT(>watchpoints);
>  
>  cpu->trace_dstate = bitmap_new(trace_get_vcpu_event_count());
> +
> +cpu_exec_initfn(cpu);
>  }
>  
>  static void cpu_common_finalize(Object *obj)

-- 
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 v3 3/3] exec: call cpu_exec_exit() from a CPU unrealize common function

[David Gibson]

On Sat, Oct 15, 2016 at 12:52:49AM +0200, Laurent Vivier wrote:
> As cpu_exec_exit() mirrors the cpu_exec_realizefn(),
> rename it as cpu_exec_unrealizefn().
> 
> Create and register a cpu_common_unrealizefn() function for
> the CPU device class and call cpu_exec_unrealizefn() from
> this function.
> 
> Remove cpu_exec_exit() from cpu_common_finalize()
> (which mirrors init, not realize), and as x86_cpu_unrealizefn()
> overwrites the device class unrealize function, add a call to
> cpu_exec_unrealizefn() (as in ppc_cpu_unrealizefn()).
> 
> Signed-off-by: Laurent Vivier 

Reviewed-by: David Gibson 

> ---
>  exec.c  | 2 +-
>  include/qom/cpu.h   | 2 +-
>  qom/cpu.c   | 8 +++-
>  target-i386/cpu.c   | 2 ++
>  target-ppc/translate_init.c | 2 +-
>  5 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 203eb52..3cd25db 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -596,7 +596,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int 
> asidx)
>  }
>  #endif
>  
> -void cpu_exec_exit(CPUState *cpu)
> +void cpu_exec_unrealizefn(CPUState *cpu)
>  {
>  CPUClass *cc = CPU_GET_CLASS(cpu);
>  
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 5520c6c..633c3fc 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -948,7 +948,7 @@ void QEMU_NORETURN cpu_abort(CPUState *cpu, const char 
> *fmt, ...)
>  GCC_FMT_ATTR(2, 3);
>  void cpu_exec_initfn(CPUState *cpu);
>  void cpu_exec_realizefn(CPUState *cpu, Error **errp);
> -void cpu_exec_exit(CPUState *cpu);
> +void cpu_exec_unrealizefn(CPUState *cpu);
>  
>  #ifdef CONFIG_SOFTMMU
>  extern const struct VMStateDescription vmstate_cpu_common;
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 85f1132..03d9190 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -345,6 +345,12 @@ static void cpu_common_realizefn(DeviceState *dev, Error 
> **errp)
>  trace_init_vcpu(cpu);
>  }
>  
> +static void cpu_common_unrealizefn(DeviceState *dev, Error **errp)
> +{
> +CPUState *cpu = CPU(dev);
> +cpu_exec_unrealizefn(cpu);
> +}
> +
>  static void cpu_common_initfn(Object *obj)
>  {
>  CPUState *cpu = CPU(obj);
> @@ -369,7 +375,6 @@ static void cpu_common_initfn(Object *obj)
>  static void cpu_common_finalize(Object *obj)
>  {
>  CPUState *cpu = CPU(obj);
> -cpu_exec_exit(cpu);
>  g_free(cpu->trace_dstate);
>  }
>  
> @@ -403,6 +408,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
>  k->cpu_exec_exit = cpu_common_noop;
>  k->cpu_exec_interrupt = cpu_common_exec_interrupt;
>  dc->realize = cpu_common_realizefn;
> +dc->unrealize = cpu_common_unrealizefn;
>  /*
>   * Reason: CPUs still need special care by board code: wiring up
>   * IRQs, adding reset handlers, halting non-first CPUs, ...
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 3476d46..399a3e4 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -3253,6 +3253,8 @@ static void x86_cpu_unrealizefn(DeviceState *dev, Error 
> **errp)
>  object_unparent(OBJECT(cpu->apic_state));
>  cpu->apic_state = NULL;
>  }
> +
> +cpu_exec_unrealizefn(CPU(dev));
>  }
>  
>  typedef struct BitProperty {
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 40dae70..2de6a06 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9910,7 +9910,7 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error 
> **errp)
>  opc_handler_t **table, **table_2;
>  int i, j, k;
>  
> -cpu_exec_exit(CPU(dev));
> +cpu_exec_unrealizefn(CPU(dev));
>  
>  for (i = 0; i < PPC_CPU_OPCODES_LEN; i++) {
>  if (env->opcodes[i] == _handler) {

-- 
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] [PATCH 14/15] target-ppc: Use tcg_gen_extract_*

[David Gibson]

On Sat, Oct 15, 2016 at 08:37:49PM -0700, Richard Henderson wrote:
> Use the new primitives for RDWINM and RLDICL.
> 
> Cc: qemu-...@nongnu.org
> Signed-off-by: Richard Henderson 

Applied to ppc-for-2.8, thanks.

> ---
>  target-ppc/translate.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index bfc1301..724d95c 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -1977,9 +1977,8 @@ static void gen_rlwinm(DisasContext *ctx)
>  if (mb == 0 && me == (31 - sh)) {
>  tcg_gen_shli_tl(t_ra, t_rs, sh);
>  tcg_gen_ext32u_tl(t_ra, t_ra);
> -} else if (sh != 0 && me == 31 && sh == (32 - mb)) {
> -tcg_gen_ext32u_tl(t_ra, t_rs);
> -tcg_gen_shri_tl(t_ra, t_ra, mb);
> +} else if (me == 31 && (me - mb + 1) + sh <= 32) {
> +tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
>  } else {
>  target_ulong mask;
>  #if defined(TARGET_PPC64)
> @@ -2094,8 +2093,8 @@ static void gen_rldinm(DisasContext *ctx, int mb, int 
> me, int sh)
>  
>  if (sh != 0 && mb == 0 && me == (63 - sh)) {
>  tcg_gen_shli_tl(t_ra, t_rs, sh);
> -} else if (sh != 0 && me == 63 && sh == (64 - mb)) {
> -tcg_gen_shri_tl(t_ra, t_rs, mb);
> +} else if (me == 63 && (me - mb + 1) + sh <= 64) {
> +tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
>  } else {
>  tcg_gen_rotli_tl(t_ra, t_rs, sh);
>  tcg_gen_andi_tl(t_ra, t_ra, MASK(mb, me));

-- 
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] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP

[David Gibson]

On Tue, Oct 11, 2016 at 06:18:30PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> VMSTATE_WITH_TMP is for handling structures where some calculation
> or rearrangement of the data needs to be performed before the data
> hits the wire.
> For example,  where the value on the wire is an offset from a
> non-migrated base, but the data in the structure is the actual pointer.
> 
> To use it, a temporary type is created and a vmsd used on that type.
> The first element of the type must be 'parent' a pointer back to the
> type of the main structure.  VMSTATE_WITH_TMP takes care of allocating
> and freeing the temporary before running the child vmsd.
> 
> The post_load/pre_save on the child vmsd can copy things from the parent
> to the temporary using the parent pointer and do any other calculations
> needed; it can then use normal VMSD entries to do the actual data
> storage without having to fiddle around with qemu_get_*/qemu_put_*
> 
> Signed-off-by: Dr. David Alan Gilbert 

The requirement for the parent pointer is a little clunky, but I don't
quickly see a better way, and it is compile-time verified.  As noted
elsewhere I think this is a really useful approach which could allow a
bunch of internal state cleanups while preserving migration.

Reviewed-by: David Gibson 

> ---
>  include/migration/vmstate.h | 20 
>  migration/vmstate.c | 38 ++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 9500da1..efb0e90 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -259,6 +259,7 @@ extern const VMStateInfo vmstate_info_cpudouble;
>  extern const VMStateInfo vmstate_info_timer;
>  extern const VMStateInfo vmstate_info_buffer;
>  extern const VMStateInfo vmstate_info_unused_buffer;
> +extern const VMStateInfo vmstate_info_tmp;
>  extern const VMStateInfo vmstate_info_bitmap;
>  extern const VMStateInfo vmstate_info_qtailq;
>  
> @@ -651,6 +652,25 @@ extern const VMStateInfo vmstate_info_qtailq;
>  .offset = offsetof(_state, _field),  \
>  }
>  
> +/* Allocate a temporary of type 'tmp_type', set tmp->parent to _state
> + * and execute the vmsd on the temporary.  Note that we're working with
> + * the whole of _state here, not a field within it.
> + * We compile time check that:
> + *That _tmp_type contains a 'parent' member that's a pointer to the
> + *'_state' type
> + *That the pointer is right at the start of _tmp_type.
> + */
> +#define VMSTATE_WITH_TMP(_state, _tmp_type, _vmsd) { \
> +.name = "tmp",   \
> +.size = sizeof(_tmp_type) +  \
> +QEMU_BUILD_BUG_EXPR(offsetof(_tmp_type, parent) != 0) + \
> +type_check_pointer(_state,   \
> +typeof_field(_tmp_type, parent)),\
> +.vmsd = &(_vmsd),\
> +.info = _info_tmp,   \
> +.flags= VMS_LINKED,  \
> +}
> +
>  #define VMSTATE_UNUSED_BUFFER(_test, _version, _size) {  \
>  .name = "unused",\
>  .field_exists = (_test), \
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 2157997..f2563c5 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -925,6 +925,44 @@ const VMStateInfo vmstate_info_unused_buffer = {
>  .put  = put_unused_buffer,
>  };
>  
> +/* vmstate_info_tmp, see VMSTATE_WITH_TMP, the idea is that we allocate
> + * a temporary buffer and the pre_load/pre_save methods in the child vmsd
> + * copy stuff from the parent into the child and do calculations to fill
> + * in fields that don't really exist in the parent but need to be in the
> + * stream.
> + */
> +static int get_tmp(QEMUFile *f, void *pv, size_t size, VMStateField *field)
> +{
> +int ret;
> +const VMStateDescription *vmsd = field->vmsd;
> +int version_id = field->version_id;
> +void *tmp = g_malloc(size);
> +
> +/* Writes the parent field which is at the start of the tmp */
> +*(void **)tmp = pv;
> +ret = vmstate_load_state(f, vmsd, tmp, version_id);
> +g_free(tmp);
> +return ret;
> +}
> +
> +static void put_tmp(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> +QJSON *vmdesc)
> +{
> +const VMStateDescription *vmsd = field->vmsd;
> +void *tmp = g_malloc(size);
> +
> +/* Writes the parent field which is at the start of the tmp */
> +*(void **)tmp = pv;
> +vmstate_save_state(f, vmsd, tmp, vmdesc);
> +g_free(tmp);
> +}
> +
> +const 

Re: [Qemu-devel] [very-WIP 3/4] slirp: VMStatify sbuf

[David Gibson]

On Tue, Oct 11, 2016 at 06:18:32PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Convert the sbuf structure to a VMStateDescription.
> Note this uses the VMSTATE_WITH_TMP mechanism to calculate
> and reload the offsets based on the pointers.
> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: David Gibson 

> ---
>  slirp/sbuf.h  |   4 +-
>  slirp/slirp.c | 116 
> ++
>  2 files changed, 78 insertions(+), 42 deletions(-)
> 
> diff --git a/slirp/sbuf.h b/slirp/sbuf.h
> index efcec39..a722ecb 100644
> --- a/slirp/sbuf.h
> +++ b/slirp/sbuf.h
> @@ -12,8 +12,8 @@
>  #define sbspace(sb) ((sb)->sb_datalen - (sb)->sb_cc)
>  
>  struct sbuf {
> - u_int   sb_cc;  /* actual chars in buffer */
> - u_int   sb_datalen; /* Length of data  */
> + uint32_t sb_cc; /* actual chars in buffer */
> + uint32_t sb_datalen;/* Length of data  */
>   char*sb_wptr;   /* write pointer. points to where the next
>* bytes should be written in the sbuf */
>   char*sb_rptr;   /* read pointer. points to where the next
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 6276315..2f7802e 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -1185,19 +1185,72 @@ static const VMStateDescription vmstate_slirp_tcp = {
>  }
>  };
>  
> -static void slirp_sbuf_save(QEMUFile *f, struct sbuf *sbuf)
> +/* The sbuf has a pair of pointers that are migrated as offsets;
> + * we calculate the offsets and restore the pointers using
> + * pre_save/post_load on a tmp structure.
> + */
> +struct sbuf_tmp {
> +struct sbuf *parent;
> +uint32_t roff, woff;
> +};
> +
> +static void sbuf_tmp_pre_save(void *opaque)
> +{
> +struct sbuf_tmp *tmp = opaque;
> +tmp->woff = tmp->parent->sb_wptr - tmp->parent->sb_data;
> +tmp->roff = tmp->parent->sb_rptr - tmp->parent->sb_data;
> +}
> +
> +static int sbuf_tmp_post_load(void *opaque, int version)
>  {
> -uint32_t off;
> -
> -qemu_put_be32(f, sbuf->sb_cc);
> -qemu_put_be32(f, sbuf->sb_datalen);
> -off = (uint32_t)(sbuf->sb_wptr - sbuf->sb_data);
> -qemu_put_sbe32(f, off);
> -off = (uint32_t)(sbuf->sb_rptr - sbuf->sb_data);
> -qemu_put_sbe32(f, off);
> -qemu_put_buffer(f, (unsigned char*)sbuf->sb_data, sbuf->sb_datalen);
> +struct sbuf_tmp *tmp = opaque;
> +uint32_t requested_len = tmp->parent->sb_datalen;
> +
> +/* Allocate the buffer space used by the field after the tmp */
> +sbreserve(tmp->parent, tmp->parent->sb_datalen);
> +
> +if (tmp->parent->sb_datalen != requested_len) {
> +return -ENOMEM;
> +}
> +if (tmp->woff >= requested_len ||
> +tmp->roff >= requested_len) {
> +error_report("invalid sbuf offsets r/w=%u/%u len=%u",
> + tmp->roff, tmp->woff, requested_len);
> +return -EINVAL;
> +}
> +
> +tmp->parent->sb_wptr = tmp->parent->sb_data + tmp->woff;
> +tmp->parent->sb_rptr = tmp->parent->sb_data + tmp->roff;
> +
> +return 0;
>  }
>  
> +
> +static const VMStateDescription vmstate_slirp_sbuf_tmp = {
> +.name = "slirp-sbuf-tmp",
> +.post_load = sbuf_tmp_post_load,
> +.pre_save  = sbuf_tmp_pre_save,
> +.version_id = 0,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT32(woff, struct sbuf_tmp),
> +VMSTATE_UINT32(roff, struct sbuf_tmp),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> +static const VMStateDescription vmstate_slirp_sbuf = {
> +.name = "slirp-sbuf",
> +.version_id = 0,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT32(sb_cc, struct sbuf),
> +VMSTATE_UINT32(sb_datalen, struct sbuf),
> +VMSTATE_WITH_TMP(struct sbuf, struct sbuf_tmp, 
> vmstate_slirp_sbuf_tmp),
> +VMSTATE_VBUFFER_UINT32(sb_data, struct sbuf, 0, NULL, 0, sb_datalen),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> +
>  static void slirp_socket_save(QEMUFile *f, struct socket *so)
>  {
>  qemu_put_be32(f, so->so_urgc);
> @@ -1225,8 +1278,9 @@ static void slirp_socket_save(QEMUFile *f, struct 
> socket *so)
>  qemu_put_byte(f, so->so_emu);
>  qemu_put_byte(f, so->so_type);
>  qemu_put_be32(f, so->so_state);
> -slirp_sbuf_save(f, >so_rcv);
> -slirp_sbuf_save(f, >so_snd);
> +/* TODO: Build vmstate at this level */
> +vmstate_save_state(f, _slirp_sbuf, >so_rcv, 0);
> +vmstate_save_state(f, _slirp_sbuf, >so_snd, 0);
>  vmstate_save_state(f, _slirp_tcp, so->so_tcpcb, 0);
>  }
>  
> @@ -1263,31 +1317,9 @@ static void slirp_state_save(QEMUFile *f, void *opaque)
>  slirp_bootp_save(f, slirp);
>  }
>  
> -static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
> -{
> -uint32_t off, sb_cc, sb_datalen;
> -
> -sb_cc = qemu_get_be32(f);
> -sb_datalen = qemu_get_be32(f);
> -
> -sbreserve(sbuf, 

Re: [Qemu-devel] [very-WIP 2/7] tests/migration: Add test for VMSTATE_WITH_TMP

[David Gibson]

On Tue, Oct 11, 2016 at 06:18:31PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Add a test for VMSTATE_WITH_TMP to tests/test-vmstate.c
> 
> Signed-off-by: Dr. David Alan Gilbert 

Looks reasonable.  The other obvious use case to me - which involves a
little less awkwardness with command line supplied values - is
changing an internal value to an equivalent-but-scaled representation.
e.g. changing from size (in bytes, but must be page aligned), to
number of pages.

> ---
>  tests/test-vmstate.c | 97 
> +---
>  1 file changed, 93 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index d8da26f..203ab4a 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -83,7 +83,7 @@ static void save_vmstate(const VMStateDescription *desc, 
> void *obj)
>  qemu_fclose(f);
>  }
>  
> -static void compare_vmstate(uint8_t *wire, size_t size)
> +static void compare_vmstate(const uint8_t *wire, size_t size)
>  {
>  QEMUFile *f = open_test_file(false);
>  uint8_t result[size];
> @@ -106,7 +106,7 @@ static void compare_vmstate(uint8_t *wire, size_t size)
>  }
>  
>  static int load_vmstate_one(const VMStateDescription *desc, void *obj,
> -int version, uint8_t *wire, size_t size)
> +int version, const uint8_t *wire, size_t size)
>  {
>  QEMUFile *f;
>  int ret;
> @@ -130,7 +130,7 @@ static int load_vmstate_one(const VMStateDescription 
> *desc, void *obj,
>  static int load_vmstate(const VMStateDescription *desc,
>  void *obj, void *obj_clone,
>  void (*obj_copy)(void *, void*),
> -int version, uint8_t *wire, size_t size)
> +int version, const uint8_t *wire, size_t size)
>  {
>  /* We test with zero size */
>  obj_copy(obj_clone, obj);
> @@ -282,7 +282,6 @@ static void test_simple_primitive(void)
>  FIELD_EQUAL(i64_1);
>  FIELD_EQUAL(i64_2);
>  }
> -#undef FIELD_EQUAL
>  
>  typedef struct TestStruct {
>  uint32_t a, b, c, e;
> @@ -475,6 +474,95 @@ static void test_load_skip(void)
>  qemu_fclose(loading);
>  }
>  
> +typedef struct TmpTestStruct {
> +TestStruct *parent;
> +int64_t diff;
> +} TmpTestStruct;
> +
> +static void tmp_child_pre_save(void *opaque)
> +{
> +struct TmpTestStruct *tts = opaque;
> +
> +tts->diff = tts->parent->b - tts->parent->a;
> +}
> +
> +static int tmp_child_post_load(void *opaque, int version_id)
> +{
> +struct TmpTestStruct *tts = opaque;
> +
> +tts->parent->b = tts->parent->a + tts->diff;
> +
> +return 0;
> +}
> +
> +static const VMStateDescription vmstate_tmp_back_to_parent = {
> +.name = "test/tmp_child_parent",
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT64(f, TestStruct),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> +static const VMStateDescription vmstate_tmp_child = {
> +.name = "test/tmp_child",
> +.pre_save = tmp_child_pre_save,
> +.post_load = tmp_child_post_load,
> +.fields = (VMStateField[]) {
> +VMSTATE_INT64(diff, TmpTestStruct),
> +VMSTATE_STRUCT_POINTER(parent, TmpTestStruct,
> +   vmstate_tmp_back_to_parent, TestStruct),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> +static const VMStateDescription vmstate_with_tmp = {
> +.name = "test/with_tmp",
> +.version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT32(a, TestStruct),
> +VMSTATE_UINT64(d, TestStruct),
> +VMSTATE_WITH_TMP(TestStruct, TmpTestStruct, vmstate_tmp_child),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> +static void obj_tmp_copy(void *target, void *source)
> +{
> +memcpy(target, source, sizeof(TestStruct));
> +}
> +
> +static void test_tmp_struct(void)
> +{
> +TestStruct obj, obj_clone;
> +
> +uint8_t const wire_with_tmp[] = {
> +/* u32 a */ 0x00, 0x00, 0x00, 0x02,
> +/* u64 d */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
> +/* diff  */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
> +/* u64 f */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08,
> +QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely 
> */
> +};
> +
> +memset(, 0, sizeof(obj));
> +obj.a = 2;
> +obj.b = 4;
> +obj.d = 1;
> +obj.f = 8;
> +save_vmstate(_with_tmp, );
> +
> +compare_vmstate(wire_with_tmp, sizeof(wire_with_tmp));
> +
> +memset(, 0, sizeof(obj));
> +fprintf(stderr, "test_tmp_struct load\n");
> +SUCCESS(load_vmstate(_with_tmp, , _clone,
> + obj_tmp_copy, 1, wire_with_tmp,
> + sizeof(wire_with_tmp)));
> +g_assert_cmpint(obj.a, ==, 2); /* From top level vmsd */
> +g_assert_cmpint(obj.b, ==, 4); /* from the post_load */
> +

Re: [Qemu-devel] [PATCH v6 13/35] tcg: Add atomic helpers

[Emilio G. Cota]

On Sun, Oct 16, 2016 at 18:40:05 -0700, Richard Henderson wrote:
> On 10/16/2016 03:17 PM, Emilio G. Cota wrote:
> >>+#if DATA_SIZE == 1
> >>> +# define END
> >>> +#elif defined(HOST_WORDS_BIGENDIAN)
> >>> +# define END  _be
> >>> +#else
> >>> +# define END  _le
> >>> +#endif
> >It took me a while to figure out that ATOMIC_NAME needs END (ATOMIC_NAME
> >is defined later in the patch).
> >
> >It might be clearer to pass it explicitly as a suffix in the macro, as in
> >#define ATOMIC_NAME(name, suffix) to then have ATOMIC_NAME(cmpxchg, END).
> 
> Hum.  Perhaps a comment?

Yep that would do as well.

Emilio

Re: [Qemu-devel] [PULL 00/16] ppc-for-2.8 queue 20161017

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PULL 00/16] ppc-for-2.8 queue 20161017
Type: series
Message-id: 1476672219-8836-1-git-send-email-da...@gibson.dropbear.id.au

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

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

# Useful git options
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 show --no-patch --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
Switched to a new branch 'test'
aefdf38 spapr: Improved placement of PCI host bridges in guest memory map
3dfa5b2 spapr_pci: Add a 64-bit MMIO window
bfc637b spapr: Adjust placement of PCI host bridge to allow > 1TiB RAM
e7457d3 spapr_pci: Delegate placement of PCI host bridges to machine type
2598f30 libqos: Limit spapr-pci to 32-bit MMIO for now
49bf88f libqos: Correct error in PCI hole sizing for spapr
7ff1481 libqos: Isolate knowledge of spapr memory map to qpci_init_spapr()
dfee107 ppc/xics: Split ICS into ics-base and ics class
87bd979 ppc/xics: Make the ICSState a list
0068b41 spapr: fix inheritance chain for default machine options
8075496 target-ppc: implement vexts[bh]2w and vexts[bhw]2d
c8aeb3f tests/boot-sector: Increase time-out to 90 seconds
67bc2e5 tests/boot-sector: Use mkstemp() to create a unique file name
c3e6be1 tests/boot-sector: Use minimum length for the Forth boot script
d45743f qtest: ask endianness of the target in qtest_init()
fe0631e tests: minor cleanups in usb-hcd-uhci-test

=== OUTPUT BEGIN ===
Checking PATCH 1/16: tests: minor cleanups in usb-hcd-uhci-test...
Checking PATCH 2/16: qtest: ask endianness of the target in qtest_init()...
Checking PATCH 3/16: tests/boot-sector: Use minimum length for the Forth boot 
script...
Checking PATCH 4/16: tests/boot-sector: Use mkstemp() to create a unique file 
name...
Checking PATCH 5/16: tests/boot-sector: Increase time-out to 90 seconds...
Checking PATCH 6/16: target-ppc: implement vexts[bh]2w and vexts[bhw]2d...
Checking PATCH 7/16: spapr: fix inheritance chain for default machine options...
Checking PATCH 8/16: ppc/xics: Make the ICSState a list...
Checking PATCH 9/16: ppc/xics: Split ICS into ics-base and ics class...
Checking PATCH 10/16: libqos: Isolate knowledge of spapr memory map to 
qpci_init_spapr()...
Checking PATCH 11/16: libqos: Correct error in PCI hole sizing for spapr...
Checking PATCH 12/16: libqos: Limit spapr-pci to 32-bit MMIO for now...
Checking PATCH 13/16: spapr_pci: Delegate placement of PCI host bridges to 
machine type...
Checking PATCH 14/16: spapr: Adjust placement of PCI host bridge to allow > 
1TiB RAM...
Checking PATCH 15/16: spapr_pci: Add a 64-bit MMIO window...
ERROR: trailing whitespace
#237: FILE: include/hw/ppc/spapr.h:44:
+  uint64_t *buid, hwaddr *pio, $

total: 1 errors, 0 warnings, 170 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 16/16: spapr: Improved placement of PCI host bridges in guest 
memory map...
WARNING: line over 80 characters
#99: FILE: hw/ppc/spapr.c:2401:
+QEMU_BUILD_BUG_ON((SPAPR_PCI_MEM64_WIN_SIZE % SPAPR_PCI_MEM32_WIN_SIZE) != 
0);

WARNING: line over 80 characters
#102: FILE: hw/ppc/spapr.c:2404:
+QEMU_BUILD_BUG_ON((max_phbs * SPAPR_PCI_IO_WIN_SIZE) > 
SPAPR_PCI_MEM32_WIN_SIZE);

WARNING: line over 80 characters
#103: FILE: hw/ppc/spapr.c:2405:
+QEMU_BUILD_BUG_ON((max_phbs * SPAPR_PCI_MEM32_WIN_SIZE) > 
SPAPR_PCI_MEM64_WIN_SIZE);

ERROR: Macros with multiple statements should be enclosed in a do - while loop
#137: FILE: hw/ppc/spapr.c:2524:
+#define SPAPR_COMPAT_2_7\
+HW_COMPAT_2_7   \
+{   \
+.driver   = TYPE_SPAPR_PCI_HOST_BRIDGE, \
+.property = "mem_win_size", \
+.value= stringify(SPAPR_PCI_2_7_MMIO_WIN_SIZE),\
+},  \
+{   \
+.driver   = TYPE_SPAPR_PCI_HOST_BRIDGE, \
+.property = "mem64_win_size",   \
+.value= "0",\
+},

total: 1 errors, 3 warnings, 221 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback 

Re: [Qemu-devel] [PATCH 07/11] spapr: add hotplug interrupt machine options

[Bharata B Rao]

On Fri, Oct 14, 2016 at 01:04:37PM -0500, Michael Roth wrote:
> Quoting Bharata B Rao (2016-10-14 03:37:32)
> > On Wed, Oct 12, 2016 at 06:13:55PM -0500, Michael Roth wrote:
> > > This adds machine options of the form:
> > > 
> > >   -machine pseries,legacy-hotplug-events=true
> > >   -machine pseries,legacy-hotplug-events=false
> > > 
> > > to denote whether or not we wish to force the use of "legacy" style
> > > hotplug events, which are surfaced through EPOW interrupts instead of
> > > a dedicated interrupt source, and lack certain features necessary,
> > > mainly, for memory unplug support.
> > > 
> > > If false, QEMU will default to "legacy" style unless the guest
> > > advertises support for the newer events via
> > > ibm,client-architecture-support hcall during early boot.
> > > 
> > > For pseries-2.7 and earlier we default to true, for newer machine
> > > types we default to false.
> > > 
> > > Signed-off-by: Michael Roth 
> > > ---
> > >  hw/ppc/spapr.c  | 31 +++
> > >  include/hw/ppc/spapr.h  |  1 +
> > >  include/hw/ppc/spapr_ovec.h |  1 +
> > >  3 files changed, 33 insertions(+)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index f8cde92..d80a6fa 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -1816,6 +1816,11 @@ static void ppc_spapr_init(MachineState *machine)
> > > 
> > >  spapr_ovec_set(spapr->ov5, OV5_FORM1_AFFINITY);
> > > 
> > > +/* use dedicated HP event source if guest supports it */
> > > +if (spapr->use_hotplug_event_source) {
> > > +spapr_ovec_set(spapr->ov5, OV5_HP_EVT);
> > 
> > The above comment can be confusing. Here you really mean that
> > the machine type version supports OV5_HP_EVT right ? Because
> > guest support for the same is determined during cas call later.
> 
> What trying to get it across that support would only be enabled if the
> guest indicates support for it later. What about something like:
> 
> /* advertise support for dedicated HP event source to guests */

Sounds good. Thanks.

> > 
> > Regards,
> > Bharata.

[Qemu-devel] [PULL 08/16] ppc/xics: Make the ICSState a list

[David Gibson]

From: Benjamin Herrenschmidt 

Instead of an array of fixed sized blocks, use a list, as we will need
to have sources with variable number of interrupts. SPAPR only uses
a single entry. Native will create more. If performance becomes an
issue we can add some hashed lookup but for now this will do fine.

Signed-off-by: Benjamin Herrenschmidt 
[ move the initialization of list to xics_common_initfn,
  restore xirr_owner after migration and move restoring to
  icp_post_load]
Signed-off-by: Nikunj A Dadhania 
[ clg: removed the icp_post_load() changes from nikunj patchset v3:
   http://patchwork.ozlabs.org/patch/646008/ ]
Signed-off-by: Cédric Le Goater 
Signed-off-by: David Gibson 
---
 hw/intc/trace-events  |  5 +--
 hw/intc/xics.c| 83 
 hw/intc/xics_kvm.c| 27 +++-
 hw/intc/xics_spapr.c  | 88 +--
 hw/ppc/spapr_events.c |  2 +-
 hw/ppc/spapr_pci.c|  5 ++-
 hw/ppc/spapr_vio.c|  2 +-
 include/hw/ppc/xics.h | 13 
 8 files changed, 139 insertions(+), 86 deletions(-)

diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index f12192c..aa342a8 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -56,10 +56,11 @@ xics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d 
[irq %#x]"
 xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority) 
"ics_write_xive: irq %#x [src %d] server %#x prio %#x"
 xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]"
 xics_ics_eoi(int nr) "ics_eoi: irq %#x"
-xics_alloc(int src, int irq) "source#%d, irq %d"
-xics_alloc_block(int src, int first, int num, bool lsi, int align) "source#%d, 
first irq %d, %d irqs, lsi=%d, alignnum %d"
+xics_alloc(int irq) "irq %d"
+xics_alloc_block(int first, int num, bool lsi, int align) "first irq %d, %d 
irqs, lsi=%d, alignnum %d"
 xics_ics_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs"
 xics_ics_free_warn(int src, int irq) "Source#%d, irq %d is already free"
+xics_icp_post_load(uint32_t server_no, uint32_t xirr, uint64_t addr, uint8_t 
pend) "server_no %d, xirr %#x, xirr_owner 0x%" PRIx64 ", pending %d"
 
 # hw/intc/s390_flic_kvm.c
 flic_create_device(int err) "flic: create device failed %d"
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 69162f0..433af93 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -96,13 +96,16 @@ void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
 static void xics_common_reset(DeviceState *d)
 {
 XICSState *xics = XICS_COMMON(d);
+ICSState *ics;
 int i;
 
 for (i = 0; i < xics->nr_servers; i++) {
 device_reset(DEVICE(>ss[i]));
 }
 
-device_reset(DEVICE(xics->ics));
+QLIST_FOREACH(ics, >ics, list) {
+device_reset(DEVICE(ics));
+}
 }
 
 static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
@@ -134,7 +137,6 @@ static void xics_prop_set_nr_irqs(Object *obj, Visitor *v, 
const char *name,
 }
 
 assert(info->set_nr_irqs);
-assert(xics->ics);
 info->set_nr_irqs(xics, value, errp);
 }
 
@@ -174,6 +176,9 @@ static void xics_prop_set_nr_servers(Object *obj, Visitor 
*v,
 
 static void xics_common_initfn(Object *obj)
 {
+XICSState *xics = XICS_COMMON(obj);
+
+QLIST_INIT(>ics);
 object_property_add(obj, "nr_irqs", "int",
 xics_prop_get_nr_irqs, xics_prop_set_nr_irqs,
 NULL, NULL, NULL);
@@ -212,33 +217,35 @@ static void ics_reject(ICSState *ics, int nr);
 static void ics_resend(ICSState *ics);
 static void ics_eoi(ICSState *ics, int nr);
 
-static void icp_check_ipi(XICSState *xics, int server)
+static void icp_check_ipi(ICPState *ss)
 {
-ICPState *ss = xics->ss + server;
-
 if (XISR(ss) && (ss->pending_priority <= ss->mfrr)) {
 return;
 }
 
-trace_xics_icp_check_ipi(server, ss->mfrr);
+trace_xics_icp_check_ipi(ss->cs->cpu_index, ss->mfrr);
 
-if (XISR(ss)) {
-ics_reject(xics->ics, XISR(ss));
+if (XISR(ss) && ss->xirr_owner) {
+ics_reject(ss->xirr_owner, XISR(ss));
 }
 
 ss->xirr = (ss->xirr & ~XISR_MASK) | XICS_IPI;
 ss->pending_priority = ss->mfrr;
+ss->xirr_owner = NULL;
 qemu_irq_raise(ss->output);
 }
 
 static void icp_resend(XICSState *xics, int server)
 {
 ICPState *ss = xics->ss + server;
+ICSState *ics;
 
 if (ss->mfrr < CPPR(ss)) {
-icp_check_ipi(xics, server);
+icp_check_ipi(ss);
+}
+QLIST_FOREACH(ics, >ics, list) {
+ics_resend(ics);
 }
-ics_resend(xics->ics);
 }
 
 void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
@@ -256,7 +263,10 @@ void icp_set_cppr(XICSState *xics, int server, uint8_t 
cppr)
 ss->xirr &= ~XISR_MASK; /* Clear XISR */
 ss->pending_priority = 0xff;
 qemu_irq_lower(ss->output);
-  

[Qemu-devel] [PULL 09/16] ppc/xics: Split ICS into ics-base and ics class

[David Gibson]

From: Benjamin Herrenschmidt 

The existing implementation remains same and ics-base is introduced. The
type name "ics" is retained, and all the related functions renamed as
ics_simple_*

This will allow different implementations for the source controllers
such as the MSI support of PHB3 on Power8 which uses in-memory state
tables for example.

Signed-off-by: Benjamin Herrenschmidt 
Signed-off-by: Nikunj A Dadhania 
Reviewed-by: David Gibson 
[ clg: added ICS_BASE_GET_CLASS and related fixes, based on :
   http://patchwork.ozlabs.org/patch/646010/ ]
Signed-off-by: Cédric Le Goater 
Signed-off-by: David Gibson 
---
 hw/intc/trace-events  |  10 ++--
 hw/intc/xics.c| 150 +++---
 hw/intc/xics_kvm.c|  12 ++--
 hw/intc/xics_spapr.c  |  30 +-
 include/hw/ppc/xics.h |  27 +
 5 files changed, 138 insertions(+), 91 deletions(-)

diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index aa342a8..a367b46 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -50,12 +50,12 @@ xics_icp_accept(uint32_t old_xirr, uint32_t new_xirr) 
"icp_accept: XIRR %#"PRIx3
 xics_icp_eoi(int server, uint32_t xirr, uint32_t new_xirr) "icp_eoi: server %d 
given XIRR %#"PRIx32" new XIRR %#"PRIx32
 xics_icp_irq(int server, int nr, uint8_t priority) "cpu %d trying to deliver 
irq %#"PRIx32" priority %#x"
 xics_icp_raise(uint32_t xirr, uint8_t pending_priority) "raising IRQ new 
XIRR=%#x new pending priority=%#x"
-xics_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq %#x]"
+xics_ics_simple_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq 
%#x]"
 xics_masked_pending(void) "set_irq_msi: masked pending"
-xics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq %#x]"
-xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority) 
"ics_write_xive: irq %#x [src %d] server %#x prio %#x"
-xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]"
-xics_ics_eoi(int nr) "ics_eoi: irq %#x"
+xics_ics_simple_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq 
%#x]"
+xics_ics_simple_write_xive(int nr, int srcno, int server, uint8_t priority) 
"ics_write_xive: irq %#x [src %d] server %#x prio %#x"
+xics_ics_simple_reject(int nr, int srcno) "reject irq %#x [src %d]"
+xics_ics_simple_eoi(int nr) "ics_eoi: irq %#x"
 xics_alloc(int irq) "irq %d"
 xics_alloc_block(int first, int num, bool lsi, int align) "first irq %d, %d 
irqs, lsi=%d, alignnum %d"
 xics_ics_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs"
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 433af93..f40b000 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -213,9 +213,32 @@ static const TypeInfo xics_common_info = {
 #define XISR(ss)   (((ss)->xirr) & XISR_MASK)
 #define CPPR(ss)   (((ss)->xirr) >> 24)
 
-static void ics_reject(ICSState *ics, int nr);
-static void ics_resend(ICSState *ics);
-static void ics_eoi(ICSState *ics, int nr);
+static void ics_reject(ICSState *ics, uint32_t nr)
+{
+ICSStateClass *k = ICS_BASE_GET_CLASS(ics);
+
+if (k->reject) {
+k->reject(ics, nr);
+}
+}
+
+static void ics_resend(ICSState *ics)
+{
+ICSStateClass *k = ICS_BASE_GET_CLASS(ics);
+
+if (k->resend) {
+k->resend(ics);
+}
+}
+
+static void ics_eoi(ICSState *ics, int nr)
+{
+ICSStateClass *k = ICS_BASE_GET_CLASS(ics);
+
+if (k->eoi) {
+k->eoi(ics, nr);
+}
+}
 
 static void icp_check_ipi(ICPState *ss)
 {
@@ -418,7 +441,7 @@ static const TypeInfo icp_info = {
 /*
  * ICS: Source layer
  */
-static void resend_msi(ICSState *ics, int srcno)
+static void ics_simple_resend_msi(ICSState *ics, int srcno)
 {
 ICSIRQState *irq = ics->irqs + srcno;
 
@@ -431,7 +454,7 @@ static void resend_msi(ICSState *ics, int srcno)
 }
 }
 
-static void resend_lsi(ICSState *ics, int srcno)
+static void ics_simple_resend_lsi(ICSState *ics, int srcno)
 {
 ICSIRQState *irq = ics->irqs + srcno;
 
@@ -443,11 +466,11 @@ static void resend_lsi(ICSState *ics, int srcno)
 }
 }
 
-static void set_irq_msi(ICSState *ics, int srcno, int val)
+static void ics_simple_set_irq_msi(ICSState *ics, int srcno, int val)
 {
 ICSIRQState *irq = ics->irqs + srcno;
 
-trace_xics_set_irq_msi(srcno, srcno + ics->offset);
+trace_xics_ics_simple_set_irq_msi(srcno, srcno + ics->offset);
 
 if (val) {
 if (irq->priority == 0xff) {
@@ -459,31 +482,31 @@ static void set_irq_msi(ICSState *ics, int srcno, int val)
 }
 }
 
-static void set_irq_lsi(ICSState *ics, int srcno, int val)
+static void ics_simple_set_irq_lsi(ICSState *ics, int srcno, int val)
 {
 ICSIRQState *irq = ics->irqs + srcno;
 
-trace_xics_set_irq_lsi(srcno, srcno + ics->offset);
+trace_xics_ics_simple_set_irq_lsi(srcno, srcno + ics->offset);
 if (val) {
 irq->status |= 

[Qemu-devel] [PULL 16/16] spapr: Improved placement of PCI host bridges in guest memory map

[David Gibson]

Currently, the MMIO space for accessing PCI on pseries guests begins at
1 TiB in guest address space.  Each PCI host bridge (PHB) has a 64 GiB
chunk of address space in which it places its outbound PIO and 32-bit and
64-bit MMIO windows.

This scheme as several problems:
  - It limits guest RAM to 1 TiB (though we have a limited fix for this
now)
  - It limits the total MMIO window to 64 GiB.  This is not always enough
for some of the large nVidia GPGPU cards
  - Putting all the windows into a single 64 GiB area means that naturally
aligning things within there will waste more address space.
In addition there was a miscalculation in some of the defaults, which meant
that the MMIO windows for each PHB actually slightly overran the 64 GiB
region for that PHB.  We got away without nasty consequences because
the overrun fit within an unused area at the beginning of the next PHB's
region, but it's not pretty.

This patch implements a new scheme which addresses those problems, and is
also closer to what bare metal hardware and pHyp guests generally use.

Because some guest versions (including most current distro kernels) can't
access PCI MMIO above 64 TiB, we put all the PCI windows between 32 TiB and
64 TiB.  This is broken into 1 TiB chunks.  The first 1 TiB contains the
PIO (64 kiB) and 32-bit MMIO (2 GiB) windows for all of the PHBs.  Each
subsequent TiB chunk contains a naturally aligned 64-bit MMIO window for
one PHB each.

This reduces the number of allowed PHBs (without full manual configuration
of all the windows) from 256 to 31, but this should still be plenty in
practice.

We also change some of the default window sizes for manually configured
PHBs to saner values.

Finally we adjust some tests and libqos so that it correctly uses the new
default locations.  Ideally it would parse the device tree given to the
guest, but that's a more complex problem for another time.

Signed-off-by: David Gibson 
Reviewed-by: Laurent Vivier 
---
 hw/ppc/spapr.c  | 122 +---
 hw/ppc/spapr_pci.c  |   5 +-
 include/hw/pci-host/spapr.h |   8 ++-
 tests/endianness-test.c |   3 +-
 tests/libqos/pci-spapr.c|   9 ++--
 tests/spapr-phb-test.c  |   2 +-
 6 files changed, 109 insertions(+), 40 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ea5d0e6..ddb7438 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2375,31 +2375,38 @@ static void spapr_phb_placement(sPAPRMachineState 
*spapr, uint32_t index,
 hwaddr *mmio32, hwaddr *mmio64,
 unsigned n_dma, uint32_t *liobns, Error **errp)
 {
+/*
+ * New-style PHB window placement.
+ *
+ * Goals: Gives large (1TiB), naturally aligned 64-bit MMIO window
+ * for each PHB, in addition to 2GiB 32-bit MMIO and 64kiB PIO
+ * windows.
+ *
+ * Some guest kernels can't work with MMIO windows above 1<<46
+ * (64TiB), so we place up to 31 PHBs in the area 32TiB..64TiB
+ *
+ * 32TiB..(33TiB+1984kiB) contains the 64kiB PIO windows for each
+ * PHB stacked together.  (32TiB+2GiB)..(32TiB+64GiB) contains the
+ * 2GiB 32-bit MMIO windows for each PHB.  Then 33..64TiB has the
+ * 1TiB 64-bit MMIO windows for each PHB.
+ */
 const uint64_t base_buid = 0x8002000ULL;
-const hwaddr phb_spacing = 0x10ULL; /* 64 GiB */
-const hwaddr mmio_offset = 0xa000; /* 2 GiB + 512 MiB */
-const hwaddr pio_offset = 0x8000; /* 2 GiB */
-const uint32_t max_index = 255;
-const hwaddr phb0_alignment = 0x100ULL; /* 1 TiB */
-
-uint64_t ram_top = MACHINE(spapr)->ram_size;
-hwaddr phb0_base, phb_base;
+const int max_phbs =
+(SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / SPAPR_PCI_MEM64_WIN_SIZE - 1;
 int i;
 
-/* Do we have hotpluggable memory? */
-if (MACHINE(spapr)->maxram_size > ram_top) {
-/* Can't just use maxram_size, because there may be an
- * alignment gap between normal and hotpluggable memory
- * regions */
-ram_top = spapr->hotplug_memory.base +
-memory_region_size(>hotplug_memory.mr);
-}
-
-phb0_base = QEMU_ALIGN_UP(ram_top, phb0_alignment);
+/* Sanity check natural alignments */
+QEMU_BUILD_BUG_ON((SPAPR_PCI_BASE % SPAPR_PCI_MEM64_WIN_SIZE) != 0);
+QEMU_BUILD_BUG_ON((SPAPR_PCI_LIMIT % SPAPR_PCI_MEM64_WIN_SIZE) != 0);
+QEMU_BUILD_BUG_ON((SPAPR_PCI_MEM64_WIN_SIZE % SPAPR_PCI_MEM32_WIN_SIZE) != 
0);
+QEMU_BUILD_BUG_ON((SPAPR_PCI_MEM32_WIN_SIZE % SPAPR_PCI_IO_WIN_SIZE) != 0);
+/* Sanity check bounds */
+QEMU_BUILD_BUG_ON((max_phbs * SPAPR_PCI_IO_WIN_SIZE) > 
SPAPR_PCI_MEM32_WIN_SIZE);
+QEMU_BUILD_BUG_ON((max_phbs * SPAPR_PCI_MEM32_WIN_SIZE) > 
SPAPR_PCI_MEM64_WIN_SIZE);
 
-if (index > max_index) {
+if (index >= max_phbs) {
 error_setg(errp, "\"index\" for PAPR PHB is too 

[Qemu-devel] [PULL 13/16] spapr_pci: Delegate placement of PCI host bridges to machine type

[David Gibson]

The 'spapr-pci-host-bridge' represents the virtual PCI host bridge (PHB)
for a PAPR guest.  Unlike on x86, it's routine on Power (both bare metal
and PAPR guests) to have numerous independent PHBs, each controlling a
separate PCI domain.

There are two ways of configuring the spapr-pci-host-bridge device: first
it can be done fully manually, specifying the locations and sizes of all
the IO windows.  This gives the most control, but is very awkward with 6
mandatory parameters.  Alternatively just an "index" can be specified
which essentially selects from an array of predefined PHB locations.
The PHB at index 0 is automatically created as the default PHB.

The current set of default locations causes some problems for guests with
large RAM (> 1 TiB) or PCI devices with very large BARs (e.g. big nVidia
GPGPU cards via VFIO).  Obviously, for migration we can only change the
locations on a new machine type, however.

This is awkward, because the placement is currently decided within the
spapr-pci-host-bridge code, so it breaks abstraction to look inside the
machine type version.

So, this patch delegates the "default mode" PHB placement from the
spapr-pci-host-bridge device back to the machine type via a public method
in sPAPRMachineClass.  It's still a bit ugly, but it's about the best we
can do.

For now, this just changes where the calculation is done.  It doesn't
change the actual location of the host bridges, or any other behaviour.

Signed-off-by: David Gibson 
Reviewed-by: Laurent Vivier 
---
 hw/ppc/spapr.c  | 31 +++
 hw/ppc/spapr_pci.c  | 21 +++--
 include/hw/pci-host/spapr.h | 11 +--
 include/hw/ppc/spapr.h  |  3 +++
 4 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index fc4c3c9..2bfd187 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2370,6 +2370,36 @@ static HotpluggableCPUList 
*spapr_query_hotpluggable_cpus(MachineState *machine)
 return head;
 }
 
+static void spapr_phb_placement(sPAPRMachineState *spapr, uint32_t index,
+uint64_t *buid, hwaddr *pio, hwaddr *mmio,
+unsigned n_dma, uint32_t *liobns, Error **errp)
+{
+const uint64_t base_buid = 0x8002000ULL;
+const hwaddr phb0_base = 0x100ULL; /* 1 TiB */
+const hwaddr phb_spacing = 0x10ULL; /* 64 GiB */
+const hwaddr mmio_offset = 0xa000; /* 2 GiB + 512 MiB */
+const hwaddr pio_offset = 0x8000; /* 2 GiB */
+const uint32_t max_index = 255;
+
+hwaddr phb_base;
+int i;
+
+if (index > max_index) {
+error_setg(errp, "\"index\" for PAPR PHB is too large (max %u)",
+   max_index);
+return;
+}
+
+*buid = base_buid + index;
+for (i = 0; i < n_dma; ++i) {
+liobns[i] = SPAPR_PCI_LIOBN(index, i);
+}
+
+phb_base = phb0_base + index * phb_spacing;
+*pio = phb_base + pio_offset;
+*mmio = phb_base + mmio_offset;
+}
+
 static void spapr_machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
@@ -2406,6 +2436,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
 mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
 fwc->get_dev_path = spapr_get_fw_dev_path;
 nc->nmi_monitor_handler = spapr_nmi;
+smc->phb_placement = spapr_phb_placement;
 }
 
 static const TypeInfo spapr_machine_info = {
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index a7ca988..8bd7f59 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1311,7 +1311,8 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
 sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
 
 if (sphb->index != (uint32_t)-1) {
-hwaddr windows_base;
+sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
+Error *local_err = NULL;
 
 if ((sphb->buid != (uint64_t)-1) || (sphb->dma_liobn[0] != 
(uint32_t)-1)
 || (sphb->dma_liobn[1] != (uint32_t)-1 && windows_supported == 2)
@@ -1322,21 +1323,13 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
-if (sphb->index > SPAPR_PCI_MAX_INDEX) {
-error_setg(errp, "\"index\" for PAPR PHB is too large (max %u)",
-   SPAPR_PCI_MAX_INDEX);
+smc->phb_placement(spapr, sphb->index, >buid,
+   >io_win_addr, >mem_win_addr,
+   windows_supported, sphb->dma_liobn, _err);
+if (local_err) {
+error_propagate(errp, local_err);
 return;
 }
-
-sphb->buid = SPAPR_PCI_BASE_BUID + sphb->index;
-for (i = 0; i < windows_supported; ++i) {
-sphb->dma_liobn[i] = SPAPR_PCI_LIOBN(sphb->index, i);
-}
-
-windows_base = SPAPR_PCI_WINDOW_BASE
-+ 

[Qemu-devel] [PULL 11/16] libqos: Correct error in PCI hole sizing for spapr

[David Gibson]

In pci-spapr.c (as in pci-pc.c from which it was derived), the
pci_hole_start/pci_hole_size and pci_iohole_start/pci_iohole_size pairs[1]
essentially define the region of PCI (not CPU) addresses in which MMIO
or PIO BARs respectively will be allocated.

The size value is relative to the start value.  But in pci-spapr.c it is
set to the entire size of the window supported by the (emulated) hardware,
but the start values are *not* at the beginning of the emulated windows.

That means if you tried to map enough PCI BARs, we'd messily overrun the
IO windows, instead of failing in iomap as we should.

This patch corrects this by calculating the hole sizes from the location
of the window in PCI space and the hole start.

[1] Those are bad names, but that's a problem for another time.

Signed-off-by: David Gibson 
Reviewed-by: Laurent Vivier 
---
 tests/libqos/pci-spapr.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/libqos/pci-spapr.c b/tests/libqos/pci-spapr.c
index 1765a54..3192903 100644
--- a/tests/libqos/pci-spapr.c
+++ b/tests/libqos/pci-spapr.c
@@ -285,11 +285,13 @@ QPCIBus *qpci_init_spapr(QGuestAllocator *alloc)
 ret->mmio.size = SPAPR_PCI_MMIO_WIN_SIZE;
 
 ret->pci_hole_start = 0xC000;
-ret->pci_hole_size = SPAPR_PCI_MMIO_WIN_SIZE;
+ret->pci_hole_size =
+ret->mmio.pci_base + ret->mmio.size - ret->pci_hole_start;
 ret->pci_hole_alloc = 0;
 
 ret->pci_iohole_start = 0xc000;
-ret->pci_iohole_size = SPAPR_PCI_IO_WIN_SIZE;
+ret->pci_iohole_size =
+ret->pio.pci_base + ret->pio.size - ret->pci_iohole_start;
 ret->pci_iohole_alloc = 0;
 
 return >bus;
-- 
2.7.4

[Qemu-devel] [PULL 07/16] spapr: fix inheritance chain for default machine options

[David Gibson]

From: Michael Roth 

Rather than machine instances having backward-compatible option
defaults that need to be repeatedly re-enabled for every new machine
type we introduce, we set the defaults appropriate for newer machine
types, then add code to explicitly disable instance options as needed
to maintain compatibility with older machine types.

Currently pseries-2.5 does not inherit from pseries-2.6 in this
fashion, which is okay at the moment since we do not have any
instance compatibility options for pseries-2.6+ currently.

We will make use of this in future patches though, so fix it here.

Signed-off-by: Michael Roth 
[dwg: Extended to make 2.7 inherit from 2.8 as well]
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 03e3803..fc4c3c9 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2475,6 +2475,7 @@ DEFINE_SPAPR_MACHINE(2_8, "2.8", true);
 
 static void spapr_machine_2_7_instance_options(MachineState *machine)
 {
+spapr_machine_2_8_instance_options(machine);
 }
 
 static void spapr_machine_2_7_class_options(MachineClass *mc)
@@ -2501,6 +2502,7 @@ DEFINE_SPAPR_MACHINE(2_7, "2.7", false);
 
 static void spapr_machine_2_6_instance_options(MachineState *machine)
 {
+spapr_machine_2_7_instance_options(machine);
 }
 
 static void spapr_machine_2_6_class_options(MachineClass *mc)
@@ -2525,6 +2527,7 @@ DEFINE_SPAPR_MACHINE(2_6, "2.6", false);
 
 static void spapr_machine_2_5_instance_options(MachineState *machine)
 {
+spapr_machine_2_6_instance_options(machine);
 }
 
 static void spapr_machine_2_5_class_options(MachineClass *mc)
-- 
2.7.4

[Qemu-devel] [PULL 15/16] spapr_pci: Add a 64-bit MMIO window

[David Gibson]

On real hardware, and under pHyp, the PCI host bridges on Power machines
typically advertise two outbound MMIO windows from the guest's physical
memory space to PCI memory space:
  - A 32-bit window which maps onto 2GiB..4GiB in the PCI address space
  - A 64-bit window which maps onto a large region somewhere high in PCI
address space (traditionally this used an identity mapping from guest
physical address to PCI address, but that's not always the case)

The qemu implementation in spapr-pci-host-bridge, however, only supports a
single outbound MMIO window, however.  At least some Linux versions expect
the two windows however, so we arranged this window to map onto the PCI
memory space from 2 GiB..~64 GiB, then advertised it as two contiguous
windows, the "32-bit" window from 2G..4G and the "64-bit" window from
4G..~64G.

This approach means, however, that the 64G window is not naturally aligned.
In turn this limits the size of the largest BAR we can map (which does have
to be naturally aligned) to roughly half of the total window.  With some
large nVidia GPGPU cards which have huge memory BARs, this is starting to
be a problem.

This patch adds true support for separate 32-bit and 64-bit outbound MMIO
windows to the spapr-pci-host-bridge implementation, each of which can
be independently configured.  The 32-bit window always maps to 2G.. in PCI
space, but the PCI address of the 64-bit window can be configured (it
defaults to the same as the guest physical address).

So as not to break possible existing configurations, as long as a 64-bit
window is not specified, a large single window can be specified.  This
will appear the same way to the guest as the old approach, although it's
now implemented by two contiguous memory regions rather than a single one.

For now, this only adds the possibility of 64-bit windows.  The default
configuration still uses the legacy mode.

Signed-off-by: David Gibson 
Reviewed-by: Laurent Vivier 
---
 hw/ppc/spapr.c  | 10 +--
 hw/ppc/spapr_pci.c  | 70 -
 include/hw/pci-host/spapr.h |  8 --
 include/hw/ppc/spapr.h  |  3 +-
 4 files changed, 72 insertions(+), 19 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d747e58..ea5d0e6 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2371,7 +2371,8 @@ static HotpluggableCPUList 
*spapr_query_hotpluggable_cpus(MachineState *machine)
 }
 
 static void spapr_phb_placement(sPAPRMachineState *spapr, uint32_t index,
-uint64_t *buid, hwaddr *pio, hwaddr *mmio,
+uint64_t *buid, hwaddr *pio,
+hwaddr *mmio32, hwaddr *mmio64,
 unsigned n_dma, uint32_t *liobns, Error **errp)
 {
 const uint64_t base_buid = 0x8002000ULL;
@@ -2409,7 +2410,12 @@ static void spapr_phb_placement(sPAPRMachineState 
*spapr, uint32_t index,
 
 phb_base = phb0_base + index * phb_spacing;
 *pio = phb_base + pio_offset;
-*mmio = phb_base + mmio_offset;
+*mmio32 = phb_base + mmio_offset;
+/*
+ * We don't set the 64-bit MMIO window, relying on the PHB's
+ * fallback behaviour of automatically splitting a large "32-bit"
+ * window into contiguous 32-bit and 64-bit windows
+ */
 }
 
 static void spapr_machine_class_init(ObjectClass *oc, void *data)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 8bd7f59..10d5de2 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1317,14 +1317,16 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
 if ((sphb->buid != (uint64_t)-1) || (sphb->dma_liobn[0] != 
(uint32_t)-1)
 || (sphb->dma_liobn[1] != (uint32_t)-1 && windows_supported == 2)
 || (sphb->mem_win_addr != (hwaddr)-1)
+|| (sphb->mem64_win_addr != (hwaddr)-1)
 || (sphb->io_win_addr != (hwaddr)-1)) {
 error_setg(errp, "Either \"index\" or other parameters must"
" be specified for PAPR PHB, not both");
 return;
 }
 
-smc->phb_placement(spapr, sphb->index, >buid,
-   >io_win_addr, >mem_win_addr,
+smc->phb_placement(spapr, sphb->index,
+   >buid, >io_win_addr,
+   >mem_win_addr, >mem64_win_addr,
windows_supported, sphb->dma_liobn, _err);
 if (local_err) {
 error_propagate(errp, local_err);
@@ -1353,6 +1355,38 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+if (sphb->mem64_win_size != 0) {
+if (sphb->mem64_win_addr == (hwaddr)-1) {
+error_setg(errp,
+   "64-bit memory window address not specified for PHB");
+return;
+}
+
+if (sphb->mem_win_size > SPAPR_PCI_MEM32_WIN_SIZE) {
+  

[Qemu-devel] [PULL 12/16] libqos: Limit spapr-pci to 32-bit MMIO for now

[David Gibson]

Currently the functions in pci-spapr.c (like pci-pc.c on which it's based)
don't distinguish between 32-bit and 64-bit PCI MMIO.  At the moment, the
qemu side implementation is a bit weird and has a single MMIO window
straddling 32-bit and 64-bit regions, but we're likely to change that in
future.

In any case, pci-pc.c - and therefore the testcases using PCI - only handle
32-bit MMIOs for now.  For spapr despite whatever changes might happen with
the MMIO windows, the 32-bit window is likely to remain at 2..4 GiB in PCI
space.

So, explicitly limit pci-spapr.c to 32-bit MMIOs for now, we can add 64-bit
MMIO support back in when and if we need it.

Signed-off-by: David Gibson 
Reviewed-by: Laurent Vivier 
---
 tests/libqos/pci-spapr.c | 32 +++-
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/tests/libqos/pci-spapr.c b/tests/libqos/pci-spapr.c
index 3192903..558dfc3 100644
--- a/tests/libqos/pci-spapr.c
+++ b/tests/libqos/pci-spapr.c
@@ -32,8 +32,8 @@ typedef struct QPCIBusSPAPR {
 uint64_t pio_cpu_base;
 QPCIWindow pio;
 
-uint64_t mmio_cpu_base;
-QPCIWindow mmio;
+uint64_t mmio32_cpu_base;
+QPCIWindow mmio32;
 
 uint64_t pci_hole_start;
 uint64_t pci_hole_size;
@@ -58,7 +58,7 @@ static uint8_t qpci_spapr_io_readb(QPCIBus *bus, void *addr)
 if (port < s->pio.size) {
 v = readb(s->pio_cpu_base + port);
 } else {
-v = readb(s->mmio_cpu_base + port);
+v = readb(s->mmio32_cpu_base + port);
 }
 return v;
 }
@@ -71,7 +71,7 @@ static uint16_t qpci_spapr_io_readw(QPCIBus *bus, void *addr)
 if (port < s->pio.size) {
 v = readw(s->pio_cpu_base + port);
 } else {
-v = readw(s->mmio_cpu_base + port);
+v = readw(s->mmio32_cpu_base + port);
 }
 return bswap16(v);
 }
@@ -84,7 +84,7 @@ static uint32_t qpci_spapr_io_readl(QPCIBus *bus, void *addr)
 if (port < s->pio.size) {
 v = readl(s->pio_cpu_base + port);
 } else {
-v = readl(s->mmio_cpu_base + port);
+v = readl(s->mmio32_cpu_base + port);
 }
 return bswap32(v);
 }
@@ -96,7 +96,7 @@ static void qpci_spapr_io_writeb(QPCIBus *bus, void *addr, 
uint8_t value)
 if (port < s->pio.size) {
 writeb(s->pio_cpu_base + port, value);
 } else {
-writeb(s->mmio_cpu_base + port, value);
+writeb(s->mmio32_cpu_base + port, value);
 }
 }
 
@@ -108,7 +108,7 @@ static void qpci_spapr_io_writew(QPCIBus *bus, void *addr, 
uint16_t value)
 if (port < s->pio.size) {
 writew(s->pio_cpu_base + port, value);
 } else {
-writew(s->mmio_cpu_base + port, value);
+writew(s->mmio32_cpu_base + port, value);
 }
 }
 
@@ -120,7 +120,7 @@ static void qpci_spapr_io_writel(QPCIBus *bus, void *addr, 
uint32_t value)
 if (port < s->pio.size) {
 writel(s->pio_cpu_base + port, value);
 } else {
-writel(s->mmio_cpu_base + port, value);
+writel(s->mmio32_cpu_base + port, value);
 }
 }
 
@@ -235,12 +235,9 @@ static void qpci_spapr_iounmap(QPCIBus *bus, void *data)
 /* FIXME */
 }
 
-#define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x8000ULL
 #define SPAPR_PCI_WINDOW_BASE0x100ULL
-#define SPAPR_PCI_WINDOW_SPACING 0x10ULL
-#define SPAPR_PCI_MMIO_WIN_OFF   0xA000
-#define SPAPR_PCI_MMIO_WIN_SIZE  (SPAPR_PCI_WINDOW_SPACING - \
- SPAPR_PCI_MEM_WIN_BUS_OFFSET)
+#define SPAPR_PCI_MMIO32_WIN_OFF 0xA000
+#define SPAPR_PCI_MMIO32_WIN_SIZE0x8000 /* 2 GiB */
 #define SPAPR_PCI_IO_WIN_OFF 0x8000
 #define SPAPR_PCI_IO_WIN_SIZE0x1
 
@@ -280,13 +277,14 @@ QPCIBus *qpci_init_spapr(QGuestAllocator *alloc)
 ret->pio.pci_base = 0;
 ret->pio.size = SPAPR_PCI_IO_WIN_SIZE;
 
-ret->mmio_cpu_base = SPAPR_PCI_WINDOW_BASE + SPAPR_PCI_MMIO_WIN_OFF;
-ret->mmio.pci_base = SPAPR_PCI_MEM_WIN_BUS_OFFSET;
-ret->mmio.size = SPAPR_PCI_MMIO_WIN_SIZE;
+/* 32-bit portion of the MMIO window is at PCI address 2..4 GiB */
+ret->mmio32_cpu_base = SPAPR_PCI_WINDOW_BASE + SPAPR_PCI_MMIO32_WIN_OFF;
+ret->mmio32.pci_base = 0x8000; /* 2 GiB */
+ret->mmio32.size = SPAPR_PCI_MMIO32_WIN_SIZE;
 
 ret->pci_hole_start = 0xC000;
 ret->pci_hole_size =
-ret->mmio.pci_base + ret->mmio.size - ret->pci_hole_start;
+ret->mmio32.pci_base + ret->mmio32.size - ret->pci_hole_start;
 ret->pci_hole_alloc = 0;
 
 ret->pci_iohole_start = 0xc000;
-- 
2.7.4

[Qemu-devel] [PULL 10/16] libqos: Isolate knowledge of spapr memory map to qpci_init_spapr()

[David Gibson]

The libqos code for accessing PCI on the spapr machine type uses IOBASE()
and MMIOBASE() macros to determine the address in the CPU memory map of
the windows to PCI address space.

This is a detail of the implementation of PCI in the machine type, it's not
specified by the PAPR standard.  Real guests would get the addresses of the
PCI windows from the device tree.

Finding the device tree in libqos would be awkward, but we can at least
localize this knowledge of the implementation to the init function, saving
it in the QPCIBusSPAPR structure for use by the accessors.

That leaves only one place to fix if we alter the location of the PCI
windows, as we're planning to do.

Signed-off-by: David Gibson 
Reviewed-by: Laurent Vivier 
---
 tests/libqos/pci-spapr.c | 113 +++
 1 file changed, 64 insertions(+), 49 deletions(-)

diff --git a/tests/libqos/pci-spapr.c b/tests/libqos/pci-spapr.c
index 2f73bad..1765a54 100644
--- a/tests/libqos/pci-spapr.c
+++ b/tests/libqos/pci-spapr.c
@@ -18,30 +18,23 @@
 
 /* From include/hw/pci-host/spapr.h */
 
-#define SPAPR_PCI_BASE_BUID  0x8002000ULL
-
-#define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x8000ULL
-
-#define SPAPR_PCI_WINDOW_BASE0x100ULL
-#define SPAPR_PCI_WINDOW_SPACING 0x10ULL
-#define SPAPR_PCI_MMIO_WIN_OFF   0xA000
-#define SPAPR_PCI_MMIO_WIN_SIZE  (SPAPR_PCI_WINDOW_SPACING - \
- SPAPR_PCI_MEM_WIN_BUS_OFFSET)
-#define SPAPR_PCI_IO_WIN_OFF 0x8000
-#define SPAPR_PCI_IO_WIN_SIZE0x1
-
-/* index is the phb index */
-
-#define BUIDBASE(index)  (SPAPR_PCI_BASE_BUID + (index))
-#define PCIBASE(index)   (SPAPR_PCI_WINDOW_BASE + \
-  (index) * SPAPR_PCI_WINDOW_SPACING)
-#define IOBASE(index)(PCIBASE(index) + SPAPR_PCI_IO_WIN_OFF)
-#define MMIOBASE(index)  (PCIBASE(index) + SPAPR_PCI_MMIO_WIN_OFF)
+typedef struct QPCIWindow {
+uint64_t pci_base;/* window address in PCI space */
+uint64_t size;/* window size */
+} QPCIWindow;
 
 typedef struct QPCIBusSPAPR {
 QPCIBus bus;
 QGuestAllocator *alloc;
 
+uint64_t buid;
+
+uint64_t pio_cpu_base;
+QPCIWindow pio;
+
+uint64_t mmio_cpu_base;
+QPCIWindow mmio;
+
 uint64_t pci_hole_start;
 uint64_t pci_hole_size;
 uint64_t pci_hole_alloc;
@@ -59,69 +52,75 @@ typedef struct QPCIBusSPAPR {
 
 static uint8_t qpci_spapr_io_readb(QPCIBus *bus, void *addr)
 {
+QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
 uint64_t port = (uintptr_t)addr;
 uint8_t v;
-if (port < SPAPR_PCI_IO_WIN_SIZE) {
-v = readb(IOBASE(0) + port);
+if (port < s->pio.size) {
+v = readb(s->pio_cpu_base + port);
 } else {
-v = readb(MMIOBASE(0) + port);
+v = readb(s->mmio_cpu_base + port);
 }
 return v;
 }
 
 static uint16_t qpci_spapr_io_readw(QPCIBus *bus, void *addr)
 {
+QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
 uint64_t port = (uintptr_t)addr;
 uint16_t v;
-if (port < SPAPR_PCI_IO_WIN_SIZE) {
-v = readw(IOBASE(0) + port);
+if (port < s->pio.size) {
+v = readw(s->pio_cpu_base + port);
 } else {
-v = readw(MMIOBASE(0) + port);
+v = readw(s->mmio_cpu_base + port);
 }
 return bswap16(v);
 }
 
 static uint32_t qpci_spapr_io_readl(QPCIBus *bus, void *addr)
 {
+QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
 uint64_t port = (uintptr_t)addr;
 uint32_t v;
-if (port < SPAPR_PCI_IO_WIN_SIZE) {
-v = readl(IOBASE(0) + port);
+if (port < s->pio.size) {
+v = readl(s->pio_cpu_base + port);
 } else {
-v = readl(MMIOBASE(0) + port);
+v = readl(s->mmio_cpu_base + port);
 }
 return bswap32(v);
 }
 
 static void qpci_spapr_io_writeb(QPCIBus *bus, void *addr, uint8_t value)
 {
+QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
 uint64_t port = (uintptr_t)addr;
-if (port < SPAPR_PCI_IO_WIN_SIZE) {
-writeb(IOBASE(0) + port, value);
+if (port < s->pio.size) {
+writeb(s->pio_cpu_base + port, value);
 } else {
-writeb(MMIOBASE(0) + port, value);
+writeb(s->mmio_cpu_base + port, value);
 }
 }
 
 static void qpci_spapr_io_writew(QPCIBus *bus, void *addr, uint16_t value)
 {
+QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
 uint64_t port = (uintptr_t)addr;
 value = bswap16(value);
-if (port < SPAPR_PCI_IO_WIN_SIZE) {
-writew(IOBASE(0) + port, value);
+if (port < s->pio.size) {
+writew(s->pio_cpu_base + port, value);
 } else {
-writew(MMIOBASE(0) + port, value);
+writew(s->mmio_cpu_base + port, value);
 }
 }
 
 static void qpci_spapr_io_writel(QPCIBus *bus, void *addr, uint32_t value)
 {
+QPCIBusSPAPR *s = 

[Qemu-devel] [PULL 00/16] ppc-for-2.8 queue 20161017

[David Gibson]

The following changes since commit 6aa5a3679449cdf0b6fe5a6829b22e642ded57fd:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161013-1' into 
staging (2016-10-13 14:27:58 +0100)

are available in the git repository at:

  git://github.com/dgibson/qemu.git tags/ppc-for-2.8-20161017

for you to fetch changes up to 357d1e3bc7d2d80e5271bc4f3ac8537e30dc8046:

  spapr: Improved placement of PCI host bridges in guest memory map (2016-10-16 
12:04:15 +1100)


ppc patch queue 2016-10-17

Highlights:
* Significant rework of how PCI IO windows are placed for the
  pseries machine type
* A number of extra tests added for ppc
* Other tests clean up / fixed
* Some cleanups to the XICS interrupt controller in preparation
  for the 'powernv' machine type

A number of the test changes aren't strictly in ppc related code, but
are included via my tree because they're primarily focused on
improving test coverage for ppc.


Benjamin Herrenschmidt (2):
  ppc/xics: Make the ICSState a list
  ppc/xics: Split ICS into ics-base and ics class

David Gibson (7):
  libqos: Isolate knowledge of spapr memory map to qpci_init_spapr()
  libqos: Correct error in PCI hole sizing for spapr
  libqos: Limit spapr-pci to 32-bit MMIO for now
  spapr_pci: Delegate placement of PCI host bridges to machine type
  spapr: Adjust placement of PCI host bridge to allow > 1TiB RAM
  spapr_pci: Add a 64-bit MMIO window
  spapr: Improved placement of PCI host bridges in guest memory map

Laurent Vivier (2):
  tests: minor cleanups in usb-hcd-uhci-test
  qtest: ask endianness of the target in qtest_init()

Michael Roth (1):
  spapr: fix inheritance chain for default machine options

Nikunj A Dadhania (1):
  target-ppc: implement vexts[bh]2w and vexts[bhw]2d

Thomas Huth (3):
  tests/boot-sector: Use minimum length for the Forth boot script
  tests/boot-sector: Use mkstemp() to create a unique file name
  tests/boot-sector: Increase time-out to 90 seconds

 hw/intc/trace-events|  15 +--
 hw/intc/xics.c  | 231 ++--
 hw/intc/xics_kvm.c  |  37 --
 hw/intc/xics_spapr.c| 116 +++---
 hw/ppc/spapr.c  | 118 +-
 hw/ppc/spapr_events.c   |   2 +-
 hw/ppc/spapr_pci.c  |  95 ++-
 hw/ppc/spapr_vio.c  |   2 +-
 include/hw/pci-host/spapr.h |  25 ++--
 include/hw/ppc/spapr.h  |   4 +
 include/hw/ppc/xics.h   |  40 ---
 qtest.c |   7 ++
 target-ppc/helper.h |   5 +
 target-ppc/int_helper.c |  15 +++
 target-ppc/translate/vmx-impl.inc.c |   5 +
 target-ppc/translate/vmx-ops.inc.c  |   5 +
 tests/bios-tables-test.c|   2 +-
 tests/boot-sector.c |  25 ++--
 tests/boot-sector.h |   4 +-
 tests/endianness-test.c |   3 +-
 tests/libqos/pci-spapr.c| 116 ++
 tests/libqos/virtio-pci.c   |   2 +-
 tests/libqtest.c|  68 ---
 tests/libqtest.h|  16 ++-
 tests/pxe-test.c|   2 +-
 tests/spapr-phb-test.c  |   2 +-
 tests/usb-hcd-uhci-test.c   |  15 ++-
 tests/virtio-blk-test.c |   2 +-
 28 files changed, 642 insertions(+), 337 deletions(-)

[Qemu-devel] [PULL 04/16] tests/boot-sector: Use mkstemp() to create a unique file name

[David Gibson]

From: Thomas Huth 

The pxe-test is run for three different targets now (x86_64, i386
and ppc64), and the bios-tables-test is run for two targets (x86_64
and i386). But each of the tests is using an invariant name for the
disk image with the boot sector code - so if the tests are running in
parallel, there is a race condition that they destroy the disk image
of a parallel test program. Let's use mkstemp() to create unique
temporary files here instead - and since mkstemp() is returning an
integer file descriptor instead of a FILE pointer, we also switch
the fwrite() and fclose() to write() and close() instead.

Reported-by: Sascha Silbe 
Signed-off-by: Thomas Huth 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: David Gibson 
---
 tests/bios-tables-test.c |  2 +-
 tests/boot-sector.c  | 17 -
 tests/boot-sector.h  |  4 ++--
 tests/pxe-test.c |  2 +-
 4 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 6ea2b6d..812f830 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -112,7 +112,7 @@ typedef struct {
 g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
 } while (0)
 
-static const char *disk = "tests/acpi-test-disk.raw";
+static char disk[] = "tests/acpi-test-disk-XX";
 static const char *data_dir = "tests/acpi-test-data";
 #ifdef CONFIG_IASL
 static const char *iasl = stringify(CONFIG_IASL);
diff --git a/tests/boot-sector.c b/tests/boot-sector.c
index 0168fd0..8399314 100644
--- a/tests/boot-sector.c
+++ b/tests/boot-sector.c
@@ -69,12 +69,13 @@ static uint8_t boot_sector[0x7e000] = {
 };
 
 /* Create boot disk file.  */
-int boot_sector_init(const char *fname)
+int boot_sector_init(char *fname)
 {
-FILE *f = fopen(fname, "w");
+int fd, ret;
 size_t len = sizeof boot_sector;
 
-if (!f) {
+fd = mkstemp(fname);
+if (fd < 0) {
 fprintf(stderr, "Couldn't open \"%s\": %s", fname, strerror(errno));
 return 1;
 }
@@ -86,8 +87,14 @@ int boot_sector_init(const char *fname)
 HIGH(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET + 1);
 }
 
-fwrite(boot_sector, 1, len, f);
-fclose(f);
+ret = write(fd, boot_sector, len);
+close(fd);
+
+if (ret != len) {
+fprintf(stderr, "Could not write \"%s\"", fname);
+return 1;
+}
+
 return 0;
 }
 
diff --git a/tests/boot-sector.h b/tests/boot-sector.h
index f64b477..35d61c7 100644
--- a/tests/boot-sector.h
+++ b/tests/boot-sector.h
@@ -14,8 +14,8 @@
 #ifndef TEST_BOOT_SECTOR_H
 #define TEST_BOOT_SECTOR_H
 
-/* Create boot disk file.  */
-int boot_sector_init(const char *fname);
+/* Create boot disk file. fname must be a suitable string for mkstemp() */
+int boot_sector_init(char *fname);
 
 /* Loop until signature in memory is OK.  */
 void boot_sector_test(void);
diff --git a/tests/pxe-test.c b/tests/pxe-test.c
index 5d3ddbe..34282d3 100644
--- a/tests/pxe-test.c
+++ b/tests/pxe-test.c
@@ -19,7 +19,7 @@
 
 #define NETNAME "net0"
 
-static const char *disk = "tests/pxe-test-disk.raw";
+static char disk[] = "tests/pxe-test-disk-XX";
 
 static void test_pxe_one(const char *params, bool ipv6)
 {
-- 
2.7.4

[Qemu-devel] [PULL 06/16] target-ppc: implement vexts[bh]2w and vexts[bhw]2d

[David Gibson]

From: Nikunj A Dadhania 

Vector Extend Sign Instructions:

vextsb2w: Vector Extend Sign Byte To Word
vextsh2w: Vector Extend Sign Halfword To Word
vextsb2d: Vector Extend Sign Byte To Doubleword
vextsh2d: Vector Extend Sign Halfword To Doubleword
vextsw2d: Vector Extend Sign Word To Doubleword

Signed-off-by: Nikunj A Dadhania 
Signed-off-by: David Gibson 
---
 target-ppc/helper.h |  5 +
 target-ppc/int_helper.c | 15 +++
 target-ppc/translate/vmx-impl.inc.c |  5 +
 target-ppc/translate/vmx-ops.inc.c  |  5 +
 4 files changed, 30 insertions(+)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 796ad45..04c6421 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -267,6 +267,11 @@ DEF_HELPER_3(vinsertb, void, avr, avr, i32)
 DEF_HELPER_3(vinserth, void, avr, avr, i32)
 DEF_HELPER_3(vinsertw, void, avr, avr, i32)
 DEF_HELPER_3(vinsertd, void, avr, avr, i32)
+DEF_HELPER_2(vextsb2w, void, avr, avr)
+DEF_HELPER_2(vextsh2w, void, avr, avr)
+DEF_HELPER_2(vextsb2d, void, avr, avr)
+DEF_HELPER_2(vextsh2d, void, avr, avr)
+DEF_HELPER_2(vextsw2d, void, avr, avr)
 DEF_HELPER_2(vupkhpx, void, avr, avr)
 DEF_HELPER_2(vupklpx, void, avr, avr)
 DEF_HELPER_2(vupkhsb, void, avr, avr)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 202854f..5aee0a8 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -1934,6 +1934,21 @@ VEXTRACT(uw, u32)
 VEXTRACT(d, u64)
 #undef VEXTRACT
 
+#define VEXT_SIGNED(name, element, mask, cast, recast)  \
+void helper_##name(ppc_avr_t *r, ppc_avr_t *b)  \
+{   \
+int i;  \
+VECTOR_FOR_INORDER_I(i, element) {  \
+r->element[i] = (recast)((cast)(b->element[i] & mask)); \
+}   \
+}
+VEXT_SIGNED(vextsb2w, s32, UINT8_MAX, int8_t, int32_t)
+VEXT_SIGNED(vextsb2d, s64, UINT8_MAX, int8_t, int64_t)
+VEXT_SIGNED(vextsh2w, s32, UINT16_MAX, int16_t, int32_t)
+VEXT_SIGNED(vextsh2d, s64, UINT16_MAX, int16_t, int64_t)
+VEXT_SIGNED(vextsw2d, s64, UINT32_MAX, int32_t, int64_t)
+#undef VEXT_SIGNED
+
 #define VSPLTI(suffix, element, splat_type) \
 void helper_vspltis##suffix(ppc_avr_t *r, uint32_t splat)   \
 {   \
diff --git a/target-ppc/translate/vmx-impl.inc.c 
b/target-ppc/translate/vmx-impl.inc.c
index 25cd073..c8998f3 100644
--- a/target-ppc/translate/vmx-impl.inc.c
+++ b/target-ppc/translate/vmx-impl.inc.c
@@ -815,6 +815,11 @@ GEN_VXFORM_NOA(vclzb, 1, 28)
 GEN_VXFORM_NOA(vclzh, 1, 29)
 GEN_VXFORM_NOA(vclzw, 1, 30)
 GEN_VXFORM_NOA(vclzd, 1, 31)
+GEN_VXFORM_NOA_2(vextsb2w, 1, 24, 16)
+GEN_VXFORM_NOA_2(vextsh2w, 1, 24, 17)
+GEN_VXFORM_NOA_2(vextsb2d, 1, 24, 24)
+GEN_VXFORM_NOA_2(vextsh2d, 1, 24, 25)
+GEN_VXFORM_NOA_2(vextsw2d, 1, 24, 26)
 GEN_VXFORM_NOA_2(vctzb, 1, 24, 28)
 GEN_VXFORM_NOA_2(vctzh, 1, 24, 29)
 GEN_VXFORM_NOA_2(vctzw, 1, 24, 30)
diff --git a/target-ppc/translate/vmx-ops.inc.c 
b/target-ppc/translate/vmx-ops.inc.c
index ac1dc9b..68cba3e 100644
--- a/target-ppc/translate/vmx-ops.inc.c
+++ b/target-ppc/translate/vmx-ops.inc.c
@@ -215,6 +215,11 @@ GEN_VXFORM_DUAL_INV(vspltish, vinserth, 6, 13, 0x, 
0x10,
 GEN_VXFORM_DUAL_INV(vspltisw, vinsertw, 6, 14, 0x, 0x10,
PPC_ALTIVEC),
 GEN_VXFORM_300_EXT(vinsertd, 6, 15, 0x10),
+GEN_VXFORM_300_EO(vextsb2w, 0x01, 0x18, 0x10),
+GEN_VXFORM_300_EO(vextsh2w, 0x01, 0x18, 0x11),
+GEN_VXFORM_300_EO(vextsb2d, 0x01, 0x18, 0x18),
+GEN_VXFORM_300_EO(vextsh2d, 0x01, 0x18, 0x19),
+GEN_VXFORM_300_EO(vextsw2d, 0x01, 0x18, 0x1A),
 GEN_VXFORM_300_EO(vctzb, 0x01, 0x18, 0x1C),
 GEN_VXFORM_300_EO(vctzh, 0x01, 0x18, 0x1D),
 GEN_VXFORM_300_EO(vctzw, 0x01, 0x18, 0x1E),
-- 
2.7.4

[Qemu-devel] [PULL 02/16] qtest: ask endianness of the target in qtest_init()

[David Gibson]

From: Laurent Vivier 

The target endianness is not deduced anymore from
the architecture name but asked directly to the guest,
using a new qtest command: "endianness". As it can't
change (this is the value of TARGET_WORDS_BIGENDIAN),
we store it to not have to ask every time we want to
know if we have to byte-swap a value.

Signed-off-by: Laurent Vivier 
CC: Greg Kurz 
CC: David Gibson 
CC: Peter Maydell 
Signed-off-by: David Gibson 
---
 qtest.c   |  7 +
 tests/libqos/virtio-pci.c |  2 +-
 tests/libqtest.c  | 68 ---
 tests/libqtest.h  | 16 ---
 tests/virtio-blk-test.c   |  2 +-
 5 files changed, 45 insertions(+), 50 deletions(-)

diff --git a/qtest.c b/qtest.c
index 22482cc..b53b39c 100644
--- a/qtest.c
+++ b/qtest.c
@@ -537,6 +537,13 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 
 qtest_send_prefix(chr);
 qtest_send(chr, "OK\n");
+} else if (strcmp(words[0], "endianness") == 0) {
+qtest_send_prefix(chr);
+#if defined(TARGET_WORDS_BIGENDIAN)
+qtest_sendf(chr, "OK big\n");
+#else
+qtest_sendf(chr, "OK little\n");
+#endif
 #ifdef TARGET_PPC64
 } else if (strcmp(words[0], "rtas") == 0) {
 uint64_t res, args, ret;
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 18b92b9..6e005c1 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -86,7 +86,7 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, 
uint64_t addr)
 int i;
 uint64_t u64 = 0;
 
-if (qtest_big_endian()) {
+if (target_big_endian()) {
 for (i = 0; i < 8; ++i) {
 u64 |= (uint64_t)qpci_io_readb(dev->pdev,
 (void *)(uintptr_t)addr + i) << (7 - i) * 8;
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 6f6bdf1..d4e6bff 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -37,6 +37,7 @@ struct QTestState
 bool irq_level[MAX_IRQ];
 GString *rx;
 pid_t qemu_pid;  /* our child QEMU process */
+bool big_endian;
 };
 
 static GHookList abrt_hooks;
@@ -47,6 +48,8 @@ static struct sigaction sigact_old;
 g_assert_cmpint(ret, !=, -1); \
 } while (0)
 
+static int qtest_query_target_endianness(QTestState *s);
+
 static int init_socket(const char *socket_path)
 {
 struct sockaddr_un addr;
@@ -209,6 +212,10 @@ QTestState *qtest_init(const char *extra_args)
 kill(s->qemu_pid, SIGSTOP);
 }
 
+/* ask endianness of the target */
+
+s->big_endian = qtest_query_target_endianness(s);
+
 return s;
 }
 
@@ -342,6 +349,20 @@ redo:
 return words;
 }
 
+static int qtest_query_target_endianness(QTestState *s)
+{
+gchar **args;
+int big_endian;
+
+qtest_sendf(s, "endianness\n");
+args = qtest_rsp(s, 1);
+g_assert(strcmp(args[1], "big") == 0 || strcmp(args[1], "little") == 0);
+big_endian = strcmp(args[1], "big") == 0;
+g_strfreev(args);
+
+return big_endian;
+}
+
 typedef struct {
 JSONMessageParser parser;
 QDict *response;
@@ -886,50 +907,7 @@ char *hmp(const char *fmt, ...)
 return ret;
 }
 
-bool qtest_big_endian(void)
+bool qtest_big_endian(QTestState *s)
 {
-const char *arch = qtest_get_arch();
-int i;
-
-static const struct {
-const char *arch;
-bool big_endian;
-} endianness[] = {
-{ "aarch64", false },
-{ "alpha", false },
-{ "arm", false },
-{ "cris", false },
-{ "i386", false },
-{ "lm32", true },
-{ "m68k", true },
-{ "microblaze", true },
-{ "microblazeel", false },
-{ "mips", true },
-{ "mips64", true },
-{ "mips64el", false },
-{ "mipsel", false },
-{ "moxie", true },
-{ "or32", true },
-{ "ppc", true },
-{ "ppc64", true },
-{ "ppcemb", true },
-{ "s390x", true },
-{ "sh4", false },
-{ "sh4eb", true },
-{ "sparc", true },
-{ "sparc64", true },
-{ "unicore32", false },
-{ "x86_64", false },
-{ "xtensa", false },
-{ "xtensaeb", true },
-{},
-};
-
-for (i = 0; endianness[i].arch; i++) {
-if (strcmp(endianness[i].arch, arch) == 0) {
-return endianness[i].big_endian;
-}
-}
-
-return false;
+return s->big_endian;
 }
diff --git a/tests/libqtest.h b/tests/libqtest.h
index f7402e0..4be1f77 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -410,6 +410,14 @@ int64_t qtest_clock_step(QTestState *s, int64_t step);
 int64_t qtest_clock_set(QTestState *s, int64_t val);
 
 /**
+ * qtest_big_endian:
+ * @s: QTestState instance to operate on.
+ *
+ * Returns: True if the architecture under test has a big endian configuration.
+ */
+bool 

[Qemu-devel] [PULL 05/16] tests/boot-sector: Increase time-out to 90 seconds

[David Gibson]

From: Thomas Huth 

Since the PXE tester runs rather slow on ppc64 with tcg, there
is a chance that we hit the 60 seconds timeout on machines that
have a heavy CPU load. So let's increase the timeout to ease
the situation.

Signed-off-by: Thomas Huth 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: David Gibson 
---
 tests/boot-sector.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/boot-sector.c b/tests/boot-sector.c
index 8399314..e3880f4 100644
--- a/tests/boot-sector.c
+++ b/tests/boot-sector.c
@@ -106,9 +106,9 @@ void boot_sector_test(void)
 uint16_t signature;
 int i;
 
-   /* Wait at most 1 minute */
+/* Wait at most 90 seconds */
 #define TEST_DELAY (1 * G_USEC_PER_SEC / 10)
-#define TEST_CYCLES MAX((60 * G_USEC_PER_SEC / TEST_DELAY), 1)
+#define TEST_CYCLES MAX((90 * G_USEC_PER_SEC / TEST_DELAY), 1)
 
 /* Poll until code has run and modified memory.  Once it has we know BIOS
  * initialization is done.  TODO: check that IP reached the halt
-- 
2.7.4

[Qemu-devel] [PULL 03/16] tests/boot-sector: Use minimum length for the Forth boot script

[David Gibson]

From: Thomas Huth 

The pxe-test is quite slow on ppc64 with tcg. We can speed it up
a little bit by decreasing the size of the file that has to be
loaded via TFTP.

Signed-off-by: Thomas Huth 
Reviewed-by: Eric Blake 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: David Gibson 
---
 tests/boot-sector.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/boot-sector.c b/tests/boot-sector.c
index e3193c0..0168fd0 100644
--- a/tests/boot-sector.c
+++ b/tests/boot-sector.c
@@ -72,6 +72,7 @@ static uint8_t boot_sector[0x7e000] = {
 int boot_sector_init(const char *fname)
 {
 FILE *f = fopen(fname, "w");
+size_t len = sizeof boot_sector;
 
 if (!f) {
 fprintf(stderr, "Couldn't open \"%s\": %s", fname, strerror(errno));
@@ -80,13 +81,12 @@ int boot_sector_init(const char *fname)
 
 /* For Open Firmware based system, we can use a Forth script instead */
 if (strcmp(qtest_get_arch(), "ppc64") == 0) {
-memset(boot_sector, ' ', sizeof boot_sector);
-sprintf((char *)boot_sector, "\\ Bootscript\n%x %x c! %x %x c!\n",
+len = sprintf((char *)boot_sector, "\\ Bootscript\n%x %x c! %x %x 
c!\n",
 LOW(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET,
 HIGH(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET + 1);
 }
 
-fwrite(boot_sector, 1, sizeof boot_sector, f);
+fwrite(boot_sector, 1, len, f);
 fclose(f);
 return 0;
 }
-- 
2.7.4

[Qemu-devel] [PULL 14/16] spapr: Adjust placement of PCI host bridge to allow > 1TiB RAM

[David Gibson]

Currently the default PCI host bridge for the 'pseries' machine type is
constructed with its IO windows in the 1TiB..(1TiB + 64GiB) range in
guest memory space.  This means that if > 1TiB of guest RAM is specified,
the RAM will collide with the PCI IO windows, causing serious problems.

Problems won't be obvious until guest RAM goes a bit beyond 1TiB, because
there's a little unused space at the bottom of the area reserved for PCI,
but essentially this means that > 1TiB of RAM has never worked with the
pseries machine type.

This patch fixes this by altering the placement of PHBs on large-RAM VMs.
Instead of always placing the first PHB at 1TiB, it is placed at the next
1 TiB boundary after the maximum RAM address.

Technically, this changes behaviour in a migration-breaking way for
existing machines with > 1TiB maximum memory, but since having > 1 TiB
memory was broken anyway, this seems like a reasonable trade-off.

Signed-off-by: David Gibson 
Reviewed-by: Laurent Vivier 
---
 hw/ppc/spapr.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2bfd187..d747e58 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2375,15 +2375,27 @@ static void spapr_phb_placement(sPAPRMachineState 
*spapr, uint32_t index,
 unsigned n_dma, uint32_t *liobns, Error **errp)
 {
 const uint64_t base_buid = 0x8002000ULL;
-const hwaddr phb0_base = 0x100ULL; /* 1 TiB */
 const hwaddr phb_spacing = 0x10ULL; /* 64 GiB */
 const hwaddr mmio_offset = 0xa000; /* 2 GiB + 512 MiB */
 const hwaddr pio_offset = 0x8000; /* 2 GiB */
 const uint32_t max_index = 255;
+const hwaddr phb0_alignment = 0x100ULL; /* 1 TiB */
 
-hwaddr phb_base;
+uint64_t ram_top = MACHINE(spapr)->ram_size;
+hwaddr phb0_base, phb_base;
 int i;
 
+/* Do we have hotpluggable memory? */
+if (MACHINE(spapr)->maxram_size > ram_top) {
+/* Can't just use maxram_size, because there may be an
+ * alignment gap between normal and hotpluggable memory
+ * regions */
+ram_top = spapr->hotplug_memory.base +
+memory_region_size(>hotplug_memory.mr);
+}
+
+phb0_base = QEMU_ALIGN_UP(ram_top, phb0_alignment);
+
 if (index > max_index) {
 error_setg(errp, "\"index\" for PAPR PHB is too large (max %u)",
max_index);
-- 
2.7.4

[Qemu-devel] [PULL 01/16] tests: minor cleanups in usb-hcd-uhci-test

[David Gibson]

From: Laurent Vivier 

Two minor cleanups:
- exit gracefully in case on unsupported target,
- put machine command line in a constant to avoid
  to duplicate it.

Signed-off-by: Laurent Vivier 
Reviewed-by: Gerd Hoffmann 
Signed-off-by: David Gibson 
---
 tests/usb-hcd-uhci-test.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
index 4b951ce..e956b9c 100644
--- a/tests/usb-hcd-uhci-test.c
+++ b/tests/usb-hcd-uhci-test.c
@@ -77,6 +77,9 @@ static void test_usb_storage_hotplug(void)
 int main(int argc, char **argv)
 {
 const char *arch = qtest_get_arch();
+const char *cmd = "-device piix3-usb-uhci,id=uhci,addr=1d.0"
+  " -drive id=drive0,if=none,file=/dev/null,format=raw"
+  " -device usb-tablet,bus=uhci.0,port=1";
 int ret;
 
 g_test_init(, , NULL);
@@ -87,13 +90,13 @@ int main(int argc, char **argv)
 qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug);
 
 if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-qs = qtest_pc_boot("-device piix3-usb-uhci,id=uhci,addr=1d.0"
-   " -drive 
id=drive0,if=none,file=/dev/null,format=raw"
-   " -device usb-tablet,bus=uhci.0,port=1");
+qs = qtest_pc_boot(cmd);
 } else if (strcmp(arch, "ppc64") == 0) {
-qs = qtest_spapr_boot("-device piix3-usb-uhci,id=uhci,addr=1d.0"
-   " -drive 
id=drive0,if=none,file=/dev/null,format=raw"
-   " -device usb-tablet,bus=uhci.0,port=1");
+qs = qtest_spapr_boot(cmd);
+} else {
+g_printerr("usb-hcd-uhci-test tests are only "
+   "available on x86 or ppc64\n");
+exit(EXIT_FAILURE);
 }
 ret = g_test_run();
 qtest_shutdown(qs);
-- 
2.7.4

[Qemu-devel] [PULL v2 4/4] tests/docker/Makefile.include: add a generic docker-run target

[Fam Zheng]

From: Alex Bennée 

This re-factors the docker makefile to include a docker-run target which
can be controlled entirely from environment variables specified on the
make command line. This allows us to run against any given docker image
we may have in our repository, for example:

make docker-run TEST="test-quick" IMAGE="debian:arm64" \
 EXECUTABLE=./aarch64-linux-user/qemu-aarch64

The existing docker-foo@bar targets still work but the inline
verification has been dropped because we already don't hit that due to
other pattern rules in rules.mak.

Signed-off-by: Alex Bennée 

Message-Id: <20161011161625.9070-5-alex.ben...@linaro.org>
Message-Id: <20161011161625.9070-6-alex.ben...@linaro.org>
[Squash in the verification removal patch. - Fam]
Signed-off-by: Fam Zheng 
---
 tests/docker/Makefile.include | 61 +++
 1 file changed, 38 insertions(+), 23 deletions(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index b44daab..3f15d5a 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -78,6 +78,7 @@ docker:
@echo ' "IMAGE" is one of the listed container 
name."'
@echo 'docker-image:Build all images.'
@echo 'docker-image-IMAGE:  Build image "IMAGE".'
+   @echo 'docker-run:  For manually running a "TEST" with 
"IMAGE"'
@echo
@echo 'Available container images:'
@echo '$(DOCKER_IMAGES)'
@@ -101,31 +102,45 @@ docker:
@echo 'NOCACHE=1Ignore cache when build images.'
@echo 'EXECUTABLE=Include executable in image.'
 
+# This rule if for directly running against an arbitrary docker target.
+# It is called by the expanded docker targets (e.g. make
+# docker-test-foo@bar) which will do additional verification.
+#
+# For example: make docker-run TEST="test-quick" IMAGE="debian:arm64" 
EXECUTABLE=./aarch64-linux-user/qemu-aarch64
+#
+docker-run: docker-qemu-src
+   @mkdir -p "$(DOCKER_CCACHE_DIR)"
+   @if test -z "$(IMAGE)" || test -z "$(TEST)"; \
+   then echo "Invalid target $(IMAGE)/$(TEST)"; exit 1; \
+   fi
+   $(if $(EXECUTABLE), \
+   $(call quiet-command,   \
+   $(SRC_PATH)/tests/docker/docker.py update   \
+   $(IMAGE) $(EXECUTABLE), \
+   "  COPYING $(EXECUTABLE) to $(IMAGE)"))
+   $(call quiet-command,   \
+   $(SRC_PATH)/tests/docker/docker.py run  \
+   -t  \
+   $(if $V,,--rm)  \
+   $(if $(DEBUG),-i,--net=none)\
+   -e TARGET_LIST=$(TARGET_LIST)   \
+   -e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
+   -e V=$V -e J=$J -e DEBUG=$(DEBUG)   \
+   -e SHOW_ENV=$(SHOW_ENV) \
+   -e CCACHE_DIR=/var/tmp/ccache   \
+   -v $$(readlink -e 
$(DOCKER_SRC_COPY)):/var/tmp/qemu:z$(COMMA)ro \
+   -v $(DOCKER_CCACHE_DIR):/var/tmp/ccache:z   \
+   $(IMAGE)\
+   /var/tmp/qemu/run   \
+   $(TEST), "  RUN $(TEST) in ${IMAGE}")
+
+# Run targets:
+#
+# Of the form docker-TEST-FOO@IMAGE-BAR which will then be expanded into a 
call to "make docker-run"
 docker-run-%: CMD = $(shell echo '$@' | sed -e 
's/docker-run-\([^@]*\)@\(.*\)/\1/')
 docker-run-%: IMAGE = $(shell echo '$@' | sed -e 
's/docker-run-\([^@]*\)@\(.*\)/\2/')
-docker-run-%: docker-qemu-src
-   @mkdir -p "$(DOCKER_CCACHE_DIR)"
-   @if test -z "$(IMAGE)" || test -z "$(CMD)"; \
-   then echo "Invalid target"; exit 1; \
-   fi
-   $(if $(filter $(TESTS),$(CMD)),$(if $(filter $(IMAGES),$(IMAGE)), \
-   $(call quiet-command,\
-   if $(SRC_PATH)/tests/docker/docker.py images | \
-   awk '$$1=="qemu" && $$2=="$(IMAGE)"{found=1} 
END{exit(!found)}'; then \
-   $(SRC_PATH)/tests/docker/docker.py run $(if 
$V,,--rm) \
-   -t \
-   $(if $(DEBUG),-i,--net=none) \
-   -e TARGET_LIST=$(TARGET_LIST) \
-   -e EXTRA_CONFIGURE_OPTS=$(EXTRA_CONFIGURE_OPTS) 
\
-   -e V=$V -e J=$J -e DEBUG=$(DEBUG) -e 
SHOW_ENV=$(SHOW_ENV)\
-   -e 

[Qemu-devel] [PULL v2 1/4] tests/docker: add travis dockerfile

[Fam Zheng]

From: Alex Bennée 

This target grabs the latest Travis containers from their repository at
quay.io and then installs QEMU's build dependencies. With this it is
possible to run on broadly the same setup as they have on travis-ci.org.

Signed-off-by: Alex Bennée 
Message-Id: <20161011161625.9070-2-alex.ben...@linaro.org>
Signed-off-by: Fam Zheng 
---
 tests/docker/dockerfiles/travis.docker | 6 ++
 1 file changed, 6 insertions(+)
 create mode 100644 tests/docker/dockerfiles/travis.docker

diff --git a/tests/docker/dockerfiles/travis.docker 
b/tests/docker/dockerfiles/travis.docker
new file mode 100644
index 000..e4983ae
--- /dev/null
+++ b/tests/docker/dockerfiles/travis.docker
@@ -0,0 +1,6 @@
+FROM quay.io/travisci/travis-ruby
+RUN apt-get update
+RUN apt-get -y build-dep qemu
+RUN apt-get -y build-dep device-tree-compiler
+RUN apt-get -y install python2.7 dh-autoreconf
+ENV FEATURES pyyaml
-- 
2.7.4

[Qemu-devel] [PULL v2 3/4] tests/docker: make test-mingw honour TARGET_LIST

[Fam Zheng]

From: Alex Bennée 

The other builders honour this variable, so should the mingw build.

Signed-off-by: Alex Bennée 
Message-Id: <20161011161625.9070-4-alex.ben...@linaro.org>
Signed-off-by: Fam Zheng 
---
 tests/docker/test-mingw | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/docker/test-mingw b/tests/docker/test-mingw
index 3396876..2adadcb 100755
--- a/tests/docker/test-mingw
+++ b/tests/docker/test-mingw
@@ -16,9 +16,10 @@
 requires mingw dtc
 
 cd "$BUILD_DIR"
+DEF_TARGET_LIST="x86_64-softmmu,aarch64-softmmu"
 
 for prefix in x86_64-w64-mingw32- i686-w64-mingw32-; do
-TARGET_LIST=x86_64-softmmu,aarch64-softmmu \
+TARGET_LIST=${TARGET_LIST:-$DEF_TARGET_LIST} \
 build_qemu --cross-prefix=$prefix \
 --enable-trace-backends=simple \
 --enable-debug \
-- 
2.7.4

[Qemu-devel] [PULL v2 2/4] tests/docker: test-build script

[Fam Zheng]

From: Alex Bennée 

Much like test-quick but only builds. This is useful for some of the
build targets like ThreadSanitizer that don't yet pass "make check".

Signed-off-by: Alex Bennée 

Message-Id: <20161011161625.9070-3-alex.ben...@linaro.org>
Signed-off-by: Fam Zheng 
---
 tests/docker/test-build | 20 
 1 file changed, 20 insertions(+)
 create mode 100755 tests/docker/test-build

diff --git a/tests/docker/test-build b/tests/docker/test-build
new file mode 100755
index 000..031a7d9
--- /dev/null
+++ b/tests/docker/test-build
@@ -0,0 +1,20 @@
+#!/bin/bash -e
+#
+# Quick compile test without the make check step of test-quick.
+#
+# Copyright (c) 2016 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2
+# or (at your option) any later version. See the COPYING file in
+# the top-level directory.
+
+. common.rc
+
+cd "$BUILD_DIR"
+
+DEF_TARGET_LIST="x86_64-softmmu,aarch64-softmmu"
+TARGET_LIST=${TARGET_LIST:-$DEF_TARGET_LIST} \
+build_qemu
-- 
2.7.4

[Qemu-devel] [PULL v2 0/4] Docker patches

[Fam Zheng]

The following changes since commit 6aa5a3679449cdf0b6fe5a6829b22e642ded57fd:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161013-1' into 
staging (2016-10-13 14:27:58 +0100)

are available in the git repository at:

  g...@github.com:famz/qemu tags/for-upstream

for you to fetch changes up to e86c9a64f455018fb04d631e14c5f926e36c69fb:

  tests/docker/Makefile.include: add a generic docker-run target (2016-10-17 
10:05:48 +0800)



v2: Fix $(MAKE) in patch 4. [Paolo]



Alex Bennée (4):
  tests/docker: add travis dockerfile
  tests/docker: test-build script
  tests/docker: make test-mingw honour TARGET_LIST
  tests/docker/Makefile.include: add a generic docker-run target

 tests/docker/Makefile.include  | 61 +-
 tests/docker/dockerfiles/travis.docker |  6 
 tests/docker/test-build| 20 +++
 tests/docker/test-mingw|  3 +-
 4 files changed, 66 insertions(+), 24 deletions(-)
 create mode 100644 tests/docker/dockerfiles/travis.docker
 create mode 100755 tests/docker/test-build

-- 
2.7.4

Re: [Qemu-devel] [PULL 4/4] tests/docker/Makefile.include: add a generic docker-run target

[Fam Zheng]

On Sat, 10/15 15:48, Alex Bennée wrote:
> 
> Paolo Bonzini  writes:
> 
> > On 14/10/2016 17:29, Fam Zheng wrote:
> >> From: Alex Bennée 
> >>
> >> This re-factors the docker makefile to include a docker-run target which
> >> can be controlled entirely from environment variables specified on the
> >> make command line. This allows us to run against any given docker image
> >> we may have in our repository, for example:
> >>
> >> make docker-run TEST="test-quick" IMAGE="debian:arm64" \
> >>  EXECUTABLE=./aarch64-linux-user/qemu-aarch64
> >>
> >> The existing docker-foo@bar targets still work but the inline
> >> verification has been dropped because we already don't hit that due to
> >> other pattern rules in rules.mak.
> >>
> >> Signed-off-by: Alex Bennée 
> >>
> >> Message-Id: <20161011161625.9070-5-alex.ben...@linaro.org>
> >> Message-Id: <20161011161625.9070-6-alex.ben...@linaro.org>
> >> [Squash in the verification removal patch. - Fam]
> >> Signed-off-by: Fam Zheng 
> >> ---
> >>  tests/docker/Makefile.include | 61 
> >> +++
> >>  1 file changed, 38 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> >> index b44daab..925f711 100644
> >> --- a/tests/docker/Makefile.include
> >> +++ b/tests/docker/Makefile.include
> >> @@ -78,6 +78,7 @@ docker:
> >>@echo ' "IMAGE" is one of the listed container 
> >> name."'
> >>@echo 'docker-image:Build all images.'
> >>@echo 'docker-image-IMAGE:  Build image "IMAGE".'
> >> +  @echo 'docker-run:  For manually running a "TEST" with 
> >> "IMAGE"'
> >>@echo
> >>@echo 'Available container images:'
> >>@echo '$(DOCKER_IMAGES)'
> >> @@ -101,31 +102,45 @@ docker:
> >>@echo 'NOCACHE=1Ignore cache when build images.'
> >>@echo 'EXECUTABLE=Include executable in image.'
> >>
> >> +# This rule if for directly running against an arbitrary docker target.
> >> +# It is called by the expanded docker targets (e.g. make
> >> +# docker-test-foo@bar) which will do additional verification.
> >> +#
> >> +# For example: make docker-run TEST="test-quick" IMAGE="debian:arm64" 
> >> EXECUTABLE=./aarch64-linux-user/qemu-aarch64
> >> +#
> >> +docker-run: docker-qemu-src
> >> +  @mkdir -p "$(DOCKER_CCACHE_DIR)"
> >> +  @if test -z "$(IMAGE)" || test -z "$(TEST)"; \
> >> +  then echo "Invalid target $(IMAGE)/$(TEST)"; exit 1; \
> >> +  fi
> >> +  $(if $(EXECUTABLE), \
> >> +  $(call quiet-command,   \
> >> +  $(SRC_PATH)/tests/docker/docker.py update   \
> >> +  $(IMAGE) $(EXECUTABLE), \
> >> +  "  COPYING $(EXECUTABLE) to $(IMAGE)"))
> >> +  $(call quiet-command,   \
> >> +  $(SRC_PATH)/tests/docker/docker.py run  \
> >> +  -t  \
> >> +  $(if $V,,--rm)  \
> >> +  $(if $(DEBUG),-i,--net=none)\
> >> +  -e TARGET_LIST=$(TARGET_LIST)   \
> >> +  -e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
> >> +  -e V=$V -e J=$J -e DEBUG=$(DEBUG)   \
> >> +  -e SHOW_ENV=$(SHOW_ENV) \
> >> +  -e CCACHE_DIR=/var/tmp/ccache   \
> >> +  -v $$(readlink -e 
> >> $(DOCKER_SRC_COPY)):/var/tmp/qemu:z$(COMMA)ro \
> >> +  -v $(DOCKER_CCACHE_DIR):/var/tmp/ccache:z   \
> >> +  $(IMAGE)\
> >> +  /var/tmp/qemu/run   \
> >> +  $(TEST), "  RUN $(TEST) in ${IMAGE}")
> >> +
> >> +# Run targets:
> >> +#
> >> +# Of the form docker-TEST-FOO@IMAGE-BAR which will then be expanded into 
> >> a call to "make docker-run"
> >>  docker-run-%: CMD = $(shell echo '$@' | sed -e 
> >> 's/docker-run-\([^@]*\)@\(.*\)/\1/')
> >>  docker-run-%: IMAGE = $(shell echo '$@' | sed -e 
> >> 's/docker-run-\([^@]*\)@\(.*\)/\2/')
> >> -docker-run-%: docker-qemu-src
> >> -  @mkdir -p "$(DOCKER_CCACHE_DIR)"
> >> -  @if test -z "$(IMAGE)" || test -z "$(CMD)"; \
> >> -  then echo "Invalid target"; exit 1; \
> >> -  fi
> >> -  $(if $(filter $(TESTS),$(CMD)),$(if $(filter $(IMAGES),$(IMAGE)), \
> >> -  $(call quiet-command,\
> >> -  if $(SRC_PATH)/tests/docker/docker.py images | \
> >> -  awk '$$1=="qemu" && $$2=="$(IMAGE)"{found=1} 
> >> END{exit(!found)}'; then \
> >> -  $(SRC_PATH)/tests/docker/docker.py 

Re: [Qemu-devel] [PATCH v6 26/35] tests: add atomic_add-bench

[Richard Henderson]

On 10/14/2016 02:19 PM, Emilio G. Cota wrote:

On Tue, Oct 11, 2016 at 14:40:52 -0500, Richard Henderson wrote:

> From: "Emilio G. Cota" 
>
> With this microbenchmark we can measure the overhead of emulating atomic
> instructions with a configurable degree of contention.
>
> The benchmark spawns $n threads, each performing $o atomic ops (additions)
> in a loop. Each atomic operation is performed on a different cache line
> (assuming lines are 64b long) that is randomly selected from a range [0, $r).
>
> [ Note: each $foo corresponds to a -foo flag ]
>
> Signed-off-by: Emilio G. Cota 
> Signed-off-by: Richard Henderson 
> Message-Id: <1467054136-10430-20-git-send-email-c...@braap.org>

Can you please fix up this patch with the below? It does the
same as we're doing in qht-bench:
  http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg03034.html


Done.

r~

Re: [Qemu-devel] [PATCH v6 13/35] tcg: Add atomic helpers

[Richard Henderson]

On 10/16/2016 03:17 PM, Emilio G. Cota wrote:

+#if DATA_SIZE == 1
> +# define END
> +#elif defined(HOST_WORDS_BIGENDIAN)
> +# define END  _be
> +#else
> +# define END  _le
> +#endif

It took me a while to figure out that ATOMIC_NAME needs END (ATOMIC_NAME
is defined later in the patch).

It might be clearer to pass it explicitly as a suffix in the macro, as in
#define ATOMIC_NAME(name, suffix) to then have ATOMIC_NAME(cmpxchg, END).


Hum.  Perhaps a comment?

r~

Re: [Qemu-devel] [PATCH v6 13/35] tcg: Add atomic helpers

[Richard Henderson]

On 10/16/2016 03:17 PM, Emilio G. Cota wrote:

> +/* Note that for addition, we need to use a separate cmpxchg loop instead
> +   of bswaps for the reverse-host-endian helpers.  */
> +ABI_TYPE ATOMIC_NAME(fetch_add)(CPUArchState *env, target_ulong addr,
> + ABI_TYPE val EXTRA_ARGS)
> +{
> +DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
> +DATA_TYPE ldo, ldn, ret, sto;
> +
> +ldo = *haddr;

ldo = atomic_read(haddr)
would be better here for C11 compliance (or tsan will complain).



Good point.  Fixed.


r~

Re: [Qemu-devel] [PATCH 08/11] spapr_events: add support for dedicated hotplug event source

[David Gibson]

On Fri, Oct 14, 2016 at 01:44:17PM -0500, Michael Roth wrote:
> Quoting David Gibson (2016-10-13 23:56:43)
> > On Wed, Oct 12, 2016 at 06:13:56PM -0500, Michael Roth wrote:
> > > Hotplug events were previously delivered using an EPOW interrupt
> > > and were queued by linux guests into a circular buffer. For traditional
> > > EPOW events like shutdown/resets, this isn't an issue, but for hotplug
> > > events there are cases where this buffer can be exhausted, resulting
> > > in the loss of hotplug events, resets, etc.
> > > 
> > > Newer-style hotplug event are delivered using a dedicated event source.
> > > We enable this in supported guests by adding standard an additional
> > > event source in the guest device-tree via /event-sources, and, if
> > > the guest advertises support for the newer-style hotplug events,
> > > using the corresponding interrupt to signal the available of
> > > hotplug/unplug events.
> > > 
> > > Signed-off-by: Michael Roth 
> > 
> > So.. are you saying that as well as allowing new event types, the new
> > special hotplug event souce effectively allows for a bigger queue?
> > 
> > Does that mean that we didn't even necessarily need the base+length
> > unplug events, because we could now have sent the many single-LMB
> > unplug requests that were necessary?  Or does it not increase the
> > effective queue enough for that?
> 
> I assume there are still some internal limits, but the events
> are now processed using a workqueue which should provide a bit more
> headroom than the RTAS event buffer used for EPOW events. Maybe
> John (on cc:) can provide more insight into what the actual limits
> 
> In either case, the possibility for huge memory hotplug/unplug
> situations still warrant the use of count+index I think, and since
> support for the new event delivery mechanism is negotiated using the
> same option bit as count+index, there's not really any reason not to
> use it when we can. For situations where we can't (CPU/PCI), it
> might give us a bit of breathing room there as well.

Ok, makes sense.  Thanks for the extra information.

> 
> > 
> > > ---
> > >  hw/ppc/spapr.c |  10 ++--
> > >  hw/ppc/spapr_events.c  | 148 
> > > ++---
> > >  include/hw/ppc/spapr.h |   3 +-
> > >  3 files changed, 120 insertions(+), 41 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index d80a6fa..2037222 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -275,8 +275,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
> > > hwaddr initrd_size,
> > > hwaddr kernel_size,
> > > bool little_endian,
> > > -   const char *kernel_cmdline,
> > > -   uint32_t epow_irq)
> > > +   const char *kernel_cmdline)
> > >  {
> > >  void *fdt;
> > >  uint32_t start_prop = cpu_to_be32(initrd_base);
> > > @@ -437,7 +436,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
> > >  _FDT((fdt_end_node(fdt)));
> > >  
> > >  /* event-sources */
> > > -spapr_events_fdt_skel(fdt, epow_irq);
> > > +spapr_events_fdt_skel(fdt);
> > >  
> > >  /* /hypervisor node */
> > >  if (kvm_enabled()) {
> > > @@ -1944,7 +1943,7 @@ static void ppc_spapr_init(MachineState *machine)
> > >  }
> > >  g_free(filename);
> > >  
> > > -/* Set up EPOW events infrastructure */
> > > +/* Set up RTAS event infrastructure */
> > >  spapr_events_init(spapr);
> > >  
> > >  /* Set up the RTC RTAS interfaces */
> > > @@ -2076,8 +2075,7 @@ static void ppc_spapr_init(MachineState *machine)
> > >  /* Prepare the device tree */
> > >  spapr->fdt_skel = spapr_create_fdt_skel(initrd_base, initrd_size,
> > >  kernel_size, kernel_le,
> > > -kernel_cmdline,
> > > -spapr->check_exception_irq);
> > > +kernel_cmdline);
> > >  assert(spapr->fdt_skel != NULL);
> > >  
> > >  /* used by RTAS */
> > > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> > > index 4c7b6ae..f8bbec6 100644
> > > --- a/hw/ppc/spapr_events.c
> > > +++ b/hw/ppc/spapr_events.c
> > > @@ -40,6 +40,7 @@
> > >  #include "hw/ppc/spapr_drc.h"
> > >  #include "qemu/help_option.h"
> > >  #include "qemu/bcd.h"
> > > +#include "hw/ppc/spapr_ovec.h"
> > >  #include 
> > >  
> > >  struct rtas_error_log {
> > > @@ -206,28 +207,104 @@ struct hp_log_full {
> > >  struct rtas_event_log_v6_hp hp;
> > >  } QEMU_PACKED;
> > >  
> > > -#define EVENT_MASK_INTERNAL_ERRORS   0x8000
> > > -#define EVENT_MASK_EPOW  0x4000
> > > -#define EVENT_MASK_HOTPLUG   0x1000
> > > 

Re: [Qemu-devel] [PATCH v4 17/20] ppc/pnv: Add cut down PSI bridge model and hookup external interrupt

[David Gibson]

On Fri, Oct 14, 2016 at 10:07:53AM +0200, Cédric Le Goater wrote:
> >> --- a/hw/ppc/pnv.c
> >> +++ b/hw/ppc/pnv.c
> >> @@ -318,15 +318,24 @@ static void ppc_powernv_reset(void)
> >>   * have a CPLD that will collect the SerIRQ and shoot them as a
> >>   * single level interrupt to the P8 chip. So let's setup a hook
> >>   * for doing just that.
> >> - *
> >> - * Note: The actual interrupt input isn't emulated yet, this will
> >> - * come with the PSI bridge model.
> >>   */
> >>  static void pnv_lpc_isa_irq_handler_cpld(void *opaque, int n, int level)
> >>  {
> >> -/* We don't yet emulate the PSI bridge which provides the external
> >> - * interrupt, so just drop interrupts on the floor
> >> - */
> >> +static uint32_t irqstate;
> > 
> > Hmm.. static local with important state?  That it's not clear whether
> > it should be per-chip or not?
> > 
> > I'm not averse to hacks for early bringup, but it should at least have
> > a FIXME comment on it.
> 
> yes. I will see if I can make a "irq_cpld' attribute of the chip instead. 
> It should be cleaner.

Wouldn't it be in the machine, not the chip?  IIUC there's only one
CPLD on the whole board.

-- 
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 v4 15/20] ppc/xics: Add "native" XICS subclass

[David Gibson]

On Fri, Oct 14, 2016 at 11:40:30AM +0200, Cédric Le Goater wrote:
> On 10/14/2016 08:10 AM, David Gibson wrote:
> > On Mon, Oct 03, 2016 at 09:24:51AM +0200, Cédric Le Goater wrote:
> >> From: Benjamin Herrenschmidt 
> >>
> >> This provides access to the MMIO based Interrupt Presentation
> >> Controllers (ICP) as found on a POWER8 system.
> >>
> >> A new XICSNative class is introduced to hold the MMIO region of the
> >> ICPs. It also makes use of a hash table to associate the ICPState of a
> >> CPU with a HW processor id, as this is the server number presented in
> >> the XIVEs.
> >>
> >> The class routine 'find_icp' provide the way to do the lookups when
> >> needed in the XICS base class.
> >>
> >> Signed-off-by: Benjamin Herrenschmidt 
> >> [clg: - naming cleanups
> >>   - replaced the use of xics_get_cpu_index_by_dt_id() by 
> >> xics_find_icp()
> >>   - added some qemu logging in case of error  
> >>   - introduced a xics_native_find_icp routine to map icp index to
> >> cpu index  
> >>   - moved sysbus mapping to chip ]
> >> Signed-off-by: Cédric Le Goater 
> >> ---
> >>
> >>  checkpatch complains on this one, but it seems to be a false positive :
> >>  
> >>  ERROR: spaces required around that '&' (ctx:WxV)
> >>  #314: FILE: hw/intc/xics_native.c:246:
> >>  +(gpointer) >ss[cs->cpu_index]);
> >>
> >>  default-configs/ppc64-softmmu.mak |   3 +-
> >>  hw/intc/Makefile.objs |   1 +
> >>  hw/intc/xics_native.c | 327 
> >> ++
> >>  include/hw/ppc/pnv.h  |  19 +++
> >>  include/hw/ppc/xics.h |  24 +++
> >>  5 files changed, 373 insertions(+), 1 deletion(-)
> >>  create mode 100644 hw/intc/xics_native.c
> >>
> >> diff --git a/default-configs/ppc64-softmmu.mak 
> >> b/default-configs/ppc64-softmmu.mak
> >> index 67a9bcaa67fa..a22c93a48686 100644
> >> --- a/default-configs/ppc64-softmmu.mak
> >> +++ b/default-configs/ppc64-softmmu.mak
> >> @@ -48,8 +48,9 @@ CONFIG_PLATFORM_BUS=y
> >>  CONFIG_ETSEC=y
> >>  CONFIG_LIBDECNUMBER=y
> >>  # For pSeries
> >> -CONFIG_XICS=$(CONFIG_PSERIES)
> >> +CONFIG_XICS=$(or $(CONFIG_PSERIES),$(CONFIG_POWERNV))
> >>  CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
> >> +CONFIG_XICS_NATIVE=$(CONFIG_POWERNV)
> >>  CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
> >>  # For PReP
> >>  CONFIG_MC146818RTC=y
> >> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> >> index 05ec21b21e0e..7be5dfc8347b 100644
> >> --- a/hw/intc/Makefile.objs
> >> +++ b/hw/intc/Makefile.objs
> >> @@ -31,6 +31,7 @@ obj-$(CONFIG_RASPI) += bcm2835_ic.o bcm2836_control.o
> >>  obj-$(CONFIG_SH4) += sh_intc.o
> >>  obj-$(CONFIG_XICS) += xics.o
> >>  obj-$(CONFIG_XICS_SPAPR) += xics_spapr.o
> >> +obj-$(CONFIG_XICS_NATIVE) += xics_native.o
> >>  obj-$(CONFIG_XICS_KVM) += xics_kvm.o
> >>  obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o
> >>  obj-$(CONFIG_S390_FLIC) += s390_flic.o
> >> diff --git a/hw/intc/xics_native.c b/hw/intc/xics_native.c
> >> new file mode 100644
> >> index ..16413d807f65
> >> --- /dev/null
> >> +++ b/hw/intc/xics_native.c
> >> @@ -0,0 +1,327 @@
> >> +/*
> >> + * QEMU PowerPC PowerNV machine model
> >> + *
> >> + * Native version of ICS/ICP
> >> + *
> >> + * Copyright (c) 2016, IBM Corporation.
> >> + *
> >> + * This library is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU Lesser General Public
> >> + * License as published by the Free Software Foundation; either
> >> + * version 2 of the License, or (at your option) any later version.
> >> + *
> >> + * This library is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >> + * Lesser General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU Lesser General Public
> >> + * License along with this library; if not, see 
> >> .
> >> + */
> >> +
> >> +#include "qemu/osdep.h"
> >> +#include "qapi/error.h"
> >> +#include "qemu-common.h"
> >> +#include "cpu.h"
> >> +#include "hw/hw.h"
> >> +#include "qemu/log.h"
> >> +#include "qapi/error.h"
> >> +
> >> +#include "hw/ppc/fdt.h"
> >> +#include "hw/ppc/xics.h"
> >> +#include "hw/ppc/pnv.h"
> >> +
> >> +#include 
> >> +
> >> +static void xics_native_reset(void *opaque)
> >> +{
> >> +device_reset(DEVICE(opaque));
> >> +}
> >> +
> >> +static void xics_native_initfn(Object *obj)
> >> +{
> >> +XICSState *xics = XICS_COMMON(obj);
> >> +
> >> +QLIST_INIT(>ics);
> > 
> > This shouldn't be necessary, because it's done in the base class
> > initfn (which is called before the subclass initfns).
> 
> True. This is a left over. 
> 
> >> +/*
> >> + * Let's not forget to register a reset handler else the ICPs
> 

Re: [Qemu-devel] [PATCH v4 10/20] ppc/xics: Make the ICSState a list

[David Gibson]

On Fri, Oct 14, 2016 at 09:35:46AM +0200, Cédric Le Goater wrote:
> On 10/14/2016 07:32 AM, David Gibson wrote:
> > On Mon, Oct 03, 2016 at 09:24:46AM +0200, Cédric Le Goater wrote:
> >> From: Benjamin Herrenschmidt 
> >>
> >> Instead of an array of fixed sized blocks, use a list, as we will need
> >> to have sources with variable number of interrupts. SPAPR only uses
> >> a single entry. Native will create more. If performance becomes an
> >> issue we can add some hashed lookup but for now this will do fine.
> >>
> >> Signed-off-by: Benjamin Herrenschmidt 
> >> [ move the initialization of list to xics_common_initfn,
> >>   restore xirr_owner after migration and move restoring to
> >>   icp_post_load]
> >> Signed-off-by: Nikunj A Dadhania 
> >> [ clg: removed the icp_post_load() changes from nikunj patchset v3:
> >>http://patchwork.ozlabs.org/patch/646008/ ]
> >> Signed-off-by: Cédric Le Goater 
> > 
> > I think this and 11/20 are good enough and sufficiently standalone
> > that I've merged them into ppc-for-2.8.
> 
> These are a prereq for the patchset.

Sorry, I wasn't clear.  I realize they're prerequisites for the rest
of the patchset.  What I meant is that they don't require the rest of
the patchset to apply themselves, and they make some sense even
without the rest of the series.

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

[Qemu-devel] [PATCH] e1000e: Don't zero out buffer address in rx descriptor

[Kevin Wolf]

The e1000e emulation zeroes out any used rx descriptor and then writes a
completely newly constructed value there. By doing this, it doesn't only
update the write-back area of the descriptors (as it's supposed to do),
but it also clears the buffer address, which real hardware doesn't do.

The spec explicitly mentions in chapter 7.1.8 that it is valid for a
driver to reuse a descriptor and only update the status field while
doing so, i.e. reusing the old buffer address:

If software statically allocates buffers, and uses memory read to
check for completed descriptors, it simply has to zero the status
byte in the descriptor to make it ready for reuse by hardware.

This patch fixes the behaviour to leave the buffer address in
descriptors unchanged even after the descriptor has been used.

Signed-off-by: Kevin Wolf 
---
 hw/net/e1000e_core.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 9fa4116..a5ca97d 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1280,11 +1280,10 @@ e1000e_write_lgcy_rx_descr(E1000ECore *core, uint8_t 
*desc,
 
 struct e1000_rx_desc *d = (struct e1000_rx_desc *) desc;
 
-memset(d, 0, sizeof(*d));
-
 assert(!rss_info->enabled);
 
 d->length = cpu_to_le16(length);
+d->csum = 0;
 
 e1000e_build_rx_metadata(core, pkt, pkt != NULL,
  rss_info,
@@ -1293,6 +1292,7 @@ e1000e_write_lgcy_rx_descr(E1000ECore *core, uint8_t 
*desc,
  >special);
 d->errors = (uint8_t) (le32_to_cpu(status_flags) >> 24);
 d->status = (uint8_t) le32_to_cpu(status_flags);
+d->special = 0;
 }
 
 static inline void
@@ -1303,7 +1303,7 @@ e1000e_write_ext_rx_descr(E1000ECore *core, uint8_t *desc,
 {
 union e1000_rx_desc_extended *d = (union e1000_rx_desc_extended *) desc;
 
-memset(d, 0, sizeof(*d));
+memset(>wb, 0, sizeof(d->wb));
 
 d->wb.upper.length = cpu_to_le16(length);
 
@@ -1327,7 +1327,7 @@ e1000e_write_ps_rx_descr(E1000ECore *core, uint8_t *desc,
 union e1000_rx_desc_packet_split *d =
 (union e1000_rx_desc_packet_split *) desc;
 
-memset(d, 0, sizeof(*d));
+memset(>wb, 0, sizeof(d->wb));
 
 d->wb.middle.length0 = cpu_to_le16((*written)[0]);
 
-- 
2.1.4

Re: [Qemu-devel] [PATCH v6 00/35] cmpxchg-based emulation of atomics

[Emilio G. Cota]

On Tue, Oct 11, 2016 at 14:40:26 -0500, Richard Henderson wrote:
> Sixth time is the charm, right?  This time I'm certain that it
> compiles with centos6, and contains the previously missing update
> from Emilio to atomic_add-bench.

For patches 03-16 (including the elusive patch 06 for which I reviewed 1bfe0cdf8
from your atomic-4 branch on github):

  Reviewed-by: Emilio G. Cota 

I just tested the patchset by running concurrencykit's ck_pr regression test 
(which
tests lock'ed ops) for [guest-on-host bits, all x86] 64-on-64, 32-on-32 and
64-on-32. I ran it with TCG debugging enabled. It passes all tests.

I also checked (on x86_64 for several target architectures) that all patches
build OK.

Thanks,

Emilio

Re: [Qemu-devel] [PATCH v6 13/35] tcg: Add atomic helpers

[Emilio G. Cota]

On Tue, Oct 11, 2016 at 14:40:39 -0500, Richard Henderson wrote:
> Add all of cmpxchg, op_fetch, fetch_op, and xchg.
> Handle both endian-ness, and sizes up to 8.
> Handle expanding non-atomically, when emulating in serial.
> 
> Signed-off-by: Richard Henderson 
> ---
>  Makefile.objs |   2 +-
>  Makefile.target   |   1 +
>  atomic_template.h | 173 ++
>  cputlb.c  | 112 -
>  include/qemu/atomic.h |  43 ---

I'd have the changes to atomic.h as a separate patch--this one is
pretty big already.

>  tcg-runtime.c |  49 ++--
>  tcg/tcg-op.c  | 328 
> ++
>  tcg/tcg-op.h  |  44 +++
>  tcg/tcg-runtime.h |  75 
>  tcg/tcg.h |  53 
>  10 files changed, 848 insertions(+), 32 deletions(-)
>  create mode 100644 atomic_template.h
(snip)
> diff --git a/atomic_template.h b/atomic_template.h
> new file mode 100644
> index 000..d2c8a08
> --- /dev/null
> +++ b/atomic_template.h
(snip)
> +#if DATA_SIZE == 1
> +# define END
> +#elif defined(HOST_WORDS_BIGENDIAN)
> +# define END  _be
> +#else
> +# define END  _le
> +#endif

It took me a while to figure out that ATOMIC_NAME needs END (ATOMIC_NAME
is defined later in the patch).

It might be clearer to pass it explicitly as a suffix in the macro, as in
#define ATOMIC_NAME(name, suffix) to then have ATOMIC_NAME(cmpxchg, END).

(snip)
> +
> +/* Note that for addition, we need to use a separate cmpxchg loop instead
> +   of bswaps for the reverse-host-endian helpers.  */
> +ABI_TYPE ATOMIC_NAME(fetch_add)(CPUArchState *env, target_ulong addr,
> + ABI_TYPE val EXTRA_ARGS)
> +{
> +DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
> +DATA_TYPE ldo, ldn, ret, sto;
> +
> +ldo = *haddr;

ldo = atomic_read(haddr)
would be better here for C11 compliance (or tsan will complain).

> +while (1) {
> +ret = BSWAP(ldo);
> +sto = BSWAP(ret + val);
> +ldn = atomic_cmpxchg__nocheck(haddr, ldo, sto);
> +if (ldn == ldo) {
> +return ret;
> +}
> +ldo = ldn;
> +}
> +}
> +
> +ABI_TYPE ATOMIC_NAME(add_fetch)(CPUArchState *env, target_ulong addr,
> + ABI_TYPE val EXTRA_ARGS)
> +{
> +DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP;
> +DATA_TYPE ldo, ldn, ret, sto;
> +
> +ldo = *haddr;

ditto.

No more comments. Have to say though that I really like how with the
tables in tcg-op.c, code ends up looking neat!

Emilio

[Qemu-devel] [PATCH] include/qemu: Add documentation to functions in include/qemu/id.h

[Veronia Bahaa]

Add documentation to the functions id_generate and id_wellformed in 
include/qemu/id.h

Signed-off-by: Veronia Bahaa 
---
 include/qemu/id.h |   23 +++
 1 file changed, 23 insertions(+)

diff --git a/include/qemu/id.h b/include/qemu/id.h
index 40c7010..7bbcdc0 100644
--- a/include/qemu/id.h
+++ b/include/qemu/id.h
@@ -7,7 +7,30 @@ typedef enum IdSubSystems {
 ID_MAX  /* last element, used as array size */
 } IdSubSystems;
 
+/**
+ * id_generate: Generates an ID of the form PREFIX SUBSYSTEM NUMBER
+ *  where:
+ *
+ *  - PREFIX is the reserved character '#'
+ *  - SUBSYSTEM identifies the subsystem creating the ID
+ *  - NUMBER is a decimal number unique within SUBSYSTEM.
+ *
+ *Example: "#block146"
+ *
+ * Returns the generated id string for the subsystem
+ *
+ * @id: the subsystem to generate an id for
+ */
 char *id_generate(IdSubSystems id);
+
+/**
+ * id_wellformed: checks that an id starts with a letter
+ *  followed by numbers, digits, '-','.', or '_'
+ *
+ * Returns %true if the id is well-formed
+ *
+ * @id: the id to be checked
+ */
 bool id_wellformed(const char *id);
 
 #endif
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH] linux-user: Fix do_store_exclusive for shared memory of interprocess.

[Emilio G. Cota]

(Adding Richard to Cc)

On Sat, Oct 15, 2016 at 23:53:48 +0800, Heiher wrote:
> From: Heiher 
> 
> test case: http://pastebin.com/raw/x2GW4xNW

You should check out this patchset and use it as a base for working on this
topic:

  http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg02341.html

In particular, the added tests/atomic_add-bench does a very similar thing
to what you're doing with your test case -- although with pthreads instead of
mmap(MAP_SHARED) + fork.

(more comments below)

> Signed-off-by: Heiher 
> ---
>  linux-user/main.c | 24 ++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 0e31dad..81b0a49 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -2312,6 +2312,23 @@ static const uint8_t mips_syscall_args[] = {
>  #  undef MIPS_SYS
>  # endif /* O32 */
>  
> +#define cmpxchg_user(old, new, gaddr, target_type)   \
> +({   \
> +abi_ulong __gaddr = (gaddr); \
> +target_type *__hptr; \
> +abi_long __ret = 0;  
> \
> +if ((__hptr = lock_user(VERIFY_WRITE, __gaddr, sizeof(target_type), 0))) 
> { \
> +if ((old) != atomic_cmpxchg(__hptr, (old), (new)))   \
> +__ret = -TARGET_EAGAIN;  \
> +unlock_user(__hptr, __gaddr, sizeof(target_type));   \
> +} else   \
> +__ret = -TARGET_EFAULT;  
> \
> +__ret;   \
> +})
> +
> +#define cmpxchg_user_u32(old, new, gaddr) cmpxchg_user((old), (new), 
> (gaddr), uint32_t)
> +#define cmpxchg_user_u64(old, new, gaddr) cmpxchg_user((old), (new), 
> (gaddr), uint64_t)
> +
>  static int do_store_exclusive(CPUMIPSState *env)
>  {
>  target_ulong addr;
> @@ -2342,12 +2359,15 @@ static int do_store_exclusive(CPUMIPSState *env)
>  env->active_tc.gpr[reg] = 0;
>  } else {
>  if (d) {
> -segv = put_user_u64(env->llnewval, addr);
> +segv = cmpxchg_user_u64(env->llval, env->llnewval, addr);
>  } else {
> -segv = put_user_u32(env->llnewval, addr);
> +segv = cmpxchg_user_u32(env->llval, env->llnewval, addr);
>  }
>  if (!segv) {
>  env->active_tc.gpr[reg] = 1;
> +} else if (-TARGET_EAGAIN == segv) {
> +segv = 0;
> +env->active_tc.gpr[reg] = 0;
>  }
>  }
>  }

With the atomic cmpxchg patch series referenced above, we should directly
translate to cmpxchg, thereby removing the exception--just like this
patch does for the Alpha architecture:
  http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg02373.html

Thanks

Emilio

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Peter Maydell]

On 16 October 2016 at 15:31, Michael Tokarev  wrote:
> 13.10.2016 21:29, Stefan Weil wrote:
>> scripts/tracetool generates a C preprocessor macro from the name of the
>> build directory. Any characters which are possible in a directory name
>> but not allowed in a macro name must be substituted, otherwise builds
>> will fail.
>
> Applied to -trivial, thank you!

Consensus on this and the other thread seemed to be
that we should just fix the bug where tracetool is
putting the build directory name into symbols
rather than working around that...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/4] Added error checking for msix_init.

[Stefan Hajnoczi]

On Sun, Oct 16, 2016 at 12:53:16AM -, Shreya Shrivastava wrote:
> Add checks for negative return value to uses of msix_init.
> Signed-off-by: Shreya Shrivastava 
> 
> ---
>  hw/usb/hcd-xhci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 4acf0c6..cb854aa 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -3705,10 +3705,11 @@ static void usb_xhci_realize(struct PCIDevice
> *dev, Error **errp)
> 
>  if (xhci->msix != ON_OFF_AUTO_OFF) {
>  /* TODO check for errors */
> -msix_init(dev, xhci->numintrs,
> +ret = msix_init(dev, xhci->numintrs,
>>mem, 0, OFF_MSIX_TABLE,
>>mem, 0, OFF_MSIX_PBA,
>0x90);
> +assert(ret >= 0);
>  }
>  }

Please handle the error instead of adding an assert:

1. Use error_setg_errno(errp, -ret, "MSI-X initialization failed") so
   the caller knows an error occurred.

2. Look at the resources initialized by this function and pick
   corresponding cleanup calls from usb_xhci_exit() so they are freed.
   For example xhci->mfwrap_timer was created so timer_free() needs to
   be called.

   (This is easier if you can move the msix_init() call further up
   before resources are initialized just like the msi_init() call.  But
   you need to check if msix_init() relies on anything before moving the
   call up.)


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 3/4] Add error checking for event_notifier_init.

[Stefan Hajnoczi]

On Sun, Oct 16, 2016 at 12:50:58AM -, Shreya Shrivastava wrote:
> Add checks for negative return value to uses of event_notifier_init.
> Signed-off-by: Shreya Shrivastava 
> 
> ---
>  tests/test-aio.c | 59
> 
>  1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/tests/test-aio.c b/tests/test-aio.c
> index 03aa846..49b99f6 100644
> --- a/tests/test-aio.c
> +++ b/tests/test-aio.c
> @@ -137,8 +137,8 @@ static void test_acquire(void)
>  AcquireTestData data;
> 
>  /* Dummy event notifier ensures aio_poll() will block */
> -event_notifier_init(, false);
> -set_event_notifier(ctx, , dummy_notifier_read);
> +if (event_notifier_init(, false))
> +set_event_notifier(ctx, , dummy_notifier_read);

Silently ignoring the error is not useful.

You could define a helper function:

static void check_event_notifier_init(EventNotifier *notifier)
{
int ret = event_notifier_init(notifier, false);

g_assert(ret == 0);
}

Then use the helper instead of calling event_notifier_init() directly.
That way the test case will fail loudly if event_notifier_int() failed.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 2/4] Add error checking for load_image_targphys.

[Stefan Hajnoczi]

On Sun, Oct 16, 2016 at 12:48:24AM -, Shreya Shrivastava wrote:
> Add checks for negative return value to uses of load_image_targphys.
> Signed-off-by: Shreya Shrivastava 
> 
> ---
>  hw/arm/nseries.c  |  9 +++--
>  hw/lm32/milkymist.c   | 19 +++
>  hw/ppc/virtex_ml507.c |  5 +
>  3 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
> index c86cf80..e0b0ae5 100644
> --- a/hw/arm/nseries.c
> +++ b/hw/arm/nseries.c
> @@ -1306,6 +1306,7 @@ static int n810_atag_setup(const struct
> arm_boot_info *info, void *p)

Your email client is wrapping lines.  git-am(1) cannot apply this patch.

Please use git-send-email(1) as recommended in the guidelines:
http://qemu-project.org/Contribute/SubmitAPatch

>  static void n8x0_init(MachineState *machine,
>struct arm_boot_info *binfo, int model)
>  {
> +int rom_size;
>  MemoryRegion *sysmem = get_system_memory();
>  struct n800_s *s = (struct n800_s *) g_malloc0(sizeof(*s));
>  int sdram_size = binfo->ram_size;
> @@ -1379,10 +1380,14 @@ static void n8x0_init(MachineState *machine,
>   *
>   * The code above is for loading the `zImage' file from Nokia
>   * images.  */
> -load_image_targphys(option_rom[0].name,
> + rom_size = load_image_targphys(option_rom[0].name,
>  OMAP2_Q2_BASE + 0x40,
>  sdram_size - 0x40);
> -
> + if (rom_size < 0) {
> +   fprintf(stderr, "qemu: could not load rom file '%s'\n",
> +   option_rom[0].name);
> +   exit(1);
> +}

QEMU coding style uses 4-space indentation.  Please run your patches
through scripts/checkpatch.pl to identify coding style issues.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-block] [PATCH v2] rbd: make the code more readable

[Stefan Hajnoczi]

On Sat, Oct 15, 2016 at 04:26:13PM +0800, Xiubo Li wrote:
> Make it a bit clearer and more readable.
> 
> Signed-off-by: Xiubo Li 
> CC: John Snow 
> ---
> 
> V2:
> - Advice from John Snow. Thanks.
> 
> 
>  block/rbd.c | 25 -
>  1 file changed, 12 insertions(+), 13 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-block] [PATCH 18/18] aio: convert from RFifoLock to QemuRecMutex

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 07:34:22PM +0200, Paolo Bonzini wrote:
> It is simpler and a bit faster, and QEMU does not need the contention
> callbacks (and thus the fairness) anymore.
> 
> Reviewed-by: Fam Zheng 
> Signed-off-by: Paolo Bonzini 
> ---
>  async.c  |  8 ++---
>  include/block/aio.h  |  3 +-
>  include/qemu/rfifolock.h | 54 
>  tests/.gitignore |  1 -
>  tests/Makefile.include   |  2 --
>  tests/test-rfifolock.c   | 91 
> 
>  util/Makefile.objs   |  1 -
>  util/rfifolock.c | 78 -
>  8 files changed, 5 insertions(+), 233 deletions(-)
>  delete mode 100644 include/qemu/rfifolock.h
>  delete mode 100644 tests/test-rfifolock.c
>  delete mode 100644 util/rfifolock.c

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-block] [PATCH 15/18] block: only call aio_poll on the current thread's AioContext

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 07:34:19PM +0200, Paolo Bonzini wrote:
> aio_poll is not thread safe; for example bdrv_drain can hang if
> the last in-flight I/O operation is completed in the I/O thread after
> the main thread has checked bs->in_flight.
> 
> The bug remains latent as long as all of it is called within
> aio_context_acquire/aio_context_release, but this will change soon.
> 
> To fix this, if bdrv_drain is called from outside the I/O thread,
> signal the main AioContext through a dummy bottom half.  The event
> loop then only runs in the I/O thread.
> 
> Reviewed-by: Fam Zheng 
> Signed-off-by: Paolo Bonzini 
> ---
>  async.c   |  1 +
>  block.c   |  2 ++
>  block/io.c|  7 +++
>  include/block/block.h | 24 +---
>  include/block/block_int.h |  1 +
>  5 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/async.c b/async.c
> index f30d011..fb37b03 100644
> --- a/async.c
> +++ b/async.c
> @@ -61,6 +61,7 @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc 
> *cb, void *opaque)
>  smp_wmb();
>  ctx->first_bh = bh;
>  qemu_mutex_unlock(>bh_lock);
> +aio_notify(ctx);
>  }
>  
>  QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
> diff --git a/block.c b/block.c
> index fbe485c..a17baab 100644
> --- a/block.c
> +++ b/block.c
> @@ -2090,7 +2090,9 @@ int bdrv_reopen_multiple(AioContext *ctx, 
> BlockReopenQueue *bs_queue, Error **er
>  
>  assert(bs_queue != NULL);
>  
> +aio_context_release(ctx);
>  bdrv_drain_all();
> +aio_context_acquire(ctx);
>  
>  QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
>  if (bdrv_reopen_prepare(_entry->state, bs_queue, _err)) {
> diff --git a/block/io.c b/block/io.c
> index 7d3dcfc..cd28909 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -474,8 +474,15 @@ void bdrv_inc_in_flight(BlockDriverState *bs)
>  atomic_inc(>in_flight);
>  }
>  
> +static void dummy_bh_cb(void *opaque)
> +{
> +}
> +
>  void bdrv_wakeup(BlockDriverState *bs)
>  {
> +if (bs->wakeup) {
> +aio_bh_schedule_oneshot(qemu_get_aio_context(), dummy_bh_cb, NULL);
> +}
>  }

Why use a dummy BH instead of aio_notify()?

>  
>  void bdrv_dec_in_flight(BlockDriverState *bs)
> diff --git a/include/block/block.h b/include/block/block.h
> index ba4318b..72d5d8e 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -343,9 +343,27 @@ void bdrv_drain_all(void);
>  #define bdrv_poll_while(bs, cond) ({   \
>  bool waited_ = false;  \
>  BlockDriverState *bs_ = (bs);  \
> -while ((cond)) {   \
> -aio_poll(bdrv_get_aio_context(bs_), true); \
> -waited_ = true;\
> +AioContext *ctx_ = bdrv_get_aio_context(bs_);  \
> +if (aio_context_in_iothread(ctx_)) {   \
> +while ((cond)) {   \
> +aio_poll(ctx_, true);  \
> +waited_ = true;\
> +}  \
> +} else {   \
> +assert(qemu_get_current_aio_context() ==   \
> +   qemu_get_aio_context());\

The assumption is that IOThread #1 will never call bdrv_poll_while() on
IOThread #2's AioContext.  I believe that is true today.  Is this what
you had in mind?

Please add a comment since it's not completely obvious from the assert
expression.

> +/* Ask bdrv_dec_in_flight to wake up the main  \
> + * QEMU AioContext.\
> + */\
> +assert(!bs_->wakeup);  \
> +bs_->wakeup = true;\
> +while ((cond)) {   \
> +aio_context_release(ctx_); \
> +aio_poll(qemu_get_aio_context(), true);\
> +aio_context_acquire(ctx_); \
> +waited_ = true;\
> +}  \
> +bs_->wakeup = false;   \
>  }  \
>  waited_; })
>  
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 11f877b..0516f62 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -470,6 +470,7 @@ struct BlockDriverState {
>  NotifierWithReturnList before_write_notifiers;
>  
>  /* number of in-flight requests; overall and serialising */
> +bool wakeup;
>  unsigned int in_flight;
>  unsigned int 

Re: [Qemu-devel] [Qemu-block] [PATCH 14/18] block: prepare bdrv_reopen_multiple to release AioContext

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 07:34:18PM +0200, Paolo Bonzini wrote:
> After the next patch bdrv_drain_all will have to be called without holding any
> AioContext.  Prepare to do this by adding an AioContext argument to
> bdrv_reopen_multiple.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block.c   | 4 ++--
>  block/commit.c| 2 +-
>  block/replication.c   | 3 ++-
>  include/block/block.h | 2 +-
>  qemu-io-cmds.c| 2 +-
>  5 files changed, 7 insertions(+), 6 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 13/18] replication: pass BlockDriverState to reopen_backing_file

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 07:34:17PM +0200, Paolo Bonzini wrote:
> This will be needed in the next patch to retrieve the AioContext.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block/replication.c | 21 -
>  1 file changed, 12 insertions(+), 9 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-block] [PATCH 11/18] aio: introduce qemu_get_current_aio_context

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 07:34:15PM +0200, Paolo Bonzini wrote:
> This will be used by bdrv_poll_while (and thus by bdrv_drain)
> to choose how to wait for I/O completion.
> 
> Reviewed-by: Fam Zheng 
> Signed-off-by: Paolo Bonzini 
> ---
>  include/block/aio.h | 15 +++
>  iothread.c  |  9 +
>  stubs/Makefile.objs |  1 +
>  stubs/iothread.c|  8 
>  4 files changed, 33 insertions(+)
>  create mode 100644 stubs/iothread.c
> 
> diff --git a/include/block/aio.h b/include/block/aio.h
> index b9fe2cb..60a4f21 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -453,6 +453,21 @@ static inline bool aio_node_check(AioContext *ctx, bool 
> is_external)
>  }
>  
>  /**
> + * Return the AioContext whose event loop runs in the current I/O thread.
> + */
> +AioContext *qemu_get_current_aio_context(void);

This doc comment left me wondering what "I/O thread" means.  Looking at
the implementation this function returns the current IOThread
AioContext, otherwise it returns the QEMU main loop AioContext.

I think the following captures the semantics:

"Return the AioContext whose event loop runs in the current thread.

If called from an IOThread this will be the IOThread's AioContext.  If
called from another thread it will be the main loop AioContext."

> +
> +/**
> + * @ctx: the aio context
> + *
> + * Return whether we are running in the I/O thread that manages @ctx.

s%I/O %%

This function works within an IOThread but also in any other thread.

> + */
> +static inline bool aio_context_in_iothread(AioContext *ctx)
> +{
> +return ctx == qemu_get_current_aio_context();
> +}
> +
> +/**
>   * aio_context_setup:
>   * @ctx: the aio context
>   *
> diff --git a/iothread.c b/iothread.c
> index fbeb8de..62c8796 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -20,6 +20,7 @@
>  #include "qmp-commands.h"
>  #include "qemu/error-report.h"
>  #include "qemu/rcu.h"
> +#include "qemu/main-loop.h"
>  
>  typedef ObjectClass IOThreadClass;
>  
> @@ -28,6 +29,13 @@ typedef ObjectClass IOThreadClass;
>  #define IOTHREAD_CLASS(klass) \
> OBJECT_CLASS_CHECK(IOThreadClass, klass, TYPE_IOTHREAD)
>  
> +static __thread IOThread *my_iothread;
> +
> +AioContext *qemu_get_current_aio_context(void)
> +{
> +return my_iothread ? my_iothread->ctx : qemu_get_aio_context();
> +}
> +
>  static void *iothread_run(void *opaque)
>  {
>  IOThread *iothread = opaque;
> @@ -35,6 +43,7 @@ static void *iothread_run(void *opaque)
>  
>  rcu_register_thread();
>  
> +my_iothread = iothread;
>  qemu_mutex_lock(>init_done_lock);
>  iothread->thread_id = qemu_get_thread_id();
>  qemu_cond_signal(>init_done_cond);
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index c5850e8..84b9d9e 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -17,6 +17,7 @@ stub-obj-y += gdbstub.o
>  stub-obj-y += get-fd.o
>  stub-obj-y += get-next-serial.o
>  stub-obj-y += get-vm-name.o
> +stub-obj-y += iothread.o
>  stub-obj-y += iothread-lock.o
>  stub-obj-y += is-daemonized.o
>  stub-obj-y += machine-init-done.o
> diff --git a/stubs/iothread.c b/stubs/iothread.c
> new file mode 100644
> index 000..8cc9e28
> --- /dev/null
> +++ b/stubs/iothread.c
> @@ -0,0 +1,8 @@
> +#include "qemu/osdep.h"
> +#include "block/aio.h"
> +#include "qemu/main-loop.h"
> +
> +AioContext *qemu_get_current_aio_context(void)
> +{
> +return qemu_get_aio_context();
> +}
> -- 
> 2.7.4
> 
> 
> 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-block] [PATCH 10/18] sheepdog: use bdrv_poll_while and bdrv_wakeup

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 07:34:14PM +0200, Paolo Bonzini wrote:
> These ensure that bdrv_poll_while will exit for a BlockDriverState
> that is not in the main AioContext.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block/sheepdog.c | 67 
> 
>  1 file changed, 38 insertions(+), 29 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v4 08/14] tcg/s390: Add support for fence

[Pranith Kumar]

On Sun, Oct 16, 2016 at 4:47 AM, Stefan Hajnoczi  wrote:
> On Thu, Jul 14, 2016 at 04:20:20PM -0400, Pranith Kumar wrote:
>> Cc: Alexander Graf 
>> Signed-off-by: Pranith Kumar 
>> Signed-off-by: Richard Henderson 
>> ---
>>  tcg/s390/tcg-target.inc.c | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c
>> index 5a7495b..01aae35 100644
>> --- a/tcg/s390/tcg-target.inc.c
>> +++ b/tcg/s390/tcg-target.inc.c
>> @@ -343,6 +343,7 @@ static tcg_insn_unit *tb_ret_addr;
>>  #define FACILITY_EXT_IMM (1ULL << (63 - 21))
>>  #define FACILITY_GEN_INST_EXT(1ULL << (63 - 34))
>>  #define FACILITY_LOAD_ON_COND   (1ULL << (63 - 45))
>> +#define FACILITY_FAST_BCR_SER   FACILITY_LOAD_ON_COND
>>
>>  static uint64_t facilities;
>>
>> @@ -2172,6 +2173,15 @@ static inline void tcg_out_op(TCGContext *s, 
>> TCGOpcode opc,
>>  tgen_deposit(s, args[0], args[2], args[3], args[4]);
>>  break;
>>
>> +case INDEX_op_mb:
>> +/* The host memory model is quite strong, we simply need to
>> +   serialize the instruction stream.  */
>> +if (args[0] == TCG_MO_ALL || args[0] == TCG_MO_ST_LD) {
>> +tcg_out_insn(s, RR, BCR,
>> + facilities & FACILITY_FAST_BCR_SER ? 14 : 15, 0);
>> +}
>
> args[0] == TCG_MO_ALL is always false since frontends bitwise-OR
> TCG_BAR_SC.
>
> Did you mean:
>
> switch (args[0] & TCG_MO_ALL) {
> case TCG_MO_ALL: /* fall-through */
> case TCG_MO_ST_LD:
> tcg_out_insn(s, RR, BCR,
>  facilities & FACILITY_FAST_BCR_SER ? 14 : 15, 0);
> break;
> }

Yup, that is what is intended. It looks like this patch was fixed by
rth when he merged it to do the correct thing. phew :)

-- 
Pranith

Re: [Qemu-devel] [Qemu-block] [PATCH 09/18] nfs: use bdrv_poll_while and bdrv_wakeup

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 07:34:13PM +0200, Paolo Bonzini wrote:
> These will make it possible to use nfs_get_allocated_file_size on
> a file that is not in the main AioContext.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block/nfs.c | 47 +--
>  1 file changed, 29 insertions(+), 18 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] MAINTAINERS: Update PPC status and maintainer

[Pranith Kumar]

Ping, looks like this patch fell through.

On Tue, Aug 2, 2016 at 10:52 AM, Pranith Kumar  wrote:
> Richard agreed to make odd fixes to PPC tcg parts[1]. This patch makes
> the change.
>
> [1] https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00657.html
>
> CC: Richard Henderson 
> Signed-off-by: Pranith Kumar 
> ---
>  MAINTAINERS | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b6fb84e..e581114 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1389,8 +1389,8 @@ F: tcg/mips/
>  F: disas/mips.c
>
>  PPC
> -M: Vassili Karpov (malc) 
> -S: Maintained
> +M: Richard Henderson 
> +S: Odd Fixes
>  F: tcg/ppc/
>  F: disas/ppc.c
>
> --
> 2.9.2
>



-- 
Pranith

Re: [Qemu-devel] [PATCH] scripts/hxtool: fix undefined behavour of echo

[Daniel Shahaf]

Michael Tokarev wrote on Sun, Oct 16, 2016 at 17:30:28 +0300:
> From: Daniel Shahaf 
> 
> Avoid undefined behaviour of echo(1) with backslashes in arguments
> The behaviour is implementation-defined, different /bin/sh's behave
> differently.
> 

Signed-off-by: Daniel Shahaf 

> Signed-off-By: Michael Tokarev 
> ---
> Submitting this patch upstream, thank you very much!
> Daniel, can you please add your Signed-off-By line?

Like this?

Thank you for forwarding the patch upstream.

Daniel

Re: [Qemu-devel] [PATCH] scripts/hxtool: fix undefined behavour of echo

[Michael Tokarev]

16.10.2016 18:22, Daniel Shahaf wrote:

> Signed-off-by: Daniel Shahaf 
> Like this?

Yes, exactly, thank you! :)

/mjt

Re: [Qemu-devel] [PATCH] scripts/hxtool: fix undefined behavour of echo

[no-reply]

Hi,

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

Type: series
Message-id: 1476628228-7425-1-git-send-email-...@msgid.tls.msk.ru
Subject: [Qemu-devel] [PATCH] scripts/hxtool: fix undefined behavour of echo

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

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

# Useful git options
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 show --no-patch --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
Switched to a new branch 'test'
af0a15c scripts/hxtool: fix undefined behavour of echo

=== OUTPUT BEGIN ===
Checking PATCH 1/1: scripts/hxtool: fix undefined behavour of echo...
ERROR: The correct form is "Signed-off-by"
#10: 
Signed-off-By: Michael Tokarev 

total: 1 errors, 0 warnings, 68 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Michael Tokarev]

13.10.2016 21:29, Stefan Weil wrote:
> scripts/tracetool generates a C preprocessor macro from the name of the
> build directory. Any characters which are possible in a directory name
> but not allowed in a macro name must be substituted, otherwise builds
> will fail.

Applied to -trivial, thank you!

/mjt

[Qemu-devel] [PATCH] scripts/hxtool: fix undefined behavour of echo

[Michael Tokarev]

From: Daniel Shahaf 

Avoid undefined behaviour of echo(1) with backslashes in arguments
The behaviour is implementation-defined, different /bin/sh's behave
differently.

Signed-off-By: Michael Tokarev 
---
Submitting this patch upstream, thank you very much!
Daniel, can you please add your Signed-off-By line?

 scripts/hxtool | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/scripts/hxtool b/scripts/hxtool
index 995bb7f..04f7d7b 100644
--- a/scripts/hxtool
+++ b/scripts/hxtool
@@ -26,32 +26,32 @@ hxtotexi()
 ;;
 STEXI*)
 if test $flag -eq 1 ; then
-echo "line $line: syntax error: expected ETEXI, found $str" >&2
+printf "line %d: syntax error: expected ETEXI, found '%s'\n" 
"$line" "$str" >&2
 exit 1
 fi
 flag=1
 ;;
 ETEXI*)
 if test $flag -ne 1 ; then
-echo "line $line: syntax error: expected STEXI, found $str" >&2
+printf "line %d: syntax error: expected STEXI, found '%s'\n" 
"$line" "$str" >&2
 exit 1
 fi
 flag=0
 ;;
 SQMP*|EQMP*)
 if test $flag -eq 1 ; then
-echo "line $line: syntax error: expected ETEXI, found $str" >&2
+printf "line %d: syntax error: expected ETEXI, found '%s'\n" 
"$line" "$str" >&2
 exit 1
 fi
 ;;
 DEFHEADING*)
-echo "$(expr "$str" : "DEFHEADING(\(.*\))")"
+printf '%s\n' "$(expr "$str" : "DEFHEADING(\(.*\))")"
 ;;
 ARCHHEADING*)
-echo "$(expr "$str" : "ARCHHEADING(\(.*\),.*)")"
+printf '%s\n' "$(expr "$str" : "ARCHHEADING(\(.*\),.*)")"
 ;;
 *)
-test $flag -eq 1 && echo "$str"
+test $flag -eq 1 && printf '%s\n' "$str"
 ;;
 esac
 line=$((line+1))
@@ -69,26 +69,26 @@ hxtoqmp()
 ;;
 SQMP*)
 if test $flag -eq 1 ; then
-echo "line $line: syntax error: expected EQMP, found $str" >&2
+printf "line %d: syntax error: expected EQMP, found '%s'\n" 
"$line" "$str" >&2
 exit 1
 fi
 flag=1
 ;;
 EQMP*)
 if test $flag -ne 1 ; then
-echo "line $line: syntax error: expected SQMP, found $str" >&2
+printf "line %d: syntax error: expected SQMP, found '%s'\n" 
"$line" "$str" >&2
 exit 1
 fi
 flag=0
 ;;
 STEXI*|ETEXI*)
 if test $flag -eq 1 ; then
-echo "line $line: syntax error: expected EQMP, found $str" >&2
+printf "line %d: syntax error: expected EQMP, found '%s'\n" 
"$line" "$str" >&2
 exit 1
 fi
 ;;
 *)
-test $flag -eq 1 && echo "$str"
+test $flag -eq 1 && printf '%s\n' "$str"
 ;;
 esac
 line=$((line+1))
-- 
2.1.4

[Qemu-devel] [PATCH] qemu-options.hx: set: fix copy-paste error

[Michael Tokarev]

Reported-By: Daniel Shahaf 
Signed-off-by: Michael Tokarev 
---
 qemu-options.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index c209b53..f296406 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -172,7 +172,7 @@ DEF("set", HAS_ARG, QEMU_OPTION_set,
 STEXI
 @item -set @var{group}.@var{id}.@var{arg}=@var{value}
 @findex -set
-Set parameter @var{arg} for item @var{id} of type @var{group}\n"
+Set parameter @var{arg} for item @var{id} of type @var{group}
 ETEXI
 
 DEF("global", HAS_ARG, QEMU_OPTION_global,
-- 
2.1.4

Re: [Qemu-devel] [PATCH v3] usb: Change *_exitfn return type from int to void

[Michael Tokarev]

13.10.2016 23:21, Akanksha Srivastava wrote:
> The *_exitfn functions cannot fail and should not be
> returning int.
...

Applied to -trivial, thank you!

/mjt

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Stefan Hajnoczi]

On Fri, Oct 14, 2016 at 10:05:18PM +0200, Stefan Weil wrote:
> On 10/14/16 12:01, Stefan Hajnoczi wrote:
> > dirname should only contain QEMU source tree subdirectory paths (e.g.
> > hw/net).  How did you get a comma in there?
> > 
> > I tried an out-of-tree build in a directory called 'a,b' but it doesn't
> > trigger the issue.
> > 
> > I'd like to understand how to reproduce the problem in case I missed
> > something.
> > 
> > Stefan
> 
> I can reproduce it like this from the QEMU base directory:
> 
> $ mkdir a,b
> $ cd a,b
> $ ../configure
> [...]
> $ make
> [...]
> make: *** [trace/generated-tracers.o] Error 1
> $ head trace/generated-tracers.h
> /* This file is autogenerated by tracetool, do not edit. */
> 
> #ifndef TRACE_A,B_GENERATED_TRACERS_H
> #define TRACE_A,B_GENERATED_TRACERS_H

Interesting!  I missed it because my a,b directory was not inside the
source directory.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support

[Stefan Hajnoczi]

On Fri, Oct 14, 2016 at 09:40:40AM -0500, Eric Blake wrote:
> On 10/14/2016 04:00 AM, Stefan Hajnoczi wrote:
> > Add the AF_VSOCK address family so that qemu-ga will be able to use
> > virtio-vsock.
> > 
> > The AF_VSOCK address family uses  address tuples.  The cid is
> > the unique identifier comparable to an IP address.  AF_VSOCK does not
> > use name resolution so it's easy to convert between struct sockaddr_vm
> > and strings.
> > 
> > This patch defines a VsockSocketAddress instead of trying to piggy-back
> > on InetSocketAddress.  This is cleaner in the long run since it avoids
> > lots of IPv4 vs IPv6 vs vsock special casing.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> 
> >  ##
> > +# @VsockSocketAddress
> > +#
> > +# Captures a socket address in the vsock namespace.
> > +#
> > +# @cid: unique host identifier
> > +# @port: port
> > +#
> > +# Note that string types are used to allow for possible future hostname or
> > +# service resolution support.
> > +#
> > +# Since 2.8
> > +##
> > +{ 'struct': 'VsockSocketAddress',
> > +  'data': {
> > +'cid': 'str',
> > +'port': 'str' } }
> 
> It would also be possible to do this now:
> 
> { 'struct': 'VsockSocketAddress',
>   'data': { 'cid': 'int', 'port': 'int' } }
> 
> then down the road expand to:
> 
> { 'alternate': 'StrOrInt',
>   'data': { 's': 'str', 'i': 'int' } }
> { 'struct': 'VsockSocketAddress',
>   'data': { 'cid': 'StrOrInt', 'port': 'StrOrInt' } }
> 
> although the C code to do type-safe access vsock->cid.u.i or
> vsock->cid.u.s based on vsock->cid.type is a bit more verbose than just
> accessing vsock->cid as a string and converting every time around.
> Where it gets really interesting is that using the alternate would allow
> all of these QMP forms:
> 
> { "socket": { "cid": 1, "port": 2 } }
> { "socket": { "cid": "1", "port": "2" } }
> { "socket": { "cid": "host", "port": "service" } }
> 
> It MIGHT be worth going the type-safe route, especially if we WANT to
> convert the existing SocketAddress uses to use the alternate rather than
> always managing things as a string.  But it doesn't have to be in this
> patch.
> 
> > +static VsockSocketAddress *vsock_parse(const char *str, Error **errp)
> > +{
> > +VsockSocketAddress *addr = NULL;
> > +char cid[33];
> > +char port[33];
> > +int n;
> > +
> > +if (sscanf(str, "%32[^:]:%32[^,]%n", cid, port, ) != 2) {
> 
> This says stop at the first comma after the colon...
> 
> > +error_setg(errp, "error parsing address '%s'", str);
> > +return NULL;
> > +}
> > +if (str[n] != '\0') {
> > +error_setg(errp, "trailing characters in address '%s'", str);
> 
> ...but this rejects a trailing comma.  Is a trailing comma possible base
> on how QemuOpts work? If so, do you need to handle it here?

Actually I just wanted to grab characters up until the end of string.
It wasn't clear from the sscanf(3) man page what the best way to do that
was, so I kept the comma which is also used in tcp addresses (because
they support additional comma-separated options).

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v7 12/12] virtio-crypto: perfect algorithms chainning support

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 03:12:06PM +0800, Gonglei wrote:
> diff --git a/include/standard-headers/linux/virtio_crypto.h 
> b/include/standard-headers/linux/virtio_crypto.h
> index f2a059e..9ae02fb 100644
> --- a/include/standard-headers/linux/virtio_crypto.h
> +++ b/include/standard-headers/linux/virtio_crypto.h
> @@ -326,7 +326,15 @@ struct virtio_crypto_mac_data_req {
>  };
>  
>  struct virtio_crypto_alg_chain_data_para {
> -struct virtio_crypto_cipher_para cipher;
> +__virtio32 iv_len;
> +/* Length of source data */
> +__virtio32 src_data_len;
> +/* Length of destination data */
> +__virtio32 dst_data_len;
> +/* Starting point for cipher processing in source data */
> +__virtio32 cipher_start_src_offset;
> +/* Length of the source data that the cipher will be computed on */
> +__virtio32 len_to_cipher;
>  /* Starting point for hash processing in source data */
>  __virtio32 hash_start_src_offset;
>  /* Length of the source data that the hash will be computed on */
> @@ -335,6 +343,7 @@ struct virtio_crypto_alg_chain_data_para {
>  __virtio32 aad_len;
>  /* Length of the hash result */
>  __virtio32 hash_result_len;
> +__virtio32 reserved;
>  };
>  
>  struct virtio_crypto_alg_chain_data_req {

This is supposed to be an external header file from Linux.  QEMU patches
normally don't change the file except to import an updated version.

Please import the final version once in the patch series and don't
modify it.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v7 10/12] cryptodev: introduce an unified wrapper for crypto operation

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 03:12:04PM +0800, Gonglei wrote:
> We use an opaque point to the VirtIOCryptoReq which
> can support different packets based on different
> algorithms.
> 
> Signed-off-by: Gonglei 
> ---
>  backends/cryptodev-builtin.c |  2 +-
>  backends/cryptodev.c | 28 ++--
>  hw/virtio/virtio-crypto.c| 10 +-
>  include/sysemu/cryptodev.h   | 13 +++--
>  4 files changed, 39 insertions(+), 14 deletions(-)
> 
> diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
> index 34c45be..dc0a364 100644
> --- a/backends/cryptodev-builtin.c
> +++ b/backends/cryptodev-builtin.c
> @@ -286,7 +286,7 @@ static int cryptodev_builtin_sym_operation(
>  return -VIRTIO_CRYPTO_ERR;
>  }
>  }
> -return 0;
> +return VIRTIO_CRYPTO_OK;
>  }

This looks unrelated to the commit description.  Please squash the
VIRTIO_CRYPTO_OK changes into the patches that introduced the old code
that you are replacing.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v7 09/12] virtio-crypto: add data queue processing handler

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 03:12:03PM +0800, Gonglei wrote:
> Introduces VirtIOCryptoReq structure to store
> crypto request so that we can support sync and async
> crypto operation in the future.

What do you mean by "sync and async" operations?

> 
> At present, we only support cipher and algorithm
> chainning.

s/chainning/chaining/

> 
> Signed-off-by: Gonglei 
> ---
>  hw/virtio/virtio-crypto.c | 338 
> +-
>  include/hw/virtio/virtio-crypto.h |  10 +-
>  2 files changed, 345 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> index db86796..094bc48 100644
> --- a/hw/virtio/virtio-crypto.c
> +++ b/hw/virtio/virtio-crypto.c
> @@ -306,6 +306,342 @@ static void virtio_crypto_handle_ctrl(VirtIODevice 
> *vdev, VirtQueue *vq)
>  } /* end for loop */
>  }
>  
> +static void virtio_crypto_init_request(VirtIOCrypto *vcrypto, VirtQueue *vq,
> +VirtIOCryptoReq *req)
> +{
> +req->vcrypto = vcrypto;
> +req->vq = vq;
> +req->in = NULL;
> +req->in_iov = NULL;
> +req->in_num = 0;
> +req->in_len = 0;
> +req->flags = CRYPTODEV_BACKEND_ALG__MAX;
> +req->u.sym_op_info = NULL;
> +}
> +
> +static void virtio_crypto_free_request(VirtIOCryptoReq *req)
> +{
> +if (req) {
> +if (req->flags == CRYPTODEV_BACKEND_ALG_SYM) {
> +g_free(req->u.sym_op_info);
> +}
> +g_free(req);
> +}
> +}
> +
> +static void
> +virtio_crypto_sym_input_data_helper(VirtIODevice *vdev,
> +VirtIOCryptoReq *req,
> +uint32_t status,
> +CryptoDevBackendSymOpInfo *sym_op_info)
> +{
> +size_t s, len;
> +
> +if (status != VIRTIO_CRYPTO_OK) {
> +return;
> +}
> +
> +len = sym_op_info->dst_len;
> +/* Save the cipher result */
> +s = iov_from_buf(req->in_iov, req->in_num, 0, sym_op_info->dst, len);
> +if (s != len) {
> +virtio_error(vdev, "virtio-crypto dest data incorrect");
> +return;
> +}
> +
> +iov_discard_front(>in_iov, >in_num, len);
> +
> +if (sym_op_info->op_type ==
> +  VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) {
> +/* Save the digest result */
> +s = iov_from_buf(req->in_iov, req->in_num, 0,
> + sym_op_info->digest_result,
> + sym_op_info->digest_result_len);
> +if (s != sym_op_info->digest_result_len) {
> +virtio_error(vdev, "virtio-crypto digest result incorrect");
> +}
> +}
> +}
> +
> +static void virtio_crypto_req_complete(VirtIOCryptoReq *req, uint32_t status)
> +{
> +VirtIOCrypto *vcrypto = req->vcrypto;
> +VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
> +
> +if (req->flags == CRYPTODEV_BACKEND_ALG_SYM) {
> +virtio_crypto_sym_input_data_helper(vdev, req, status,
> +req->u.sym_op_info);
> +}
> +stl_he_p(>in->status, status);

This should be virtio_stl_p().

> +virtqueue_push(req->vq, >elem, req->in_len);
> +virtio_notify(vdev, req->vq);
> +}
> +
> +static VirtIOCryptoReq *
> +virtio_crypto_get_request(VirtIOCrypto *s, VirtQueue *vq)
> +{
> +VirtIOCryptoReq *req = virtqueue_pop(vq, sizeof(VirtIOCryptoReq));
> +
> +if (req) {
> +virtio_crypto_init_request(s, vq, req);
> +}
> +return req;
> +}
> +
> +static CryptoDevBackendSymOpInfo *
> +virtio_crypto_sym_op_helper(VirtIODevice *vdev,
> +   struct virtio_crypto_cipher_para *para,
> +   uint32_t aad_len,
> +   struct iovec *iov, unsigned int out_num,
> +   uint32_t hash_result_len,
> +   uint32_t hash_start_src_offset)
> +{
> +CryptoDevBackendSymOpInfo *op_info;
> +uint32_t src_len, dst_len;
> +uint32_t iv_len;
> +size_t max_len, curr_size = 0;
> +size_t s;
> +
> +iv_len = virtio_ldl_p(vdev, >iv_len);
> +src_len = virtio_ldl_p(vdev, >src_data_len);
> +dst_len = virtio_ldl_p(vdev, >dst_data_len);
> +
> +max_len = iv_len + aad_len + src_len + dst_len + hash_result_len;

Integer overflow checks are needed here to prevent memory corruption
later on.

Imagine a 32-bit host where sizeof(max_len) == 4 and iv_len + aad_len +
... == UINT32_MAX + 1.  The malloc below will allocate just 1 byte
instead of UINT32_MAX + 1 due to overflow.

> +op_info = g_malloc0(sizeof(CryptoDevBackendSymOpInfo) + max_len);
> +op_info->iv_len = iv_len;
> +op_info->src_len = src_len;
> +op_info->dst_len = dst_len;
> +op_info->aad_len = aad_len;
> +op_info->digest_result_len = hash_result_len;
> +op_info->hash_start_src_offset = hash_start_src_offset;
> +/* Handle the initilization vector */
> +if (op_info->iv_len > 0) {
> +DPRINTF("iv_len=%" PRIu32 "\n", op_info->iv_len);
> +op_info->iv = op_info->data + curr_size;
> +
> +s = iov_to_buf(iov, out_num, 

Re: [Qemu-devel] [PATCH v7 08/12] virtio-crypto: add control queue handler

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 03:12:02PM +0800, Gonglei wrote:
> +static int64_t
> +virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
> +   struct virtio_crypto_sym_create_session_req *sess_req,
> +   uint32_t queue_id,
> +   uint32_t opcode,
> +   struct iovec *iov, unsigned int out_num)
> +{
> +VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
> +CryptoDevBackendSymSessionInfo info;
> +int64_t session_id;
> +int queue_index;
> +uint32_t op_type;
> +Error *local_err = NULL;
> +int ret;
> +
> +memset(, 0, sizeof(info));
> +op_type = virtio_ldl_p(vdev, _req->op_type);
> +info.op_type = op_type;
> +info.op_code = opcode;
> +
> +if (op_type == VIRTIO_CRYPTO_SYM_OP_CIPHER) {
> +ret = virtio_crypto_cipher_session_helper(vdev, ,
> +   _req->u.cipher.para,
> +   , _num);
> +if (ret < 0) {
> +return -EFAULT;

info.cipher_key is leaked here.

> +}
> +} else if (op_type == VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) {
> +size_t s;
> +/* cipher part */
> +ret = virtio_crypto_cipher_session_helper(vdev, ,
> +   _req->u.chain.para.cipher_param,
> +   , _num);
> +if (ret < 0) {
> +return -EFAULT;

info.cipher_key is leaked here.

> +}
> +/* hash part */
> +info.alg_chain_order = virtio_ldl_p(vdev,
> +   
> _req->u.chain.para.alg_chain_order);
> +info.add_len = virtio_ldl_p(vdev, _req->u.chain.para.aad_len);
> +info.hash_mode = virtio_ldl_p(vdev, 
> _req->u.chain.para.hash_mode);
> +if (info.hash_mode == VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH) {
> +info.hash_alg = virtio_ldl_p(vdev,
> +   _req->u.chain.para.u.mac_param.algo);
> +info.auth_key_len = virtio_ldl_p(vdev,
> + 
> _req->u.chain.para.u.mac_param.auth_key_len);
> +info.hash_result_len = virtio_ldl_p(vdev,
> +   
> _req->u.chain.para.u.mac_param.hash_result_len);
> +/* get auth key */
> +if (info.auth_key_len > 0) {
> +DPRINTF("auth_keylen=%" PRIu32 "\n", info.auth_key_len);
> +info.auth_key = g_malloc(info.auth_key_len);
> +s = iov_to_buf(iov, out_num, 0, info.auth_key,
> +   info.auth_key_len);
> +if (unlikely(s != info.auth_key_len)) {
> +virtio_error(vdev,
> +  "virtio-crypto authenticated key incorrect");
> +ret = -EFAULT;
> +goto err;
> +}
> +iov_discard_front(, _num, info.auth_key_len);
> +}
> +} else if (info.hash_mode == VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN) {
> +info.hash_alg = virtio_ldl_p(vdev,
> + _req->u.chain.para.u.hash_param.algo);
> +info.hash_result_len = virtio_ldl_p(vdev,
> +
> _req->u.chain.para.u.hash_param.hash_result_len);
> +} else {
> +/* VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED */
> +error_report("unsupported hash mode");

Why is error_report() used instead of virtio_error()?  The same applies
elsewhere.

> +ret = -VIRTIO_CRYPTO_NOTSUPP;
> +goto err;
> +}
> +} else {
> +/* VIRTIO_CRYPTO_SYM_OP_NONE */
> +error_report("unsupported cipher op_type: 
> VIRTIO_CRYPTO_SYM_OP_NONE");
> +ret = -VIRTIO_CRYPTO_NOTSUPP;
> +goto err;
> +}
> +
> +queue_index = virtio_crypto_vq2q(queue_id);
> +session_id = cryptodev_backend_sym_create_session(
> + vcrypto->cryptodev,
> + , queue_index, _err);
> +if (session_id >= 0) {
> +DPRINTF("create session_id=%" PRIu64 " successfully\n",
> +session_id);
> +
> +ret = session_id;
> +} else {
> +if (local_err) {
> +error_report_err(local_err);
> +}
> +ret = -VIRTIO_CRYPTO_ERR;
> +}
> +
> +err:
> +g_free(info.cipher_key);
> +g_free(info.auth_key);
> +return ret;
> +}
> +
> +static uint32_t
> +virtio_crypto_handle_close_session(VirtIOCrypto *vcrypto,
> + struct virtio_crypto_destroy_session_req *close_sess_req,
> + uint32_t queue_id)
> +{
> +VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
> +int ret;
> +uint64_t session_id;
> +uint32_t status;
> +Error *local_err = NULL;
> +
> +session_id = virtio_ldq_p(vdev, _sess_req->session_id);
> +DPRINTF("close session, id=%" PRIu64 "\n", session_id);
> +
> +ret = cryptodev_backend_sym_close_session(
> +  vcrypto->cryptodev, session_id, queue_id, _err);
> +if (ret == 0) {
> +  

Re: [Qemu-devel] [Qemu-block] [PATCH 08/18] nfs: move nfs_set_events out of the while loops

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 07:34:12PM +0200, Paolo Bonzini wrote:
> nfs_set_events only needs to be called once before entering the
> while loop; afterwards, nfs_process_read and nfs_process_write
> take care of it.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block/nfs.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3] usb: Change *_exitfn return type from int to void

[Stefan Hajnoczi]

On Fri, Oct 14, 2016 at 01:51:31AM +0530, Akanksha Srivastava wrote:
> The *_exitfn functions cannot fail and should not be
> returning int.
> This also removes the passthru_exitfn since this callback
> does nothing as of now.
> This was suggested as a Bite-sized task for code cleanup.
> Signed-off-by: Akanksha Srivastava 
> ---
>  hw/usb/ccid-card-emulated.c   |  3 +--
>  hw/usb/ccid-card-passthru.c   |  6 --
>  hw/usb/ccid.h |  2 +-
>  hw/usb/dev-smartcard-reader.c | 11 +--
>  4 files changed, 7 insertions(+), 15 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-block] [PATCH 07/18] block: introduce bdrv_poll_while and bdrv_wakeup

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 07:34:11PM +0200, Paolo Bonzini wrote:
> @@ -485,9 +474,14 @@ void bdrv_inc_in_flight(BlockDriverState *bs)
>  atomic_inc(>in_flight);
>  }
>  
> +void bdrv_wakeup(BlockDriverState *bs)
> +{
> +}

Please write a doc comment explaining the semantics of this new API.

Since it's a nop here it may be better to introduce bdrv_wakeup() in
another patch.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-block] [PATCH 02/18] blockjob: introduce .drain callback for jobs

[Stefan Hajnoczi]

On Thu, Oct 13, 2016 at 07:34:06PM +0200, Paolo Bonzini wrote:
> +static void backup_drain(BlockJob *job)
> +{
> +BackupBlockJob *s = container_of(job, BackupBlockJob, common);
> +
> +/* Need to keep a reference in case blk_drain triggers execution
> + * of backup_complete...
> + */
> +if (s->target) {
> +blk_ref(s->target);
> +blk_drain(s->target);
> +blk_unref(s->target);
> +}
[...]
> @@ -331,6 +346,7 @@ static void backup_complete(BlockJob *job, void *opaque)
>  BackupCompleteData *data = opaque;
>  
>  blk_unref(s->target);
> +s->target = NULL;

Will blk_unref(s->target) segfault since backup_complete() has set it to
NULL?  I expected backup_drain() to stash the pointer in a local
variable to avoid using s->target.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] trace: fix group name generation

[Greg Kurz]

On Sat, 15 Oct 2016 13:03:10 +0100
Peter Maydell  wrote:

> On 14 October 2016 at 23:06, Greg Kurz  wrote:
> > On Fri, 14 Oct 2016 16:31:01 -0500
> > Eric Blake  wrote:
> >  
> >> On 10/14/2016 04:26 PM, Greg Kurz wrote:  
> >> > Since commit "80dd5c4918ab trace: introduce a formal group name for trace
> >> > events", tracetool generates C variable names and macro definitions out
> >> > of the path to the trace-events-all file.  
> 
> >> > diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> >> > index 629b2593c846..b81b834db924 100755
> >> > --- a/scripts/tracetool.py
> >> > +++ b/scripts/tracetool.py
> >> > @@ -70,7 +70,7 @@ def make_group_name(filename):
> >> >
> >> >  if dirname == "":
> >> >  return "common"
> >> > -return re.sub(r"/|-", "_", dirname)
> >> > +return "_" + re.sub(r"[^\w]", "_", dirname)  
> >>
> >> This STILL doesn't solve the complaint that the build is now dependent
> >> on the location.  Why can't we STRIP off any prefix prior to the in-tree
> >> portion of the naming that we know is sane, instead of munging the
> >> prefix but in the process creating source code that generates with
> >> different lengths?
> >>  
> >
> > Heh, because the complaint was about the build break :)
> >  
> >> Ideally, compiling twice, once in directory 'a', and the second time in
> >> directory '', should not make a noticeable
> >> difference in the final size of the executable due to the difference in
> >> lengths of the debugging symbols used to record the longer name of the
> >> second directory being encoded into lots of macro names.
> >>  
> >
> > You're right. I'm okay to look for a way to fix the symbol variable size
> > issue, but I think this patch still makes sense as it makes tracetool
> > less fragile in case we add a directory with an illegal character to
> > the QEMU tree... and it fixes annoying build breaks (Paolo also hit
> > this and asked to revert 80dd5c4918ab in another mail).  
> 
> I agree with Eric that we should just not be putting
> the build directory name in these variables -- this
> looks like it's simply a bug to me, and we should fix it.
> 

As I was saying in the previous mail, I also agree with Eric :)

And it isn't even about the variable size of the debugging info,
but rather about the random names of the generated symbols...

> I think the chances of us adding a directory to the QEMU
> tree itself with a silly name are quite low (not least because
> if you do that then you get a handy build failure to tell
> you not to do that ;-))
> 

Fair enough, even if the error is a bit obscure at first sight... and
we already have a silly named directory (9pfs), which would be supported
with a leading '_'... but hopefully, it is not at the top level ;)

> thanks
> -- PMM

Cheers.

--
Greg

Re: [Qemu-devel] [v2 2/5] block/ssh: Add InetSocketAddress and accept it

[Ashijeet Acharya]

>>>  /* Check the remote host's key against known_hosts. */
>>> -ret = check_host_key(s, host, port, host_key_check, errp);
>>> +ret = check_host_key(s, s->inet->host, port, host_key_check,
>>
>> But then you're still using the port here... And I can't come up with a
>> way (not even a bad one) to get the numeric port. Maybe interpret the
>> addrinfo in inet_connect_saddr()? But getting that information out would
>> be ugly, if even possible...
>>
>
> Will using strtol() do any good?

Better to use qemu_strtol() from cutils.c ?

Ashijeet

Re: [Qemu-devel] [v2 2/5] block/ssh: Add InetSocketAddress and accept it

[Ashijeet Acharya]

On Sun, Oct 16, 2016 at 4:00 AM, Max Reitz  wrote:
> On 15.10.2016 11:04, Ashijeet Acharya wrote:
>> Add InetSocketAddress compatibility to SSH driver.
>>
>> Add a new option "server" to the SSH block driver which then accepts
>> a InetSocketAddress.
>>
>> "host" and "port" are supported as legacy options and are mapped to
>> their InetSocketAddress representation.
>>
>> Signed-off-by: Ashijeet Acharya 
>> ---
>>  block/ssh.c | 83 
>> ++---
>>  1 file changed, 74 insertions(+), 9 deletions(-)
>>
>> diff --git a/block/ssh.c b/block/ssh.c
>> index 75cb7bc..3b18907 100644
>> --- a/block/ssh.c
>> +++ b/block/ssh.c
>> @@ -30,10 +30,14 @@
>>  #include "block/block_int.h"
>>  #include "qapi/error.h"
>>  #include "qemu/error-report.h"
>> +#include "qemu/cutils.h"
>>  #include "qemu/sockets.h"
>>  #include "qemu/uri.h"
>> +#include "qapi-visit.h"
>>  #include "qapi/qmp/qint.h"
>>  #include "qapi/qmp/qstring.h"
>> +#include "qapi/qobject-input-visitor.h"
>> +#include "qapi/qobject-output-visitor.h"
>>
>>  /* DEBUG_SSH=1 enables the DPRINTF (debugging printf) statements in
>>   * this block driver code.
>> @@ -74,6 +78,8 @@ typedef struct BDRVSSHState {
>>   */
>>  LIBSSH2_SFTP_ATTRIBUTES attrs;
>>
>> +InetSocketAddress *inet;
>> +
>>  /* Used to warn if 'flush' is not supported. */
>>  char *hostport;
>>  bool unsafe_flush_warning;
>> @@ -263,7 +269,9 @@ static bool ssh_has_filename_options_conflict(QDict 
>> *options, Error **errp)
>>  !strcmp(qe->key, "port") ||
>>  !strcmp(qe->key, "path") ||
>>  !strcmp(qe->key, "user") ||
>> -!strcmp(qe->key, "host_key_check"))
>> +!strcmp(qe->key, "host_key_check") ||
>> +!strcmp(qe->key, "server") ||
>
> I know I've done the same in my series, but I'll actually drop this
> condition from the next version; I'm not actually handling the case
> where the destination address is not flattened, and neither are you
> (we're both using qdict_extract_subqdict() instead of testing whether
> "server" is an object on its own), so I think you should drop it as well
> and just test for the prefix.
>
> It doesn't harm to test for "server" itself, but I think it's a bit
> confusing still, since you (we) are not dealing with that possibility
> anywhere else.

Yes, I have dropped this.

>> +!strstart(qe->key, "server.", NULL))
>
> It should be just "strstart", not "!strstart", because the function
> returns 1 if the prefix matches.

Fixed.

>
>>  {
>>  error_setg(errp, "Option '%s' cannot be used with a file name",
>> qe->key);
>> @@ -555,13 +563,66 @@ static QemuOptsList ssh_runtime_opts = {
>>  },
>>  };
>>
>> +static bool ssh_process_legacy_socket_options(QDict *output_opts,
>> +  QemuOpts *legacy_opts,
>> +  Error **errp)
>> +{
>> +const char *host = qemu_opt_get(legacy_opts, "host");
>> +const char *port = qemu_opt_get(legacy_opts, "port");
>> +
>> +if (!host && port) {
>> +error_setg(errp, "port may not be used without host");
>> +return false;
>> +} else {
>> +qdict_put(output_opts, "server.host", qstring_from_str(host));
>
> This will segfault if host is NULL. You shouldn't go into this branch at
> all if host is not set.

Yes, I have put this in a different 'if' like:

if (host) {
   qdict_put();
   ...
}

>
>> +qdict_put(output_opts, "server.port",
>> +  qstring_from_str(port ?: stringify(22)));
>> +}
>> +
>> +return true;
>> +}
>> +
>> +static InetSocketAddress *ssh_config(BDRVSSHState *s, QDict *options,
>> + Error **errp)
>> +{
>> +InetSocketAddress *inet = NULL;
>> +QDict *addr = NULL;
>> +QObject *crumpled_addr = NULL;
>> +Visitor *iv = NULL;
>> +Error *local_error = NULL;
>> +
>> +qdict_extract_subqdict(options, , "server.");
>> +if (!qdict_size(addr)) {
>> +error_setg(errp, "SSH server address missing");
>> +goto out;
>> +}
>> +
>> +crumpled_addr = qdict_crumple(addr, true, errp);
>> +if (!crumpled_addr) {
>> +goto out;
>> +}
>> +
>> +iv = qobject_input_visitor_new_autocast(crumpled_addr, true, 1, true);
>
> In contrast to what Kevin said in v1, I think you do not want to use
> autocast here.
>
> Or, to be more specific, it's difficult. The thing is that the autocast
> documentation says: "Any scalar values in the @obj input data structure
> should always be represented as strings".
>
> So if you do use the autocast version, command line works great because
> from there everything comes as a string. But blockdev-add no longer
> works because from there everything comes with the correct type (and you
> cannot give it the wrong type). Case in point, this happens 

Re: [Qemu-devel] [PATCH v4 08/14] tcg/s390: Add support for fence

[Stefan Hajnoczi]

On Thu, Jul 14, 2016 at 04:20:20PM -0400, Pranith Kumar wrote:
> Cc: Alexander Graf 
> Signed-off-by: Pranith Kumar 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/s390/tcg-target.inc.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c
> index 5a7495b..01aae35 100644
> --- a/tcg/s390/tcg-target.inc.c
> +++ b/tcg/s390/tcg-target.inc.c
> @@ -343,6 +343,7 @@ static tcg_insn_unit *tb_ret_addr;
>  #define FACILITY_EXT_IMM (1ULL << (63 - 21))
>  #define FACILITY_GEN_INST_EXT(1ULL << (63 - 34))
>  #define FACILITY_LOAD_ON_COND   (1ULL << (63 - 45))
> +#define FACILITY_FAST_BCR_SER   FACILITY_LOAD_ON_COND
>  
>  static uint64_t facilities;
>  
> @@ -2172,6 +2173,15 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode 
> opc,
>  tgen_deposit(s, args[0], args[2], args[3], args[4]);
>  break;
>  
> +case INDEX_op_mb:
> +/* The host memory model is quite strong, we simply need to
> +   serialize the instruction stream.  */
> +if (args[0] == TCG_MO_ALL || args[0] == TCG_MO_ST_LD) {
> +tcg_out_insn(s, RR, BCR,
> + facilities & FACILITY_FAST_BCR_SER ? 14 : 15, 0);
> +}

args[0] == TCG_MO_ALL is always false since frontends bitwise-OR
TCG_BAR_SC.

Did you mean:

switch (args[0] & TCG_MO_ALL) {
case TCG_MO_ALL: /* fall-through */
case TCG_MO_ST_LD:
tcg_out_insn(s, RR, BCR,
 facilities & FACILITY_FAST_BCR_SER ? 14 : 15, 0);
break;
}

?


signature.asc
Description: PGP signature

[Qemu-devel] -Werror=tautological-compare error with -save-temps on fedora 24 gcc 6.2.1

[Anand J]

Hi,

I'm using gcc 6.2.1 for compiling qemu with following options in Fedora 24

../../../configure --enable-debug --extra-cflags="-save-temps"
make

and getting following error.

*  CC  ui/gtk.o*
*qemu/ui/gtk.c: In function ‘gd_map_keycode’:*
*qemu/ui/gtk.c:1030:21: error: self-comparison always evaluates to true
[-Werror=tautological-compare]*
* } else if (GDK_IS_X11_DISPLAY(dpy) && gdk_keycode < 158) {*
* ^~*
*qemu/ui/gtk.c: In function ‘gd_set_keycode_type’:*
*qemu/ui/gtk.c:2123:18: error: self-comparison always evaluates to true
[-Werror=tautological-compare]*
* if (GDK_IS_X11_DISPLAY(display)) {*
*  ^~*
*cc1: all warnings being treated as errors*
*qemu/rules.mak:60: recipe for target 'ui/gtk.o' failed*
*make: *** [ui/gtk.o] Error 1*



Build works fine without -save-temps options. Does anybody know how to fix
this?

Thanks,
Anand