[Qemu-devel] [PULL 12/31] ftgmac100: remove check on runt messages
From: Cédric Le Goater
This is a ethernet wire limitation not needed in emulation. It breaks
U-Boot n/w stack also.
Signed-off-by: Cédric Le Goater
Message-id: 20180530061711.23673-5-...@kaod.org
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/net/ftgmac100.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 8a7f274dc11..909c1182eeb 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -822,12 +822,6 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const
uint8_t *buf,
return size;
}
-if (size < 64 && !(s->maccr & FTGMAC100_MACCR_RX_RUNT)) {
-qemu_log_mask(LOG_GUEST_ERROR, "%s: dropped runt frame of %zd bytes\n",
- __func__, size);
-return size;
-}
-
if (!ftgmac100_filter(s, buf, size)) {
return size;
}
--
2.17.1
[Qemu-devel] [PULL 17/31] xilinx-dp: Add trailing '\n' to qemu_log() call
From: Philippe Mathieu-Daudé Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-id: 20180606152128.449-4-f4...@amsat.org Signed-off-by: Peter Maydell --- hw/display/xlnx_dp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c index 6715b9cc2b9..c32ab083f83 100644 --- a/hw/display/xlnx_dp.c +++ b/hw/display/xlnx_dp.c @@ -1074,7 +1074,9 @@ static void xlnx_dp_avbufm_write(void *opaque, hwaddr offset, uint64_t value, case AV_BUF_STC_SNAPSHOT1: case AV_BUF_HCOUNT_VCOUNT_INT0: case AV_BUF_HCOUNT_VCOUNT_INT1: -qemu_log_mask(LOG_UNIMP, "avbufm: unimplmented"); +qemu_log_mask(LOG_UNIMP, "avbufm: unimplemented register 0x%04" + PRIx64 "\n", + offset << 2); break; default: s->avbufm_registers[offset] = value; -- 2.17.1
[Qemu-devel] [PATCH v6 22/49] tests/tcg: enable building for ARM
This allows us to use the docker cross compiler image to build these tests. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- v5 - add EXTRA_RUNS for mmap tests --- tests/tcg/arm/Makefile.include | 8 tests/tcg/arm/Makefile.target | 5 - 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/arm/Makefile.include diff --git a/tests/tcg/arm/Makefile.include b/tests/tcg/arm/Makefile.include new file mode 100644 index 00..8e7eac008f --- /dev/null +++ b/tests/tcg/arm/Makefile.include @@ -0,0 +1,8 @@ +# Makefile.include for all ARM targets +# +# We don't have any bigendian build tools so we only use this for armhf + +ifeq ($(TARGET_NAME),arm) +DOCKER_IMAGE=debian-armhf-cross +DOCKER_CROSS_COMPILER=arm-linux-gnueabihf-gcc +endif diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target index bc6962ecc6..0312293dca 100644 --- a/tests/tcg/arm/Makefile.target +++ b/tests/tcg/arm/Makefile.target @@ -1,6 +1,6 @@ # -*- Mode: makefile -*- # -# ARM - included from tests/tcg/Makefile.target +# ARM - included from tests/tcg/Makefile # ARM_SRC=$(SRC_PATH)/tests/tcg/arm @@ -11,3 +11,6 @@ VPATH += $(ARM_SRC) hello-arm: CFLAGS+=-marm -ffreestanding hello-arm: LDFLAGS+=-nostdlib + +# On ARM Linux only supports 4k pages +EXTRA_RUNS+=run-test-mmap-4096 -- 2.17.1
[Qemu-devel] [PULL 31/31] sdcard: Disable CMD19/CMD23 for Spec v2
From: Philippe Mathieu-Daudé
These commands got introduced by Spec v3
(see 0c3fb03f7ec and 4481bbc79d2).
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20180607180641.874-7-f4...@amsat.org
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/sd/sd.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 4e49a3827a3..540bccb8d13 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1179,6 +1179,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
break;
case 19:/* CMD19: SEND_TUNING_BLOCK (SD) */
+if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+break;
+}
if (sd->state == sd_transfer_state) {
sd->state = sd_sendingdata_state;
sd->data_offset = 0;
@@ -1187,6 +1190,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
break;
case 23:/* CMD23: SET_BLOCK_COUNT */
+if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+break;
+}
switch (sd->state) {
case sd_transfer_state:
sd->multi_blk_cnt = req.arg;
--
2.17.1
[Qemu-devel] [PULL 25/31] target/xtensa: Add trailing '\n' to qemu_log() calls
From: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
Acked-by: Max Filippov
Message-id: 20180606152128.449-12-f4...@amsat.org
Signed-off-by: Peter Maydell
---
target/xtensa/translate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 89db23852b7..a11162eebe0 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -2234,7 +2234,7 @@ static void translate_rur(DisasContext *dc, const
uint32_t arg[],
if (uregnames[par[0]].name) {
tcg_gen_mov_i32(cpu_R[arg[0]], cpu_UR[par[0]]);
} else {
-qemu_log_mask(LOG_UNIMP, "RUR %d not implemented, ", par[0]);
+qemu_log_mask(LOG_UNIMP, "RUR %d not implemented\n", par[0]);
}
}
}
@@ -2375,7 +2375,7 @@ static void translate_slli(DisasContext *dc, const
uint32_t arg[],
{
if (gen_window_check2(dc, arg[0], arg[1])) {
if (arg[2] == 32) {
-qemu_log_mask(LOG_GUEST_ERROR, "slli a%d, a%d, 32 is undefined",
+qemu_log_mask(LOG_GUEST_ERROR, "slli a%d, a%d, 32 is undefined\n",
arg[0], arg[1]);
}
tcg_gen_shli_i32(cpu_R[arg[0]], cpu_R[arg[1]], arg[2] & 0x1f);
@@ -2571,7 +2571,7 @@ static void translate_wur(DisasContext *dc, const
uint32_t arg[],
if (uregnames[par[0]].name) {
gen_wur(par[0], cpu_R[arg[0]]);
} else {
-qemu_log_mask(LOG_UNIMP, "WUR %d not implemented, ", par[0]);
+qemu_log_mask(LOG_UNIMP, "WUR %d not implemented\n", par[0]);
}
}
}
--
2.17.1
[Qemu-devel] [PULL 13/31] hw/arm: Remove the deprecated xlnx-ep108 machine
From: Thomas Huth
It has been marked as deprecated since QEMU v2.11, so it is time to
remove this now. The xlnx-zcu102 machine is very much the same and
can be used as a replacement instead.
Signed-off-by: Thomas Huth
Reviewed-by: Alistair Francis
Signed-off-by: Peter Maydell
---
hw/arm/xlnx-zcu102.c | 62 ++--
qemu-doc.texi| 5
2 files changed, 2 insertions(+), 65 deletions(-)
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index c70278c8c10..f26fd8eb919 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -39,10 +39,6 @@ typedef struct XlnxZCU102 {
#define ZCU102_MACHINE(obj) \
OBJECT_CHECK(XlnxZCU102, (obj), TYPE_ZCU102_MACHINE)
-#define TYPE_EP108_MACHINE MACHINE_TYPE_NAME("xlnx-ep108")
-#define EP108_MACHINE(obj) \
-OBJECT_CHECK(XlnxZCU102, (obj), TYPE_EP108_MACHINE)
-
static struct arm_boot_info xlnx_zcu102_binfo;
static bool zcu102_get_secure(Object *obj, Error **errp)
@@ -73,8 +69,9 @@ static void zcu102_set_virt(Object *obj, bool value, Error
**errp)
s->virt = value;
}
-static void xlnx_zynqmp_init(XlnxZCU102 *s, MachineState *machine)
+static void xlnx_zcu102_init(MachineState *machine)
{
+XlnxZCU102 *s = ZCU102_MACHINE(machine);
int i;
uint64_t ram_size = machine->ram_size;
@@ -183,60 +180,6 @@ static void xlnx_zynqmp_init(XlnxZCU102 *s, MachineState
*machine)
arm_load_kernel(s->soc.boot_cpu_ptr, _zcu102_binfo);
}
-static void xlnx_ep108_init(MachineState *machine)
-{
-XlnxZCU102 *s = EP108_MACHINE(machine);
-
-if (!qtest_enabled()) {
-info_report("The Xilinx EP108 machine is deprecated, please use the "
-"ZCU102 machine (which has the same features) instead.");
-}
-
-xlnx_zynqmp_init(s, machine);
-}
-
-static void xlnx_ep108_machine_instance_init(Object *obj)
-{
-XlnxZCU102 *s = EP108_MACHINE(obj);
-
-/* EP108, we don't support setting secure or virt */
-s->secure = false;
-s->virt = false;
-}
-
-static void xlnx_ep108_machine_class_init(ObjectClass *oc, void *data)
-{
-MachineClass *mc = MACHINE_CLASS(oc);
-
-mc->desc = "Xilinx ZynqMP EP108 board (Deprecated, please use
xlnx-zcu102)";
-mc->init = xlnx_ep108_init;
-mc->block_default_type = IF_IDE;
-mc->units_per_default_bus = 1;
-mc->ignore_memory_transaction_failures = true;
-mc->max_cpus = XLNX_ZYNQMP_NUM_APU_CPUS + XLNX_ZYNQMP_NUM_RPU_CPUS;
-mc->default_cpus = XLNX_ZYNQMP_NUM_APU_CPUS;
-}
-
-static const TypeInfo xlnx_ep108_machine_init_typeinfo = {
-.name = MACHINE_TYPE_NAME("xlnx-ep108"),
-.parent = TYPE_MACHINE,
-.class_init = xlnx_ep108_machine_class_init,
-.instance_init = xlnx_ep108_machine_instance_init,
-.instance_size = sizeof(XlnxZCU102),
-};
-
-static void xlnx_ep108_machine_init_register_types(void)
-{
-type_register_static(_ep108_machine_init_typeinfo);
-}
-
-static void xlnx_zcu102_init(MachineState *machine)
-{
-XlnxZCU102 *s = ZCU102_MACHINE(machine);
-
-xlnx_zynqmp_init(s, machine);
-}
-
static void xlnx_zcu102_machine_instance_init(Object *obj)
{
XlnxZCU102 *s = ZCU102_MACHINE(obj);
@@ -289,4 +232,3 @@ static void xlnx_zcu102_machine_init_register_types(void)
}
type_init(xlnx_zcu102_machine_init_register_types)
-type_init(xlnx_ep108_machine_init_register_types)
diff --git a/qemu-doc.texi b/qemu-doc.texi
index f00706b9996..2effe66d6bc 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2965,11 +2965,6 @@ support page sizes < 4096 any longer.
@section System emulator machines
-@subsection Xilinx EP108 (since 2.11.0)
-
-The ``xlnx-ep108'' machine has been replaced by the ``xlnx-zcu102'' machine.
-The ``xlnx-zcu102'' machine has the same features and capabilites in QEMU.
-
@section Block device options
@subsection "backing": "" (since 2.12.0)
--
2.17.1
[Qemu-devel] [PULL 23/31] target/m68k: Add trailing '\n' to qemu_log() call
From: Philippe Mathieu-Daudé Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Laurent Vivier Message-id: 20180606152128.449-10-f4...@amsat.org Signed-off-by: Peter Maydell --- target/m68k/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 37d6ffd8531..4b5dbdb51c9 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -1556,7 +1556,7 @@ DISAS_INSN(undef) /* ??? This is both instructions that are as yet unimplemented for the 680x0 series, as well as those that are implemented but actually illegal for CPU32 or pre-68020. */ -qemu_log_mask(LOG_UNIMP, "Illegal instruction: %04x @ %08x", +qemu_log_mask(LOG_UNIMP, "Illegal instruction: %04x @ %08x\n", insn, s->insn_pc); gen_exception(s, s->insn_pc, EXCP_UNSUPPORTED); } -- 2.17.1
[Qemu-devel] [PATCH v6 34/49] tests/tcg: enable building for m68k
As before, using Debian SID compilers. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- v5 - add EXTRA_RUNS for mmap tests --- tests/docker/Makefile.include | 1 + tests/docker/dockerfiles/debian-m68k-cross.docker | 12 tests/tcg/m68k/Makefile.include | 2 ++ tests/tcg/m68k/Makefile.target| 7 +++ 4 files changed, 22 insertions(+) create mode 100644 tests/docker/dockerfiles/debian-m68k-cross.docker create mode 100644 tests/tcg/m68k/Makefile.include create mode 100644 tests/tcg/m68k/Makefile.target diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index a83e979e6d..b1ae717010 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -71,6 +71,7 @@ docker-image-debian-sid: NOCACHE=1 docker-image-debian-alpha-cross: docker-image-debian-sid docker-image-debian-hppa-cross: docker-image-debian-sid +docker-image-debian-m68k-cross: docker-image-debian-sid docker-image-travis: NOUSER=1 # Specialist build images, sometimes very limited tools diff --git a/tests/docker/dockerfiles/debian-m68k-cross.docker b/tests/docker/dockerfiles/debian-m68k-cross.docker new file mode 100644 index 00..21ba3b0132 --- /dev/null +++ b/tests/docker/dockerfiles/debian-m68k-cross.docker @@ -0,0 +1,12 @@ +# +# Docker cross-compiler target +# +# This docker target builds on the debian sid base image which +# contains cross compilers for Debian "ports" targets. +# +FROM qemu:debian-sid + +RUN DEBIAN_FRONTEND=noninteractive eatmydata \ +apt-get install -y --no-install-recommends \ +gcc-m68k-linux-gnu \ +libc6-dev-m68k-cross diff --git a/tests/tcg/m68k/Makefile.include b/tests/tcg/m68k/Makefile.include new file mode 100644 index 00..cd7c6bf50d --- /dev/null +++ b/tests/tcg/m68k/Makefile.include @@ -0,0 +1,2 @@ +DOCKER_IMAGE=debian-m68k-cross +DOCKER_CROSS_COMPILER=m68k-linux-gnu-gcc diff --git a/tests/tcg/m68k/Makefile.target b/tests/tcg/m68k/Makefile.target new file mode 100644 index 00..62f109eef4 --- /dev/null +++ b/tests/tcg/m68k/Makefile.target @@ -0,0 +1,7 @@ +# -*- Mode: makefile -*- +# +# m68k specific tweaks - specifically masking out broken tests +# + +# On m68k Linux supports 4k and 8k pages (but 8k is currently broken) +EXTRA_RUNS+=run-test-mmap-4096 # run-test-mmap-8192 -- 2.17.1
[Qemu-devel] [PATCH v6 12/49] tests/tcg/multiarch: move most output to stdout
The default test run outputs to stdout so it can be re-directed.
Errors are still reported to stderr.
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
---
v4
- remove space in fprintf () to keep checkpatch happy
---
tests/tcg/multiarch/test-mmap.c | 40 -
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/tests/tcg/multiarch/test-mmap.c b/tests/tcg/multiarch/test-mmap.c
index cdefadfa4c..5c0afe6e49 100644
--- a/tests/tcg/multiarch/test-mmap.c
+++ b/tests/tcg/multiarch/test-mmap.c
@@ -36,7 +36,7 @@
do \
{ \
if (!(x)) { \
-fprintf (stderr, "FAILED at %s:%d\n", __FILE__, __LINE__); \
+fprintf(stderr, "FAILED at %s:%d\n", __FILE__, __LINE__); \
exit (EXIT_FAILURE); \
}\
} while (0)
@@ -57,7 +57,7 @@ void check_aligned_anonymous_unfixed_mmaps(void)
uintptr_t p;
int i;
- fprintf (stderr, "%s", __func__);
+ fprintf(stdout, "%s", __func__);
for (i = 0; i < 0x1fff; i++)
{
size_t len;
@@ -106,7 +106,7 @@ void check_aligned_anonymous_unfixed_mmaps(void)
munmap (p4, len);
munmap (p5, len);
}
- fprintf (stderr, " passed\n");
+ fprintf(stdout, " passed\n");
}
void check_large_anonymous_unfixed_mmap(void)
@@ -115,7 +115,7 @@ void check_large_anonymous_unfixed_mmap(void)
uintptr_t p;
size_t len;
- fprintf (stderr, "%s", __func__);
+ fprintf(stdout, "%s", __func__);
len = 0x0200;
p1 = mmap(NULL, len, PROT_READ,
@@ -130,7 +130,7 @@ void check_large_anonymous_unfixed_mmap(void)
/* Make sure we can read from the entire area. */
memcpy (dummybuf, p1, pagesize);
munmap (p1, len);
- fprintf (stderr, " passed\n");
+ fprintf(stdout, " passed\n");
}
void check_aligned_anonymous_unfixed_colliding_mmaps(void)
@@ -141,7 +141,7 @@ void check_aligned_anonymous_unfixed_colliding_mmaps(void)
uintptr_t p;
int i;
- fprintf (stderr, "%s", __func__);
+ fprintf(stdout, "%s", __func__);
for (i = 0; i < 0x2fff; i++)
{
int nlen;
@@ -180,7 +180,7 @@ void check_aligned_anonymous_unfixed_colliding_mmaps(void)
munmap (p2, pagesize);
munmap (p3, nlen);
}
- fprintf (stderr, " passed\n");
+ fprintf(stdout, " passed\n");
}
void check_aligned_anonymous_fixed_mmaps(void)
@@ -194,7 +194,7 @@ void check_aligned_anonymous_fixed_mmaps(void)
addr = mmap(NULL, pagesize * 40, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS,
-1, 0);
- fprintf (stderr, "%s addr=%p", __func__, addr);
+ fprintf(stdout, "%s addr=%p", __func__, addr);
fail_unless (addr != MAP_FAILED);
for (i = 0; i < 40; i++)
@@ -212,7 +212,7 @@ void check_aligned_anonymous_fixed_mmaps(void)
munmap (p1, pagesize);
addr += pagesize;
}
- fprintf (stderr, " passed\n");
+ fprintf(stdout, " passed\n");
}
void check_aligned_anonymous_fixed_mmaps_collide_with_host(void)
@@ -225,8 +225,8 @@ void
check_aligned_anonymous_fixed_mmaps_collide_with_host(void)
/* Find a suitable address to start with. Right were the x86 hosts
stack is. */
addr = ((void *)0x8000);
- fprintf (stderr, "%s addr=%p", __func__, addr);
- fprintf (stderr, "FIXME: QEMU fails to track pages used by the host.");
+ fprintf(stdout, "%s addr=%p", __func__, addr);
+ fprintf(stdout, "FIXME: QEMU fails to track pages used by the host.");
for (i = 0; i < 20; i++)
{
@@ -243,7 +243,7 @@ void
check_aligned_anonymous_fixed_mmaps_collide_with_host(void)
munmap (p1, pagesize);
addr += pagesize;
}
- fprintf (stderr, " passed\n");
+ fprintf(stdout, " passed\n");
}
void check_file_unfixed_mmaps(void)
@@ -252,7 +252,7 @@ void check_file_unfixed_mmaps(void)
uintptr_t p;
int i;
- fprintf (stderr, "%s", __func__);
+ fprintf(stdout, "%s", __func__);
for (i = 0; i < 0x10; i++)
{
size_t len;
@@ -294,7 +294,7 @@ void check_file_unfixed_mmaps(void)
munmap (p2, len);
munmap (p3, len);
}
- fprintf (stderr, " passed\n");
+ fprintf(stdout, " passed\n");
}
void check_file_unfixed_eof_mmaps(void)
@@ -304,7 +304,7 @@ void check_file_unfixed_eof_mmaps(void)
uintptr_t p;
int i;
- fprintf (stderr, "%s", __func__);
+
[Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
From: "Dr. David Alan Gilbert" Allow a bunch of the info commands to be used in preconfig. version, chardev, name, uuid,memdev, iothreads Were enabled in QMP in the previous patch from Igor status, hotpluggable_cpus Was enabled in the original allow-preconfig series history is HMP specific usbhost, qom-tree, numa Don't have a QMP equivalent Signed-off-by: Dr. David Alan Gilbert --- hmp-commands-info.hx | 12 hmp-commands.hx | 1 + 2 files changed, 13 insertions(+) diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index ddfcd5adcc..f4d0d7d2d0 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -19,6 +19,7 @@ ETEXI .params = "", .help = "show the version of QEMU", .cmd= hmp_info_version, +.flags = "p", }, STEXI @@ -47,6 +48,7 @@ ETEXI .params = "", .help = "show the character devices", .cmd= hmp_info_chardev, +.flags = "p", }, STEXI @@ -165,6 +167,7 @@ ETEXI .params = "", .help = "show the command line history", .cmd= hmp_info_history, +.flags = "p", }, STEXI @@ -315,6 +318,7 @@ ETEXI .params = "", .help = "show NUMA information", .cmd= hmp_info_numa, +.flags = "p", }, STEXI @@ -343,6 +347,7 @@ ETEXI .params = "", .help = "show host USB devices", .cmd= hmp_info_usbhost, +.flags = "p", }, STEXI @@ -399,6 +404,7 @@ ETEXI .params = "", .help = "show the current VM status (running|paused)", .cmd= hmp_info_status, +.flags = "p", }, STEXI @@ -457,6 +463,7 @@ ETEXI .params = "", .help = "show the current VM name", .cmd= hmp_info_name, +.flags = "p", }, STEXI @@ -471,6 +478,7 @@ ETEXI .params = "", .help = "show the current VM UUID", .cmd= hmp_info_uuid, +.flags = "p", }, STEXI @@ -613,6 +621,7 @@ ETEXI .params = "[path]", .help = "show QOM composition tree", .cmd= hmp_info_qom_tree, +.flags = "p", }, STEXI @@ -671,6 +680,7 @@ ETEXI .params = "", .help = "show memory backends", .cmd= hmp_info_memdev, +.flags = "p", }, STEXI @@ -699,6 +709,7 @@ ETEXI .params = "", .help = "show iothreads", .cmd= hmp_info_iothreads, +.flags = "p", }, STEXI @@ -829,6 +840,7 @@ ETEXI .params = "", .help = "Show information about hotpluggable CPUs", .cmd= hmp_hotpluggable_cpus, +.flags = "p", }, STEXI diff --git a/hmp-commands.hx b/hmp-commands.hx index 8bf590ae4b..dc82ed526f 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1856,6 +1856,7 @@ ETEXI .help = "show various information about the system state", .cmd= hmp_info_help, .sub_table = info_cmds, +.flags = "p", }, STEXI -- 2.17.0
[Qemu-devel] [PATCH v6 43/49] tests/Makefile.include: add [build|clean|check]-tcg targets
This will ensure all linux-user targets build their guest test programs and ensure check-tcg will run the respective tests. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- v2 - use -include instead of complex macro stuff - also include TARGET_BASE_ARCH/Makefile v3 - add build-tcg, make check-tcg actually run tests --- tests/Makefile.include | 26 +- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index 0eaa835b8a..ca00247e36 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -10,6 +10,7 @@ check-help: @echo " $(MAKE) check-speed Run qobject speed tests" @echo " $(MAKE) check-qapi-schemaRun QAPI schema tests" @echo " $(MAKE) check-block Run block tests" + @echo " $(MAKE) check-tcgRun TCG tests" @echo " $(MAKE) check-report.htmlGenerates an HTML test report" @echo " $(MAKE) check-clean Clean the tests" @echo @@ -923,6 +924,30 @@ check-report.xml: $(patsubst %,check-report-qtest-%.xml, $(QTEST_TARGETS)) check check-report.html: check-report.xml $(call quiet-command,gtester-report $< > $@,"GEN","$@") +# Per guest TCG tests + +LINUX_USER_TARGETS=$(filter %-linux-user,$(TARGET_LIST)) +BUILD_TCG_TARGET_RULES=$(patsubst %,build-tcg-tests-%, $(LINUX_USER_TARGETS)) +CLEAN_TCG_TARGET_RULES=$(patsubst %,clean-tcg-tests-%, $(LINUX_USER_TARGETS)) +RUN_TCG_TARGET_RULES=$(patsubst %,run-tcg-tests-%, $(LINUX_USER_TARGETS)) + +build-tcg-tests-%: + $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" TARGET_DIR="$*/" guest-tests,) + +run-tcg-tests-%: build-tcg-tests-% + $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" TARGET_DIR="$*/" run-guest-tests,) + +clean-tcg-tests-%: + $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" TARGET_DIR="$*/" clean-guest-tests,) + +.PHONY: build-tcg +build-tcg: $(BUILD_TCG_TARGET_RULES) + +.PHONY: check-tcg +check-tcg: $(RUN_TCG_TARGET_RULES) + +.PHONY: clean-tcg +clean-tcg: $(CLEAN_TCG_TARGET_RULES) # Other tests @@ -965,7 +990,6 @@ check-speed: $(patsubst %,check-%, $(check-speed-y)) check-block: $(patsubst %,check-%, $(check-block-y)) check: check-qapi-schema check-unit check-qtest check-decodetree check-clean: - $(MAKE) -C tests/tcg clean rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y) rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), $(check-qtest-$(target)-y)) $(check-qtest-generic-y)) rm -f tests/test-qapi-gen-timestamp -- 2.17.1
[Qemu-devel] [PATCH v6 18/49] tests/tcg/x86_64: add Makefile.target
The sources for x86_64 are shared in the i386 directory which will be included thanks to TARGET_BASE_ARCH. However not all sources build so we need to filter out the ones we can't build in the 64 bit world and those that can't be built for 32 bit. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- v4 - update MAINTAINERS v5 - merge with disable i386 version of test-i386-ssse --- MAINTAINERS | 1 + tests/tcg/i386/Makefile.target | 4 ++-- tests/tcg/x86_64/Makefile.target | 15 +++ 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 tests/tcg/x86_64/Makefile.target diff --git a/MAINTAINERS b/MAINTAINERS index e795b8186e..1063c4d60f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -284,6 +284,7 @@ M: Eduardo Habkost S: Maintained F: target/i386/ F: tests/tcg/i386/ +F: tests/tcg/x86_64/ F: hw/i386/ F: disas/i386.c T: git git://github.com/ehabkost/qemu.git x86-next diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target index 64d241cfdf..cd173363ee 100644 --- a/tests/tcg/i386/Makefile.target +++ b/tests/tcg/i386/Makefile.target @@ -7,9 +7,9 @@ VPATH += $(I386_SRC) I386_SRCS=$(notdir $(wildcard $(I386_SRC)/*.c)) I386_TESTS=$(I386_SRCS:.c=) - +I386_ONLY_TESTS=$(filter-out test-i386-ssse3, $(I386_TESTS)) # Update TESTS -TESTS+=$(I386_TESTS) +TESTS+=$(I386_ONLY_TESTS) ifneq ($(TARGET_NAME),x86_64) CFLAGS+=-m32 diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target new file mode 100644 index 00..74f170b9ed --- /dev/null +++ b/tests/tcg/x86_64/Makefile.target @@ -0,0 +1,15 @@ +# -*- Mode: makefile -*- +# +# x86_64 tests - included from tests/tcg/Makefile.target +# +# Currently we only build test-x86_64 and test-i386-ssse3 from +# $(SRC)/tests/tcg/i386/ +# + +X86_64_TESTS=$(filter-out $(I386_ONLY_TESTS), $(TESTS)) +X86_64_TESTS+=test-x86_64 +TESTS:=$(X86_64_TESTS) + +test-x86_64: LDFLAGS+=-lm -lc +test-x86_64: test-i386.c test-i386.h test-i386-shift.h test-i386-muldiv.h + $(CC) $(CFLAGS) $< -o $@ $(LDFLAGS) -- 2.17.1
[Qemu-devel] [PATCH v4 02/10] block: Add blklogwrites
From: Aapo Vienamo
Implements a block device write logging system, similar to Linux kernel
device mapper dm-log-writes. The write operations that are performed
on a block device are logged to a file or another block device. The
write log format is identical to the dm-log-writes format. Currently,
log markers are not supported.
This functionality can be used for crash consistency and fs consistency
testing. By implementing it in qemu, tests utilizing write logs can be
be used to test non-Linux drivers and older kernels.
The implementation is based on the blkverify and blkdebug block drivers.
Signed-off-by: Aapo Vienamo
Signed-off-by: Ari Sundholm
---
MAINTAINERS | 6 +
block/Makefile.objs | 1 +
block/blklogwrites.c | 389 +++
qapi/block-core.json | 30 +++-
4 files changed, 420 insertions(+), 6 deletions(-)
create mode 100644 block/blklogwrites.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 4c73c16..4b8d1a8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2015,6 +2015,12 @@ S: Supported
F: block/quorum.c
L: qemu-bl...@nongnu.org
+blklogwrites
+M: Ari Sundholm
+L: qemu-bl...@nongnu.org
+S: Supported
+F: block/blklogwrites.c
+
blkverify
M: Stefan Hajnoczi
L: qemu-bl...@nongnu.org
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 899bfb5..c8337bf 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -5,6 +5,7 @@ block-obj-y += qed-check.o
block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
block-obj-y += quorum.o
block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o
+block-obj-y += blklogwrites.o
block-obj-y += block-backend.o snapshot.o qapi.o
block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
block-obj-$(CONFIG_POSIX) += file-posix.o
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
new file mode 100644
index 000..1b969b0
--- /dev/null
+++ b/block/blklogwrites.c
@@ -0,0 +1,389 @@
+/*
+ * Write logging blk driver based on blkverify and blkdebug.
+ *
+ * Copyright (c) 2017 Tuomas Tynkkynen
+ * Copyright (c) 2018 Aapo Vienamo
+ * Copyright (c) 2018 Ari Sundholm
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/sockets.h" /* for EINPROGRESS on Windows */
+#include "block/block_int.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qstring.h"
+#include "qemu/cutils.h"
+#include "qemu/option.h"
+
+/* Disk format stuff - taken from Linux drivers/md/dm-log-writes.c */
+
+#define LOG_FLUSH_FLAG (1 << 0)
+#define LOG_FUA_FLAG (1 << 1)
+#define LOG_DISCARD_FLAG (1 << 2)
+#define LOG_MARK_FLAG (1 << 3)
+
+#define WRITE_LOG_VERSION 1ULL
+#define WRITE_LOG_MAGIC 0x6a736677736872ULL
+
+/* All fields are little-endian. */
+struct log_write_super {
+uint64_t magic;
+uint64_t version;
+uint64_t nr_entries;
+uint32_t sectorsize;
+};
+
+struct log_write_entry {
+uint64_t sector;
+uint64_t nr_sectors;
+uint64_t flags;
+uint64_t data_len;
+};
+
+/* End of disk format structures. */
+
+typedef struct {
+BdrvChild *log_file;
+uint64_t cur_log_sector;
+uint64_t nr_entries;
+} BDRVBlkLogWritesState;
+
+static int blk_log_writes_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
+{
+BDRVBlkLogWritesState *s = bs->opaque;
+Error *local_err = NULL;
+int ret;
+
+/* Open the raw file */
+bs->file = bdrv_open_child(NULL, options, "raw", bs, _file, false,
+ _err);
+if (local_err) {
+ret = -EINVAL;
+error_propagate(errp, local_err);
+goto fail;
+}
+
+s->cur_log_sector = 1;
+s->nr_entries = 0;
+
+/* Open the log file */
+s->log_file = bdrv_open_child(NULL, options, "log", bs, _file, false,
+ _err);
+if (local_err) {
+ret = -EINVAL;
+error_propagate(errp, local_err);
+goto fail;
+}
+
+ret = 0;
+fail:
+if (ret < 0) {
+bdrv_unref_child(bs, bs->file);
+bs->file = NULL;
+}
+return ret;
+}
+
+static void blk_log_writes_close(BlockDriverState *bs)
+{
+BDRVBlkLogWritesState *s = bs->opaque;
+
+bdrv_unref_child(bs, s->log_file);
+s->log_file = NULL;
+}
+
+static int64_t blk_log_writes_getlength(BlockDriverState *bs)
+{
+return bdrv_getlength(bs->file->bs);
+}
+
+static void blk_log_writes_refresh_filename(BlockDriverState *bs,
+QDict *options)
+{
+BDRVBlkLogWritesState *s = bs->opaque;
+
+/* bs->file->bs has already been refreshed */
+bdrv_refresh_filename(s->log_file->bs);
+
+if (bs->file->bs->full_open_options
+&& s->log_file->bs->full_open_options)
+{
+QDict *opts = qdict_new();
+qdict_put_obj(opts, "driver",
+ QOBJECT(qstring_from_str("blklogwrites")));
+
+
[Qemu-devel] [PATCH v4 06/10] hw/block/virtio-blk: Always apply block configuration to block driver
This allows the block driver to use the block configuration of the new VirtIO block device. One use for this information is to set request limits using this information. Signed-off-by: Ari Sundholm --- hw/block/virtio-blk.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 50b5c86..51e7b86 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -975,6 +975,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s); blk_set_dev_ops(s->blk, _block_ops, s); blk_set_guest_block_size(s->blk, s->conf.conf.logical_block_size); +blkconf_apply_to_blkdrv(>conf.conf); blk_iostatus_enable(s->blk); } -- 2.7.4
[Qemu-devel] [PATCH v6 09/49] tests/tcg: move architecture independent tests into subdir
We will want to build these for all supported guest architectures so
lets move them all into one place. We also drop test_path at this
point because it needs qemu utils and glib bits which is hard to
support for cross compiling.
Signed-off-by: Alex Bennée
Reviewed-by: Thomas Huth
Reviewed-by: Richard Henderson
---
v2
- move VPATH and TESTs setup into multiarch/Makefile.target
- remove moved bits from tests/tcg/Makefile
v3
- use LDFLAGS+= for testthread linking
v4
- make myself MAINTAINER for tests/tcg/multiarch
v5
- split test-mmap into run-test-mmap and run-test-mmap-%
---
MAINTAINERS| 4 +
tests/tcg/Makefile | 31 -
tests/tcg/README | 10 +-
tests/tcg/multiarch/Makefile.target| 36 ++
tests/tcg/multiarch/README | 1 +
tests/tcg/{ => multiarch}/linux-test.c | 0
tests/tcg/{ => multiarch}/sha1.c | 0
tests/tcg/{ => multiarch}/test-mmap.c | 0
tests/tcg/{ => multiarch}/testthread.c | 0
tests/tcg/test_path.c | 157 -
10 files changed, 45 insertions(+), 194 deletions(-)
create mode 100644 tests/tcg/multiarch/Makefile.target
create mode 100644 tests/tcg/multiarch/README
rename tests/tcg/{ => multiarch}/linux-test.c (100%)
rename tests/tcg/{ => multiarch}/sha1.c (100%)
rename tests/tcg/{ => multiarch}/test-mmap.c (100%)
rename tests/tcg/{ => multiarch}/testthread.c (100%)
delete mode 100644 tests/tcg/test_path.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 4c73c16fee..b8f6c90122 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -303,6 +303,10 @@ F: target/tricore/
F: hw/tricore/
F: include/hw/tricore/
+Multiarch Linux User Tests
+M: Alex Bennée
+F: tests/tcg/multiarch/
+
Guest CPU Cores (KVM):
--
diff --git a/tests/tcg/Makefile b/tests/tcg/Makefile
index 89e3342f3d..e12395117a 100644
--- a/tests/tcg/Makefile
+++ b/tests/tcg/Makefile
@@ -18,12 +18,9 @@ LDFLAGS=
# also, pi_10.com runs indefinitely
I386_TESTS=hello-i386 \
- linux-test \
- testthread \
sha1-i386 \
test-i386 \
test-i386-fprem \
- test-mmap \
# runcom
# native i386 compilers sometimes are not biarch. assume cross-compilers are
@@ -47,8 +44,6 @@ run-%: %
-$(QEMU) ./$*
run-hello-i386: hello-i386
-run-linux-test: linux-test
-run-testthread: testthread
run-sha1-i386: sha1-i386
run-test-i386: test-i386
@@ -66,11 +61,6 @@ run-test-x86_64: test-x86_64
-$(QEMU_X86_64) test-x86_64 > test-x86_64.out
@if diff -u test-x86_64.ref test-x86_64.out ; then echo "Auto Test OK";
fi
-run-test-mmap: test-mmap
- -$(QEMU) ./test-mmap
- -$(QEMU) -p 8192 ./test-mmap 8192
- -$(QEMU) -p 16384 ./test-mmap 16384
- -$(QEMU) -p 32768 ./test-mmap 32768
run-runcom: runcom
-$(QEMU) ./runcom $(SRC_PATH)/tests/pi_10.com
@@ -80,17 +70,10 @@ run-test_path: test_path
# rules to compile tests
-test_path: test_path.o
-
-test_path.o: test_path.c
-
hello-i386: hello-i386.c
$(CC_I386) -nostdlib $(CFLAGS) -static $(LDFLAGS) -o $@ $<
strip $@
-testthread: testthread.c
- $(CC_I386) $(CFLAGS) $(LDFLAGS) -o $@ $< -lpthread
-
# i386/x86_64 emulation test (test various opcodes) */
test-i386: test-i386.c test-i386-code16.S test-i386-vm86.S \
test-i386.h test-i386-shift.h test-i386-muldiv.h
@@ -104,28 +87,14 @@ test-x86_64: test-i386.c \
test-i386.h test-i386-shift.h test-i386-muldiv.h
$(CC_X86_64) $(QEMU_INCLUDES) $(CFLAGS) $(LDFLAGS) -o $@
$( test-mmap.out, "TEST", \
+ "$< (default) on $(TARGET_NAME)")
+
+# additional page sizes (defined by each architecture adding to EXTRA_RUNS)
+run-test-mmap-%: test-mmap
+ $(call quiet-command, $(QEMU) -p $* $< > test-mmap-$*.out, "TEST", \
+ "$< ($* byte pages) on $(TARGET_NAME)")
diff --git a/tests/tcg/multiarch/README b/tests/tcg/multiarch/README
new file mode 100644
index 00..522c9d2ea3
--- /dev/null
+++ b/tests/tcg/multiarch/README
@@ -0,0 +1 @@
+Multi-architecture linux-user tests
diff --git a/tests/tcg/linux-test.c b/tests/tcg/multiarch/linux-test.c
similarity index 100%
rename from tests/tcg/linux-test.c
rename to tests/tcg/multiarch/linux-test.c
diff --git a/tests/tcg/sha1.c b/tests/tcg/multiarch/sha1.c
similarity index 100%
rename from tests/tcg/sha1.c
rename to tests/tcg/multiarch/sha1.c
diff --git a/tests/tcg/test-mmap.c b/tests/tcg/multiarch/test-mmap.c
similarity index 100%
rename from tests/tcg/test-mmap.c
rename to tests/tcg/multiarch/test-mmap.c
diff --git a/tests/tcg/testthread.c b/tests/tcg/multiarch/testthread.c
similarity index 100%
rename from tests/tcg/testthread.c
rename to tests/tcg/multiarch/testthread.c
diff --git a/tests/tcg/test_path.c b/tests/tcg/test_path.c
deleted file mode 100644
index 1c29bce263..00
--- a/tests/tcg/test_path.c
+++ /dev/null
@@ -1,157 +0,0 @@
-/* Test
[Qemu-devel] [PATCH v4 00/10] New block driver: blklogwrites
This patch series adds a new block driver, blklogwrites, to QEMU. The driver is given two block devices: a raw device backed by an image or a host block device, and a log device, typically backed by a file, on which writes to the raw device are logged. The logging format used is the same as in the dm-log-writes target of the Linux kernel device mapper. The log reflects the writes that have been performed on the guest block device and flushed. To be strict, the log may contain writes that have not been flushed yet, but they are technically outside the bounds of the log until the next flush updates the metadata in the log superblock. We believe these semantics to be useful even though they may not be completely identical to those of dm-log-writes. This functionality can be used for crash consistency and fs consistency testing in filesystem drivers, including on non-Linux guests and older guests running Linux versions without support for dm-log-writes. This is simple and useful. Admittedly this and more advanced things could perhaps be done by extending the quorum driver, but this approach would require re-engineering the functionality and involve a more complicated setup, so we offer this simple solution which we have found useful internally. In the first patch of this series, two block permission constants are moved from block.c to include/block/block.h to make them available outside of block.c. The next patch uses these constants. The driver requires all requests to be aligned to the sector size. In the second patch of this series, which includes the bulk of the blklogwrites driver, this sector size is assumed to be equal to BDRV_SECTOR_SIZE for simplicity. In patches 3-8, a mechanism to pass the block configuration of a block backend to a block driver is introduced, and this mechanism is integrated in various hw/block drivers. In patch 9, blklogwrites is changed to use the block configuration of the relevant block backend to set its block limits. Finally, in patch 10, blklogwrites is improved to use the logical sector size of the block backend for logging. This means that the sector size in the log metadata will be set to the logical sector size, and offsets and sizes of writes will be expressed as a multiple of that instead of BDRV_SECTOR_SIZE. v4: - Check return value of snprintf() (flagged by patchew) - Don't use C99 for loop initial declaration (flagged by patchew, but wasn't QEMU supposed to be C99? Oh well.) - Add proper "Since" fields for blklogwrites in block-core.json - Rebase on current upstream master v3: - Rebase on current upstream master v2: - Incorporate review feedback - Add patches to apply block configurations in more block device drivers Aapo Vienamo (1): block: Add blklogwrites Ari Sundholm (9): block: Move two block permission constants to the relevant enum block: Add a mechanism for passing a block driver a block configuration hw/scsi/scsi-disk: Always apply block configuration to block driver hw/ide/qdev: Always apply block configuration to block driver hw/block/virtio-blk: Always apply block configuration to block driver hw/block/nvme: Always apply block configuration to block driver hw/block/fdc: Always apply block configuration to block driver block/blklogwrites: Use block limits from the backend block configuration block/blklogwrites: Use the block device logical sector size when logging writes MAINTAINERS | 6 + block.c | 6 - block/Makefile.objs | 1 + block/blklogwrites.c | 426 ++ block/io.c| 22 +++ hw/block/block.c | 12 +- hw/block/fdc.c| 4 + hw/block/nvme.c | 1 + hw/block/virtio-blk.c | 1 + hw/ide/qdev.c | 2 + hw/scsi/scsi-disk.c | 1 + include/block/block.h | 17 ++ include/block/block_int.h | 9 + include/hw/block/block.h | 1 + qapi/block-core.json | 30 +++- 15 files changed, 526 insertions(+), 13 deletions(-) create mode 100644 block/blklogwrites.c -- 2.7.4
[Qemu-devel] [PATCH v6 37/49] tests/tcg: enable building for mips64
As before, using Debian SID compilers. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- tests/docker/Makefile.include | 1 + tests/docker/dockerfiles/debian-mips64-cross.docker | 12 tests/tcg/mips/Makefile.include | 3 +++ 3 files changed, 16 insertions(+) create mode 100644 tests/docker/dockerfiles/debian-mips64-cross.docker diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 2080a528a3..654c769f9a 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -74,6 +74,7 @@ docker-image-debian-hppa-cross: docker-image-debian-sid docker-image-debian-m68k-cross: docker-image-debian-sid docker-image-debian-sh4-cross: docker-image-debian-sid docker-image-debian-sparc64-cross: docker-image-debian-sid +docker-image-debian-mips64-cross: docker-image-debian-sid docker-image-travis: NOUSER=1 # Specialist build images, sometimes very limited tools diff --git a/tests/docker/dockerfiles/debian-mips64-cross.docker b/tests/docker/dockerfiles/debian-mips64-cross.docker new file mode 100644 index 00..ed1ce0e919 --- /dev/null +++ b/tests/docker/dockerfiles/debian-mips64-cross.docker @@ -0,0 +1,12 @@ +# +# Docker cross-compiler target +# +# This docker target builds on the debian sid base image which +# contains cross compilers for Debian "ports" targets. +# +FROM qemu:debian-sid + +RUN DEBIAN_FRONTEND=noninteractive eatmydata \ +apt-get install -y --no-install-recommends \ +gcc-mips64-linux-gnuabi64 \ +libc6-dev-mips64-cross diff --git a/tests/tcg/mips/Makefile.include b/tests/tcg/mips/Makefile.include index a9beceb623..4a14fc078d 100644 --- a/tests/tcg/mips/Makefile.include +++ b/tests/tcg/mips/Makefile.include @@ -8,6 +8,9 @@ ifeq ($(TARGET_NAME),mips64el) DOCKER_IMAGE=debian-mips64el-cross DOCKER_CROSS_COMPILER=mips64el-linux-gnuabi64-gcc +else ifeq ($(TARGET_NAME),mips64) +DOCKER_IMAGE=debian-mips64-cross +DOCKER_CROSS_COMPILER=mips64-linux-gnuabi64-gcc else ifeq ($(TARGET_NAME),mipsel) DOCKER_IMAGE=debian-mipsel-cross DOCKER_CROSS_COMPILER=mipsel-linux-gnu-gcc -- 2.17.1
[Qemu-devel] [PATCH v6 08/49] docker: Makefile.include introduce DOCKER_SCRIPT
Define this in one place to make it easy to re-use. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé --- tests/docker/Makefile.include | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 74fd51c22c..8afb383478 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -15,6 +15,8 @@ DOCKER_TESTS := $(notdir $(shell \ DOCKER_TOOLS := travis +DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py + TESTS ?= % IMAGES ?= % @@ -38,7 +40,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker echo WARNING: EXECUTABLE is not set, debootstrap may fail. 2>&1 ; \ fi $(call quiet-command,\ - $(SRC_PATH)/tests/docker/docker.py build qemu:$* $< \ + $(DOCKER_SCRIPT) build qemu:$* $< \ $(if $V,,--quiet) $(if $(NOCACHE),--no-cache) \ $(if $(NOUSER),,--add-current-user) \ $(if $(EXTRA_FILES),--extra-files $(EXTRA_FILES))\ @@ -133,11 +135,11 @@ docker-run: docker-qemu-src fi $(if $(EXECUTABLE), \ $(call quiet-command, \ - $(SRC_PATH)/tests/docker/docker.py update \ + $(DOCKER_SCRIPT) update \ $(IMAGE) $(EXECUTABLE), \ " COPYING $(EXECUTABLE) to $(IMAGE)")) $(call quiet-command, \ - $(SRC_PATH)/tests/docker/docker.py run \ + $(DOCKER_SCRIPT) run\ $(if $(NOUSER),,-u $(shell id -u)) \ --security-opt seccomp=unconfined \ $(if $V,,--rm) \ @@ -167,4 +169,4 @@ docker-run-%: @$(MAKE) docker-run TEST=$(CMD) IMAGE=qemu:$(IMAGE) docker-clean: - $(call quiet-command, $(SRC_PATH)/tests/docker/docker.py clean) + $(call quiet-command, $(DOCKER_SCRIPT) clean) -- 2.17.1
[Qemu-devel] [PATCH v6 13/49] tests/tcg: move i386 specific tests into subdir
These only need to be built for i386 guests. This includes a stub
tests/tcg/i386/Makfile.target which absorbs some of what was in
tests/tcg/Makefile.
Signed-off-by: Alex Bennée
Reviewed-by: Thomas Huth
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
---
v2
- move VPATH and TESTs setup into i386/Makefile.target
- set CFLAGS+=-m32 for cross building
v4
- update MAINATINERS
---
MAINTAINERS | 1 +
tests/tcg/README| 39
tests/tcg/i386/Makefile.target | 30 ++
tests/tcg/i386/README | 38 +++
tests/tcg/{ => i386}/hello-i386.c | 0
tests/tcg/{ => i386}/pi_10.com | Bin
tests/tcg/{ => i386}/runcom.c | 0
tests/tcg/{ => i386}/test-i386-code16.S | 0
tests/tcg/{ => i386}/test-i386-fprem.c | 0
tests/tcg/{ => i386}/test-i386-muldiv.h | 0
tests/tcg/{ => i386}/test-i386-shift.h | 0
tests/tcg/{ => i386}/test-i386-ssse3.c | 0
tests/tcg/{ => i386}/test-i386-vm86.S | 0
tests/tcg/{ => i386}/test-i386.c| 0
tests/tcg/{ => i386}/test-i386.h| 0
15 files changed, 69 insertions(+), 39 deletions(-)
create mode 100644 tests/tcg/i386/Makefile.target
create mode 100644 tests/tcg/i386/README
rename tests/tcg/{ => i386}/hello-i386.c (100%)
rename tests/tcg/{ => i386}/pi_10.com (100%)
rename tests/tcg/{ => i386}/runcom.c (100%)
rename tests/tcg/{ => i386}/test-i386-code16.S (100%)
rename tests/tcg/{ => i386}/test-i386-fprem.c (100%)
rename tests/tcg/{ => i386}/test-i386-muldiv.h (100%)
rename tests/tcg/{ => i386}/test-i386-shift.h (100%)
rename tests/tcg/{ => i386}/test-i386-ssse3.c (100%)
rename tests/tcg/{ => i386}/test-i386-vm86.S (100%)
rename tests/tcg/{ => i386}/test-i386.c (100%)
rename tests/tcg/{ => i386}/test-i386.h (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index b8f6c90122..e795b8186e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -283,6 +283,7 @@ M: Richard Henderson
M: Eduardo Habkost
S: Maintained
F: target/i386/
+F: tests/tcg/i386/
F: hw/i386/
F: disas/i386.c
T: git git://github.com/ehabkost/qemu.git x86-next
diff --git a/tests/tcg/README b/tests/tcg/README
index 0890044cf0..469504c4cb 100644
--- a/tests/tcg/README
+++ b/tests/tcg/README
@@ -3,45 +3,6 @@ regression testing. Tests are either multi-arch, meaning they
can be
built for all guest architectures that support linux-user executable,
or they are architecture specific.
-i386
-
-
-test-i386
--
-
-This program executes most of the 16 bit and 32 bit x86 instructions and
-generates a text output, for comparison with the output obtained with
-a real CPU or another emulator.
-
-The Linux system call modify_ldt() is used to create x86 selectors
-to test some 16 bit addressing and 32 bit with segmentation cases.
-
-The Linux system call vm86() is used to test vm86 emulation.
-
-Various exceptions are raised to test most of the x86 user space
-exception reporting.
-
-linux-test
---
-
-This program tests various Linux system calls. It is used to verify
-that the system call parameters are correctly converted between target
-and host CPUs.
-
-test-i386-fprem
-
-runcom
---
-
-test-mmap
--
-
-sha1
-
-
-hello-i386
---
ARM
diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target
new file mode 100644
index 00..2f27b65e2d
--- /dev/null
+++ b/tests/tcg/i386/Makefile.target
@@ -0,0 +1,30 @@
+# i386 cross compile notes
+
+I386_SRC=$(SRC_PATH)/tests/tcg/i386
+
+# Set search path for all sources
+VPATH += $(I386_SRC)
+
+I386_SRCS=$(notdir $(wildcard $(I386_SRC)/*.c))
+I386_TESTS=$(I386_SRCS:.c=)
+
+# Update TESTS
+TESTS+=$(I386_TESTS)
+
+ifneq ($(TARGET_NAME),x86_64)
+CFLAGS+=-m32
+endif
+
+#
+# hello-i386 is a barebones app
+#
+hello-i386: CFLAGS+=-ffreestanding
+hello-i386: LDFLAGS+=-nostdlib
+
+#
+# test-386 includes a couple of additional objects that need to be linked
together
+#
+
+test-i386: test-i386.c test-i386-code16.S test-i386-vm86.S test-i386.h
test-i386-shift.h test-i386-muldiv.h
+ $(CC) $(CFLAGS) $(LDFLAGS) -o $@ \
+ $(
[Qemu-devel] [PATCH v6 40/49] tests/tcg: enable building for PowerPC
Now we have restored debian-image-powerpc-cross using Debian SID compilers we can build for 32 bit powerpc. Although PPC32 supports a range of pages sizes currently only 4k works so the others are commented out for now. We can also merge the ppc64 support under the base architecture directory to avoid too much proliferation of directories. Signed-off-by: Alex Bennée --- v5 - new for v5 --- tests/tcg/ppc/Makefile.include | 7 +++ tests/tcg/ppc/Makefile.target | 12 tests/tcg/ppc64le/Makefile.include | 2 -- 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 tests/tcg/ppc/Makefile.include create mode 100644 tests/tcg/ppc/Makefile.target delete mode 100644 tests/tcg/ppc64le/Makefile.include diff --git a/tests/tcg/ppc/Makefile.include b/tests/tcg/ppc/Makefile.include new file mode 100644 index 00..b062c30dd3 --- /dev/null +++ b/tests/tcg/ppc/Makefile.include @@ -0,0 +1,7 @@ +ifeq ($(TARGET_NAME),ppc) +DOCKER_IMAGE=debian-powerpc-cross +DOCKER_CROSS_COMPILER=powerpc-linux-gnu-gcc +else ifeq ($(TARGET_NAME),ppc64le) +DOCKER_IMAGE=debian-ppc64el-cross +DOCKER_CROSS_COMPILER=powerpc64le-linux-gnu-gcc +endif diff --git a/tests/tcg/ppc/Makefile.target b/tests/tcg/ppc/Makefile.target new file mode 100644 index 00..f5e08c7376 --- /dev/null +++ b/tests/tcg/ppc/Makefile.target @@ -0,0 +1,12 @@ +# -*- Mode: makefile -*- +# +# PPC - included from tests/tcg/Makefile +# + +ifneq (,$(findstring 64,$(TARGET_NAME))) +# On PPC64 Linux can be configured with 4k (default) or 64k pages (currently broken) +EXTRA_RUNS+=run-test-mmap-4096 #run-test-mmap-65536 +else +# On PPC32 Linux supports 4K/16K/64K/256K (but currently only 4k works) +EXTRA_RUNS+=run-test-mmap-4096 #run-test-mmap-16384 run-test-mmap-65536 run-test-mmap-262144 +endif diff --git a/tests/tcg/ppc64le/Makefile.include b/tests/tcg/ppc64le/Makefile.include deleted file mode 100644 index d71cfc9aa7..00 --- a/tests/tcg/ppc64le/Makefile.include +++ /dev/null @@ -1,2 +0,0 @@ -DOCKER_IMAGE=debian-ppc64el-cross -DOCKER_CROSS_COMPILER=powerpc64le-linux-gnu-gcc -- 2.17.1
[Qemu-devel] [PATCH v6 20/49] tests/tcg/i386/test-i386: fix printf format
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
---
tests/tcg/i386/test-i386.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/tcg/i386/test-i386.c b/tests/tcg/i386/test-i386.c
index caef4da176..a29b41e764 100644
--- a/tests/tcg/i386/test-i386.c
+++ b/tests/tcg/i386/test-i386.c
@@ -2258,7 +2258,7 @@ SSE_OP(a ## sd);
"pop %0\n"\
: "=rm" (eflags)\
: "x" (a.dq), "x" (b.dq));\
-printf("%-9s: a=%f b=%f cc=%04x\n",\
+printf("%-9s: a=%f b=%f cc=%04lx\n",\
#op, a1, b1,\
eflags & (CC_C | CC_P | CC_Z | CC_S | CC_O | CC_A));\
}
--
2.17.1
[Qemu-devel] [PULL 09/31] ftgmac100: compute maximum frame size depending on the protocol
From: Cédric Le Goater
The maximum frame size includes the CRC and depends if a VLAN tag is
inserted or not. Adjust the frame size limit in the transmit handler
using on the FTGMAC100State buffer size and in the receive handler use
the packet protocol.
Signed-off-by: Cédric Le Goater
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20180530061711.23673-2-...@kaod.org
Signed-off-by: Peter Maydell
---
include/hw/net/ftgmac100.h | 7 ++-
hw/net/ftgmac100.c | 23 ---
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h
index d9bc589fbf7..94cfe053329 100644
--- a/include/hw/net/ftgmac100.h
+++ b/include/hw/net/ftgmac100.h
@@ -16,6 +16,11 @@
#include "hw/sysbus.h"
#include "net/net.h"
+/*
+ * Max frame size for the receiving buffer
+ */
+#define FTGMAC100_MAX_FRAME_SIZE9220
+
typedef struct FTGMAC100State {
/*< private >*/
SysBusDevice parent_obj;
@@ -26,7 +31,7 @@ typedef struct FTGMAC100State {
qemu_irq irq;
MemoryRegion iomem;
-uint8_t *frame;
+uint8_t frame[FTGMAC100_MAX_FRAME_SIZE];
uint32_t irq_state;
uint32_t isr;
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 3300e8ef4a8..425ac36cff8 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -207,16 +207,18 @@ typedef struct {
/*
* Max frame size for the receiving buffer
*/
-#define FTGMAC100_MAX_FRAME_SIZE10240
+#define FTGMAC100_MAX_FRAME_SIZE9220
/* Limits depending on the type of the frame
*
* 9216 for Jumbo frames (+ 4 for VLAN)
* 1518 for other frames (+ 4 for VLAN)
*/
-static int ftgmac100_max_frame_size(FTGMAC100State *s)
+static int ftgmac100_max_frame_size(FTGMAC100State *s, uint16_t proto)
{
-return (s->maccr & FTGMAC100_MACCR_JUMBO_LF ? 9216 : 1518) + 4;
+int max = (s->maccr & FTGMAC100_MACCR_JUMBO_LF ? 9216 : 1518);
+
+return max + (proto == ETH_P_VLAN ? 4 : 0);
}
static void ftgmac100_update_irq(FTGMAC100State *s)
@@ -408,7 +410,6 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t
tx_ring,
uint8_t *ptr = s->frame;
uint32_t addr = tx_descriptor;
uint32_t flags = 0;
-int max_frame_size = ftgmac100_max_frame_size(s);
while (1) {
FTGMAC100Desc bd;
@@ -427,11 +428,12 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t
tx_ring,
flags = bd.des1;
}
-len = bd.des0 & 0x3FFF;
-if (frame_size + len > max_frame_size) {
+len = FTGMAC100_TXDES0_TXBUF_SIZE(bd.des0);
+if (frame_size + len > sizeof(s->frame)) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %d bytes\n",
__func__, len);
-len = max_frame_size - frame_size;
+s->isr |= FTGMAC100_INT_XPKT_LOST;
+len = sizeof(s->frame) - frame_size;
}
if (dma_memory_read(_space_memory, bd.des3, ptr, len)) {
@@ -788,7 +790,8 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const
uint8_t *buf,
uint32_t buf_len;
size_t size = len;
uint32_t first = FTGMAC100_RXDES0_FRS;
-int max_frame_size = ftgmac100_max_frame_size(s);
+uint16_t proto = be16_to_cpu(PKT_GET_ETH_HDR(buf)->h_proto);
+int max_frame_size = ftgmac100_max_frame_size(s, proto);
if ((s->maccr & (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN))
!= (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN)) {
@@ -820,9 +823,9 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const
uint8_t *buf,
/* Huge frames are truncated. */
if (size > max_frame_size) {
-size = max_frame_size;
qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %zd bytes\n",
__func__, size);
+size = max_frame_size;
flags |= FTGMAC100_RXDES0_FTL;
}
@@ -940,8 +943,6 @@ static void ftgmac100_realize(DeviceState *dev, Error
**errp)
object_get_typename(OBJECT(dev)), DEVICE(dev)->id,
s);
qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
-
-s->frame = g_malloc(FTGMAC100_MAX_FRAME_SIZE);
}
static const VMStateDescription vmstate_ftgmac100 = {
--
2.17.1
[Qemu-devel] [PULL 15/31] hw/sd/milkymist-memcard: Add trailing '\n' to qemu_log() call
From: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20180606152128.449-2-f4...@amsat.org
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/sd/milkymist-memcard.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index fe1a761..fcbccf54eaf 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -140,7 +140,7 @@ static uint64_t memcard_read(void *opaque, hwaddr addr,
r = s->response[s->response_read_ptr++];
if (s->response_read_ptr > s->response_len) {
qemu_log_mask(LOG_GUEST_ERROR, "milkymist_memcard: "
- "read more cmd bytes than available. Clipping.");
+ "read more cmd bytes than available:
clipping\n");
s->response_read_ptr = 0;
}
}
--
2.17.1
[Qemu-devel] [PATCH v6 41/49] tests/tcg/Makefile: update to be called from Makefile.target
This make is now invoked from each individual target make with the appropriate CC and EXTRA_CFLAGS set for each guest. It then includes additional Makefile.targets from: - tests/tcg/multiarch (always) - tests/tcg/$(TARGET_BASE_ARCH) (if available) - tests/tcg/$(TARGET_NAME) The order is important as the later Makefile's may want to suppress TESTS from its base arch profile. Each included Makefile.target is responsible for adding TESTS as well as defining any special build instructions for individual tests. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- v2 - cleaner approach to include sub makefiles - move TESTS/VPATH manipulation into sub-makefile - avoid double inclusion when TARGET_BASE_ARCH==TARGET_NAME v4 - add timeout to default runner - clean-up comments about build flags - update to handle BUILD_STATIC - add MAINTAINERS entry v5 - support EXTRA_RUNS for run variants v6 - extend timeout for DEBUG_TCG --- MAINTAINERS| 5 ++ tests/tcg/Makefile | 183 +++-- 2 files changed, 81 insertions(+), 107 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index b8fbef495b..4761f8cab4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2144,6 +2144,11 @@ W: https://travis-ci.org/qemu/qemu W: https://app.shippable.com/github/qemu/qemu W: http://patchew.org/QEMU/ +Guest Test Compilation Support +M: Alex Bennée +F: tests/tcg/Makefile +L: qemu-devel@nongnu.org + Documentation - Build system architecture diff --git a/tests/tcg/Makefile b/tests/tcg/Makefile index e12395117a..e7dbcdb5bf 100644 --- a/tests/tcg/Makefile +++ b/tests/tcg/Makefile @@ -1,125 +1,94 @@ +# -*- Mode: makefile -*- +# +# TCG tests +# +# These are complicated by the fact we want to build them for guest +# systems. This requires knowing what guests we are building and which +# ones we have cross-compilers for or docker images with +# cross-compilers. +# +# The tests themselves should be as minimal as possible as +# cross-compilers don't always have a large amount of libraries +# available. +# +# We only include the host build system for SRC_PATH and we don't +# bother with the common rules.mk. We expect the following: +# +# CC - the C compiler command +# EXTRA_CFLAGS - any extra CFLAGS +# BUILD_STATIC - are we building static binaries +# +# By default all tests are statically compiled but some host systems +# may not package static libraries by default. If an external +# cross-compiler can only build dynamic libraries the user might need +# to make extra efforts to ensure ld.so can link at runtime when the +# tests are run. +# +# We also accept SPEED=slow to enable slower running tests +# +# We also expect to be in the tests build dir for the FOO-linux-user. +# + -include ../../config-host.mak --include $(SRC_PATH)/rules.mak +-include ../config-target.mak -$(call set-vpath, $(SRC_PATH)/tests/tcg) +quiet-command = $(if $(V),$1,$(if $(2),@printf " %-7s %s\n" $2 $3 && $1, @$1)) -QEMU=../../i386-linux-user/qemu-i386 -QEMU_X86_64=../../x86_64-linux-user/qemu-x86_64 -CC_X86_64=$(CC_I386) -m64 +# Tests we are building +TESTS= -QEMU_INCLUDES += -I../.. -CFLAGS=-Wall -O2 -g -fno-strict-aliasing -#CFLAGS+=-msse2 +# Start with a blank slate, the build targets get to add stuff first +CFLAGS= +QEMU_CFLAGS= LDFLAGS= -# TODO: automatically detect ARM and MIPS compilers, and run those too - -# runcom maps page 0, so it requires root privileges -# also, pi_10.com runs indefinitely - -I386_TESTS=hello-i386 \ - sha1-i386 \ - test-i386 \ - test-i386-fprem \ - # runcom +# The QEMU for this TARGET +QEMU=../qemu-$(TARGET_NAME) -# native i386 compilers sometimes are not biarch. assume cross-compilers are -ifneq ($(ARCH),i386) -I386_TESTS+=run-test-x86_64 +# If TCG debugging is enabled things are a lot slower +ifeq ($(CONFIG_DEBUG_TCG),y) +TIMEOUT=45 +else +TIMEOUT=15 endif -TESTS = test_path -ifneq ($(call find-in-path, $(CC_I386)),) -TESTS += $(I386_TESTS) +# The order we include is important. We include multiarch, base arch +# and finally arch if it's not the same as base arch. +-include $(SRC_PATH)/tests/tcg/multiarch/Makefile.target +-include $(SRC_PATH)/tests/tcg/$(TARGET_BASE_ARCH)/Makefile.target +ifneq ($(TARGET_BASE_ARCH),$(TARGET_NAME)) +-include $(SRC_PATH)/tests/tcg/$(TARGET_NAME)/Makefile.target endif -all: $(patsubst %,run-%,$(TESTS)) -test: all - -# rules to run tests - -.PHONY: $(patsubst %,run-%,$(TESTS)) - -run-%: % - -$(QEMU) ./$* - -run-hello-i386: hello-i386 -run-sha1-i386: sha1-i386 - -run-test-i386: test-i386 - ./test-i386 > test-i386.ref - -$(QEMU) test-i386 > test-i386.out - @if diff -u test-i386.ref test-i386.out ; then echo "Auto Test OK"; fi - -run-test-i386-fprem: test-i386-fprem - ./test-i386-fprem > test-i386-fprem.ref - -$(QEMU) test-i386-fprem > test-i386-fprem.out
[Qemu-devel] [PULL 18/31] ppc/pnv: Add trailing '\n' to qemu_log() calls
From: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
Acked-by: David Gibson
Message-id: 20180606152128.449-5-f4...@amsat.org
Signed-off-by: Peter Maydell
---
hw/ppc/pnv_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index cbb64ad9e7e..13ad7d9e047 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -97,7 +97,7 @@ static uint64_t pnv_core_xscom_read(void *opaque, hwaddr addr,
val = 0x24full;
break;
default:
-qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx,
+qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx "\n",
addr);
}
@@ -107,7 +107,7 @@ static uint64_t pnv_core_xscom_read(void *opaque, hwaddr
addr,
static void pnv_core_xscom_write(void *opaque, hwaddr addr, uint64_t val,
unsigned int width)
{
-qemu_log_mask(LOG_UNIMP, "Warning: writing to reg=0x%" HWADDR_PRIx,
+qemu_log_mask(LOG_UNIMP, "Warning: writing to reg=0x%" HWADDR_PRIx "\n",
addr);
}
--
2.17.1
[Qemu-devel] [PATCH v2 1/6] spapr: no need to verify the node
The node property can always be queried and the value has already been
verified in pc_dimm_realize().
Acked-by: David Gibson
Reviewed-by: Greg Kurz
Signed-off-by: David Hildenbrand
---
hw/ppc/spapr.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2375cbee12..f16a0b2870 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3578,14 +3578,8 @@ static void spapr_machine_device_plug(HotplugHandler
*hotplug_dev,
error_setg(errp, "Memory hotplug not supported for this machine");
return;
}
-node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP, errp);
-if (*errp) {
-return;
-}
-if (node < 0 || node >= MAX_NODES) {
-error_setg(errp, "Invaild node %d", node);
-return;
-}
+node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
+_abort);
spapr_memory_plug(hotplug_dev, dev, node, errp);
} else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
--
2.17.0
[Qemu-devel] [PATCH v2 2/6] spapr: move lookup of the node into spapr_memory_plug()
Let's clean the hotplug handler up by moving lookup of the node into
the function where it is actually being used.
Signed-off-by: David Hildenbrand
---
hw/ppc/spapr.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f16a0b2870..1f577b274b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3136,7 +3136,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t
addr_start, uint64_t size,
}
static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
- uint32_t node, Error **errp)
+ Error **errp)
{
Error *local_err = NULL;
sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
@@ -3144,6 +3144,7 @@ static void spapr_memory_plug(HotplugHandler
*hotplug_dev, DeviceState *dev,
PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
MemoryRegion *mr;
uint64_t align, size, addr;
+uint32_t node;
mr = ddc->get_memory_region(dimm, _err);
if (local_err) {
@@ -3163,6 +3164,8 @@ static void spapr_memory_plug(HotplugHandler
*hotplug_dev, DeviceState *dev,
goto out_unplug;
}
+node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
+_abort);
spapr_add_lmbs(dev, addr, size, node,
spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT),
_err);
@@ -3572,16 +3575,11 @@ static void spapr_machine_device_plug(HotplugHandler
*hotplug_dev,
sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(ms);
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
-int node;
-
if (!smc->dr_lmb_enabled) {
error_setg(errp, "Memory hotplug not supported for this machine");
return;
}
-node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
-_abort);
-
-spapr_memory_plug(hotplug_dev, dev, node, errp);
+spapr_memory_plug(hotplug_dev, dev, errp);
} else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
spapr_core_plug(hotplug_dev, dev, errp);
}
--
2.17.0
[Qemu-devel] [PULL 01/31] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR
From: Shannon Zhao
While we skip the GIC_INTERNAL irqs, we don't change the register offset
accordingly. This will overlap the GICR registers value and leave the
last GIC_INTERNAL irq's registers out of update.
Fix this by skipping the registers banked by GICR.
Also for migration compatibility if the migration source (old version
qemu) doesn't send gicd_no_migration_shift_bug = 1 to destination, then
we shift the data of PPI to get the right data for SPI.
Fixes: 367b9f527becdd20ddf116e17a3c0c2bbc486920
Cc: qemu-sta...@nongnu.org
Reviewed-by: Eric Auger
Reviewed-by: Peter Maydell
Signed-off-by: Shannon Zhao
Message-id: 1527816987-16108-1-git-send-email-zhaoshengl...@huawei.com
Signed-off-by: Peter Maydell
---
include/hw/intc/arm_gicv3_common.h | 1 +
hw/intc/arm_gicv3_common.c | 79 ++
hw/intc/arm_gicv3_kvm.c| 38 ++
3 files changed, 118 insertions(+)
diff --git a/include/hw/intc/arm_gicv3_common.h
b/include/hw/intc/arm_gicv3_common.h
index bccdfe17c66..d75b49d5581 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -217,6 +217,7 @@ struct GICv3State {
uint32_t revision;
bool security_extn;
bool irq_reset_nonsecure;
+bool gicd_no_migration_shift_bug;
int dev_fd; /* kvm device fd if backed by kvm vgic support */
Error *migration_blocker;
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 7b54d523762..864b7c6515f 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -27,6 +27,7 @@
#include "hw/intc/arm_gicv3_common.h"
#include "gicv3_internal.h"
#include "hw/arm/linux-boot-if.h"
+#include "sysemu/kvm.h"
static int gicv3_pre_save(void *opaque)
{
@@ -141,6 +142,79 @@ static const VMStateDescription vmstate_gicv3_cpu = {
}
};
+static int gicv3_gicd_no_migration_shift_bug_pre_load(void *opaque)
+{
+GICv3State *cs = opaque;
+
+ /*
+* The gicd_no_migration_shift_bug flag is used for migration compatibility
+* for old version QEMU which may have the GICD bmp shift bug under KVM
mode.
+* Strictly, what we want to know is whether the migration source is using
+* KVM. Since we don't have any way to determine that, we look at whether
the
+* destination is using KVM; this is close enough because for the older QEMU
+* versions with this bug KVM -> TCG migration didn't work anyway. If the
+* source is a newer QEMU without this bug it will transmit the migration
+* subsection which sets the flag to true; otherwise it will remain set to
+* the value we select here.
+*/
+if (kvm_enabled()) {
+cs->gicd_no_migration_shift_bug = false;
+}
+
+return 0;
+}
+
+static int gicv3_gicd_no_migration_shift_bug_post_load(void *opaque,
+ int version_id)
+{
+GICv3State *cs = opaque;
+
+if (cs->gicd_no_migration_shift_bug) {
+return 0;
+}
+
+/* Older versions of QEMU had a bug in the handling of state save/restore
+ * to the KVM GICv3: they got the offset in the bitmap arrays wrong,
+ * so that instead of the data for external interrupts 32 and up
+ * starting at bit position 32 in the bitmap, it started at bit
+ * position 64. If we're receiving data from a QEMU with that bug,
+ * we must move the data down into the right place.
+ */
+memmove(cs->group, (uint8_t *)cs->group + GIC_INTERNAL / 8,
+sizeof(cs->group) - GIC_INTERNAL / 8);
+memmove(cs->grpmod, (uint8_t *)cs->grpmod + GIC_INTERNAL / 8,
+sizeof(cs->grpmod) - GIC_INTERNAL / 8);
+memmove(cs->enabled, (uint8_t *)cs->enabled + GIC_INTERNAL / 8,
+sizeof(cs->enabled) - GIC_INTERNAL / 8);
+memmove(cs->pending, (uint8_t *)cs->pending + GIC_INTERNAL / 8,
+sizeof(cs->pending) - GIC_INTERNAL / 8);
+memmove(cs->active, (uint8_t *)cs->active + GIC_INTERNAL / 8,
+sizeof(cs->active) - GIC_INTERNAL / 8);
+memmove(cs->edge_trigger, (uint8_t *)cs->edge_trigger + GIC_INTERNAL / 8,
+sizeof(cs->edge_trigger) - GIC_INTERNAL / 8);
+
+/*
+ * While this new version QEMU doesn't have this kind of bug as we fix it,
+ * so it needs to set the flag to true to indicate that and it's necessary
+ * for next migration to work from this new version QEMU.
+ */
+cs->gicd_no_migration_shift_bug = true;
+
+return 0;
+}
+
+const VMStateDescription vmstate_gicv3_gicd_no_migration_shift_bug = {
+.name = "arm_gicv3/gicd_no_migration_shift_bug",
+.version_id = 1,
+.minimum_version_id = 1,
+.pre_load = gicv3_gicd_no_migration_shift_bug_pre_load,
+.post_load = gicv3_gicd_no_migration_shift_bug_post_load,
+.fields = (VMStateField[]) {
+VMSTATE_BOOL(gicd_no_migration_shift_bug, GICv3State),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const VMStateDescription vmstate_gicv3 = {
.name =
[Qemu-devel] [PULL 21/31] stellaris: Add trailing '\n' to qemu_log() calls
From: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20180606152128.449-8-f4...@amsat.org
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/arm/stellaris.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index e886f54976a..502a20842c0 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -203,11 +203,11 @@ static uint64_t gptm_read(void *opaque, hwaddr offset,
return s->rtc;
}
qemu_log_mask(LOG_UNIMP,
- "GPTM: read of TAR but timer read not supported");
+ "GPTM: read of TAR but timer read not supported\n");
return 0;
case 0x4c: /* TBR */
qemu_log_mask(LOG_UNIMP,
- "GPTM: read of TBR but timer read not supported");
+ "GPTM: read of TBR but timer read not supported\n");
return 0;
default:
qemu_log_mask(LOG_GUEST_ERROR,
@@ -836,11 +836,12 @@ static void stellaris_i2c_write(void *opaque, hwaddr
offset,
break;
case 0x20: /* MCR */
if (value & 1) {
-qemu_log_mask(LOG_UNIMP, "stellaris_i2c: Loopback not
implemented");
+qemu_log_mask(LOG_UNIMP,
+ "stellaris_i2c: Loopback not implemented\n");
}
if (value & 0x20) {
qemu_log_mask(LOG_UNIMP,
- "stellaris_i2c: Slave mode not implemented");
+ "stellaris_i2c: Slave mode not implemented\n");
}
s->mcr = value & 0x31;
break;
@@ -1124,7 +1125,7 @@ static void stellaris_adc_write(void *opaque, hwaddr
offset,
s->sspri = value;
break;
case 0x28: /* PSSI */
-qemu_log_mask(LOG_UNIMP, "ADC: sample initiate unimplemented");
+qemu_log_mask(LOG_UNIMP, "ADC: sample initiate unimplemented\n");
break;
case 0x30: /* SAC */
s->sac = value;
--
2.17.1
[Qemu-devel] [PATCH v2 5/6] spapr: handle pc-dimm unplug via hotplug handler chain
Factor out memory unplug into separate function from spapr_lmb_release().
Then use generic hotplug_handler_unplug() to trigger memory unplug,
which will call spapr_machine_device_unplug() -> spapr_memory_unplug()
in the end.
This way unplug operation is not buried in lmb internals and located in
the same place like in other targets, following similar logic/call chain
across targets.
Acked-by: David Gibson
Reviewed-by: Greg Kurz
Reviewed-by: Igor Mammedov
Signed-off-by: David Hildenbrand
---
hw/ppc/spapr.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c45f8bc75b..404d887f4e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3299,7 +3299,8 @@ static sPAPRDIMMState
*spapr_recover_pending_dimm_state(sPAPRMachineState *ms,
/* Callback to be called during DRC release. */
void spapr_lmb_release(DeviceState *dev)
{
-sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_hotplug_handler(dev));
+HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(dev);
+sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_ctrl);
sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr, PC_DIMM(dev));
/* This information will get lost if a migration occurs
@@ -3317,9 +3318,17 @@ void spapr_lmb_release(DeviceState *dev)
/*
* Now that all the LMBs have been removed by the guest, call the
- * pc-dimm unplug handler to cleanup up the pc-dimm device.
+ * unplug handler chain. This can never fail.
*/
-pc_dimm_memory_unplug(dev, MACHINE(spapr));
+hotplug_handler_unplug(hotplug_ctrl, dev, _abort);
+}
+
+static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
+{
+sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
+sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr, PC_DIMM(dev));
+
+pc_dimm_memory_unplug(dev, MACHINE(hotplug_dev));
object_unparent(OBJECT(dev));
spapr_pending_dimm_unplugs_remove(spapr, ds);
}
@@ -3587,6 +3596,9 @@ static void spapr_machine_device_plug(HotplugHandler
*hotplug_dev,
static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
+if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+spapr_memory_unplug(hotplug_dev, dev);
+}
}
static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
--
2.17.0
[Qemu-devel] [PATCH v6 36/49] tests/tcg: enable building for sparc64
As before, using Debian SID compilers. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- v5 - add Makefile with EXTRA_RUNS for mmap tests --- tests/docker/Makefile.include| 1 + tests/docker/dockerfiles/debian-sparc64-cross.docker | 12 tests/tcg/sparc64/Makefile.include | 2 ++ tests/tcg/sparc64/Makefile.target| 6 ++ 4 files changed, 21 insertions(+) create mode 100644 tests/docker/dockerfiles/debian-sparc64-cross.docker create mode 100644 tests/tcg/sparc64/Makefile.include create mode 100644 tests/tcg/sparc64/Makefile.target diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index bad3d28103..2080a528a3 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -73,6 +73,7 @@ docker-image-debian-alpha-cross: docker-image-debian-sid docker-image-debian-hppa-cross: docker-image-debian-sid docker-image-debian-m68k-cross: docker-image-debian-sid docker-image-debian-sh4-cross: docker-image-debian-sid +docker-image-debian-sparc64-cross: docker-image-debian-sid docker-image-travis: NOUSER=1 # Specialist build images, sometimes very limited tools diff --git a/tests/docker/dockerfiles/debian-sparc64-cross.docker b/tests/docker/dockerfiles/debian-sparc64-cross.docker new file mode 100644 index 00..1e2c809274 --- /dev/null +++ b/tests/docker/dockerfiles/debian-sparc64-cross.docker @@ -0,0 +1,12 @@ +# +# Docker cross-compiler target +# +# This docker target builds on the debian sid base image which +# contains cross compilers for Debian "ports" targets. +# +FROM qemu:debian-sid + +RUN DEBIAN_FRONTEND=noninteractive eatmydata \ +apt-get install -y --no-install-recommends \ +gcc-sparc64-linux-gnu \ +libc6-dev-sparc64-cross diff --git a/tests/tcg/sparc64/Makefile.include b/tests/tcg/sparc64/Makefile.include new file mode 100644 index 00..95fc8dee9f --- /dev/null +++ b/tests/tcg/sparc64/Makefile.include @@ -0,0 +1,2 @@ +DOCKER_IMAGE=debian-sparc64-cross +DOCKER_CROSS_COMPILER=sparc64-linux-gnu-gcc diff --git a/tests/tcg/sparc64/Makefile.target b/tests/tcg/sparc64/Makefile.target new file mode 100644 index 00..408dace783 --- /dev/null +++ b/tests/tcg/sparc64/Makefile.target @@ -0,0 +1,6 @@ +# -*- Mode: makefile -*- +# +# sparc specific tweaks + +# On Sparc64 Linux support 8k pages +EXTRA_RUNS+=run-test-mmap-8192 -- 2.17.1
[Qemu-devel] [PATCH v3 1/7] hmp: Add flag for preconfig commands
From: "Dr. David Alan Gilbert"
Add a flag to command definitions to allow them to be used in preconfig
and check it.
If users try to use commands that aren't available, tell them to use
the exit_preconfig comand we're adding in a few patches.
Signed-off-by: Dr. David Alan Gilbert
---
monitor.c | 20
1 file changed, 20 insertions(+)
diff --git a/monitor.c b/monitor.c
index 6d0cec552e..f4a16e6a03 100644
--- a/monitor.c
+++ b/monitor.c
@@ -128,6 +128,7 @@ typedef struct mon_cmd_t {
const char *args_type;
const char *params;
const char *help;
+const char *flags; /* p=preconfig */
void (*cmd)(Monitor *mon, const QDict *qdict);
/* @sub_table is a list of 2nd level of commands. If it does not exist,
* cmd should be used. If it exists, sub_table[?].cmd should be
@@ -936,6 +937,19 @@ static int parse_cmdline(const char *cmdline,
return -1;
}
+/*
+ * Returns true if the command can be executed in preconfig mode
+ * i.e. it has the 'p' flag.
+ */
+static bool cmd_can_preconfig(const mon_cmd_t *cmd)
+{
+if (!cmd->flags) {
+return false;
+}
+
+return strchr(cmd->flags, 'p');
+}
+
static void help_cmd_dump_one(Monitor *mon,
const mon_cmd_t *cmd,
char **prefix_args,
@@ -2976,6 +2990,12 @@ static const mon_cmd_t *monitor_parse_command(Monitor
*mon,
(int)(p - cmdp_start), cmdp_start);
return NULL;
}
+if (runstate_check(RUN_STATE_PRECONFIG) && !cmd_can_preconfig(cmd)) {
+monitor_printf(mon, "Command '%.*s' not available with -preconfig; "
+"use exit_preconfig after configuration.\n",
+ (int)(p - cmdp_start), cmdp_start);
+return NULL;
+}
/* filter out following useless space */
while (qemu_isspace(*p)) {
--
2.17.0
Re: [Qemu-devel] [PATCH v4 04/14] pc: prepare for multi stage hotplug handlers
On Fri, Jun 08, 2018 at 02:32:09PM +0200, David Hildenbrand wrote:
>
> >>> if (TYPE_PC_DIMM) {
> >>> pc_dimm_plug()
> >>> /* do here additional concrete machine specific things */
> >>> } else if (TYPE_VIRTIO_MEM) {
> >>> virtio_mem_plug() <- do forwarding in there
> >>> /* and do here additional concrete machine specific things */
> >>> } else if (TYPE_CPU) {
> >>> cpu_plug()
> >>> /* do here additional concrete machine specific things */
> >>> }
> >>
> >> That will result in a lot of duplicate code - for every machine we
> >> support (dimm/virtio-mem/virtio-pmem/*add more memory devices here*) -
> >> virtio-mem and virtio-pmem could most probably share the code.
> > maybe or maybe not, depending on if pmem endups as memory device or
> > separate controller. And even with duplication, machine code would
> > be easy to follow just down one explicit call chain.
>
> Not 100% convinced but I am now going into that direction.
Can this go into DeviceClass? Failover has the same need to
allocate/free resources for vfio without a full realize/unrealize.
> --
>
> Thanks,
>
> David / dhildenb
Re: [Qemu-devel] [PATCH v2 00/10] python: futurize --stage1 (Python 3 compatibility)
Hi, This series failed docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. Type: series Message-id: 20180608122952.2009-1-ehabk...@redhat.com Subject: [Qemu-devel] [PATCH v2 00/10] python: futurize --stage1 (Python 3 compatibility) === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc # Let docker tests dump environment info export SHOW_ENV=1 export J=8 time make docker-test-mingw@fedora === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 034a17b732 python: futurize -f lib2to3.fixes.fix_numliterals 24815c6337 python: futurize -f lib2to3.fixes.fix_except 84c2eb4a4a python: futurize -f lib2to3.fixes.fix_renames 8e21c5b4fd python: futurize -f lib2to3.fixes.fix_tuple_params 8314382c8c python: futurize -f lib2to3.fixes.fix_reduce db6bd54016 python: futurize -f lib2to3.fixes.fix_standarderror 26518cce56 python: futurize -f lib2to3.fixes.fix_has_key 710ff909f0 python: futurize -f libfuturize.fixes.fix_next_call 332ca44612 python: futurize -f libfuturize.fixes.fix_absolute_import ebdf9a3fe6 python: futurize -f libfuturize.fixes.fix_print_with_import === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-difi82ih/src/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' BUILD fedora File "./tests/docker/docker.py", line 403 print "yes" ^ SyntaxError: invalid syntax make: *** [tests/docker/Makefile.include:38: docker-image-fedora] Error 1 real0m0.560s user0m0.159s sys 0m0.175s === OUTPUT END === Test command exited with code: 2 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH RFC 2/2] vfio-ccw: support for halt/clear subchannel
On 06/08/2018 02:20 PM, Cornelia Huck wrote: My proposal is to do the same copying to scsw(r) again, which would mean we get a request with both the halt and the start bit set. The vfio code now needs to do a hsch (instead of a ssch). The real channel subsystem should figure this out, as we can't reliably check whether the start function has concluded already (there's always a race window). This I do not agree scsw(r) is part of the driver. The interface here is not a device interface anymore but a driver interface. SCSW is a status, it is at its place in QEMU device interface with the guest but here pwrite() sends a command. Hm, I rather consider that "we write a status, and the backend figures out what to do based on that status". The status of what? Kind of a target status? I think this approach is the source of lots of complications. For instance take xsch. How are we supposed to react to a guest xsch (in QEMU and in the kernel module)? My guess is that the right thing to do is to issue an xsch in the vfio-ccw kernel module on the passed through subchannel. But there is no bit in fctl for cancel. Bottom line is: I'm not happy with the current design but I'm not sure if it's practical to do something about it (i.e. change it radically). Regards, Halil
Re: [Qemu-devel] [PATCH v10 5/7] monitor: remove event_clock_type
On Fri, Jun 08, 2018 at 11:55:09AM +0800, Peter Xu wrote: > Instead, use a dynamic function to detect which clock we'll use. The > problem is that the old code will let monitor initialization depend on > configure_accelerator() (that's where qtest_enabled() start to take > effect). After this change, we don't have such a dependency any more. > We just need to make sure configure_accelerator() is called when we > start to use it. Now it's only used in monitor_qapi_event_queue() and > monitor_qapi_event_handler(), so we're good. > > Suggested-by: Markus Armbruster > Signed-off-by: Peter Xu > --- > monitor.c | 25 - > 1 file changed, 16 insertions(+), 9 deletions(-) With Markus' suggestion: Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[Qemu-devel] [PULL 3/5] slirp: fix domainname version availability
The change missed the 2.12 deadline. Signed-off-by: Samuel Thibault Reviewed-by: Eric Blake --- qapi/net.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qapi/net.json b/qapi/net.json index 32681a1af7..6b7d93cb59 100644 --- a/qapi/net.json +++ b/qapi/net.json @@ -161,7 +161,7 @@ # to the guest # # @domainname: guest-visible domain name of the virtual nameserver -# (since 2.12) +# (since 3.0) # # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since # 2.6). The network prefix is given in the usual -- 2.17.1
[Qemu-devel] [PULL 2/5] slirp: Add Samuel Thibault's staging tree for slirp
Signed-off-by: Samuel Thibault Acked-by: Thomas Huth --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 41cd3736a9..4c73c16fee 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1675,6 +1675,7 @@ S: Maintained F: slirp/ F: net/slirp.c F: include/net/slirp.h +T: git https://people.debian.org/~sthibault/qemu.git slirp T: git git://git.kiszka.org/qemu.git queues/slirp Stubs -- 2.17.1
[Qemu-devel] [PULL 0/5] slirp updates
The following changes since commit 9be4af13305f24d2dabf94bb53e6b65c76d08bb2: Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2018-06-01 14:58:53 +0100) are available in the Git repository at: https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to c22098c74a09164797fae6511c5eaf68f32c4dd8: slirp: reformat m_inc routine (2018-06-08 09:08:30 +0300) slirp updates Prasad J Pandit (2): slirp: Fix buffer overflow on packet reassembling Samuel Thibault (3): slirp: Add Samuel Thibault's staging tree for slirp slirp: fix domainname version availability Prasad J Pandit (2): slirp: correct size computation while concatenating mbuf slirp: reformat m_inc routine Samuel Thibault (3): slirp: Fix spurious error report when sending directly slirp: Add Samuel Thibault's staging tree for slirp slirp: fix domainname version availability MAINTAINERS| 1 + qapi/net.json | 2 +- slirp/mbuf.c | 39 ++- slirp/mbuf.h | 8 +++- slirp/socket.c | 14 +++--- 5 files changed, 30 insertions(+), 34 deletions(-)
[Qemu-devel] [PULL 5/5] slirp: reformat m_inc routine
From: Prasad J Pandit
Coding style changes to the m_inc routine and minor refactoring.
Reported-by: ZDI Disclosures
Signed-off-by: Prasad J Pandit
Signed-off-by: Samuel Thibault
---
slirp/mbuf.c | 34 --
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/slirp/mbuf.c b/slirp/mbuf.c
index 18cbf759a7..0c189e1a7b 100644
--- a/slirp/mbuf.c
+++ b/slirp/mbuf.c
@@ -151,27 +151,25 @@ m_cat(struct mbuf *m, struct mbuf *n)
void
m_inc(struct mbuf *m, int size)
{
- int datasize;
+int datasize;
- /* some compiles throw up on gotos. This one we can fake. */
-if(m->m_size>size) return;
+/* some compilers throw up on gotos. This one we can fake. */
+if (m->m_size > size) {
+return;
+}
-if (m->m_flags & M_EXT) {
- datasize = m->m_data - m->m_ext;
- m->m_ext = g_realloc(m->m_ext, size + datasize);
- m->m_data = m->m_ext + datasize;
-} else {
- char *dat;
- datasize = m->m_data - m->m_dat;
- dat = g_malloc(size + datasize);
- memcpy(dat, m->m_dat, m->m_size);
-
- m->m_ext = dat;
- m->m_data = m->m_ext + datasize;
- m->m_flags |= M_EXT;
-}
+if (m->m_flags & M_EXT) {
+datasize = m->m_data - m->m_ext;
+m->m_ext = g_realloc(m->m_ext, size + datasize);
+} else {
+datasize = m->m_data - m->m_dat;
+m->m_ext = g_malloc(size + datasize);
+memcpy(m->m_ext, m->m_dat, m->m_size);
+m->m_flags |= M_EXT;
+}
-m->m_size = size + datasize;
+m->m_data = m->m_ext + datasize;
+m->m_size = size + datasize;
}
--
2.17.1
Re: [Qemu-devel] [PATCH v2 2/4] hw/ppc/spapr_drc: Replace error_setg(_abort) by abort()
David Gibson writes: > On Fri, Jun 08, 2018 at 12:54:36AM -0300, Philippe Mathieu-Daudé wrote: >> Hi David, >> >> On 06/08/2018 12:03 AM, David Gibson wrote: >> > On Thu, Jun 07, 2018 at 11:46:43AM -0300, Philippe Mathieu-Daudé wrote: >> >> Use abort() instead of error_setg(_abort), >> >> as suggested by the "qapi/error.h" documentation: >> >> >> >> Please don't error_setg(_fatal, ...), use error_report() and >> >> exit(), because that's more obvious. >> >> Likewise, don't error_setg(_abort, ...), use assert(). >> >> >> >> Use abort() instead of the suggested assert() because the assertion is >> >> already verified by the switch case. >> > >> > I think g_assert_not_reached() would be the right thing here. >> >> I try to follow Eric advice (who recalled Markus). >> As I understand: >> http://lists.gnu.org/archive/html/qemu-devel/2017-08/msg03605.html >> >> "glib-Testing [...] should not be used outside of tests/." > > Oh, ok, go with that then. Most g_assert_FOO() are indeed tests-only, but g_assert_not_reached() is actually acceptable elsewhere, see commit 6e9389563e5, and its review thread https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg05499.html Message-Id: <20170427165526.19836-1-dgilb...@redhat.com> That said, I fail to see the value of this kind of lipstick, too. > Acked-by: David Gibson > >> I can respin if you prefer. I can replace by g_assert_not_reached() on commit if you prefer.
Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues
Peter Xu writes: > On Thu, Jun 07, 2018 at 01:53:01PM +0200, Markus Armbruster wrote: >> Peter Xu writes: >> >> > Previously we cleanup the queues when we got CLOSED event. It was used >> >> we clean up >> >> > to make sure we won't leftover replies/events of a old client to a new >> >> we won't send leftover replies/events of an old client >> >> > client. Now this patch postpones that until OPENED. >> >> What if OPENED never comes? > > Then we clean that up until destruction of the monitor. IMHO it's > fine, but I'm not sure whether that's an overall acceptable approach. See below. >> > In most cases, it does not really matter much since either way will make >> > sure that the new client won't receive unexpected old events/responses. >> > However there can be a very rare race condition with the old way when we >> > use QMP with stdio and pipelines (which is quite common in iotests). >> > The problem is that, we can easily have something like this in scripts: >> > >> > cat $QMP_COMMANDS | qemu -qmp stdio ... | filter_commands >> > >> > In most cases, a QMP session will be based on a bidirectional >> > channel (read/write to a TCP port, for example), so IN and OUT are >> > sharing some fundamentally same thing. However that's untrue for pipes >> >> Suggest "are fundamentally the same thing". >> >> > above - the IN is the "cat" program, while the "OUT" is directed to the >> > "filter_commands" which is actually another process. >> >> Regarding 'IN is the "cat" program': a pipe is not a program. Suggest >> 'IN is connected to "cat", while OUT is connected to "filter_commands", >> which are separate processes. >> >> > Now when we received the CLOSED event, we cleanup the queues (including >> >> we clean up >> >> > the QMP response queue). That means, if we kill/stop the "cat" process >> > faster than the filter_commands process, the filter_commands process is >> > possible to miss some responses/events that should belong to it. >> >> I'm not sure I understand the error scenario. Can you explain it in >> more concrete terms? Like "cat terminates, QEMU reads EOF, QEMU does >> this, QEMU does that, oops, it shouldn't have done that". > > One condition could be this (after "quit" command is executed and QEMU > quits the main loop): > > 1. [main thread] QEMU queues one SHUTDOWN event into response queue > > 2. "cat" terminates (to distinguish it from the animal, I quote it). >Logically it can terminate even earlier, but let's just assume it's >here. > > 3. [monitor iothread] QEMU reads EOF from stdin, which connects to the >"cat" process > > 4. [monitor iothread] QEMU calls the CLOSED event hook for the monitor, >which will clean up the response queue of the monitor, then the >SHUTDOWN event is dropped > > 5. [main thread] clean up the monitors in monitor_cleanup(), when >trying to flush pending responses, it sees nothing. SHUTDOWN is >lost forever Got it, thanks! The actual problem is that we drop the output queue when we see EOF on input. Here's what I think we should do on such EOF: 1. Shut down input Stop reading input. If input has its own file descriptor (as opposed to a read/write fd shared with output), close it. 2. Flush output Continue to write output until the queue becomes empty or we run into an error (such as EPIPE, telling us the output's consumer went away). Works exactly as before, except we proceed to step 3 when the queue becomes empty. 3. Shut down output Close the output file descriptor. > Note that before the monitor iothread was introduced, step [4]/[5] > could never happen since the main loop was the only place to detect > the EOF event of stdin and run the CLOSED event hooks. I can't quite see what ensured response queue is empty before main loop runs the CLOSED event hook. > Now things can > happen in parallel in the iothread. Perhaps that just made the bug more likely to bite. > I can add these into commit message. > >> >> > In practise, I encountered a very strange SHUTDOWN event missing when >> >> practice >> >> May I suggest use of a spell checker? ;) > > Sorry for that. TODO added: "Find a spell checker for Emacs". C-h i M emacs RET M spelling RET >> > running test with iotest 087. Without this patch, iotest 078 will have >> >> 087 or 078? > > 087. Err. Even a spell checker won't help me here! Need to leave something for reviewers to do ;) >> > ~10% chance to miss the SHUTDOWN event and fail when with Out-Of-Band >> > enabled: >> > >> > 087 8s ... - output mismatch (see 087.out.bad) > > I'll take all the rest comments. Thanks for reviewing. I'm not sure this patch is the proper solution. Can we explore the one I sketched?
Re: [Qemu-devel] [PATCH v1 2/8] spapr: no need to verify the node
On 08.06.2018 09:34, Greg Kurz wrote:
> On Thu, 7 Jun 2018 18:52:12 +0200
> David Hildenbrand wrote:
>
>> The node property can always be queried and the value has already been
>> verified in pc_dimm_realize().
>>
>> Signed-off-by: David Hildenbrand
>> ---
>> hw/ppc/spapr.c | 9 +
>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 2375cbee12..d038f3243e 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -3578,14 +3578,7 @@ static void spapr_machine_device_plug(HotplugHandler
>> *hotplug_dev,
>> error_setg(errp, "Memory hotplug not supported for this
>> machine");
>> return;
>> }
>> -node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
>> errp);
>> -if (*errp) {
>
> Good riddance :)
>
>> -return;
>> -}
>> -if (node < 0 || node >= MAX_NODES) {
>> -error_setg(errp, "Invaild node %d", node);
>> -return;
>> -}
>> +node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
>> NULL);
>
> Maybe pass _abort ?
I'm using the same access scheme as in hw/acpi/memory_hotplug.c
("error ignored" vs. "error leads to an abort") - but this will actually
never fail. But I can use error_abort here, does not matter.
Thanks!
>
>>
>> spapr_memory_plug(hotplug_dev, dev, node, errp);
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
>
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [PATCH v1 8/8] s390x: local error handling in hotplug handler functions
On 06/08/2018 09:40 AM, David Hildenbrand wrote:
> On 08.06.2018 09:27, Christian Borntraeger wrote:
>>
>>
>> On 06/08/2018 09:25 AM, Cornelia Huck wrote:
>>> On Thu, 7 Jun 2018 18:52:18 +0200
>>> David Hildenbrand wrote:
>>>
Let's introduce and use local error variables in the hotplug handler
functions.
Signed-off-by: David Hildenbrand
---
hw/s390x/s390-virtio-ccw.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 7ae5fb38dd..29ea50a177 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -434,18 +434,23 @@ static void s390_machine_reset(void)
static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
+Error *local_err = NULL;
+
if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
-s390_cpu_plug(hotplug_dev, dev, errp);
+s390_cpu_plug(hotplug_dev, dev, _err);
}
+error_propagate(errp, local_err);
}
static void s390_machine_device_unplug_request(HotplugHandler
*hotplug_dev,
DeviceState *dev, Error
**errp)
{
+Error *local_err = NULL;
+
if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
-error_setg(errp, "CPU hot unplug not supported on this machine");
-return;
+error_setg(_err, "CPU hot unplug not supported on this
machine");
}
+error_propagate(errp, local_err);
}
static CpuInstanceProperties s390_cpu_index_to_props(MachineState *ms,
>>>
>>> Just seeing this patch by itself, it does not really make much sense.
>>> Even if this is a split out clean-up series, I'd prefer this to go
>>> together with a patch that actually adds something more to the
>>> plug/unplug functions.
>>
>> +1. It is hard to see the "why". Maybe a better patch description could help
>> here?
>>
>
> When checking for an error (*errp) we should make sure that we don't
> dereference the NULL pointer. I will be doing that in the future (memory
> devices), but as you both don't seem to like this patch, I'll drop it
> for now.
With such a patch description (making the handler safe against NULL errp) the
patch
suddenly makes sense on its own.
Re: [Qemu-devel] [Qemu-block] [RFC PATCH V4] qemu-img: make convert async
On Thu, Jun 07, 2018 at 01:19:29PM +0300, Vladimir Sementsov-Ogievskiy wrote: > 07.06.2018 13:10, Stefan Hajnoczi wrote: > > On Fri, Jun 01, 2018 at 07:16:14PM +0300, Vladimir Sementsov-Ogievskiy > > wrote: > > > 20.02.2017 17:59, Peter Lieven wrote: > > > > Am 20.02.2017 um 15:50 schrieb Stefan Hajnoczi: > > > > > On Fri, Feb 17, 2017 at 05:00:24PM +0100, Peter Lieven wrote: > > > > > > this is something I have been thinking about for almost 2 years now. > > > > > > we heavily have the following two use cases when using qemu-img > > > > > > convert. > > > > > > > > > > > > > > > [...] > > > > > > > > Does this patch work with compressed images? Especially the > > > > > out-of-order write mode may be problematic with a compressed qcow2 > > > > > image. > > > > It does, but you are right, out-of-order writes and compression should > > > > be mutually exclusive. > > > > > > Sorry for being late, but can you please explain to me, why? > > There are image format-specific limitations on compressed writes. For > > some reason I thought they were append-only in qcow2, but I was wrong. > > > > Stefan > > And what are limitations for compressed writes in qcow2? Writes must be cluster-aligned and the size must be 1 cluster (except for the last cluster in an image). qemu-img convert honors this, so it's not a problem. > We can't write asynchronously? Why? Async compressed writes are supported nowadays. I think my original comment was wrong. It should be fine to use out-of-order compressed writes. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v1 3/8] spapr: move all DIMM checks into spapr_memory_plug
On 08.06.2018 10:05, Greg Kurz wrote:
> On Thu, 7 Jun 2018 18:52:13 +0200
> David Hildenbrand wrote:
>
>> Let's clean the hotplug handler up by moving everything into
>> spapr_memory_plug().
>>
>> Signed-off-by: David Hildenbrand
>> ---
>> hw/ppc/spapr.c | 23 ++-
>> 1 file changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index d038f3243e..a12be24ca9 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -3136,14 +3136,22 @@ static void spapr_add_lmbs(DeviceState *dev,
>> uint64_t addr_start, uint64_t size,
>> }
>>
>> static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>> - uint32_t node, Error **errp)
>> + Error **errp)
>> {
>> Error *local_err = NULL;
>> sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
>> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(ms);
>> PCDIMMDevice *dimm = PC_DIMM(dev);
>> PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
>> MemoryRegion *mr;
>> uint64_t align, size, addr;
>> +uint32_t node;
>> +
>> +if (!smc->dr_lmb_enabled) {
>> +error_setg(_err, "Memory hotplug not supported for this
>> machine");
>> +goto out;
>> +}
>
> Wouldn't it be more appropriate to move this check to spapr_memory_pre_plug()
> ?
Think you're right (and as spapr_memory_pre_plug() already exists, it's
easy), other opinions? Thanks.
>
>> +node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP, NULL);
>>
>> mr = ddc->get_memory_region(dimm, _err);
>> if (local_err) {
>> @@ -3568,19 +3576,8 @@ out:
>> static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
>>DeviceState *dev, Error **errp)
>> {
>> -MachineState *ms = MACHINE(hotplug_dev);
>> -sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(ms);
>> -
>> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>> -int node;
>> -
>> -if (!smc->dr_lmb_enabled) {
>> -error_setg(errp, "Memory hotplug not supported for this
>> machine");
>> -return;
>> -}
>> -node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
>> NULL);
>> -
>> -spapr_memory_plug(hotplug_dev, dev, node, errp);
>> +spapr_memory_plug(hotplug_dev, dev, errp);
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
>> spapr_core_plug(hotplug_dev, dev, errp);
>> }
>
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [qemu-s390x] [PATCH v1 1/8] pc: local error handling in hotplug handler functions
On 08.06.2018 10:04, Thomas Huth wrote:
> On 07.06.2018 18:52, David Hildenbrand wrote:
>> Let's introduce and use local error variables in the hotplug handler
>> functions.
>
> Why? You don't check local_err in the functions, so I fail to see why
> this is needed? If you need this in a later patch, I think this should
> simply be part of that later patch instead.
See the mail I sent a couple of minutes ago as reply to the cover
letter. Thanks.
>
> Thomas
>
>
>> Signed-off-by: David Hildenbrand
>> ---
>> hw/i386/pc.c | 32 ++--
>> 1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index f3befe6721..8075c6af15 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -2006,45 +2006,57 @@ static void pc_cpu_pre_plug(HotplugHandler
>> *hotplug_dev,
>> static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>DeviceState *dev, Error **errp)
>> {
>> +Error *local_err = NULL;
>> +
>> if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>> -pc_cpu_pre_plug(hotplug_dev, dev, errp);
>> +pc_cpu_pre_plug(hotplug_dev, dev, _err);
>> }
>> +error_propagate(errp, local_err);
>> }
>>
>> static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>>DeviceState *dev, Error **errp)
>> {
>> +Error *local_err = NULL;
>> +
>> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>> -pc_dimm_plug(hotplug_dev, dev, errp);
>> +pc_dimm_plug(hotplug_dev, dev, _err);
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>> -pc_cpu_plug(hotplug_dev, dev, errp);
>> +pc_cpu_plug(hotplug_dev, dev, _err);
>> }
>> +error_propagate(errp, local_err);
>> }
>>
>> static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>> DeviceState *dev, Error
>> **errp)
>> {
>> +Error *local_err = NULL;
>> +
>> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>> -pc_dimm_unplug_request(hotplug_dev, dev, errp);
>> +pc_dimm_unplug_request(hotplug_dev, dev, _err);
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>> -pc_cpu_unplug_request_cb(hotplug_dev, dev, errp);
>> +pc_cpu_unplug_request_cb(hotplug_dev, dev, _err);
>> } else {
>> -error_setg(errp, "acpi: device unplug request for not supported
>> device"
>> - " type: %s", object_get_typename(OBJECT(dev)));
>> +error_setg(_err, "acpi: device unplug request for not
>> supported"
>> + " device type: %s", object_get_typename(OBJECT(dev)));
>> }
>> +error_propagate(errp, local_err);
>> }
>>
>> static void pc_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
>> DeviceState *dev, Error **errp)
>> {
>> +Error *local_err = NULL;
>> +
>> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>> -pc_dimm_unplug(hotplug_dev, dev, errp);
>> +pc_dimm_unplug(hotplug_dev, dev, _err);
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>> -pc_cpu_unplug_cb(hotplug_dev, dev, errp);
>> +pc_cpu_unplug_cb(hotplug_dev, dev, _err);
>> } else {
>> -error_setg(errp, "acpi: device unplug for not supported device"
>> +error_setg(_err, "acpi: device unplug for not supported
>> device"
>> " type: %s", object_get_typename(OBJECT(dev)));
>> }
>> +error_propagate(errp, local_err);
>> }
>>
>> static HotplugHandler *pc_get_hotpug_handler(MachineState *machine,
>>
>
--
Thanks,
David / dhildenb
[Qemu-devel] [PATCH v8 2/6] migration: use bitmap_mutex in migration_bitmap_clear_dirty
The bitmap mutex is used to synchronize threads to update the dirty
bitmap and the migration_dirty_pages counter. For example, the free
page optimization clears bits of free pages from the bitmap in an
iothread context. This patch makes migration_bitmap_clear_dirty update
the bitmap and counter under the mutex.
Signed-off-by: Wei Wang
CC: Dr. David Alan Gilbert
CC: Juan Quintela
CC: Michael S. Tsirkin
CC: Peter Xu
---
migration/ram.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index c53e836..2eabbe9 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1093,11 +1093,14 @@ static inline bool
migration_bitmap_clear_dirty(RAMState *rs,
{
bool ret;
+qemu_mutex_lock(>bitmap_mutex);
ret = test_and_clear_bit(page, rb->bmap);
if (ret) {
rs->migration_dirty_pages--;
}
+qemu_mutex_unlock(>bitmap_mutex);
+
return ret;
}
--
1.8.3.1
Re: [Qemu-devel] [qemu-s390x] [PATCH v1 2/8] spapr: no need to verify the node
On Fri, 8 Jun 2018 10:41:36 +0200
David Hildenbrand wrote:
> On 08.06.2018 10:39, Igor Mammedov wrote:
> > On Fri, 8 Jun 2018 10:07:31 +0200
> > Thomas Huth wrote:
> >
> >> On 08.06.2018 09:48, David Hildenbrand wrote:
> >>> On 08.06.2018 09:46, Greg Kurz wrote:
> On Fri, 8 Jun 2018 09:42:48 +0200
> David Hildenbrand wrote:
>
> > On 08.06.2018 09:34, Greg Kurz wrote:
> >> On Thu, 7 Jun 2018 18:52:12 +0200
> >> David Hildenbrand wrote:
> >>
> >>> The node property can always be queried and the value has already been
> >>> verified in pc_dimm_realize().
> >>>
> >>> Signed-off-by: David Hildenbrand
> >>> ---
> >>> hw/ppc/spapr.c | 9 +
> >>> 1 file changed, 1 insertion(+), 8 deletions(-)
> >>>
> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >>> index 2375cbee12..d038f3243e 100644
> >>> --- a/hw/ppc/spapr.c
> >>> +++ b/hw/ppc/spapr.c
> >>> @@ -3578,14 +3578,7 @@ static void
> >>> spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> >>> error_setg(errp, "Memory hotplug not supported for this
> >>> machine");
> >>> return;
> >>> }
> >>> -node = object_property_get_uint(OBJECT(dev),
> >>> PC_DIMM_NODE_PROP, errp);
> >>> -if (*errp) {
> >>
> >> Good riddance :)
> >>
> >>> -return;
> >>> -}
> >>> -if (node < 0 || node >= MAX_NODES) {
> >>> -error_setg(errp, "Invaild node %d", node);
> >>> -return;
> >>> -}
> >>
> >> Maybe turn that into an assert() instead? ... just for the paranoids ;-)
> >>
> >>> +node = object_property_get_uint(OBJECT(dev),
> >>> PC_DIMM_NODE_PROP, NULL);
> >>
> >> Maybe pass _abort ?
> >
> > I'm using the same access scheme as in hw/acpi/memory_hotplug.c
> >
> > ("error ignored" vs. "error leads to an abort") - but this will actually
> > never fail. But I can use error_abort here, does not matter.
> >
>
> Heh, /me paranoid but this is David's call and he acked that already
> so I guess it's okay.
> >>>
> >>> NULL makes it fit into a single line :)
> >>
> >> +1 for error_abort, even if it takes another line.
> > +1 for error_abort
> > call shouldn't fail, but if does it won't be silently ignored
> > and introduce undefined behavior.
>
> Maybe we should fix the others that pass in NULL.
>
> (no, not me :D - I'm already busy with your requested pre_plug handling)
Add it to wiki page for bite sized tasks?
>
> >
> >>
> >> Thomas
> >
>
>
[Qemu-devel] [PATCH v3 5/6] ramfb: enable vgabios
---
hw/display/ramfb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
index 258783fe3b..477316a14d 100644
--- a/hw/display/ramfb.c
+++ b/hw/display/ramfb.c
@@ -88,6 +88,7 @@ RAMFBState *ramfb_setup(Error **errp)
s = g_new0(RAMFBState, 1);
+rom_add_vga("vgabios-ramfb.bin");
fw_cfg_add_file_callback(fw_cfg, "etc/ramfb",
NULL, ramfb_fw_cfg_write, s,
>cfg, sizeof(s->cfg), false);
--
2.9.3
[Qemu-devel] [PATCH] hw/arm/smmuv3: Fix translate error handling
From: Jia He
In case the STE's config is "Bypass" we currently don't set the
IOMMUTLBEntry perm flags and the access does not succeed. Also
if the config is 0b0xx (Aborted/Reserved), decode_ste and
smmuv3_decode_config currently returns -EINVAL and we don't enter
the expected code path: we record an event whereas we should not.
This patch fixes those bugs and simplifies the error handling.
decode_ste and smmuv3_decode_config now return 0 if aborted or
bypassed config was found. Only bad config info produces negative
error values. In smmuv3_translate we more clearly differentiate
errors, bypass/smmu disabled, aborted and success cases. Also
trace points are differentiated.
Fixes: 9bde7f0674fe ("hw/arm/smmuv3: Implement translate callback")
Reported-by: jia...@hxt-semitech.com
Signed-off-by: jia...@hxt-semitech.com
Signed-off-by: Eric Auger
---
hw/arm/smmuv3-internal.h | 12 +--
hw/arm/smmuv3.c | 93
hw/arm/trace-events | 7 ++--
3 files changed, 77 insertions(+), 35 deletions(-)
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index a9d714b..bab25d6 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -23,6 +23,14 @@
#include "hw/arm/smmu-common.h"
+typedef enum SMMUTranslationStatus {
+SMMU_TRANS_DISABLE,
+SMMU_TRANS_ABORT,
+SMMU_TRANS_BYPASS,
+SMMU_TRANS_ERROR,
+SMMU_TRANS_SUCCESS,
+} SMMUTranslationStatus;
+
/* MMIO Registers */
REG32(IDR0,0x0)
@@ -315,7 +323,7 @@ enum { /* Command completion notification */
/* Events */
typedef enum SMMUEventType {
-SMMU_EVT_OK = 0x00,
+SMMU_EVT_NONE = 0x00,
SMMU_EVT_F_UUT,
SMMU_EVT_C_BAD_STREAMID ,
SMMU_EVT_F_STE_FETCH ,
@@ -337,7 +345,7 @@ typedef enum SMMUEventType {
} SMMUEventType;
static const char *event_stringify[] = {
-[SMMU_EVT_OK] = "SMMU_EVT_OK",
+[SMMU_EVT_NONE] = "no recorded event",
[SMMU_EVT_F_UUT]= "SMMU_EVT_F_UUT",
[SMMU_EVT_C_BAD_STREAMID] = "SMMU_EVT_C_BAD_STREAMID",
[SMMU_EVT_F_STE_FETCH] = "SMMU_EVT_F_STE_FETCH",
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index b3026de..c5e7a7c 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -154,7 +154,7 @@ void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo
*info)
EVT_SET_SID(, info->sid);
switch (info->type) {
-case SMMU_EVT_OK:
+case SMMU_EVT_NONE:
return;
case SMMU_EVT_F_UUT:
EVT_SET_SSID(, info->u.f_uut.ssid);
@@ -312,12 +312,11 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t
ssid,
return 0;
}
-/* Returns <0 if the caller has no need to continue the translation */
+/* Returns < 0 in case of invalid STE, 0 otherwise */
static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
STE *ste, SMMUEventInfo *event)
{
uint32_t config;
-int ret = -EINVAL;
if (!STE_VALID(ste)) {
goto bad_ste;
@@ -326,13 +325,13 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
config = STE_CONFIG(ste);
if (STE_CFG_ABORT(config)) {
-cfg->aborted = true; /* abort but don't record any event */
-return ret;
+cfg->aborted = true;
+return 0;
}
if (STE_CFG_BYPASS(config)) {
cfg->bypassed = true;
-return ret;
+return 0;
}
if (STE_CFG_S2_ENABLED(config)) {
@@ -509,7 +508,7 @@ bad_cd:
* the different configuration decoding steps
* @event: must be zero'ed by the caller
*
- * return < 0 if the translation needs to be aborted (@event is filled
+ * return < 0 in case of config decoding error (@event is filled
* accordingly). Return 0 otherwise.
*/
static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg,
@@ -518,19 +517,26 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr,
SMMUTransCfg *cfg,
SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
uint32_t sid = smmu_get_sid(sdev);
SMMUv3State *s = sdev->smmu;
-int ret = -EINVAL;
+int ret;
STE ste;
CD cd;
-if (smmu_find_ste(s, sid, , event)) {
+ret = smmu_find_ste(s, sid, , event);
+if (ret) {
return ret;
}
-if (decode_ste(s, cfg, , event)) {
+ret = decode_ste(s, cfg, , event);
+if (ret) {
return ret;
}
-if (smmu_get_cd(s, , 0 /* ssid */, , event)) {
+if (cfg->aborted || cfg->bypassed) {
+return 0;
+}
+
+ret = smmu_get_cd(s, , 0 /* ssid */, , event);
+if (ret) {
return ret;
}
@@ -543,8 +549,9 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion
*mr, hwaddr addr,
SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
SMMUv3State *s = sdev->smmu;
uint32_t sid = smmu_get_sid(sdev);
-SMMUEventInfo event = {.type =
Re: [Qemu-devel] [PATCH v7 4/5] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
On 06/08/2018 10:17 AM, Peter Xu wrote: On Thu, Jun 07, 2018 at 07:59:22PM +0800, Wei Wang wrote: Not necessarily _need_ to share it, I meant it can be shared using qemu command line. Live migration doesn't happen all the time, and that optimization doesn't run that long, if users want to have other BHs run in this iothread context, they can only create one iothread via the qemu cmd line. IMO iothreads and aiocontexts are for event-driven model. To me it's just a thread which polls for submitted callbacks to run. When migration reaches the place that needs to submit the optimization function, it calls start() to submit it. I'm not sure why there is a worry about what's inside the callback. Busy loop is not an event-driven model. Here if we want a busy loop I'll create a thread when start page hinting, then join the thread when done. The old (v4) implementation worked that way as you mentioned above, and Michael suggested to use iothread in the previous discussion. I'm fine with both actually. For the virtio part, we've had many discussions, I would take the choice I had with Michael before, unless there is an obvious advantage (e.g. proved better performance). But I'll stop commenting on this. Please prepare a more clear interface for migration in your next post. I'll read that. Sure, thanks. The new version is coming soon. I'm a bit curious on how much time will it use to do one round of the free page hints (e.g., an idle guest with 8G mem, or any configuration you tested)? I suppose during that time the iothread will be held steady with 100% cpu usage, am I right? Compared to the time spent by the legacy migration to send free pages, that small amount of CPU usage spent on filtering free pages could be neglected. Grinding a chopper will not hold up the work of cutting firewood :) Sorry I didn't express myself clearly. My question was that, have you measured how long time it will take from starting of the free page hints (when balloon state is set to FREE_PAGE_REPORT_S_REQUESTED), until it completes (when QEMU receives the VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID, then set the status to FREE_PAGE_REPORT_S_STOP)? I vaguely remember it's several ms (for around 7.5G free pages) long time ago. What would be the concern behind that number you want to know? Because roughly I know the time between two bitmap syncs. Then I will know how possible a free page hinting process won't stop until the next bitmap sync happens. We have a function, stop(), to stop the optimization before the next bitmap sync if the optimization is still running. But I never saw that case happens (the free page hinting finishes itself before that). Best, Wei
Re: [Qemu-devel] [PATCH v1 8/8] s390x: local error handling in hotplug handler functions
On 06/08/2018 09:25 AM, Cornelia Huck wrote:
> On Thu, 7 Jun 2018 18:52:18 +0200
> David Hildenbrand wrote:
>
>> Let's introduce and use local error variables in the hotplug handler
>> functions.
>>
>> Signed-off-by: David Hildenbrand
>> ---
>> hw/s390x/s390-virtio-ccw.c | 11 ---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 7ae5fb38dd..29ea50a177 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -434,18 +434,23 @@ static void s390_machine_reset(void)
>> static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
>> DeviceState *dev, Error **errp)
>> {
>> +Error *local_err = NULL;
>> +
>> if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>> -s390_cpu_plug(hotplug_dev, dev, errp);
>> +s390_cpu_plug(hotplug_dev, dev, _err);
>> }
>> +error_propagate(errp, local_err);
>> }
>>
>> static void s390_machine_device_unplug_request(HotplugHandler *hotplug_dev,
>> DeviceState *dev, Error
>> **errp)
>> {
>> +Error *local_err = NULL;
>> +
>> if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>> -error_setg(errp, "CPU hot unplug not supported on this machine");
>> -return;
>> +error_setg(_err, "CPU hot unplug not supported on this
>> machine");
>> }
>> +error_propagate(errp, local_err);
>> }
>>
>> static CpuInstanceProperties s390_cpu_index_to_props(MachineState *ms,
>
> Just seeing this patch by itself, it does not really make much sense.
> Even if this is a split out clean-up series, I'd prefer this to go
> together with a patch that actually adds something more to the
> plug/unplug functions.
+1. It is hard to see the "why". Maybe a better patch description could help
here?
Re: [Qemu-devel] [PATCH v1 8/8] s390x: local error handling in hotplug handler functions
On 08.06.2018 09:27, Christian Borntraeger wrote:
>
>
> On 06/08/2018 09:25 AM, Cornelia Huck wrote:
>> On Thu, 7 Jun 2018 18:52:18 +0200
>> David Hildenbrand wrote:
>>
>>> Let's introduce and use local error variables in the hotplug handler
>>> functions.
>>>
>>> Signed-off-by: David Hildenbrand
>>> ---
>>> hw/s390x/s390-virtio-ccw.c | 11 ---
>>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index 7ae5fb38dd..29ea50a177 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -434,18 +434,23 @@ static void s390_machine_reset(void)
>>> static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
>>> DeviceState *dev, Error **errp)
>>> {
>>> +Error *local_err = NULL;
>>> +
>>> if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>>> -s390_cpu_plug(hotplug_dev, dev, errp);
>>> +s390_cpu_plug(hotplug_dev, dev, _err);
>>> }
>>> +error_propagate(errp, local_err);
>>> }
>>>
>>> static void s390_machine_device_unplug_request(HotplugHandler *hotplug_dev,
>>> DeviceState *dev, Error
>>> **errp)
>>> {
>>> +Error *local_err = NULL;
>>> +
>>> if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>>> -error_setg(errp, "CPU hot unplug not supported on this machine");
>>> -return;
>>> +error_setg(_err, "CPU hot unplug not supported on this
>>> machine");
>>> }
>>> +error_propagate(errp, local_err);
>>> }
>>>
>>> static CpuInstanceProperties s390_cpu_index_to_props(MachineState *ms,
>>
>> Just seeing this patch by itself, it does not really make much sense.
>> Even if this is a split out clean-up series, I'd prefer this to go
>> together with a patch that actually adds something more to the
>> plug/unplug functions.
>
> +1. It is hard to see the "why". Maybe a better patch description could help
> here?
>
When checking for an error (*errp) we should make sure that we don't
dereference the NULL pointer. I will be doing that in the future (memory
devices), but as you both don't seem to like this patch, I'll drop it
for now.
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [PATCH v1 2/8] spapr: no need to verify the node
On 08.06.2018 09:46, Greg Kurz wrote:
> On Fri, 8 Jun 2018 09:42:48 +0200
> David Hildenbrand wrote:
>
>> On 08.06.2018 09:34, Greg Kurz wrote:
>>> On Thu, 7 Jun 2018 18:52:12 +0200
>>> David Hildenbrand wrote:
>>>
The node property can always be queried and the value has already been
verified in pc_dimm_realize().
Signed-off-by: David Hildenbrand
---
hw/ppc/spapr.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2375cbee12..d038f3243e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3578,14 +3578,7 @@ static void
spapr_machine_device_plug(HotplugHandler *hotplug_dev,
error_setg(errp, "Memory hotplug not supported for this
machine");
return;
}
-node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
errp);
-if (*errp) {
>>>
>>> Good riddance :)
>>>
-return;
-}
-if (node < 0 || node >= MAX_NODES) {
-error_setg(errp, "Invaild node %d", node);
-return;
-}
+node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
NULL);
>>>
>>> Maybe pass _abort ?
>>
>> I'm using the same access scheme as in hw/acpi/memory_hotplug.c
>>
>> ("error ignored" vs. "error leads to an abort") - but this will actually
>> never fail. But I can use error_abort here, does not matter.
>>
>
> Heh, /me paranoid but this is David's call and he acked that already
> so I guess it's okay.
NULL makes it fit into a single line :)
Thanks!
>
> Reviewed-by: Greg Kurz
>
>> Thanks!
>>
>>>
spapr_memory_plug(hotplug_dev, dev, node, errp);
} else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
>>>
>>
>>
>
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [qemu-s390x] [PATCH v1 1/8] pc: local error handling in hotplug handler functions
On 07.06.2018 18:52, David Hildenbrand wrote:
> Let's introduce and use local error variables in the hotplug handler
> functions.
Why? You don't check local_err in the functions, so I fail to see why
this is needed? If you need this in a later patch, I think this should
simply be part of that later patch instead.
Thomas
> Signed-off-by: David Hildenbrand
> ---
> hw/i386/pc.c | 32 ++--
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f3befe6721..8075c6af15 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2006,45 +2006,57 @@ static void pc_cpu_pre_plug(HotplugHandler
> *hotplug_dev,
> static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>DeviceState *dev, Error **errp)
> {
> +Error *local_err = NULL;
> +
> if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> -pc_cpu_pre_plug(hotplug_dev, dev, errp);
> +pc_cpu_pre_plug(hotplug_dev, dev, _err);
> }
> +error_propagate(errp, local_err);
> }
>
> static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>DeviceState *dev, Error **errp)
> {
> +Error *local_err = NULL;
> +
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> -pc_dimm_plug(hotplug_dev, dev, errp);
> +pc_dimm_plug(hotplug_dev, dev, _err);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> -pc_cpu_plug(hotplug_dev, dev, errp);
> +pc_cpu_plug(hotplug_dev, dev, _err);
> }
> +error_propagate(errp, local_err);
> }
>
> static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error
> **errp)
> {
> +Error *local_err = NULL;
> +
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> -pc_dimm_unplug_request(hotplug_dev, dev, errp);
> +pc_dimm_unplug_request(hotplug_dev, dev, _err);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> -pc_cpu_unplug_request_cb(hotplug_dev, dev, errp);
> +pc_cpu_unplug_request_cb(hotplug_dev, dev, _err);
> } else {
> -error_setg(errp, "acpi: device unplug request for not supported
> device"
> - " type: %s", object_get_typename(OBJECT(dev)));
> +error_setg(_err, "acpi: device unplug request for not
> supported"
> + " device type: %s", object_get_typename(OBJECT(dev)));
> }
> +error_propagate(errp, local_err);
> }
>
> static void pc_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error **errp)
> {
> +Error *local_err = NULL;
> +
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> -pc_dimm_unplug(hotplug_dev, dev, errp);
> +pc_dimm_unplug(hotplug_dev, dev, _err);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> -pc_cpu_unplug_cb(hotplug_dev, dev, errp);
> +pc_cpu_unplug_cb(hotplug_dev, dev, _err);
> } else {
> -error_setg(errp, "acpi: device unplug for not supported device"
> +error_setg(_err, "acpi: device unplug for not supported device"
> " type: %s", object_get_typename(OBJECT(dev)));
> }
> +error_propagate(errp, local_err);
> }
>
> static HotplugHandler *pc_get_hotpug_handler(MachineState *machine,
>
Re: [Qemu-devel] [PATCH 4/6] hmp: Add info commands for preconfig
* Markus Armbruster (arm...@redhat.com) wrote: > "Dr. David Alan Gilbert" writes: > > > * Markus Armbruster (arm...@redhat.com) wrote: > >> "Dr. David Alan Gilbert" writes: > >> > >> > * Markus Armbruster (arm...@redhat.com) wrote: > >> >> Peter Xu writes: > >> >> > >> >> > On Tue, Jun 05, 2018 at 01:26:34PM +0100, Dr. David Alan Gilbert > >> >> > (git) wrote: > >> >> >> From: "Dr. David Alan Gilbert" > >> >> >> > >> >> >> Allow a bunch of the info commands to be used in preconfig. > >> >> >> Could probably add most of them. > >> >> > > >> >> > I guess some of them may not work yet during preconfig. E.g.: > >> >> > > >> >> > $ ./x86_64-softmmu/qemu-system-x86_64 -preconfig -monitor stdio > >> >> > QEMU 2.12.50 monitor - type 'help' for more information > >> >> > (qemu) info mtree > >> >> > address-space: memory > >> >> > - (prio 0, i/o): system > >> >> > > >> >> > address-space: I/O > >> >> > - (prio 0, i/o): io > >> >> > > >> >> > But it's fine to enable that I guess. > >> >> > > >> >> > (Which "info" command would you want to use during preconfig?) > >> >> > > >> >> >> > >> >> >> Signed-off-by: Dr. David Alan Gilbert > >> >> > > >> >> > Reviewed-by: Peter Xu > >> >> > >> >> The reason for having -preconfig is us despairing of making -S do the > >> >> right thing. We'd have to *understand* the tangled mess that is our > >> >> startup, and rearrange it so QMP becomes available early enough for > >> >> configuring NUMA (and other things), yet late enough for everything to > >> >> work. > >> >> > >> >> -preconfig is a cheap hack to avoid this headache, by bypassing almost > >> >> all of "everything". > >> >> > >> >> Now you bring back some of "everything". Dangerous. You better show it > >> >> actually works. Until you do: > >> >> > >> >> NAK > >> > > >> > Well I did test each command in here to make sure it didn't > >> > crash/produce complete junk; but here's the output with the v2 of this > >> > patch that Igor R-b: > >> [...] > >> > >> For the sake of the argument, let's assume these commands all work in > >> preconfig state. Are their QMP equivalents all available in preconfig > >> state? > > > > That I don't know; I was happy to fix my list to the ones > > Igor recommended. If you object to some particular entries I'll > > be happy to change them. > > HMP must not provide more functionality than QMP. Specifically, we may > provide "info FOO" only when we also provide query-FOO. > > There are exceptions to this rule. I don't think they apply here. I'm > prepared to discuss them, of course. No, that's strictly not true; HMP can provide anything that helps a human debug stuff. The requirement is that if a tool needs it then it must be provided in QMP. > I wish there was a way to automate "provide command in HMP when its > buddy is available in QMP", but since the buddies are only connected by > code, that seems infeasible. > > Without such automation, the two sets of available commands need to be > kept consistent manually. The larger they are, the more of a bother. > > Bother is fine when it provides commensurate value to users. Options in > increasing order of value provided: > > (1) HMP becomes ready only after we exit preconfig state (what I > proposed in Message-ID: <87603cxob9@dusky.pond.sub.org>. > > (2) HMP provides help, quit, exit-preconfig. > > (3) HMP provides (a subset of) the commands QMP provides. > > I figure the maintenance cost of (1) and (2) will be negligible, but (3) > could be a drag. Are you sure it's worthwhile? I'm not prepared to restrict to (2), and I'm not prepared to restrict HMP to a subset of QMP; As I said previously, if there's a command that you think is incorrect/broken that I've enabled then I'm happy to remove it. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v2 2/8] ppc4xx_i2c: Move register state to private struct and remove unimplemented sdata and intr registers
On Wed, Jun 06, 2018 at 03:31:48PM +0200, BALATON Zoltan wrote:
> Signed-off-by: BALATON Zoltan
It's not clear to me why this is preferable to having the registers
embedded in the state structure. The latter is pretty standard
practice for qemu.
> ---
> hw/i2c/ppc4xx_i2c.c | 75
> +
> include/hw/i2c/ppc4xx_i2c.h | 19 ++--
> 2 files changed, 43 insertions(+), 51 deletions(-)
>
> diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
> index d1936db..a68b5f7 100644
> --- a/hw/i2c/ppc4xx_i2c.c
> +++ b/hw/i2c/ppc4xx_i2c.c
> @@ -3,7 +3,7 @@
> *
> * Copyright (c) 2007 Jocelyn Mayer
> * Copyright (c) 2012 François Revol
> - * Copyright (c) 2016 BALATON Zoltan
> + * Copyright (c) 2016-2018 BALATON Zoltan
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> * of this software and associated documentation files (the "Software"), to
> deal
> @@ -46,9 +46,26 @@
>
> #define IIC_XTCNTLSS_SRST (1 << 0)
>
> +typedef struct {
> +uint8_t mdata;
> +uint8_t lmadr;
> +uint8_t hmadr;
> +uint8_t cntl;
> +uint8_t mdcntl;
> +uint8_t sts;
> +uint8_t extsts;
> +uint8_t lsadr;
> +uint8_t hsadr;
> +uint8_t clkdiv;
> +uint8_t intrmsk;
> +uint8_t xfrcnt;
> +uint8_t xtcntlss;
> +uint8_t directcntl;
> +} PPC4xxI2CRegs;
> +
> static void ppc4xx_i2c_reset(DeviceState *s)
> {
> -PPC4xxI2CState *i2c = PPC4xx_I2C(s);
> +PPC4xxI2CRegs *i2c = PPC4xx_I2C(s)->regs;
>
> /* FIXME: Should also reset bus?
> *if (s->address != ADDR_RESET) {
> @@ -63,7 +80,6 @@ static void ppc4xx_i2c_reset(DeviceState *s)
> i2c->mdcntl = 0;
> i2c->sts = 0;
> i2c->extsts = 0x8f;
> -i2c->sdata = 0;
> i2c->lsadr = 0;
> i2c->hsadr = 0;
> i2c->clkdiv = 0;
> @@ -71,7 +87,6 @@ static void ppc4xx_i2c_reset(DeviceState *s)
> i2c->xfrcnt = 0;
> i2c->xtcntlss = 0;
> i2c->directcntl = 0xf;
> -i2c->intr = 0;
> }
>
> static inline bool ppc4xx_i2c_is_master(PPC4xxI2CState *i2c)
> @@ -81,13 +96,14 @@ static inline bool ppc4xx_i2c_is_master(PPC4xxI2CState
> *i2c)
>
> static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr addr, unsigned int
> size)
> {
> -PPC4xxI2CState *i2c = PPC4xx_I2C(opaque);
> +PPC4xxI2CState *s = PPC4xx_I2C(opaque);
> +PPC4xxI2CRegs *i2c = s->regs;
> uint64_t ret;
>
> switch (addr) {
> case 0:
> ret = i2c->mdata;
> -if (ppc4xx_i2c_is_master(i2c)) {
> +if (ppc4xx_i2c_is_master(s)) {
> ret = 0xff;
>
> if (!(i2c->sts & IIC_STS_MDBS)) {
> @@ -98,7 +114,7 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr
> addr, unsigned int size)
> int pending = (i2c->cntl >> 4) & 3;
>
> /* get the next byte */
> -int byte = i2c_recv(i2c->bus);
> +int byte = i2c_recv(s->bus);
>
> if (byte < 0) {
> qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: read failed "
> @@ -113,13 +129,13 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr
> addr, unsigned int size)
>
> if (!pending) {
> i2c->sts &= ~IIC_STS_MDBS;
> -/*i2c_end_transfer(i2c->bus);*/
> +/*i2c_end_transfer(s->bus);*/
> /*} else if (i2c->cntl & (IIC_CNTL_RPST | IIC_CNTL_CHT)) {*/
> } else if (pending) {
> /* current smbus implementation doesn't like
> multibyte xfer repeated start */
> -i2c_end_transfer(i2c->bus);
> -if (i2c_start_transfer(i2c->bus, i2c->lmadr >> 1, 1)) {
> +i2c_end_transfer(s->bus);
> +if (i2c_start_transfer(s->bus, i2c->lmadr >> 1, 1)) {
> /* if non zero is returned, the adress is not valid
> */
> i2c->sts &= ~IIC_STS_PT;
> i2c->sts |= IIC_STS_ERR;
> @@ -139,9 +155,6 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr
> addr, unsigned int size)
>TYPE_PPC4xx_I2C, __func__);
> }
> break;
> -case 2:
> -ret = i2c->sdata;
> -break;
> case 4:
> ret = i2c->lmadr;
> break;
> @@ -181,9 +194,6 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr
> addr, unsigned int size)
> case 16:
> ret = i2c->directcntl;
> break;
> -case 17:
> -ret = i2c->intr;
> -break;
> default:
> if (addr < PPC4xx_I2C_MEM_SIZE) {
> qemu_log_mask(LOG_UNIMP, "%s: Unimplemented register 0x%"
> @@ -201,14 +211,15 @@ static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr
> addr, unsigned int size)
> static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr, uint64_t value,
>unsigned int size)
>
Re: [Qemu-devel] [PATCH v1 6/8] spapr: handle pc-dimm unplug via hotplug handler chain
On Thu, 7 Jun 2018 18:52:16 +0200
David Hildenbrand wrote:
> Let's handle it via hotplug_handler_unplug(). E.g. necessary to hotplug/
> unplug memory devices (which a pc-dimm is) later.
Perhaps something like following would be better:
Factor out memory unplug into separate function from spapr_lmb_release().
Then use generic hotplug_handler_unplug() to trigger memory unplug,
which would call spapr_machine_device_unplug() -> spapr_memory_unplug()
in the end .
This way unplug operation is not buried in lmb internals and located
in the same place like in other targets, following similar
logic/call chain across targets.
> Signed-off-by: David Hildenbrand
> ---
> hw/ppc/spapr.c | 18 +++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index bcb72d9fa7..0a8a3455d6 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3298,7 +3298,8 @@ static sPAPRDIMMState
> *spapr_recover_pending_dimm_state(sPAPRMachineState *ms,
> /* Callback to be called during DRC release. */
> void spapr_lmb_release(DeviceState *dev)
> {
> -sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_hotplug_handler(dev));
> +HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_ctrl);
> sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr,
> PC_DIMM(dev));
>
> /* This information will get lost if a migration occurs
> @@ -3316,9 +3317,17 @@ void spapr_lmb_release(DeviceState *dev)
>
> /*
> * Now that all the LMBs have been removed by the guest, call the
> - * pc-dimm unplug handler to cleanup up the pc-dimm device.
> + * unplug handler chain. This can never fail.
> */
> -pc_dimm_memory_unplug(dev, MACHINE(spapr));
> +hotplug_handler_unplug(hotplug_ctrl, dev, _abort);
> +}
> +
> +static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState
> *dev)
> +{
> +sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
> +sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr,
> PC_DIMM(dev));
> +
> +pc_dimm_memory_unplug(dev, MACHINE(hotplug_dev));
> object_unparent(OBJECT(dev));
> spapr_pending_dimm_unplugs_remove(spapr, ds);
> }
> @@ -3589,6 +3598,9 @@ static void spapr_machine_device_plug(HotplugHandler
> *hotplug_dev,
> static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error **errp)
> {
> +if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> +spapr_memory_unplug(hotplug_dev, dev);
> +}
> }
>
> static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
Re: [Qemu-devel] [PATCH v1 5/8] spapr: introduce machine unplug handler
On Thu, 7 Jun 2018 18:52:15 +0200
David Hildenbrand wrote:
> We'll be handling unplug of e.g. CPUs and PCDIMMs via the general
> hotplug handler soon, so let's add that handler function.
>
> Signed-off-by: David Hildenbrand
> ---
Reviewed-by: Greg Kurz
> hw/ppc/spapr.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4447cb197f..bcb72d9fa7 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3586,6 +3586,11 @@ static void spapr_machine_device_plug(HotplugHandler
> *hotplug_dev,
> error_propagate(errp, local_err);
> }
>
> +static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
> +DeviceState *dev, Error **errp)
> +{
> +}
> +
> static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error
> **errp)
> {
> @@ -3988,6 +3993,7 @@ static void spapr_machine_class_init(ObjectClass *oc,
> void *data)
> mc->get_default_cpu_node_id = spapr_get_default_cpu_node_id;
> mc->possible_cpu_arch_ids = spapr_possible_cpu_arch_ids;
> hc->unplug_request = spapr_machine_device_unplug_request;
> +hc->unplug = spapr_machine_device_unplug;
>
> smc->dr_lmb_enabled = true;
> mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
Re: [Qemu-devel] [PULL 0/7] 9p patches 2018-06-07
On 7 June 2018 at 16:21, Greg Kurz wrote: > The following changes since commit 5d328d7d2f1fd4fb160bcfb6e4eb838720274438: > > Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20180605.0' > into staging (2018-06-07 08:59:28 +0100) > > are available in the Git repository at: > > https://github.com/gkurz/qemu.git tags/for-upstream > > for you to fetch changes up to aca6897fba149a2a650dcdf5a5e1ae828371f4aa: > > 9p: xattr: Properly translate xattrcreate flags (2018-06-07 12:17:22 +0200) > > > Mostly bug fixes and code sanitization motivated by the upcoming > support for Darwin hosts. Thanks to Keno Fischer. > > > Keno Fischer (7): > 9p: proxy: Fix size passed to `connect` > 9p: local: Properly set errp in fstatfs error path > 9p: Move a couple xattr functions to 9p-util > 9p: xattr: Fix crashes due to free of uninitialized value > 9p: local: Avoid warning if FS_IOC_GETVERSION is not defined > 9p: Properly check/translate flags in unlinkat > 9p: xattr: Properly translate xattrcreate flags Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v2] Show values and description when using "qom-list"
* Andreas Färber (afaer...@suse.de) wrote: > Am 01.06.2018 um 17:39 schrieb Ricardo Perez Blanco: > > For debugging purposes it is very useful to: > > - See the description of the field. This information is already filled > >in but not shown in "qom-list" command. > > No objection on this part. > > > - Display value of the field. > > That is by definition the qom-get operation, not qom-list. Just like the > ls command does not show file contents, there's cat etc. for that. For > debugging purposes we had a qom-tree (?) command that would combine > both. I'm not too bothered about distinguishing between the two commands; but it would be nice - one reason I'm not too bothered is because we've failed to get a qom-get in multiple years of trying. > There might be unmerged patches on qemu-devel related to display > of certain data types. Which ones? Dave > Regards, > Andreas > > > > > Signed-off-by: Ricardo Perez Blanco > > --- > > hmp.c | 13 +++-- > > qapi/misc.json | 6 -- > > qmp.c | 7 +++ > > qom/object.c | 8 +++- > > 4 files changed, 25 insertions(+), 9 deletions(-) > [snip] > > -- > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PULL 0/5] slirp updates
On 8 June 2018 at 07:13, Samuel Thibault wrote: > The following changes since commit 9be4af13305f24d2dabf94bb53e6b65c76d08bb2: > > Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into > staging (2018-06-01 14:58:53 +0100) > > are available in the Git repository at: > > https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault > > for you to fetch changes up to c22098c74a09164797fae6511c5eaf68f32c4dd8: > > slirp: reformat m_inc routine (2018-06-08 09:08:30 +0300) > > > slirp updates > > Prasad J Pandit (2): > slirp: Fix buffer overflow on packet reassembling > > Samuel Thibault (3): > slirp: Add Samuel Thibault's staging tree for slirp > slirp: fix domainname version availability > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v2 0/3] glib: update the min required version
On 6 June 2018 at 18:32, Daniel P. Berrangé wrote: > The previous patch to bump glib to 2.42 hit problems with Peter's build > environment for testing merge: > > https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg02557.html > > This posting drops back to 2.40, which allows Ubuntu 14.04 from GLibC > compile farm to be supported. > > It does NOT try to go back to 2.34, because it is hoped that the mxe.cc > Debian packages will be suitable for Peter to test Windows > cross-compile. Alternatively the docker environments provided in tree > can be used for mingw build testing on any host able to run docker. > > I also dropped some more GLIB_CHECK_VERSION checks that are redundant > given the new min version. Note that updating to MXE is still on my todo list; I'll let you know when I get to it... thanks -- PMM
Re: [Qemu-devel] [RFC PATCH v2 7/7] hw/sd/ssi-sd: Force cards connected in SPI mode to use Spec v1.10
On 7 June 2018 at 19:06, Philippe Mathieu-Daudé wrote:
> Due to physical restriction in SPI mode the maximum transfer
> speed is limited. All the extensions added after Spec v3 are
> simply not supported in SPI mode:
You say here that SPI mode doesn't support extensions added
"after spec v3"...
> 7.1 Introduction
>
> The SPI mode consists of a secondary communication protocol
> that is offered by Flash-based SD Memory Cards. This mode is
> a subset of the SD Memory Card protocol, designed to communicate
> with a SPI channel, commonly found in Motorola's (and lately a
> few other vendors') microcontrollers. The interface is selected
> during the first reset command after power up (CMD0) and cannot
> be changed once the part is powered on.
> The SPI standard defines the physical link only, and not the
> complete data transfer protocol. The SD Memory Card SPI
> implementation uses a subset of the SD Memory Card protocol and
> command set. The advantage of the SPI mode is the capability of
> using an off-the-shelf host, hence reducing the design-in effort
> to minimum. The disadvantage is the loss of performance of the
> SPI mode versus SD mode (e.g. Single data line and hardware CS
> signal per card).
> The commands and functions in SD mode defined after the Version
> 2.00 are not supported in SPI mode.
...but here quote the spec which says that commands defined after
version 2.00 are not supported in SPI mode...
and then the patch itself enforces spec 1.10, rather than 2.
So I'm confused.
> The card may respond to the
> commands and functions even if the card is in SPI mode but host
> should not use them in SPI mode.
>
> Some firmwares use the CMD8 in SPI mode to poll which Spec version
> the SD card supports.
>
> 7.2.1 Mode Selection and Initialization (SPI mode)
>
> The SD Card is powered up in the SD mode. It will enter SPI mode
> if the CS signal is asserted (negative) during the reception of
> the reset command (CMD0). If the card recognizes that the SD mode
> is required it will not respond to the command and remain in the
> SD mode. If SPI mode is required, the card will switch to SPI and
> respond with the SPI mode R1 response.
> The only way to return to the SD mode is by entering the power
> cycle. In SPI mode, the SD Card protocol state machine in SD mode
> is not observed. All the SD Card commands supported in SPI mode
> are always available. [...]
> If the card indicates an illegal command, the card is legacy and
> does not support CMD8. If the card supports CMD8 and can operate
> on the supplied voltage, the response echoes back the supply
> voltage and the check pattern that were set in the command
> argument.
>
> The NuttX RTOS use it too:
>
> /* Check for SDHC Version 2.x. CMD 8 is reserved on SD version 1.0 and
> * MMC.
> */
> finfo("Send CMD8\n");
> result = mmcsd_sendcmd(slot, _cmd8, 0x1aa);
> if (result == MMCSD_SPIR1_IDLESTATE)
> ...
> /* Check for SDC version 1.x or MMC */
> else
> ...
>
> See
> https://bitbucket.org/nuttx/nuttx/src/nuttx-7.25/drivers/mmcsd/mmcsd_spi.c?mmcsd_spi.c-1645#mmcsd_spi.c-1645
This seems reasonable guest behaviour, and it's what the spec says
to do. Why do we need to enforce 1.10 to get things to work?
thanks
-- PMM
[Qemu-devel] [Bug 1636217] Re: qemu-kvm 2.7 does not boot kvm VMs with virtio on top of VMware ESX
Marking as fixed, according to comment #13 ** Changed in: qemu Status: New => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1636217 Title: qemu-kvm 2.7 does not boot kvm VMs with virtio on top of VMware ESX Status in QEMU: Fix Released Bug description: After todays Proxmox update all my Linux VMs stopped booting. # How to reproduce - Have KVM on top of VMware ESX (I use VMware ESX 6) - Boot Linux VM with virtio Disk drive. # Result virtio based VMs do not boot anymore: root@demotuxdc:/etc/pve/nodes/demotuxdc/qemu-server# grep virtio0 100.conf bootdisk: virtio0 virtio0: pvestorage:100/vm-100-disk-1.raw,discard=on,size=20G (initially with cache=writethrough, but that doesn´t matter) What happens instead is: - BIOS displays "Booting from harddisk..." - kvm process of VM loops at about 140% of Intel(R) Core(TM) i5-6260U CPU @ 1.80GHz Skylake dual core CPU Disk of course has valid bootsector: root@demotuxdc:/srv/pvestorage/images/100# file -sk vm-100-disk-1.raw vm-100-disk-1.raw: DOS/MBR boot sector DOS/MBR boot sector DOS executable (COM), boot code root@demotuxdc:/srv/pvestorage/images/100# head -c 2048 vm-100-disk-1.raw | hd | grep GRUB 0170 be 94 7d e8 2e 00 cd 18 eb fe 47 52 55 42 20 00 |..}...GRUB .| # Workaround 1 - Change disk from virtio0 to scsi0 - Debian boots out of the box after this change - SLES 12 needs a rebuilt initrd - CentOS 7 too, but it seems that is not even enough and it still fails (even in hostonly="no" mode for dracut) # Workaround 2 Downgrade pve-qemu-kvm 2.7.0-3 to 2.6.2-2. # Expected results Disk boots just fine via virtio like it did before. # Downstream bug report Downstream suggests an issue with upstream qemu-kvm: https://bugzilla.proxmox.com/show_bug.cgi?id=1181 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1636217/+subscriptions
Re: [Qemu-devel] [OpenRISC] OpenRISC: SMP support for more than 2 cores
On 07-06-2018 12:56, Richard Henderson wrote: On 06/07/2018 06:27 AM, Davidson Francis wrote: Dear all, Currently Qemu supports only 2 cores when SMP enabled for or1k architecure, so I would like to know if there is a quick way to increase the number of cores by changing a few lines of code or to accomplish this, will requires significant changes in the source code? Probably not significant changes. The limit of 2 seems to be the way the interrupts are wired up. That can probably be extended relatively easily. r~ Thank you Richard, I will investigate how the interrupts are wired. Kind regards, Davidson Francis.
Re: [Qemu-devel] [PATCH v1 8/8] s390x: local error handling in hotplug handler functions
On Thu, 7 Jun 2018 18:52:18 +0200
David Hildenbrand wrote:
> Let's introduce and use local error variables in the hotplug handler
> functions.
>
> Signed-off-by: David Hildenbrand
> ---
> hw/s390x/s390-virtio-ccw.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 7ae5fb38dd..29ea50a177 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -434,18 +434,23 @@ static void s390_machine_reset(void)
> static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error **errp)
> {
> +Error *local_err = NULL;
> +
> if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> -s390_cpu_plug(hotplug_dev, dev, errp);
> +s390_cpu_plug(hotplug_dev, dev, _err);
> }
> +error_propagate(errp, local_err);
> }
>
> static void s390_machine_device_unplug_request(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error
> **errp)
> {
> +Error *local_err = NULL;
> +
> if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> -error_setg(errp, "CPU hot unplug not supported on this machine");
> -return;
> +error_setg(_err, "CPU hot unplug not supported on this
> machine");
> }
> +error_propagate(errp, local_err);
> }
>
> static CpuInstanceProperties s390_cpu_index_to_props(MachineState *ms,
Just seeing this patch by itself, it does not really make much sense.
Even if this is a split out clean-up series, I'd prefer this to go
together with a patch that actually adds something more to the
plug/unplug functions.
Re: [Qemu-devel] [PATCH v1 3/8] spapr: move all DIMM checks into spapr_memory_plug
On Thu, 7 Jun 2018 18:52:13 +0200
David Hildenbrand wrote:
> Let's clean the hotplug handler up by moving everything into
> spapr_memory_plug().
>
> Signed-off-by: David Hildenbrand
> ---
> hw/ppc/spapr.c | 23 ++-
> 1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d038f3243e..a12be24ca9 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3136,14 +3136,22 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t
> addr_start, uint64_t size,
> }
>
> static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> - uint32_t node, Error **errp)
> + Error **errp)
> {
> Error *local_err = NULL;
> sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(ms);
> PCDIMMDevice *dimm = PC_DIMM(dev);
> PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> MemoryRegion *mr;
> uint64_t align, size, addr;
> +uint32_t node;
> +
> +if (!smc->dr_lmb_enabled) {
> +error_setg(_err, "Memory hotplug not supported for this
> machine");
> +goto out;
> +}
Wouldn't it be more appropriate to move this check to spapr_memory_pre_plug() ?
> +node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP, NULL);
>
> mr = ddc->get_memory_region(dimm, _err);
> if (local_err) {
> @@ -3568,19 +3576,8 @@ out:
> static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
>DeviceState *dev, Error **errp)
> {
> -MachineState *ms = MACHINE(hotplug_dev);
> -sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(ms);
> -
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> -int node;
> -
> -if (!smc->dr_lmb_enabled) {
> -error_setg(errp, "Memory hotplug not supported for this
> machine");
> -return;
> -}
> -node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
> NULL);
> -
> -spapr_memory_plug(hotplug_dev, dev, node, errp);
> +spapr_memory_plug(hotplug_dev, dev, errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> spapr_core_plug(hotplug_dev, dev, errp);
> }
Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues
On Fri, Jun 08, 2018 at 12:42:35PM +0800, Peter Xu wrote: > On Thu, Jun 07, 2018 at 01:53:01PM +0200, Markus Armbruster wrote: > > Peter Xu writes: > > > > > Previously we cleanup the queues when we got CLOSED event. It was used > > > > we clean up > > > > > to make sure we won't leftover replies/events of a old client to a new > > > > we won't send leftover replies/events of an old client > > > > > client. Now this patch postpones that until OPENED. > > > > What if OPENED never comes? > > Then we clean that up until destruction of the monitor. IMHO it's > fine, but I'm not sure whether that's an overall acceptable approach. I think this patch fixes the problem at the wrong level. Marc-André's fix seemed like a cleaner solution. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v1 3/8] spapr: move all DIMM checks into spapr_memory_plug
On Fri, 8 Jun 2018 10:07:59 +0200
David Hildenbrand wrote:
> On 08.06.2018 10:05, Greg Kurz wrote:
> > On Thu, 7 Jun 2018 18:52:13 +0200
> > David Hildenbrand wrote:
> >
> >> Let's clean the hotplug handler up by moving everything into
> >> spapr_memory_plug().
> >>
> >> Signed-off-by: David Hildenbrand
> >> ---
> >> hw/ppc/spapr.c | 23 ++-
> >> 1 file changed, 10 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index d038f3243e..a12be24ca9 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -3136,14 +3136,22 @@ static void spapr_add_lmbs(DeviceState *dev,
> >> uint64_t addr_start, uint64_t size,
> >> }
> >>
> >> static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState
> >> *dev,
> >> - uint32_t node, Error **errp)
> >> + Error **errp)
> >> {
> >> Error *local_err = NULL;
> >> sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
> >> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(ms);
> >> PCDIMMDevice *dimm = PC_DIMM(dev);
> >> PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> >> MemoryRegion *mr;
> >> uint64_t align, size, addr;
> >> +uint32_t node;
> >> +
> >> +if (!smc->dr_lmb_enabled) {
> >> +error_setg(_err, "Memory hotplug not supported for this
> >> machine");
> >> +goto out;
> >> +}
> >
> > Wouldn't it be more appropriate to move this check to
> > spapr_memory_pre_plug() ?
>
> Think you're right (and as spapr_memory_pre_plug() already exists, it's
> easy), other opinions? Thanks.
I also think that it should go to preplug
>
> >
> >> +node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP, NULL);
> >>
> >> mr = ddc->get_memory_region(dimm, _err);
> >> if (local_err) {
> >> @@ -3568,19 +3576,8 @@ out:
> >> static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> >>DeviceState *dev, Error **errp)
> >> {
> >> -MachineState *ms = MACHINE(hotplug_dev);
> >> -sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(ms);
> >> -
> >> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> >> -int node;
> >> -
> >> -if (!smc->dr_lmb_enabled) {
> >> -error_setg(errp, "Memory hotplug not supported for this
> >> machine");
> >> -return;
> >> -}
> >> -node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
> >> NULL);
> >> -
> >> -spapr_memory_plug(hotplug_dev, dev, node, errp);
> >> +spapr_memory_plug(hotplug_dev, dev, errp);
> >> } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> >> spapr_core_plug(hotplug_dev, dev, errp);
> >> }
> >
>
>
Re: [Qemu-devel] [RFC v2 04/12] Add vhost-user-backend
On Fri, Jun 08, 2018 at 12:34:15AM +0200, Marc-André Lureau wrote:
> Hi
>
> On Mon, Jun 4, 2018 at 11:36 AM, Daniel P. Berrangé
> wrote:
> > On Fri, Jun 01, 2018 at 06:27:41PM +0200, Marc-André Lureau wrote:
> >> Create a vhost-user-backend object that holds a connection to a
> >> vhost-user backend and can be referenced from virtio devices that
> >> support it. See later patches for input & gpu usage.
> >>
> >> A chardev can be specified to communicate with the vhost-user backend,
> >> ex: -chardev socket,id=char0,path=/tmp/foo.sock -object
> >> vhost-user-backend,id=vuid,chardev=char0.
> >>
> >> Alternatively, an executable with its arguments may be given as 'cmd'
> >> property, ex: -object
> >> vhost-user-backend,id=vui,cmd="./vhost-user-input /dev/input..". The
> >> executable is then spawn and, by convention, the vhost-user socket is
> >> passed as fd=3. It may be considered a security breach to allow
> >> creating processes that may execute arbitrary executables, so this may
> >> be restricted to some known executables (via signature etc) or
> >> directory.
> >
> > Passing a binary and args as a string blob.
> >
> >> +static int
> >> +vhost_user_backend_spawn_cmd(VhostUserBackend *b, int vhostfd, Error
> >> **errp)
> >> +{
> >> +int devnull = open("/dev/null", O_RDWR);
> >> +pid_t pid;
> >> +
> >> +assert(!b->child);
> >> +
> >> +if (!b->cmd) {
> >> +error_setg_errno(errp, errno, "Missing cmd property");
> >> +return -1;
> >> +}
> >> +if (devnull < 0) {
> >> +error_setg_errno(errp, errno, "Unable to open /dev/null");
> >> +return -1;
> >> +}
> >> +
> >> +pid = qemu_fork(errp);
> >> +if (pid < 0) {
> >> +close(devnull);
> >> +return -1;
> >> +}
> >> +
> >> +if (pid == 0) { /* child */
> >> +int fd, maxfd = sysconf(_SC_OPEN_MAX);
> >> +
> >> +dup2(devnull, STDIN_FILENO);
> >> +dup2(devnull, STDOUT_FILENO);
> >> +dup2(vhostfd, 3);
> >> +
> >> +signal(SIGINT, SIG_IGN);
> >
> > Why ignore SIGINT ? Surely we want this extra process to be killed
> > someone ctrl-c's the parent QEMU.
>
> leftover, removed
>
> >
> >> +
> >> +for (fd = 4; fd < maxfd; fd++) {
> >> +close(fd);
> >> +}
> >> +
> >> +execlp("/bin/sh", "sh", "-c", b->cmd, NULL);
> >
> > ...which is then interpreted by the shell is a recipe for security
> > flaws. There needs to be a way to pass the command + arguments
> > to QEMU as an argv[] we can directly exec without involving the
> > shell.
> >
>
> For now, I use g_shell_parse_argv(). Do you have a better idea?
Accept individual args at the cli level is far preferrable - we don't
want anything to be parsing shell strings:
vhost-user-backend,id=vui,binary=/sbin/vhost-user-input,arg=/dev/input,arg=foo,arg=bar
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v2 1/8] ppc4xx_i2c: Clean up and improve error logging
On Fri, 8 Jun 2018, David Gibson wrote: On Wed, Jun 06, 2018 at 12:56:32PM -0300, Philippe Mathieu-Daudé wrote: On 06/06/2018 10:31 AM, BALATON Zoltan wrote: Make it more readable by converting register indexes to decimal (avoids lot of superfluous 0x0) and distinguish errors caused by accessing non-existent vs. unimplemented registers. No functional change. Signed-off-by: BALATON Zoltan --- hw/i2c/ppc4xx_i2c.c | 94 + 1 file changed, 51 insertions(+), 43 deletions(-) diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c index ab64d19..d1936db 100644 --- a/hw/i2c/ppc4xx_i2c.c +++ b/hw/i2c/ppc4xx_i2c.c @@ -31,7 +31,7 @@ #include "hw/hw.h" #include "hw/i2c/ppc4xx_i2c.h" -#define PPC4xx_I2C_MEM_SIZE 0x12 +#define PPC4xx_I2C_MEM_SIZE 18 This looks weird, it seems all memory range sizes are in hex in other QEMU devices. [snip] @@ -263,32 +267,31 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr, uint64_t value, } } break; -case 0x07: -i2c->mdcntl = value & 0xDF; +case 7: +i2c->mdcntl = value & 0xdf; break; -case 0x08: -i2c->sts &= ~(value & 0x0A); +case 8: +i2c->sts &= ~(value & 0xa); 'value & 0x0a' implicitly denotes than 'value' is a 8-bit register. Matter of taste... I tend to prefer the forms you suggest, Philippe, but not by enough to delay this otherwise good cleanup. Especially since Balaton is taking on this long neglected area of the code. I prefer code that does not have unneeded chars so it's easier to read, that's why I've removed all 0x0 from this file which made it more comprehensible. But I'll add the 0 back to this place in next respin as you both seem to prefer that and since other bit masks are two digit too it's also consistent that way.
Re: [Qemu-devel] [Qemu-ppc] [PATCH v3] target/ppc: Allow PIR read in privileged mode
On Wed, Jun 06, 2018 at 11:19:22AM +0200, Greg Kurz wrote: > On Wed, 6 Jun 2018 10:53:17 +1000 > David Gibson wrote: > > > On Tue, Jun 05, 2018 at 06:46:12PM +0200, Greg Kurz wrote: > > > On Mon, 4 Jun 2018 10:53:22 +1000 > > > David Gibson wrote: > > > > > > > On Mon, May 07, 2018 at 01:52:42PM -0300, luporl wrote: > > > > > According to PowerISA, the PIR register should be readable in > > > > > privileged > > > > > mode also, not only in hypervisor privileged mode. > > > > > > > > > > PowerISA 3.0 - 4.3.3 Processor Identification Register > > > > > > > > > > "Read access to the PIR is privileged; write access is not > > > > > provided." > > > > > > > > Yes... but a little further down it says "The PIR is a hypervisor > > > > resource". Looking at the older 2.07 ISA, it says that > > > > guest-supervisor mode reads to the PIR should be redirected to the > > > > GPIR register, which this change won't accomplish. > > > > > > > > > > Hmmm, there are two definitions for the PIR, one in Book III-S (4.3.3) > > > and one in Book III-E (5.3.3). It looks like you're referring to the > > > latter... > > > > > > [Category:Embedded.Hypervisor] > > > Read accesses to the PIR in guest supervisor state are > > > mapped to the GPIR. > > > > > > The Book III-S definition doesn't mention the GPIR. > > > > Oops, sorry. Yes the GPIR stuff is only for BookE. The statement > > about the PIR being a hypervisor resource is definitely in the BookS > > section, however (both 2.07 and 3.0). > > > > Yes it is, but IIUC, this means that the guest cannot modify it, eg, > do mtspr. Section 4.4.4 in Book III-S has a list of SPRs that seem to > indicate that mfspr doesn't require hypervisor state with the PIR. Ah, yes, I was looking for a summary that covered that, but hadn't found it yet. The patch doesn't actually apply clean to the current tree any more, due to a rename. So can you repost, and I'll apply. -- 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 v1 6/8] spapr: handle pc-dimm unplug via hotplug handler chain
On Fri, 8 Jun 2018 11:02:23 +0200
David Hildenbrand wrote:
> On 08.06.2018 10:56, Igor Mammedov wrote:
> > On Thu, 7 Jun 2018 18:52:16 +0200
> > David Hildenbrand wrote:
> >
> >> Let's handle it via hotplug_handler_unplug(). E.g. necessary to hotplug/
> >> unplug memory devices (which a pc-dimm is) later.
> > Perhaps something like following would be better:
> >
> > Factor out memory unplug into separate function from spapr_lmb_release().
> > Then use generic hotplug_handler_unplug() to trigger memory unplug,
> > which would call spapr_machine_device_unplug() -> spapr_memory_unplug()
> > in the end .
> > This way unplug operation is not buried in lmb internals and located
> > in the same place like in other targets, following similar
> > logic/call chain across targets.
>
> Can this be an addon patch? Sounds like factoring out more and moving more.
I've suggested ^^^ it as this patch description instead of the current one
that doesn't really makes the sense on it's own.
> >> Signed-off-by: David Hildenbrand
> >> ---
> >> hw/ppc/spapr.c | 18 +++---
> >> 1 file changed, 15 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index bcb72d9fa7..0a8a3455d6 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -3298,7 +3298,8 @@ static sPAPRDIMMState
> >> *spapr_recover_pending_dimm_state(sPAPRMachineState *ms,
> >> /* Callback to be called during DRC release. */
> >> void spapr_lmb_release(DeviceState *dev)
> >> {
> >> -sPAPRMachineState *spapr =
> >> SPAPR_MACHINE(qdev_get_hotplug_handler(dev));
> >> +HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(dev);
> >> +sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_ctrl);
> >> sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr,
> >> PC_DIMM(dev));
> >>
> >> /* This information will get lost if a migration occurs
> >> @@ -3316,9 +3317,17 @@ void spapr_lmb_release(DeviceState *dev)
> >>
> >> /*
> >> * Now that all the LMBs have been removed by the guest, call the
> >> - * pc-dimm unplug handler to cleanup up the pc-dimm device.
> >> + * unplug handler chain. This can never fail.
> >> */
> >> -pc_dimm_memory_unplug(dev, MACHINE(spapr));
> >> +hotplug_handler_unplug(hotplug_ctrl, dev, _abort);
> >> +}
> >> +
> >> +static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState
> >> *dev)
> >> +{
> >> +sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
> >> +sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr,
> >> PC_DIMM(dev));
> >> +
> >> +pc_dimm_memory_unplug(dev, MACHINE(hotplug_dev));
> >> object_unparent(OBJECT(dev));
> >> spapr_pending_dimm_unplugs_remove(spapr, ds);
> >> }
> >> @@ -3589,6 +3598,9 @@ static void spapr_machine_device_plug(HotplugHandler
> >> *hotplug_dev,
> >> static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
> >> DeviceState *dev, Error **errp)
> >> {
> >> +if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> >> +spapr_memory_unplug(hotplug_dev, dev);
> >> +}
> >> }
> >>
> >> static void spapr_machine_device_unplug_request(HotplugHandler
> >> *hotplug_dev,
> >
>
>
Re: [Qemu-devel] [qemu-s390x] [PATCH v1 2/8] spapr: no need to verify the node
On Fri, 8 Jun 2018 13:28:01 +0200
David Hildenbrand wrote:
> On 08.06.2018 12:52, Greg Kurz wrote:
> > On Fri, 8 Jun 2018 11:24:51 +0200
> > David Hildenbrand wrote:
> >
> >> +1 for error_abort, even if it takes another line.
> > +1 for error_abort
> > call shouldn't fail, but if does it won't be silently ignored
> > and introduce undefined behavior.
>
> Maybe we should fix the others that pass in NULL.
>
> (no, not me :D - I'm already busy with your requested pre_plug handling)
>
> >>> Add it to wiki page for bite sized tasks?
> >>
> >> Done.
> >>
> >>
> >
> > FWIW, I've also added a line to check and possibly fix places where we do
> > 'if (*errp)', which would cause QEMU to crash if the caller passes NULL.
> >
> > $ git grep 'if (\*errp)'
> > hmp.c:if (*errp) {
> > hw/ipmi/isa_ipmi_bt.c:if (*errp)
> > hw/ipmi/isa_ipmi_kcs.c:if (*errp)
> > hw/mem/memory-device.c:if (*errp) {
> > hw/mem/memory-device.c:if (*errp) {
> > hw/ppc/spapr.c:if (*errp) {
> > hw/s390x/event-facility.c:if (*errp) {
> > include/qapi/error.h: * if (*errp) { // WRONG!
> > qga/commands-posix.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > target/s390x/cpu_models.c:if (*errp) {
> > tests/test-crypto-tlscredsx509.c:if (*errp) {
> > tests/test-io-channel-tls.c:if (*errp) {
> >
>
> I think the more important part is actually looking out for people that
> use NULL instead of error_abort. This way we won't silently ignore errors.
I think we can assume that the callers here all pass in !NULL. Would
probably make sense to change these anyway because (a) better safe than
sorry, and (b) make sure new code does not copy it.
Re: [Qemu-devel] [PATCH v3 0/6] ramfb: simple boot framebuffer, no legacy vga
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20180608112001.14729-1-kra...@redhat.com
Subject: [Qemu-devel] [PATCH v3 0/6] ramfb: simple boot framebuffer, no legacy
vga
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
a674da0ab7..bac5ba3dc5 master -> master
* [new tag] patchew/20180608112001.14729-1-kra...@redhat.com ->
patchew/20180608112001.14729-1-kra...@redhat.com
Switched to a new branch 'test'
b3b768bdcf bochs-display: enable vgabios
be08ed14be ramfb: enable vgabios
f2f2069625 hw/vfio/display: add ramfb support
717edb2f4d hw/display: add virtio-ramfb device
cfb87c9b3b hw/display: add standalone ramfb device
add39ce518 hw/display: add ramfb, a simple boot framebuffer living in guest ram
=== OUTPUT BEGIN ===
Checking PATCH 1/6: hw/display: add ramfb, a simple boot framebuffer living in
guest ram...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41:
new file mode 100644
total: 0 errors, 1 warnings, 109 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 2/6: hw/display: add standalone ramfb device...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#71:
new file mode 100644
total: 0 errors, 1 warnings, 141 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 3/6: hw/display: add virtio-ramfb device...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32:
new file mode 100644
total: 0 errors, 1 warnings, 157 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 4/6: hw/vfio/display: add ramfb support...
ERROR: braces {} are necessary for all arms of this statement
#31: FILE: hw/vfio/display.c:187:
+if (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
[...]
ERROR: braces {} are necessary for all arms of this statement
#50: FILE: hw/vfio/display.c:311:
+if (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
[...]
total: 2 errors, 0 warnings, 72 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 5/6: ramfb: enable vgabios...
Checking PATCH 6/6: bochs-display: enable vgabios...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
[Qemu-devel] [PATCH 5/6] block: Add backing passthrough implementations for copy_range
Similar to bdrv_co_block_status_from_backing we add the two passthrough
callbacks for copy_range. This will be used by the block driver filters
so that they can support copy offloading.
Signed-off-by: Fam Zheng
---
block/io.c| 24
include/block/block_int.h | 22 ++
2 files changed, 46 insertions(+)
diff --git a/block/io.c b/block/io.c
index d8039793c2..d1559c9cd5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1900,6 +1900,30 @@ int coroutine_fn
bdrv_co_block_status_from_backing(BlockDriverState *bs,
return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID;
}
+int coroutine_fn bdrv_co_copy_range_from_backing(BlockDriverState *bs,
+ BdrvChild *src, uint64_t
src_offset,
+ BdrvChild *dst, uint64_t
dst_offset,
+ uint64_t bytes,
BdrvRequestFlags flags)
+{
+if (!src->bs) {
+return -ENOMEDIUM;
+}
+return bdrv_co_copy_range_from(src->bs->backing, src_offset, dst,
+ dst_offset, bytes, flags);
+}
+
+int coroutine_fn bdrv_co_copy_range_to_backing(BlockDriverState *bs,
+ BdrvChild *src, uint64_t
src_offset,
+ BdrvChild *dst, uint64_t
dst_offset,
+ uint64_t bytes,
BdrvRequestFlags flags)
+{
+if (!dst->bs) {
+return -ENOMEDIUM;
+}
+return bdrv_co_copy_range_to(src, src_offset, dst->bs->backing,
+ dst_offset, bytes, flags);
+}
+
/*
* Returns the allocation status of the specified sectors.
* Drivers not implementing the functionality are assumed to not support
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2c51cd420f..b488d74c1b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1118,6 +1118,28 @@ int coroutine_fn
bdrv_co_block_status_from_backing(BlockDriverState *bs,
int64_t *pnum,
int64_t *map,
BlockDriverState **file);
+
+/*
+ * Default implementation for drivers to pass bdrv_co_copy_range_from() to
+ * their backing file.
+ */
+int coroutine_fn bdrv_co_copy_range_from_backing(BlockDriverState *bs,
+ BdrvChild *src, uint64_t
src_offset,
+ BdrvChild *dst, uint64_t
dst_offset,
+ uint64_t bytes,
+ BdrvRequestFlags flags);
+
+
+/*
+ * Default implementation for drivers to pass bdrv_co_copy_range_to() to their
+ * backing file.
+ */
+int coroutine_fn bdrv_co_copy_range_to_backing(BlockDriverState *bs,
+ BdrvChild *src, uint64_t
src_offset,
+ BdrvChild *dst, uint64_t
dst_offset,
+ uint64_t bytes,
+ BdrvRequestFlags flags);
+
const char *bdrv_get_parent_name(const BlockDriverState *bs);
void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp);
bool blk_dev_has_removable_media(BlockBackend *blk);
--
2.17.0
[Qemu-devel] [PATCH 1/6] file-posix: Fix EINTR handling
EINTR should be checked against errno, not ret. While fixing the bug,
collecting the branches with a switch block.
Signed-off-by: Fam Zheng
---
block/file-posix.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 513d371bb1..c6dae38f94 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1473,20 +1473,20 @@ static ssize_t handle_aiocb_copy_range(RawPosixAIOData
*aiocb)
ssize_t ret = copy_file_range(aiocb->aio_fildes, _off,
aiocb->aio_fd2, _off,
bytes, 0);
-if (ret == -EINTR) {
-continue;
-}
-if (ret < 0) {
-if (errno == ENOSYS) {
+if (ret <= 0) {
+switch (errno) {
+case 0:
+/* No progress (e.g. when beyond EOF), let the caller fall back
+ * to buffer I/O. */
+/* fall through */
+case ENOSYS:
return -ENOTSUP;
-} else {
+case EINTR:
+continue;
+default:
return -errno;
}
}
-if (!ret) {
-/* No progress (e.g. when beyond EOF), fall back to buffer I/O. */
-return -ENOTSUP;
-}
bytes -= ret;
}
return 0;
--
2.17.0
[Qemu-devel] [PATCH 0/6] mirror: Use copy offloading
This is the third part of copy offloading work. The first patches are fixes and improvements in preparation for enabling mirror job. The last patch does a similar change to the backup patch: it inserts a blk_aio_copy_range call before the usual bounce buffer code in mirror_iteration. Fam Zheng (6): file-posix: Fix EINTR handling block: Check if block drivers can do copy offloading block-backend: Refactor AIO emulation block-backend: Add blk_aio_copy_range block: Add backing passthrough implementations for copy_range mirror: Use copy offloading block.c| 12 ++ block/block-backend.c | 247 +++-- block/file-posix.c | 29 ++-- block/io.c | 27 block/iscsi.c | 8 ++ block/mirror.c | 71 +- block/qcow2.c | 11 ++ block/raw-format.c | 6 + block/trace-events | 1 + include/block/block_int.h | 26 include/sysemu/block-backend.h | 4 + 11 files changed, 354 insertions(+), 88 deletions(-) -- 2.17.0
Re: [Qemu-devel] [PATCH v10 5/7] monitor: remove event_clock_type
Peter Xu writes: > On Fri, Jun 08, 2018 at 07:38:11AM +0200, Markus Armbruster wrote: > > [...] > >> > +/* >> > + * This should never be called before configure_accelerator() since >> > + * only until then could we know whether qtest was enabled or not. >> >> Uh, we know it after then, not until then. What about >> >>/* >> * Return the clock to use for recording an event's time. >> * Beware: result is invalid before configure_accelerator(). > > Oh, another Chinglish from me. :( Useful code, occasional Chinglish, the humility to go with it, and a sense of humor --- you're doing fine. [...]
Re: [Qemu-devel] [PATCH v1 2/8] spapr: no need to verify the node
On Thu, 7 Jun 2018 18:52:12 +0200
David Hildenbrand wrote:
> The node property can always be queried and the value has already been
> verified in pc_dimm_realize().
>
> Signed-off-by: David Hildenbrand
> ---
> hw/ppc/spapr.c | 9 +
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 2375cbee12..d038f3243e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3578,14 +3578,7 @@ static void spapr_machine_device_plug(HotplugHandler
> *hotplug_dev,
> error_setg(errp, "Memory hotplug not supported for this
> machine");
> return;
> }
> -node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
> errp);
> -if (*errp) {
Good riddance :)
> -return;
> -}
> -if (node < 0 || node >= MAX_NODES) {
> -error_setg(errp, "Invaild node %d", node);
> -return;
> -}
> +node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
> NULL);
Maybe pass _abort ?
>
> spapr_memory_plug(hotplug_dev, dev, node, errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
Re: [Qemu-devel] [qemu-s390x] [PATCH v2 2/4] pc-bios/s390-ccw/net: Update code for the latest changes in SLOF
On 06/07/2018 02:22 PM, Thomas Huth wrote:
> The ip_version information now has to be stored in the filename_ip_t
> structure, and there is now a common function called tftp_get_error_info()
> which can be used to get the error string for a TFTP error code.
> We can also get rid of some superfluous "(char *)" casts now.
>
> Signed-off-by: Thomas Huth
Acked-by: Christian Borntraeger
> ---
> pc-bios/s390-ccw/netboot.mak | 2 +-
> pc-bios/s390-ccw/netmain.c | 86
> +---
> 2 files changed, 18 insertions(+), 70 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
> index 4f64128..a73be36 100644
> --- a/pc-bios/s390-ccw/netboot.mak
> +++ b/pc-bios/s390-ccw/netboot.mak
> @@ -34,7 +34,7 @@ STDLIB_OBJS = atoi.o atol.o strtoul.o strtol.o rand.o
> malloc.o free.o
> %.o : $(SLOF_DIR)/lib/libc/stdlib/%.c
> $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@
> $<,"CC","$(TARGET_DIR)$@")
>
> -STDIO_OBJS = sprintf.o vfprintf.o vsnprintf.o vsprintf.o fprintf.o \
> +STDIO_OBJS = sprintf.o snprintf.o vfprintf.o vsnprintf.o vsprintf.o
> fprintf.o \
>printf.o putc.o puts.o putchar.o stdchnls.o fileno.o
> %.o : $(SLOF_DIR)/lib/libc/stdio/%.c
> $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@
> $<,"CC","$(TARGET_DIR)$@")
> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
> index 6000241..d007fb7 100644
> --- a/pc-bios/s390-ccw/netmain.c
> +++ b/pc-bios/s390-ccw/netmain.c
> @@ -47,7 +47,6 @@ IplParameterBlock iplb __attribute__((aligned(PAGE_SIZE)));
> static char cfgbuf[2048];
>
> static SubChannelId net_schid = { .one = 1 };
> -static int ip_version = 4;
> static uint64_t dest_timer;
>
> static uint64_t get_timer_ms(void)
> @@ -100,10 +99,10 @@ static int dhcp(struct filename_ip *fn_ip, int retries)
> printf("\nGiving up after %d DHCP requests\n", retries);
> return -1;
> }
> -ip_version = 4;
> +fn_ip->ip_version = 4;
> rc = dhcpv4(NULL, fn_ip);
> if (rc == -1) {
> -ip_version = 6;
> +fn_ip->ip_version = 6;
> set_ipv6_address(fn_ip->fd, 0);
> rc = dhcpv6(NULL, fn_ip);
> if (rc == 0) {
> @@ -137,8 +136,7 @@ static int tftp_load(filename_ip_t *fnip, void *buffer,
> int len)
> tftp_err_t tftp_err;
> int rc;
>
> -rc = tftp(fnip, buffer, len, DEFAULT_TFTP_RETRIES, _err, 1, 1428,
> - ip_version);
> +rc = tftp(fnip, buffer, len, DEFAULT_TFTP_RETRIES, _err);
>
> if (rc < 0) {
> /* Make sure that error messages are put into a new line */
> @@ -149,61 +147,11 @@ static int tftp_load(filename_ip_t *fnip, void *buffer,
> int len)
> printf(" TFTP: Received %s (%d KBytes)\n", fnip->filename, rc /
> 1024);
> } else if (rc > 0) {
> printf(" TFTP: Received %s (%d Bytes)\n", fnip->filename, rc);
> -} else if (rc == -1) {
> -puts("unknown TFTP error");
> -} else if (rc == -2) {
> -printf("TFTP buffer of %d bytes is too small for %s\n",
> -len, fnip->filename);
> -} else if (rc == -3) {
> -printf("file not found: %s\n", fnip->filename);
> -} else if (rc == -4) {
> -puts("TFTP access violation");
> -} else if (rc == -5) {
> -puts("illegal TFTP operation");
> -} else if (rc == -6) {
> -puts("unknown TFTP transfer ID");
> -} else if (rc == -7) {
> -puts("no such TFTP user");
> -} else if (rc == -8) {
> -puts("TFTP blocksize negotiation failed");
> -} else if (rc == -9) {
> -puts("file exceeds maximum TFTP transfer size");
> -} else if (rc <= -10 && rc >= -15) {
> -const char *icmp_err_str;
> -switch (rc) {
> -case -ICMP_NET_UNREACHABLE - 10:
> -icmp_err_str = "net unreachable";
> -break;
> -case -ICMP_HOST_UNREACHABLE - 10:
> -icmp_err_str = "host unreachable";
> -break;
> -case -ICMP_PROTOCOL_UNREACHABLE - 10:
> -icmp_err_str = "protocol unreachable";
> -break;
> -case -ICMP_PORT_UNREACHABLE - 10:
> -icmp_err_str = "port unreachable";
> -break;
> -case -ICMP_FRAGMENTATION_NEEDED - 10:
> -icmp_err_str = "fragmentation needed and DF set";
> -break;
> -case -ICMP_SOURCE_ROUTE_FAILED - 10:
> -icmp_err_str = "source route failed";
> -break;
> -default:
> -icmp_err_str = " UNKNOWN";
> -break;
> -}
> -printf("ICMP ERROR \"%s\"\n", icmp_err_str);
> -} else if (rc == -40) {
> -printf("TFTP error occurred after %d bad packets received",
> -tftp_err.bad_tftp_packets);
> -} else if (rc == -41) {
> -printf("TFTP error occurred after missing %d responses",
> -tftp_err.no_packets);
> -
Re: [Qemu-devel] [PATCH v1 2/8] spapr: no need to verify the node
On Fri, 8 Jun 2018 09:42:48 +0200
David Hildenbrand wrote:
> On 08.06.2018 09:34, Greg Kurz wrote:
> > On Thu, 7 Jun 2018 18:52:12 +0200
> > David Hildenbrand wrote:
> >
> >> The node property can always be queried and the value has already been
> >> verified in pc_dimm_realize().
> >>
> >> Signed-off-by: David Hildenbrand
> >> ---
> >> hw/ppc/spapr.c | 9 +
> >> 1 file changed, 1 insertion(+), 8 deletions(-)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 2375cbee12..d038f3243e 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -3578,14 +3578,7 @@ static void
> >> spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> >> error_setg(errp, "Memory hotplug not supported for this
> >> machine");
> >> return;
> >> }
> >> -node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
> >> errp);
> >> -if (*errp) {
> >
> > Good riddance :)
> >
> >> -return;
> >> -}
> >> -if (node < 0 || node >= MAX_NODES) {
> >> -error_setg(errp, "Invaild node %d", node);
> >> -return;
> >> -}
> >> +node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
> >> NULL);
> >
> > Maybe pass _abort ?
>
> I'm using the same access scheme as in hw/acpi/memory_hotplug.c
>
> ("error ignored" vs. "error leads to an abort") - but this will actually
> never fail. But I can use error_abort here, does not matter.
>
Heh, /me paranoid but this is David's call and he acked that already
so I guess it's okay.
Reviewed-by: Greg Kurz
> Thanks!
>
> >
> >>
> >> spapr_memory_plug(hotplug_dev, dev, node, errp);
> >> } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> >
>
>
[Qemu-devel] [PATCH v8 3/6] migration: API to clear bits of guest free pages from the dirty bitmap
This patch adds an API to clear bits corresponding to guest free pages
from the dirty bitmap. Spilt the free page block if it crosses the QEMU
RAMBlock boundary.
Signed-off-by: Wei Wang
CC: Dr. David Alan Gilbert
CC: Juan Quintela
CC: Michael S. Tsirkin
CC: Peter Xu
---
include/migration/misc.h | 2 ++
migration/migration.c| 2 +-
migration/migration.h| 1 +
migration/ram.c | 48
4 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 4ebf24c..113320e 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -14,11 +14,13 @@
#ifndef MIGRATION_MISC_H
#define MIGRATION_MISC_H
+#include "exec/cpu-common.h"
#include "qemu/notify.h"
/* migration/ram.c */
void ram_mig_init(void);
+void qemu_guest_free_page_hint(void *addr, size_t len);
/* migration/block.c */
diff --git a/migration/migration.c b/migration/migration.c
index 05aec2c..220ff48 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -647,7 +647,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error
**errp)
* Return true if we're already in the middle of a migration
* (i.e. any of the active or setup states)
*/
-static bool migration_is_setup_or_active(int state)
+bool migration_is_setup_or_active(int state)
{
switch (state) {
case MIGRATION_STATUS_ACTIVE:
diff --git a/migration/migration.h b/migration/migration.h
index 8f0c821..5a74740 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -230,6 +230,7 @@ void migrate_fd_error(MigrationState *s, const Error
*error);
void migrate_fd_connect(MigrationState *s, Error *error_in);
void migrate_init(MigrationState *s);
+bool migration_is_setup_or_active(int state);
bool migration_is_blocked(Error **errp);
/* True if outgoing migration has entered postcopy phase */
bool migration_in_postcopy(void);
diff --git a/migration/ram.c b/migration/ram.c
index 2eabbe9..237f11e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2530,6 +2530,54 @@ static void ram_state_resume_prepare(RAMState *rs,
QEMUFile *out)
}
/*
+ * This function clears bits of the free pages reported by the caller from the
+ * migration dirty bitmap. @addr is the host address corresponding to the
+ * start of the continuous guest free pages, and @len is the total bytes of
+ * those pages.
+ */
+void qemu_guest_free_page_hint(void *addr, size_t len)
+{
+RAMBlock *block;
+ram_addr_t offset;
+size_t used_len, start, npages;
+MigrationState *s = migrate_get_current();
+
+/* This function is currently expected to be used during live migration */
+if (!migration_is_setup_or_active(s->state)) {
+return;
+}
+
+for (; len > 0; len -= used_len) {
+block = qemu_ram_block_from_host(addr, false, );
+assert(block);
+
+/*
+ * This handles the case that the RAMBlock is resized after the free
+ * page hint is reported.
+ */
+if (unlikely(offset > block->used_length)) {
+return;
+}
+
+if (len <= block->used_length - offset) {
+used_len = len;
+} else {
+used_len = block->used_length - offset;
+addr += used_len;
+}
+
+start = offset >> TARGET_PAGE_BITS;
+npages = used_len >> TARGET_PAGE_BITS;
+
+qemu_mutex_lock(_state->bitmap_mutex);
+ram_state->migration_dirty_pages -=
+ bitmap_count_one_with_offset(block->bmap, start, npages);
+bitmap_clear(block->bmap, start, npages);
+qemu_mutex_unlock(_state->bitmap_mutex);
+}
+}
+
+/*
* Each of ram_save_setup, ram_save_iterate and ram_save_complete has
* long-running RCU critical section. When rcu-reclaims in the code
* start to become numerous it will be necessary to reduce the
--
1.8.3.1
[Qemu-devel] [PATCH v8 0/6] virtio-balloon: free page hint reporting support
This is the deivce part implementation to add a new feature,
VIRTIO_BALLOON_F_FREE_PAGE_HINT to the virtio-balloon device. The device
receives the guest free page hints from the driver and clears the
corresponding bits in the dirty bitmap, so that those free pages are
not transferred by the migration thread to the destination.
- Test Environment
Host: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
Guest: 8G RAM, 4 vCPU
Migration setup: migrate_set_speed 100G, migrate_set_downtime 2 second
- Test Results
- Idle Guest Live Migration Time (results are averaged over 10 runs):
- Optimization v.s. Legacy = 271ms vs 1769ms --> ~86% reduction
- Guest with Linux Compilation Workload (make bzImage -j4):
- Live Migration Time (average)
Optimization v.s. Legacy = 1265ms v.s. 2634ms --> ~51% reduction
- Linux Compilation Time
Optimization v.s. Legacy = 4min56s v.s. 5min3s
--> no obvious difference
- Source Code
- QEMU: https://github.com/wei-w-wang/qemu-free-page-lm.git
- Linux: https://github.com/wei-w-wang/linux-free-page-lm.git
ChangeLog:
v7->v8:
1) migration:
- qemu_guest_free_page_hint:
- check if this function is called during migration;
- assert when the block of a given hint is not found;
- add a ram save state notifier list;
- move migrate_postcopy to an outside header for other
subsystems (e.g. notifier callbacks) to use;
2) virtio-balloon:
- start/stop the optimization via a notifier, and reduce the
related global balloon interfaces;
- virtio_balloon_poll_free_page_hints: move the while loop into
a function;
- put page poison value into a separate vmsd subsection.
v6->v7:
virtio-balloon/virtio_balloo_poll_free_page_hints:
- add virtio_notify() at the end to notify the driver that
the optimization is done, which indicates that the entries
have all been put back to the vq and ready to detach them.
v5->v6:
virtio-balloon: use iothread to get free page hint
v4->v5:
1) migration:
- bitmap_clear_dirty: update the dirty bitmap and dirty page
count under the bitmap mutex as what other functions are doing;
- qemu_guest_free_page_hint:
- add comments for this function;
- check the !block case;
- check "offset > block->used_length" before proceed;
- assign used_len inside the for{} body;
- update the dirty bitmap and dirty page counter under the
bitmap mutex;
- ram_state_reset:
- rs->free_page_support: && with use "migrate_postcopy"
instead of migration_in_postcopy;
- clear the ram_bulk_stage flag if free_page_support is true;
2) balloon:
- add the usage documentation of balloon_free_page_start and
balloon_free_page_stop in code;
- the optimization thread is named "balloon_fpo" to meet the
requirement of "less than 14 characters";
- virtio_balloon_poll_free_page_hints:
- run on condition when runstate_is_running() is true;
- add a qemu spin lock to synchronize accesses to the free
page reporting related fields shared among the migration
thread and the optimization thread;
- virtio_balloon_free_page_start: just return if
runstate_is_running is false;
- virtio_balloon_free_page_stop: access to the free page
reporting related fields under a qemu spin lock;
- virtio_balloon_device_unrealize/reset: call
virtio_balloon_free_page_stop is the free page hint feature is
used;
- virtio_balloon_set_status: call irtio_balloon_free_page_stop
in case the guest is stopped by qmp when the optimization is
running;
v3->v4:
1) bitmap: add a new API to count 1s starting from an offset of a
bitmap
2) migration:
- qemu_guest_free_page_hint: calculate
ram_state->migration_dirty_pages by counting how many bits of
free pages are truely cleared. If some of the bits were
already 0, they shouldn't be deducted by
ram_state->migration_dirty_pages. This wasn't needed for
previous versions since we optimized bulk stage only,
where all bits are guaranteed to be set. It's needed now
because we extened the usage of this optimizaton to all stages
except the last stop stage. From 2nd stage onward, there
are possibilities that some bits of free pages are already 0.
3) virtio-balloon:
- virtio_balloon_free_page_report_status: introduce a new status,
FREE_PAGE_REPORT_S_EXIT. This status indicates that the
optimization thread has exited. FREE_PAGE_REPORT_S_STOP means
the reporting is stopped, but the
Re: [Qemu-devel] [PATCH v1 6/8] spapr: handle pc-dimm unplug via hotplug handler chain
On 08.06.2018 10:56, Igor Mammedov wrote:
> On Thu, 7 Jun 2018 18:52:16 +0200
> David Hildenbrand wrote:
>
>> Let's handle it via hotplug_handler_unplug(). E.g. necessary to hotplug/
>> unplug memory devices (which a pc-dimm is) later.
> Perhaps something like following would be better:
>
> Factor out memory unplug into separate function from spapr_lmb_release().
> Then use generic hotplug_handler_unplug() to trigger memory unplug,
> which would call spapr_machine_device_unplug() -> spapr_memory_unplug()
> in the end .
> This way unplug operation is not buried in lmb internals and located
> in the same place like in other targets, following similar
> logic/call chain across targets.
Can this be an addon patch? Sounds like factoring out more and moving more.
>
>
>> Signed-off-by: David Hildenbrand
>> ---
>> hw/ppc/spapr.c | 18 +++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index bcb72d9fa7..0a8a3455d6 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -3298,7 +3298,8 @@ static sPAPRDIMMState
>> *spapr_recover_pending_dimm_state(sPAPRMachineState *ms,
>> /* Callback to be called during DRC release. */
>> void spapr_lmb_release(DeviceState *dev)
>> {
>> -sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_hotplug_handler(dev));
>> +HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(dev);
>> +sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_ctrl);
>> sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr,
>> PC_DIMM(dev));
>>
>> /* This information will get lost if a migration occurs
>> @@ -3316,9 +3317,17 @@ void spapr_lmb_release(DeviceState *dev)
>>
>> /*
>> * Now that all the LMBs have been removed by the guest, call the
>> - * pc-dimm unplug handler to cleanup up the pc-dimm device.
>> + * unplug handler chain. This can never fail.
>> */
>> -pc_dimm_memory_unplug(dev, MACHINE(spapr));
>> +hotplug_handler_unplug(hotplug_ctrl, dev, _abort);
>> +}
>> +
>> +static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState
>> *dev)
>> +{
>> +sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
>> +sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr,
>> PC_DIMM(dev));
>> +
>> +pc_dimm_memory_unplug(dev, MACHINE(hotplug_dev));
>> object_unparent(OBJECT(dev));
>> spapr_pending_dimm_unplugs_remove(spapr, ds);
>> }
>> @@ -3589,6 +3598,9 @@ static void spapr_machine_device_plug(HotplugHandler
>> *hotplug_dev,
>> static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
>> DeviceState *dev, Error **errp)
>> {
>> +if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>> +spapr_memory_unplug(hotplug_dev, dev);
>> +}
>> }
>>
>> static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
>
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [PATCH v1 7/8] spapr: handle cpu core unplug via hotplug handler chain
On Thu, 7 Jun 2018 18:52:17 +0200
David Hildenbrand wrote:
> Let's handle it via hotplug_handler_unplug() to make plug/unplug code
> look symmetrical.
>
> Acked-by: Igor Mammedov
> Signed-off-by: David Hildenbrand
> ---
Reviewed-by: Greg Kurz
> hw/ppc/spapr.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0a8a3455d6..994deea8cf 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3415,7 +3415,15 @@ static void *spapr_populate_hotplug_cpu_dt(CPUState
> *cs, int *fdt_offset,
> /* Callback to be called during DRC release. */
> void spapr_core_release(DeviceState *dev)
> {
> -MachineState *ms = MACHINE(qdev_get_hotplug_handler(dev));
> +HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +
> +/* Call the unplug handler chain. This can never fail. */
> +hotplug_handler_unplug(hotplug_ctrl, dev, _abort);
> +}
> +
> +static void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
> +{
> +MachineState *ms = MACHINE(hotplug_dev);
> sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(ms);
> CPUCore *cc = CPU_CORE(dev);
> CPUArchId *core_slot = spapr_find_cpu_slot(ms, cc->core_id, NULL);
> @@ -3600,6 +3608,8 @@ static void spapr_machine_device_unplug(HotplugHandler
> *hotplug_dev,
> {
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> spapr_memory_unplug(hotplug_dev, dev);
> +} else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> +spapr_core_unplug(hotplug_dev, dev);
> }
> }
>
Re: [Qemu-devel] [PATCH v1 6/8] spapr: handle pc-dimm unplug via hotplug handler chain
On 08.06.2018 11:35, Igor Mammedov wrote: > On Fri, 8 Jun 2018 11:02:23 +0200 > David Hildenbrand wrote: > >> On 08.06.2018 10:56, Igor Mammedov wrote: >>> On Thu, 7 Jun 2018 18:52:16 +0200 >>> David Hildenbrand wrote: >>> Let's handle it via hotplug_handler_unplug(). E.g. necessary to hotplug/ unplug memory devices (which a pc-dimm is) later. >>> Perhaps something like following would be better: >>> >>> Factor out memory unplug into separate function from spapr_lmb_release(). >>> Then use generic hotplug_handler_unplug() to trigger memory unplug, >>> which would call spapr_machine_device_unplug() -> spapr_memory_unplug() >>> in the end . >>> This way unplug operation is not buried in lmb internals and located >>> in the same place like in other targets, following similar >>> logic/call chain across targets. >> >> Can this be an addon patch? Sounds like factoring out more and moving more. > I've suggested ^^^ it as this patch description instead of the current one > that doesn't really makes the sense on it's own. Okay, I was definitely misreading your comment :) Will change. -- Thanks, David / dhildenb
[Qemu-devel] [PATCH] block/qcow2-bitmap: fix free_bitmap_clusters
This assert may fail, because bitmap_table is not initialized. Just
drop it, as it's obvious, that bitmap_table_load sets bitmap_table
parameter only when returning zero.
Reported-by: Pavel Butsykin
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/qcow2-bitmap.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 60d5290f10..69485aa1de 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -254,7 +254,6 @@ static int free_bitmap_clusters(BlockDriverState *bs,
Qcow2BitmapTable *tb)
ret = bitmap_table_load(bs, tb, _table);
if (ret < 0) {
-assert(bitmap_table == NULL);
return ret;
}
--
2.11.1
Re: [Qemu-devel] [PATCH v4] target/ppc: Allow PIR read in privileged mode
On Fri, Jun 08, 2018 at 11:46:55AM +0200, Greg Kurz wrote: > From: luporl > > According to PowerISA, the PIR register should be readable in privileged > mode also, not only in hypervisor privileged mode. > > PowerISA 3.0 - 4.3.3 Processor Identification Register > > "Read access to the PIR is privileged; write access is not provided." > > Figure 18 in section 4.4.4 explicitly confirms that mfspr PIR is privileged > and doesn't require hypervisor state. Applied, thanks. > > Cc: David Gibson > Cc: Alexander Graf > Cc: qemu-...@nongnu.org > Signed-off-by: Leandro Lupori > Reviewed-by: Jose Ricardo Ziviani > Reviewed-by: Greg Kurz > Signed-off-by: Greg Kurz > --- > Changes in v2: > - added my Signed-off-by, maintainers CC and Jose's Reviewed-by tags > > Changes in v3: > - added subsystem name, version tag and summary of changes > - added the section of PowerISA that describes PIR access privileges > > Changes in v4 (Greg): > - rebased against ppc-for-3.0 (ie, file is now > target/ppc/translate_init.inc.c) > - added some more context from PowerISA > --- > target/ppc/translate_init.inc.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c > index 1a89017ddea8..bb9296f5a3da 100644 > --- a/target/ppc/translate_init.inc.c > +++ b/target/ppc/translate_init.inc.c > @@ -7819,7 +7819,7 @@ static void gen_spr_book3s_ids(CPUPPCState *env) > /* Processor identification */ > spr_register_hv(env, SPR_PIR, "PIR", > SPR_NOACCESS, SPR_NOACCESS, > - SPR_NOACCESS, SPR_NOACCESS, > + _read_generic, SPR_NOACCESS, > _read_generic, NULL, > 0x); > spr_register_hv(env, SPR_HID0, "HID0", > -- 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 00/11] misc: Add trailing '\n' to qemu_log() calls
On 6 June 2018 at 16:21, Philippe Mathieu-Daudé wrote: > Nothing very exciting here. > I sometimes miss to notice some trace events, running with -d unimp,trace... > then using 'grep ^...'. This is only due to a missing '\n' :) > > Philippe Mathieu-Daudé (11): > hw/sd/milkymist-memcard: Add trailing '\n' to qemu_log() call > hw/digic: Add trailing '\n' to qemu_log() calls > xilinx-dp: Add trailing '\n' to qemu_log() call > ppc/pnv: Add trailing '\n' to qemu_log() calls > hw/core/register: Add trailing '\n' to qemu_log() call > hw/mips/boston: Add trailing '\n' to qemu_log() calls > stellaris: Add trailing '\n' to qemu_log() calls > target/arm: Add trailing '\n' to qemu_log() calls > target/m68k: Add trailing '\n' to qemu_log() call > RISC-V: Add trailing '\n' to qemu_log() calls > RFC target/xtensa: Add trailing '\n' to qemu_log() calls > > hw/arm/stellaris.c| 11 ++- > hw/char/digic-uart.c | 4 ++-- > hw/core/register.c| 2 +- > hw/display/xlnx_dp.c | 4 +++- > hw/mips/boston.c | 8 > hw/ppc/pnv_core.c | 4 ++-- > hw/sd/milkymist-memcard.c | 2 +- > hw/timer/digic-timer.c| 4 ++-- > target/arm/helper.c | 4 ++-- > target/m68k/translate.c | 2 +- > target/riscv/op_helper.c | 6 -- > target/xtensa/translate.c | 6 +++--- > 12 files changed, 31 insertions(+), 26 deletions(-) Applied to target-arm.next, since about half of these are arm and the rest are trivial and have got ack/review by the relevant maintainers. thanks -- PMM
Re: [Qemu-devel] [RFC v2 3/4] monitor: remove "x-oob", turn oob on by default
On Thu, Jun 07, 2018 at 01:40:22PM +0200, Markus Armbruster wrote:
> Peter Xu writes:
>
> > There was a regression reported by Eric Auger before with OOB:
> >
> > http://lists.gnu.org/archive/html/qemu-devel/2018-03/msg06231.html
> >
> > The fix is 951702f39c ("monitor: bind dispatch bh to iohandler context",
> > 2018-04-10), which is in master already.
> >
> > For the bug, we turned Out-Of-Band feature of monitors off for 2.12
> > release. Now we turn that on again after the 2.12 release.
> >
> > This patch partly reverts be933ffc23 (monitor: new parameter "x-oob"),
> > meanwhile turn it on again by default for non-MUX QMPs.
>
> Please add a brief explanation why OOB isn't turned on for MUX. Pointer
> to existing explanation would be fine.
Sure.
--
Peter Xu
Re: [Qemu-devel] [PATCH v1 0/8] pc/spapr/s390x: machine hotplug handler cleanups
On 07.06.2018 18:52, David Hildenbrand wrote: > I'll be messing with machine hotplug handlers of pc/spapr/s390x in the > context of > [PATCH v4 00/14] MemoryDevice: use multi stage hotplug handlers > > So this is a spin-off of the cleanup patches produced so far. > > David Hildenbrand (8): > pc: local error handling in hotplug handler functions > spapr: no need to verify the node > spapr: move all DIMM checks into spapr_memory_plug > spapr: local error handling in hotplug handler functions > spapr: introduce machine unplug handler > spapr: handle pc-dimm unplug via hotplug handler chain > spapr: handle cpu core unplug via hotplug handler chain > s390x: local error handling in hotplug handler functions > > hw/i386/pc.c | 32 +- > hw/ppc/spapr.c | 89 +- > hw/s390x/s390-virtio-ccw.c | 11 +++-- > 3 files changed, 89 insertions(+), 43 deletions(-) > I might be dropping the "local error handling" patches (as Conny and Christian properly asked for the reason). Although they are not wrong, they are only useful if performing checks on error pointers (*errp) or when relying on the error to indicate the "return value" of a function. And as I might be handling plugging of memory devices in subfunctions (to keep the handler short as requested by Igor), this might not be needed on this level. -- Thanks, David / dhildenb
Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues
Stefan Hajnoczi writes: > On Fri, Jun 08, 2018 at 12:42:35PM +0800, Peter Xu wrote: >> On Thu, Jun 07, 2018 at 01:53:01PM +0200, Markus Armbruster wrote: >> > Peter Xu writes: >> > >> > > Previously we cleanup the queues when we got CLOSED event. It was used >> > >> > we clean up >> > >> > > to make sure we won't leftover replies/events of a old client to a new >> > >> > we won't send leftover replies/events of an old client >> > >> > > client. Now this patch postpones that until OPENED. >> > >> > What if OPENED never comes? >> >> Then we clean that up until destruction of the monitor. IMHO it's >> fine, but I'm not sure whether that's an overall acceptable approach. > > I think this patch fixes the problem at the wrong level. Marc-André's > fix seemed like a cleaner solution. Is it the right solution? I proposed another one: [...] >> > Here's what I think we should do on such EOF: >> > >> > 1. Shut down input >> > >> >Stop reading input. If input has its own file descriptor (as opposed >> >to a read/write fd shared with output), close it. >> > >> > 2. Flush output >> > >> >Continue to write output until the queue becomes empty or we run into >> >an error (such as EPIPE, telling us the output's consumer went away). >> >Works exactly as before, except we proceed to step 3 when the queue exactly as before EOF >> >becomes empty. >> > >> > 3. Shut down output >> > >> >Close the output file descriptor.
Re: [Qemu-devel] [PATCH v1 2/8] spapr: no need to verify the node
On Fri, Jun 08, 2018 at 09:46:57AM +0200, Greg Kurz wrote:
> On Fri, 8 Jun 2018 09:42:48 +0200
> David Hildenbrand wrote:
>
> > On 08.06.2018 09:34, Greg Kurz wrote:
> > > On Thu, 7 Jun 2018 18:52:12 +0200
> > > David Hildenbrand wrote:
> > >
> > >> The node property can always be queried and the value has already been
> > >> verified in pc_dimm_realize().
> > >>
> > >> Signed-off-by: David Hildenbrand
> > >> ---
> > >> hw/ppc/spapr.c | 9 +
> > >> 1 file changed, 1 insertion(+), 8 deletions(-)
> > >>
> > >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > >> index 2375cbee12..d038f3243e 100644
> > >> --- a/hw/ppc/spapr.c
> > >> +++ b/hw/ppc/spapr.c
> > >> @@ -3578,14 +3578,7 @@ static void
> > >> spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> > >> error_setg(errp, "Memory hotplug not supported for this
> > >> machine");
> > >> return;
> > >> }
> > >> -node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
> > >> errp);
> > >> -if (*errp) {
> > >
> > > Good riddance :)
> > >
> > >> -return;
> > >> -}
> > >> -if (node < 0 || node >= MAX_NODES) {
> > >> -error_setg(errp, "Invaild node %d", node);
> > >> -return;
> > >> -}
> > >> +node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
> > >> NULL);
> > >
> > > Maybe pass _abort ?
> >
> > I'm using the same access scheme as in hw/acpi/memory_hotplug.c
> >
> > ("error ignored" vs. "error leads to an abort") - but this will actually
> > never fail. But I can use error_abort here, does not matter.
> >
>
> Heh, /me paranoid but this is David's call and he acked that already
> so I guess it's okay.
Actually, I missed this - error_abort is preferable. That's general
the right choice for things that shouldn't ever fail. This way if
they *do* fail we get a clear error immediately.
> Reviewed-by: Greg Kurz
--
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 v1 2/8] spapr: no need to verify the node
On 08.06.2018 10:20, David Gibson wrote:
> On Fri, Jun 08, 2018 at 09:46:57AM +0200, Greg Kurz wrote:
>> On Fri, 8 Jun 2018 09:42:48 +0200
>> David Hildenbrand wrote:
>>
>>> On 08.06.2018 09:34, Greg Kurz wrote:
On Thu, 7 Jun 2018 18:52:12 +0200
David Hildenbrand wrote:
> The node property can always be queried and the value has already been
> verified in pc_dimm_realize().
>
> Signed-off-by: David Hildenbrand
> ---
> hw/ppc/spapr.c | 9 +
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 2375cbee12..d038f3243e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3578,14 +3578,7 @@ static void
> spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> error_setg(errp, "Memory hotplug not supported for this
> machine");
> return;
> }
> -node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
> errp);
> -if (*errp) {
Good riddance :)
> -return;
> -}
> -if (node < 0 || node >= MAX_NODES) {
> -error_setg(errp, "Invaild node %d", node);
> -return;
> -}
> +node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
> NULL);
Maybe pass _abort ?
>>>
>>> I'm using the same access scheme as in hw/acpi/memory_hotplug.c
>>>
>>> ("error ignored" vs. "error leads to an abort") - but this will actually
>>> never fail. But I can use error_abort here, does not matter.
>>>
>>
>> Heh, /me paranoid but this is David's call and he acked that already
>> so I guess it's okay.
>
> Actually, I missed this - error_abort is preferable. That's general
> the right choice for things that shouldn't ever fail. This way if
> they *do* fail we get a clear error immediately.
error_abort it is :)
>
>> Reviewed-by: Greg Kurz
>
>
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [PATCH v1 4/8] spapr: local error handling in hotplug handler functions
On Thu, 7 Jun 2018 18:52:14 +0200
David Hildenbrand wrote:
> Let's introduce and use local error variables in the hotplug handler
> functions.
>
> Signed-off-by: David Hildenbrand
> ---
Reviewed-by: Greg Kurz
> hw/ppc/spapr.c | 29 -
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a12be24ca9..4447cb197f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3576,11 +3576,14 @@ out:
> static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
>DeviceState *dev, Error **errp)
> {
> +Error *local_err = NULL;
> +
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> -spapr_memory_plug(hotplug_dev, dev, errp);
> +spapr_memory_plug(hotplug_dev, dev, _err);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> -spapr_core_plug(hotplug_dev, dev, errp);
> +spapr_core_plug(hotplug_dev, dev, _err);
> }
> +error_propagate(errp, local_err);
> }
>
> static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
> @@ -3588,10 +3591,11 @@ static void
> spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
> {
> sPAPRMachineState *sms = SPAPR_MACHINE(OBJECT(hotplug_dev));
> MachineClass *mc = MACHINE_GET_CLASS(sms);
> +Error *local_err = NULL;
>
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> if (spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
> -spapr_memory_unplug_request(hotplug_dev, dev, errp);
> +spapr_memory_unplug_request(hotplug_dev, dev, _err);
> } else {
> /* NOTE: this means there is a window after guest reset, prior to
> * CAS negotiation, where unplug requests will fail due to the
> @@ -3599,25 +3603,32 @@ static void
> spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
> * the case with PCI unplug, where the events will be queued and
> * eventually handled by the guest after boot
> */
> -error_setg(errp, "Memory hot unplug not supported for this
> guest");
> +error_setg(_err,
> + "Memory hot unplug not supported for this guest");
> }
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> if (!mc->has_hotpluggable_cpus) {
> -error_setg(errp, "CPU hot unplug not supported on this machine");
> -return;
> +error_setg(_err,
> + "CPU hot unplug not supported on this machine");
> +goto out;
> }
> -spapr_core_unplug_request(hotplug_dev, dev, errp);
> +spapr_core_unplug_request(hotplug_dev, dev, _err);
> }
> +out:
> +error_propagate(errp, local_err);
> }
>
> static void spapr_machine_device_pre_plug(HotplugHandler *hotplug_dev,
>DeviceState *dev, Error **errp)
> {
> +Error *local_err = NULL;
> +
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> -spapr_memory_pre_plug(hotplug_dev, dev, errp);
> +spapr_memory_pre_plug(hotplug_dev, dev, _err);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> -spapr_core_pre_plug(hotplug_dev, dev, errp);
> +spapr_core_pre_plug(hotplug_dev, dev, _err);
> }
> +error_propagate(errp, local_err);
> }
>
> static HotplugHandler *spapr_get_hotplug_handler(MachineState *machine,
[Qemu-devel] [PATCH v8 4/6] migration/ram.c: add ram save state notifiers
This patch adds a ram save state notifier list, and expose RAMState for
the notifer callbacks to use.
Signed-off-by: Wei Wang
CC: Dr. David Alan Gilbert
CC: Juan Quintela
CC: Michael S. Tsirkin
CC: Peter Xu
---
include/migration/misc.h | 52 +++
migration/ram.c | 64 +---
2 files changed, 75 insertions(+), 41 deletions(-)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 113320e..b970d7d 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -16,9 +16,61 @@
#include "exec/cpu-common.h"
#include "qemu/notify.h"
+#include "qemu/thread.h"
/* migration/ram.c */
+typedef enum RamSaveState {
+RAM_SAVE_ERR = 0,
+RAM_SAVE_RESET = 1,
+RAM_SAVE_BEFORE_SYNC_BITMAP = 2,
+RAM_SAVE_AFTER_SYNC_BITMAP = 3,
+RAM_SAVE_MAX = 4,
+} RamSaveState;
+
+/* State of RAM for migration */
+typedef struct RAMState {
+/* QEMUFile used for this migration */
+QEMUFile *f;
+/* Last block that we have visited searching for dirty pages */
+RAMBlock *last_seen_block;
+/* Last block from where we have sent data */
+RAMBlock *last_sent_block;
+/* Last dirty target page we have sent */
+ram_addr_t last_page;
+/* last ram version we have seen */
+uint32_t last_version;
+/* We are in the first round */
+bool ram_bulk_stage;
+/* How many times we have dirty too many pages */
+int dirty_rate_high_cnt;
+/* ram save states used for notifiers */
+int ram_save_state;
+/* these variables are used for bitmap sync */
+/* last time we did a full bitmap_sync */
+int64_t time_last_bitmap_sync;
+/* bytes transferred at start_time */
+uint64_t bytes_xfer_prev;
+/* number of dirty pages since start_time */
+uint64_t num_dirty_pages_period;
+/* xbzrle misses since the beginning of the period */
+uint64_t xbzrle_cache_miss_prev;
+/* number of iterations at the beginning of period */
+uint64_t iterations_prev;
+/* Iterations since start */
+uint64_t iterations;
+/* number of dirty bits in the bitmap */
+uint64_t migration_dirty_pages;
+/* protects modification of the bitmap */
+QemuMutex bitmap_mutex;
+/* The RAMBlock used in the last src_page_requests */
+RAMBlock *last_req_rb;
+/* Queue of outstanding page requests from the destination */
+QemuMutex src_page_req_mutex;
+QSIMPLEQ_HEAD(src_page_requests, RAMSrcPageRequest) src_page_requests;
+} RAMState;
+
+void add_ram_save_state_change_notifier(Notifier *notify);
void ram_mig_init(void);
void qemu_guest_free_page_hint(void *addr, size_t len);
diff --git a/migration/ram.c b/migration/ram.c
index 237f11e..d45b5a4 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -56,6 +56,9 @@
#include "qemu/uuid.h"
#include "savevm.h"
+static NotifierList ram_save_state_notifiers =
+NOTIFIER_LIST_INITIALIZER(ram_save_state_notifiers);
+
/***/
/* ram save/restore */
@@ -267,49 +270,18 @@ struct RAMSrcPageRequest {
QSIMPLEQ_ENTRY(RAMSrcPageRequest) next_req;
};
-/* State of RAM for migration */
-struct RAMState {
-/* QEMUFile used for this migration */
-QEMUFile *f;
-/* Last block that we have visited searching for dirty pages */
-RAMBlock *last_seen_block;
-/* Last block from where we have sent data */
-RAMBlock *last_sent_block;
-/* Last dirty target page we have sent */
-ram_addr_t last_page;
-/* last ram version we have seen */
-uint32_t last_version;
-/* We are in the first round */
-bool ram_bulk_stage;
-/* How many times we have dirty too many pages */
-int dirty_rate_high_cnt;
-/* these variables are used for bitmap sync */
-/* last time we did a full bitmap_sync */
-int64_t time_last_bitmap_sync;
-/* bytes transferred at start_time */
-uint64_t bytes_xfer_prev;
-/* number of dirty pages since start_time */
-uint64_t num_dirty_pages_period;
-/* xbzrle misses since the beginning of the period */
-uint64_t xbzrle_cache_miss_prev;
-/* number of iterations at the beginning of period */
-uint64_t iterations_prev;
-/* Iterations since start */
-uint64_t iterations;
-/* number of dirty bits in the bitmap */
-uint64_t migration_dirty_pages;
-/* protects modification of the bitmap */
-QemuMutex bitmap_mutex;
-/* The RAMBlock used in the last src_page_requests */
-RAMBlock *last_req_rb;
-/* Queue of outstanding page requests from the destination */
-QemuMutex src_page_req_mutex;
-QSIMPLEQ_HEAD(src_page_requests, RAMSrcPageRequest) src_page_requests;
-};
-typedef struct RAMState RAMState;
-
static RAMState *ram_state;
+void add_ram_save_state_change_notifier(Notifier *notify)
+{
+notifier_list_add(_save_state_notifiers, notify);
+}
+
+static void notify_ram_save_state_change_notifier(void)
+{
Re: [Qemu-devel] [qemu-s390x] [PATCH v1 2/8] spapr: no need to verify the node
On Fri, 8 Jun 2018 10:07:31 +0200
Thomas Huth wrote:
> On 08.06.2018 09:48, David Hildenbrand wrote:
> > On 08.06.2018 09:46, Greg Kurz wrote:
> >> On Fri, 8 Jun 2018 09:42:48 +0200
> >> David Hildenbrand wrote:
> >>
> >>> On 08.06.2018 09:34, Greg Kurz wrote:
> On Thu, 7 Jun 2018 18:52:12 +0200
> David Hildenbrand wrote:
>
> > The node property can always be queried and the value has already been
> > verified in pc_dimm_realize().
> >
> > Signed-off-by: David Hildenbrand
> > ---
> > hw/ppc/spapr.c | 9 +
> > 1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 2375cbee12..d038f3243e 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3578,14 +3578,7 @@ static void
> > spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> > error_setg(errp, "Memory hotplug not supported for this
> > machine");
> > return;
> > }
> > -node = object_property_get_uint(OBJECT(dev),
> > PC_DIMM_NODE_PROP, errp);
> > -if (*errp) {
>
> Good riddance :)
>
> > -return;
> > -}
> > -if (node < 0 || node >= MAX_NODES) {
> > -error_setg(errp, "Invaild node %d", node);
> > -return;
> > -}
>
> Maybe turn that into an assert() instead? ... just for the paranoids ;-)
>
> > +node = object_property_get_uint(OBJECT(dev),
> > PC_DIMM_NODE_PROP, NULL);
>
> Maybe pass _abort ?
> >>>
> >>> I'm using the same access scheme as in hw/acpi/memory_hotplug.c
> >>>
> >>> ("error ignored" vs. "error leads to an abort") - but this will actually
> >>> never fail. But I can use error_abort here, does not matter.
> >>>
> >>
> >> Heh, /me paranoid but this is David's call and he acked that already
> >> so I guess it's okay.
> >
> > NULL makes it fit into a single line :)
>
> +1 for error_abort, even if it takes another line.
+1 for error_abort
call shouldn't fail, but if does it won't be silently ignored
and introduce undefined behavior.
>
> Thomas
