[PATCH not-for-merge 0/5] Instrumentation for "Fixes around device realization"
This is the instrumentation mentioned in "[PATCH 00/24] Fixes around device realization". PATCH 2/5 might have value on its own. You tell me. Shell script to smoke-test all machines: #!/bin/sh success=0 fail=0 ulimit -c 0 git-describe --dirty --match v\* git-log --oneline -1 for i in bld/*-softmmu do t=${i%-softmmu} t=${t##*/} q=$i/qemu-system-$t echo "= $t =" for m in `$q -M help | sed -n '/(alias of/d;2,$s/ .*//p'` do echo "== $m ==" echo -e 'info qom-tree\ninfo qtree\nq' | $q -S -accel qtest -display none -L smoke-mon-roms -M $m -monitor stdio if [ $? -eq 0 ] then echo "*** Success: $m ***"; let success++ else echo "*** Fail: $m"; let fail++ fi done done echo $success succeeded, $fail failed Markus Armbruster (5): qom: Instrument to detect missed realize qom: Make "info qom-tree" show children sorted qdev: Make "info qtree" show child devices sorted by QOM path qdev: Instrument to detect missed QOM parenting qdev: Instrument to detect bus mismatch hw/core/qdev.c | 17 qdev-monitor.c | 32 - qom/qom-hmp-cmds.c | 51 +- 3 files changed, 98 insertions(+), 2 deletions(-) -- 2.21.1
[PATCH not-for-merge 5/5] qdev: Instrument to detect bus mismatch
Signed-off-by: Markus Armbruster --- hw/core/qdev.c | 17 + 1 file changed, 17 insertions(+) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 9e5538aeae..936ef3988a 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -98,6 +98,23 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus) { BusState *old_parent_bus = dev->parent_bus; +DeviceClass *dc = DEVICE_GET_CLASS(dev); +if (bus) { +BusClass *bc; +for (bc = BUS_GET_CLASS(bus); + bc; + bc = (BusClass *)object_class_dynamic_cast(object_class_get_parent(OBJECT_CLASS(bc)), TYPE_BUS)) { +if (!g_strcmp0(dc->bus_type, object_class_get_name(OBJECT_CLASS(bc { +break; +} +} +if (!bc) { +printf("### bus mismatch %s is %s plugged into %s\n", + object_get_typename(OBJECT(dev)), dc->bus_type, + object_class_get_name(OBJECT_CLASS(BUS_GET_CLASS(bus; +} +} + if (old_parent_bus) { trace_qdev_update_parent_bus(dev, object_get_typename(OBJECT(dev)), old_parent_bus, object_get_typename(OBJECT(old_parent_bus)), -- 2.21.1
[PATCH not-for-merge 2/5] qom: Make "info qom-tree" show children sorted
"info qom-tree" prints children in unstable order. This is a pain when diffing output for different versions to find change. Print it sorted. Signed-off-by: Markus Armbruster --- qom/qom-hmp-cmds.c | 40 +++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c index 4a61ee1b8c..cf0af8f6b5 100644 --- a/qom/qom-hmp-cmds.c +++ b/qom/qom-hmp-cmds.c @@ -78,6 +78,35 @@ static int print_qom_composition_child(Object *obj, void *opaque) return 0; } +static int qom_composition_compare(const void *a, const void *b, void *ignore) +{ +Object *obja = (void *)a, *objb = (void *)b; +const char *namea, *nameb; + +if (obja == object_get_root()) { +namea = g_strdup(""); +} else { +namea = object_get_canonical_path_component(obja); +} + +if (objb == object_get_root()) { +nameb = g_strdup(""); +} else { +nameb = object_get_canonical_path_component(objb); +} + + +return strcmp(namea, nameb); +} + +static int insert_qom_composition_child(Object *obj, void *opaque) +{ +GQueue *children = opaque; + + g_queue_insert_sorted(children, obj, qom_composition_compare, NULL); + return 0; +} + static void print_qom_composition(Monitor *mon, Object *obj, int indent) { QOMCompositionState s = { @@ -105,7 +134,16 @@ static void print_qom_composition(Monitor *mon, Object *obj, int indent) monitor_printf(mon, "%*s/%s (%s)\n", indent, "", name, object_get_typename(obj)); g_free(name); -object_child_foreach(obj, print_qom_composition_child, ); + +GQueue children; +Object *child; +g_queue_init(); +object_child_foreach(obj, insert_qom_composition_child, ); +while ((child = g_queue_pop_head())) { +print_qom_composition(mon, child, indent + 2); +} +(void)s; +(void)print_qom_composition_child; } void hmp_info_qom_tree(Monitor *mon, const QDict *dict) -- 2.21.1
[PATCH 05/24] aspeed: Don't create unwanted "cortex-a7-arm-cpu" devices
The number of CPUs is controlled by property "num-cpus". aspeed_soc_ast2600_init() creates the maximum supported number. aspeed_soc_ast2600_realize() realizes only the wanted number. Works, although it leaves unrealized devices hanging around in the QOM composition tree. Affects machines ast2600-evb and tacoma-bmc. Make the init functions create only the wanted ones. Visible in "info qom-tree"; here's the change for ast2600-evb: /machine (ast2600-evb-machine) [...] /soc (ast2600-a1) [...] /cpu[0] (cortex-a7-arm-cpu) /unnamed-gpio-in[0] (irq) /unnamed-gpio-in[1] (irq) /unnamed-gpio-in[2] (irq) /unnamed-gpio-in[3] (irq) -/cpu[1] (cortex-a7-arm-cpu) - /unnamed-gpio-in[0] (irq) - /unnamed-gpio-in[1] (irq) - /unnamed-gpio-in[2] (irq) - /unnamed-gpio-in[3] (irq) /ehci[0] (platform-ehci-usb) Cc: "Cédric Le Goater" Cc: Peter Maydell Cc: Andrew Jeffery Cc: Joel Stanley Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- hw/arm/aspeed_ast2600.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index 0a6a77dd54..6ffa587a7f 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -287,6 +287,9 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) return; } } +for (; i < sc->num_cpus; i++) { +object_unparent(OBJECT(>cpu[i])); +} /* A7MPCORE */ object_property_set_int(OBJECT(>a7mpcore), s->num_cpus, "num-cpu", -- 2.21.1
[PATCH not-for-merge 4/5] qdev: Instrument to detect missed QOM parenting
Signed-off-by: Markus Armbruster --- qdev-monitor.c | 4 1 file changed, 4 insertions(+) diff --git a/qdev-monitor.c b/qdev-monitor.c index 07f78e9f5d..ec4e134ff7 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -801,6 +801,10 @@ static void qbus_print(Monitor *mon, BusState *bus, int indent) struct qbus_child *qc = children->data; DeviceState *dev = qc->dev; GSList *next = children->next; +if (!qc->qom_path) { +printf("### no qom path: %s, id \"%s\"\n", + object_get_typename(OBJECT(dev)), dev->id ? dev->id : ""); +} qdev_print(mon, dev, indent); g_free(qc->qom_path); g_free(qc); -- 2.21.1
[PATCH not-for-merge 1/5] qom: Instrument to detect missed realize
Signed-off-by: Markus Armbruster --- qom/qom-hmp-cmds.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c index cd08233a4c..4a61ee1b8c 100644 --- a/qom/qom-hmp-cmds.c +++ b/qom/qom-hmp-cmds.c @@ -91,6 +91,17 @@ static void print_qom_composition(Monitor *mon, Object *obj, int indent) } else { name = object_get_canonical_path_component(obj); } + +DeviceState *dev = (DeviceState *)object_dynamic_cast(obj, TYPE_DEVICE); +DeviceClass *dc = (DeviceClass *)object_class_dynamic_cast(obj->class, TYPE_DEVICE); +if (dev && !dev->realized) { +monitor_printf(mon, "### unrealized: %s (%s)\n", + name, object_get_typename(obj)); +} +if (dev && dc->bus_type && !dev->parent_bus) { +monitor_printf(mon, "### no %s bus: %s (%s)\n", + dc->bus_type, name, object_get_typename(obj)); +} monitor_printf(mon, "%*s/%s (%s)\n", indent, "", name, object_get_typename(obj)); g_free(name); -- 2.21.1
[PATCH not-for-merge 3/5] qdev: Make "info qtree" show child devices sorted by QOM path
"info qtree" shows children in reverse creation order. Show them sorted by QOM path. Signed-off-by: Markus Armbruster --- qdev-monitor.c | 28 +++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/qdev-monitor.c b/qdev-monitor.c index a4735d3bb1..07f78e9f5d 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -771,17 +771,43 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent) } } +struct qbus_child { +DeviceState *dev; +char *qom_path; +}; + +static int dev_cmp(const void *a, const void *b) +{ +return g_strcmp0(((struct qbus_child *)a)->qom_path, + ((struct qbus_child *)b)->qom_path); +} + static void qbus_print(Monitor *mon, BusState *bus, int indent) { BusChild *kid; +GSList *children = NULL; qdev_printf("bus: %s\n", bus->name); indent += 2; qdev_printf("type %s\n", object_get_typename(OBJECT(bus))); QTAILQ_FOREACH(kid, >children, sibling) { DeviceState *dev = kid->child; -qdev_print(mon, dev, indent); +struct qbus_child *qc = g_malloc(sizeof(*qc)); +qc->dev = dev; +qc->qom_path = object_get_canonical_path(OBJECT(dev)); +children = g_slist_insert_sorted(children, qc, dev_cmp); } +while (children) { +struct qbus_child *qc = children->data; +DeviceState *dev = qc->dev; +GSList *next = children->next; +qdev_print(mon, dev, indent); +g_free(qc->qom_path); +g_free(qc); +g_slist_free_1(children); +children = next; +} + } #undef qdev_printf -- 2.21.1
[PATCH 21/24] sparc/leon3: Fix to put grlib,* devices on sysbus
leon3_generic_hw_init() creates a "grlib,ahbpnp" and a "grlib,apbpnp" sysbus device in a way that leaves them unplugged. Create them the common way that puts them into the main system bus. Affects machine leon3_generic. Visible in "info qtree": bus: main-system-bus type System + dev: grlib,ahbpnp, id "" +mmio f000/1000 + dev: grlib,apbpnp, id "" +mmio 800ff000/1000 dev: grlib,irqmp, id "" Cc: Fabien Chouteau Cc: KONRAD Frederic Cc: Mark Cave-Ayland Cc: Artyom Tarasenko Signed-off-by: Markus Armbruster --- hw/sparc/leon3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c index 8f024dab7b..3facb8c2ae 100644 --- a/hw/sparc/leon3.c +++ b/hw/sparc/leon3.c @@ -213,14 +213,14 @@ static void leon3_generic_hw_init(MachineState *machine) reset_info->sp= LEON3_RAM_OFFSET + ram_size; qemu_register_reset(main_cpu_reset, reset_info); -ahb_pnp = GRLIB_AHB_PNP(object_new(TYPE_GRLIB_AHB_PNP)); +ahb_pnp = GRLIB_AHB_PNP(qdev_create(NULL, TYPE_GRLIB_AHB_PNP)); object_property_set_bool(OBJECT(ahb_pnp), true, "realized", _fatal); sysbus_mmio_map(SYS_BUS_DEVICE(ahb_pnp), 0, LEON3_AHB_PNP_OFFSET); grlib_ahb_pnp_add_entry(ahb_pnp, 0, 0, GRLIB_VENDOR_GAISLER, GRLIB_LEON3_DEV, GRLIB_AHB_MASTER, GRLIB_CPU_AREA); -apb_pnp = GRLIB_APB_PNP(object_new(TYPE_GRLIB_APB_PNP)); +apb_pnp = GRLIB_APB_PNP(qdev_create(NULL, TYPE_GRLIB_APB_PNP)); object_property_set_bool(OBJECT(apb_pnp), true, "realized", _fatal); sysbus_mmio_map(SYS_BUS_DEVICE(apb_pnp), 0, LEON3_APB_PNP_OFFSET); grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB_PNP_OFFSET, 0xFFF, -- 2.21.1
[PATCH 10/24] macio: Bury unwanted "macio-gpio" devices
These devices go with the "via-pmu" device, which is controlled by property "has-pmu". macio_newworld_init() creates it unconditionally, because the property has not been set then. macio_newworld_realize() realizes it only when the property is true. Works, although it can leave an unrealized device hanging around in the QOM composition tree. Affects machine mac99 with via=cuda (default). Bury the unwanted device by making macio_newworld_realize() unparent it. Visible in "info qom-tree": /machine (mac99-machine) [...] /unattached (container) /device[9] (macio-newworld) [...] /escc-legacy-port[8] (qemu:memory-region) /escc-legacy-port[9] (qemu:memory-region) /escc-legacy[0] (qemu:memory-region) - /gpio (macio-gpio) -/gpio[0] (qemu:memory-region) /ide[0] (macio-ide) /ide.0 (IDE) /pmac-ide[0] (qemu:memory-region) Cc: Mark Cave-Ayland Cc: David Gibson Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- hw/misc/macio/macio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c index 3779865ab2..b3dddf8be7 100644 --- a/hw/misc/macio/macio.c +++ b/hw/misc/macio/macio.c @@ -368,6 +368,8 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp) memory_region_add_subregion(>bar, 0x16000, sysbus_mmio_get_region(sysbus_dev, 0)); } else { +object_unparent(OBJECT(>gpio)); + /* CUDA */ object_initialize_child(OBJECT(s), "cuda", >cuda, sizeof(s->cuda), TYPE_CUDA, _abort, NULL); -- 2.21.1
[PATCH 13/24] ppc4xx: Drop redundant device realization
object_property_set_bool(OBJECT(dev), true, "realized", ...) right after qdev_init_nofail(dev) does nothing, because qdev_init_nofail() already realizes. Drop. Cc: BALATON Zoltan Signed-off-by: Markus Armbruster --- hw/ppc/ppc440_uc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c index b30e093cbb..dc318c7aa7 100644 --- a/hw/ppc/ppc440_uc.c +++ b/hw/ppc/ppc440_uc.c @@ -1370,12 +1370,10 @@ void ppc460ex_pcie_init(CPUPPCState *env) dev = qdev_create(NULL, TYPE_PPC460EX_PCIE_HOST); qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE0_BASE); qdev_init_nofail(dev); -object_property_set_bool(OBJECT(dev), true, "realized", NULL); ppc460ex_pcie_register_dcrs(PPC460EX_PCIE_HOST(dev), env); dev = qdev_create(NULL, TYPE_PPC460EX_PCIE_HOST); qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE1_BASE); qdev_init_nofail(dev); -object_property_set_bool(OBJECT(dev), true, "realized", NULL); ppc460ex_pcie_register_dcrs(PPC460EX_PCIE_HOST(dev), env); } -- 2.21.1
[PATCH 19/24] riscv: Fix to put "riscv.hart_array" devices on sysbus
riscv_sifive_e_soc_init(), riscv_sifive_u_soc_init(), spike_board_init(), spike_v1_10_0_board_init(), spike_v1_09_1_board_init(), and riscv_virt_board_init() create "riscv-hart_array" sysbus devices in a way that leaves them unplugged. Create them the common way that puts them into the main system bus. Affects machines sifive_e, sifive_u, spike, spike_v1.10, spike_v1.9.1, and virt. Visible in "info qtree", here's the change for sifive_e: bus: main-system-bus type System + dev: riscv.hart_array, id "" +num-harts = 1 (0x1) +hartid-base = 0 (0x0) +cpu-type = "sifive-e31-riscv-cpu" dev: sifive_soc.gpio, id "" Cc: Palmer Dabbelt Cc: Alistair Francis Cc: Sagar Karandikar Cc: Bastian Koppelmann Cc: qemu-ri...@nongnu.org Signed-off-by: Markus Armbruster --- hw/riscv/sifive_e.c | 5 ++--- hw/riscv/sifive_u.c | 14 ++ hw/riscv/spike.c| 12 ++-- hw/riscv/virt.c | 4 ++-- 4 files changed, 16 insertions(+), 19 deletions(-) diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index b53109521e..8831e6728e 100644 --- a/hw/riscv/sifive_e.c +++ b/hw/riscv/sifive_e.c @@ -120,9 +120,8 @@ static void riscv_sifive_e_soc_init(Object *obj) MachineState *ms = MACHINE(qdev_get_machine()); SiFiveESoCState *s = RISCV_E_SOC(obj); -object_initialize_child(obj, "cpus", >cpus, -sizeof(s->cpus), TYPE_RISCV_HART_ARRAY, -_abort, NULL); +sysbus_init_child_obj(obj, "cpus", >cpus, + sizeof(s->cpus), TYPE_RISCV_HART_ARRAY); object_property_set_int(OBJECT(>cpus), ms->smp.cpus, "num-harts", _abort); sysbus_init_child_obj(obj, "riscv.sifive.e.gpio0", diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index 4299bdf480..bb69fd8e48 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -491,10 +491,9 @@ static void riscv_sifive_u_soc_init(Object *obj) _abort, NULL); qdev_prop_set_uint32(DEVICE(>e_cluster), "cluster-id", 0); -object_initialize_child(OBJECT(>e_cluster), "e-cpus", ->e_cpus, sizeof(s->e_cpus), -TYPE_RISCV_HART_ARRAY, _abort, -NULL); +sysbus_init_child_obj(OBJECT(>e_cluster), "e-cpus", + >e_cpus, sizeof(s->e_cpus), + TYPE_RISCV_HART_ARRAY); qdev_prop_set_uint32(DEVICE(>e_cpus), "num-harts", 1); qdev_prop_set_uint32(DEVICE(>e_cpus), "hartid-base", 0); qdev_prop_set_string(DEVICE(>e_cpus), "cpu-type", SIFIVE_E_CPU); @@ -504,10 +503,9 @@ static void riscv_sifive_u_soc_init(Object *obj) _abort, NULL); qdev_prop_set_uint32(DEVICE(>u_cluster), "cluster-id", 1); -object_initialize_child(OBJECT(>u_cluster), "u-cpus", ->u_cpus, sizeof(s->u_cpus), -TYPE_RISCV_HART_ARRAY, _abort, -NULL); +sysbus_init_child_obj(OBJECT(>u_cluster), "u-cpus", + >u_cpus, sizeof(s->u_cpus), + TYPE_RISCV_HART_ARRAY); qdev_prop_set_uint32(DEVICE(>u_cpus), "num-harts", ms->smp.cpus - 1); qdev_prop_set_uint32(DEVICE(>u_cpus), "hartid-base", 1); qdev_prop_set_string(DEVICE(>u_cpus), "cpu-type", SIFIVE_U_CPU); diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c index d0c4843712..01d52e758e 100644 --- a/hw/riscv/spike.c +++ b/hw/riscv/spike.c @@ -169,8 +169,8 @@ static void spike_board_init(MachineState *machine) unsigned int smp_cpus = machine->smp.cpus; /* Initialize SOC */ -object_initialize_child(OBJECT(machine), "soc", >soc, sizeof(s->soc), -TYPE_RISCV_HART_ARRAY, _abort, NULL); +sysbus_init_child_obj(OBJECT(machine), "soc", >soc, sizeof(s->soc), + TYPE_RISCV_HART_ARRAY); object_property_set_str(OBJECT(>soc), machine->cpu_type, "cpu-type", _abort); object_property_set_int(OBJECT(>soc), smp_cpus, "num-harts", @@ -275,8 +275,8 @@ static void spike_v1_10_0_board_init(MachineState *machine) } /* Initialize SOC */ -object_initialize_child(OBJECT(machine), "soc", >soc, sizeof(s->soc), -TYPE_RISCV_HART_ARRAY, _abort, NULL); +sysbus_init_child_obj(OBJECT(machine), "soc", >soc, sizeof(s->soc), + TYPE_RISCV_HART_ARRAY); object_property_set_str(OBJECT(>soc), SPIKE_V1_10_0_CPU, "cpu-type", _abort); object_property_set_int(OBJECT(>soc), smp_cpus, "num-harts", @@ -365,8 +365,8 @@ static void spike_v1_09_1_board_init(MachineState *machine) } /* Initialize SOC */ -object_initialize_child(OBJECT(machine), "soc", >soc, sizeof(s->soc), -TYPE_RISCV_HART_ARRAY, _abort, NULL); +
[PATCH 24/24] qdev: Assert onboard devices all get realized properly
This would have caught some of the bugs I just fixed. Signed-off-by: Markus Armbruster --- hw/core/qdev.c | 16 1 file changed, 16 insertions(+) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 0df995eb94..fe2dea8968 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -429,6 +429,19 @@ void qdev_init_nofail(DeviceState *dev) object_unref(OBJECT(dev)); } +static int qdev_assert_realized_properly(Object *obj, void *opaque) +{ +DeviceState *dev = DEVICE(object_dynamic_cast(obj, TYPE_DEVICE)); +DeviceClass *dc; + +if (dev) { +dc = DEVICE_GET_CLASS(dev); +assert(dev->realized); +assert(dev->parent_bus || !dc->bus_type); +} +return 0; +} + void qdev_machine_creation_done(void) { /* @@ -436,6 +449,9 @@ void qdev_machine_creation_done(void) * only create hotpluggable devices */ qdev_hotplug = true; + +object_child_foreach_recursive(object_get_root(), + qdev_assert_realized_properly, NULL); } bool qdev_machine_modified(void) -- 2.21.1
[PATCH 02/24] display/xlnx_dp: Fix to realize "i2c-ddc" and "aux-to-i2c-bridge"
xlnx_dp_init() creates these two devices, but they're never realized. Affects machine xlnx-zcu102. I wonder how this ever worked. If the "device becomes real only on realize" thing actually works, then we've always been missing these two devices, yet nobody noticed. Fix by realizing them in xlnx_dp_realize(). Fixes: 58ac482a66de09a7590f705e53fc6a3fb8a055e8 Cc: KONRAD Frederic Cc: Alistair Francis Cc: "Edgar E. Iglesias" Cc: Peter Maydell Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- hw/display/xlnx_dp.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c index 3e5fb44e06..bdc229a51e 100644 --- a/hw/display/xlnx_dp.c +++ b/hw/display/xlnx_dp.c @@ -1264,9 +1264,13 @@ static void xlnx_dp_realize(DeviceState *dev, Error **errp) DisplaySurface *surface; struct audsettings as; +qdev_init_nofail(DEVICE(s->aux_bus->bridge)); + qdev_init_nofail(DEVICE(s->dpcd)); aux_map_slave(AUX_SLAVE(s->dpcd), 0x); +qdev_init_nofail(DEVICE(s->edid)); + s->console = graphic_console_init(dev, 0, _dp_gfx_ops, s); surface = qemu_console_surface(s->console); xlnx_dpdma_set_host_data_location(s->dpdma, DP_GRAPHIC_DMA_CHANNEL, -- 2.21.1
[PATCH 11/24] pnv/phb4: Bury unwanted "pnv-phb4-pec-stack" devices
The number of stacks is controlled by property "num-stacks". pnv_pec_instance_init() creates the maximum supported number, because the property has not been set then. pnv_pec_realize() realizes only the wanted number. Works, although it can leave unrealized devices hanging around in the QOM composition tree. Affects machine powernv9. Bury the unwanted devices by making pnv_pec_realize() unparent them. Visible in "info qom-tree": /machine (powernv9-machine) /chip[0] (power9_v2.0-pnv-chip) [...] /pec[0] (pnv-phb4-pec) /stack[0] (pnv-phb4-pec-stack) [...] - /stack[1] (pnv-phb4-pec-stack) -/phb (pnv-phb4) - /pcie-mmcfg-mmio[0] (qemu:memory-region) - /root (pnv-phb4-root-port) - /source (xive-source) - /stack[2] (pnv-phb4-pec-stack) -/phb (pnv-phb4) - /pcie-mmcfg-mmio[0] (qemu:memory-region) - /root (pnv-phb4-root-port) - /source (xive-source) /xscom-pec-0.0-nest[0] (qemu:memory-region) /xscom-pec-0.0-pci[0] (qemu:memory-region) /pec[1] (pnv-phb4-pec) /stack[0] (pnv-phb4-pec-stack) [...] /stack[1] (pnv-phb4-pec-stack) [...] - /stack[2] (pnv-phb4-pec-stack) -/phb (pnv-phb4) - /pcie-mmcfg-mmio[0] (qemu:memory-region) - /root (pnv-phb4-root-port) - /source (xive-source) /xscom-pec-0.1-nest[0] (qemu:memory-region) /xscom-pec-0.1-pci[0] (qemu:memory-region) /pec[2] (pnv-phb4-pec) /stack[0] (pnv-phb4-pec-stack) [...] /stack[1] (pnv-phb4-pec-stack) [...] /stack[2] (pnv-phb4-pec-stack) [...] Cc: Cédric Le Goater Cc: David Gibson Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- hw/pci-host/pnv_phb4_pec.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c index 911d147ffd..565345a018 100644 --- a/hw/pci-host/pnv_phb4_pec.c +++ b/hw/pci-host/pnv_phb4_pec.c @@ -397,6 +397,9 @@ static void pnv_pec_realize(DeviceState *dev, Error **errp) return; } } +for (; i < PHB4_PEC_MAX_STACKS; i++) { +object_unparent(OBJECT(>stacks[i])); +} /* Initialize the XSCOM regions for the PEC registers */ snprintf(name, sizeof(name), "xscom-pec-%d.%d-nest", pec->chip_id, -- 2.21.1
[PATCH 18/24] display/sm501 display/ati: Fix to realize "i2c-ddc"
sm501_init() and ati_vga_realize() create an "i2c-ddc" device, but neglect to realize it. Affects machines sam460ex, shix, r2d, and fulong2e. I wonder how this ever worked. If the "device becomes real only on realize" thing actually works, then we've always been missing the device, yet nobody noticed. Fix by realizing it right away. Visible in "info qom-tree"; here's the change for sam460ex: /machine (sam460ex-machine) [...] /unattached (container) [...] -/device[14] (sii3112) +/device[14] (i2c-ddc) +/device[15] (sii3112) [rest of device[*] renumbered...] Fixes: 4a1f253adb45ac6019971193d5077c4d5d55886a Fixes: 4a1f253adb45ac6019971193d5077c4d5d55886a Cc: BALATON Zoltan Cc: qemu-...@nongnu.org Cc: Magnus Damm Cc: Philippe Mathieu-Daudé Cc: Aleksandar Markovic Signed-off-by: Markus Armbruster --- hw/display/ati.c | 1 + hw/display/sm501.c | 1 + 2 files changed, 2 insertions(+) diff --git a/hw/display/ati.c b/hw/display/ati.c index 58ec8291d4..7c2177d7b3 100644 --- a/hw/display/ati.c +++ b/hw/display/ati.c @@ -929,6 +929,7 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp) bitbang_i2c_init(>bbi2c, i2cbus); I2CSlave *i2cddc = I2C_SLAVE(qdev_create(BUS(i2cbus), TYPE_I2CDDC)); i2c_set_slave_address(i2cddc, 0x50); +qdev_init_nofail(DEVICE(i2cddc)); /* mmio register space */ memory_region_init_io(>mm, OBJECT(s), _mm_ops, s, diff --git a/hw/display/sm501.c b/hw/display/sm501.c index acc692531a..132e75b641 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -1816,6 +1816,7 @@ static void sm501_init(SM501State *s, DeviceState *dev, /* ddc */ I2CDDCState *ddc = I2CDDC(qdev_create(BUS(s->i2c_bus), TYPE_I2CDDC)); i2c_set_slave_address(I2C_SLAVE(ddc), 0x50); +qdev_init_nofail(DEVICE(ddc)); /* mmio */ memory_region_init(>mmio_region, OBJECT(dev), "sm501.mmio", MMIO_SIZE); -- 2.21.1
[PATCH 09/24] macio: Fix to realize "mos6522-cuda" and "mos6522-pmu" devices
cuda_init() creates a "mos6522-cuda" device, but it's never realized. Affects machines mac99 with via=cuda (default) and g3beige. pmu_init() creates a "mos6522-pmu" device, but it's never realized. Affects machine mac99 with via=pmu and via=pmu-adb, I wonder how this ever worked. If the "device becomes real only on realize" thing actually works, then we've always been missing these devices, yet nobody noticed. Fix by realizing them in cuda_realize() and pmu_realize(), respectively. Fixes: 6dca62af95e0b7020aa00d0ca9b2c421f341 Cc: Laurent Vivier Signed-off-by: Markus Armbruster --- hw/misc/macio/cuda.c | 8 +++- hw/misc/macio/pmu.c | 8 +++- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c index e0cc0aac5d..6d4d135f71 100644 --- a/hw/misc/macio/cuda.c +++ b/hw/misc/macio/cuda.c @@ -523,15 +523,13 @@ static void cuda_realize(DeviceState *dev, Error **errp) { CUDAState *s = CUDA(dev); SysBusDevice *sbd; -MOS6522State *ms; -DeviceState *d; struct tm tm; +qdev_init_nofail(DEVICE(>mos6522_cuda)); + /* Pass IRQ from 6522 */ -d = DEVICE(>mos6522_cuda); -ms = MOS6522(d); sbd = SYS_BUS_DEVICE(s); -sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms)); +sysbus_pass_irq(sbd, SYS_BUS_DEVICE(>mos6522_cuda)); qemu_get_timedate(, 0); s->tick_offset = (uint32_t)mktimegm() + RTC_OFFSET; diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c index 9a9cd427e1..e29ca5e6cc 100644 --- a/hw/misc/macio/pmu.c +++ b/hw/misc/macio/pmu.c @@ -740,15 +740,13 @@ static void pmu_realize(DeviceState *dev, Error **errp) { PMUState *s = VIA_PMU(dev); SysBusDevice *sbd; -MOS6522State *ms; -DeviceState *d; struct tm tm; +qdev_init_nofail(DEVICE(>mos6522_pmu)); + /* Pass IRQ from 6522 */ -d = DEVICE(>mos6522_pmu); -ms = MOS6522(d); sbd = SYS_BUS_DEVICE(s); -sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms)); +sysbus_pass_irq(sbd, SYS_BUS_DEVICE(>mos6522_pmu)); qemu_get_timedate(, 0); s->tick_offset = (uint32_t)mktimegm() + RTC_OFFSET; -- 2.21.1
[PATCH 08/24] mac_via: Fix to realize "mos6522-q800-via*" devices
mac_via_realize() creates a "mos6522-q800-via1" and a "mos6522-q800-via2" device, but neglects to realize them. Affects machine q800. I wonder how this ever worked. If the "device becomes real only on realize" thing actually works, then we've always been missing these two devices, yet nobody noticed. Fix by realizing them right away. Fixes: 6dca62af95e0b7020aa00d0ca9b2c421f341 Cc: Laurent Vivier Signed-off-by: Markus Armbruster --- hw/misc/mac_via.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c index e05623d730..ee32f72d75 100644 --- a/hw/misc/mac_via.c +++ b/hw/misc/mac_via.c @@ -890,6 +890,9 @@ static void mac_via_realize(DeviceState *dev, Error **errp) object_property_add_alias(OBJECT(dev), "irq[1]", OBJECT(ms), SYSBUS_DEVICE_GPIO_IRQ "[0]"); +qdev_init_nofail(DEVICE(>mos6522_via1)); +qdev_init_nofail(DEVICE(>mos6522_via2)); + /* Pass through mos6522 input IRQs */ qdev_pass_gpios(DEVICE(>mos6522_via1), dev, "via1-irq"); qdev_pass_gpios(DEVICE(>mos6522_via2), dev, "via2-irq"); -- 2.21.1
[PATCH 06/24] armv7m: Bury unwanted "ARM,bitband-memory" devices
These devices are optional, and enabled by property "enable-bitband". armv7m_instance_init() creates them unconditionally, because the property has not been set then. armv7m_realize() realizes them only when the property is true. Works, although it leaves unrealized devices hanging around in the QOM composition tree. Affects machines microbit, mps2-an505, mps2-an521, musca-a, and musca-b1. Bury the unwanted devices by making armv7m_realize() unparent them. Visible in "info qom-tree"; here's the change for microbit: /machine (microbit-machine) /microbit.twi (microbit.i2c) /microbit.twi[0] (qemu:memory-region) /nrf51 (nrf51-soc) /armv6m (armv7m) /armv7m-container[0] (qemu:memory-region) - /bitband[0] (ARM,bitband-memory) -/bitband[0] (qemu:memory-region) - /bitband[1] (ARM,bitband-memory) -/bitband[0] (qemu:memory-region) /cpu (cortex-m0-arm-cpu) Cc: Peter Maydell Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- hw/arm/armv7m.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index 7da57f56d3..f930619f53 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -245,8 +245,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp) memory_region_add_subregion(>container, 0xe000e000, sysbus_mmio_get_region(sbd, 0)); -if (s->enable_bitband) { -for (i = 0; i < ARRAY_SIZE(s->bitband); i++) { +for (i = 0; i < ARRAY_SIZE(s->bitband); i++) { +if (s->enable_bitband) { Object *obj = OBJECT(>bitband[i]); SysBusDevice *sbd = SYS_BUS_DEVICE(>bitband[i]); @@ -265,6 +265,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp) memory_region_add_subregion(>container, bitband_output_addr[i], sysbus_mmio_get_region(sbd, 0)); +} else { +object_unparent(OBJECT(>bitband[i])); } } } -- 2.21.1
[PATCH 22/24] qdev: Assert devices are plugged into a bus that can take them
This would have caught some of the bugs I just fixed. Signed-off-by: Markus Armbruster --- hw/core/qdev.c | 5 + 1 file changed, 5 insertions(+) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 9e5538aeae..0df995eb94 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -97,6 +97,11 @@ static void bus_add_child(BusState *bus, DeviceState *child) void qdev_set_parent_bus(DeviceState *dev, BusState *bus) { BusState *old_parent_bus = dev->parent_bus; +DeviceClass *dc = DEVICE_GET_CLASS(dev); + +assert(dc->bus_type + ? bus && object_dynamic_cast(OBJECT(bus), dc->bus_type) + : !bus); if (old_parent_bus) { trace_qdev_update_parent_bus(dev, object_get_typename(OBJECT(dev)), -- 2.21.1
[PATCH 04/24] aspeed: Don't create unwanted "ftgmac100", "aspeed-mmi" devices
These devices are optional, and controlled by @nb_nics. aspeed_soc_ast2600_init() and aspeed_soc_init() create the maximum supported number. aspeed_soc_ast2600_realize() and aspeed_soc_realize() realize only the wanted number. Works, although it can leave unrealized devices hanging around in the QOM composition tree. Affects machines ast2500-evb, ast2600-evb, palmetto-bmc, romulus-bmc, swift-bmc, tacoma-bmc, and witherspoon-bmc. Make the init functions create only the wanted ones. Visible in "info qom-tree"; here's the change for ast2600-evb: /machine (ast2600-evb-machine) [...] /soc (ast2600-a1) [...] /ftgmac100[0] (ftgmac100) /ftgmac100[0] (qemu:memory-region) -/ftgmac100[1] (ftgmac100) -/ftgmac100[2] (ftgmac100) -/ftgmac100[3] (ftgmac100) /gpio (aspeed.gpio-ast2600) [...] /mii[0] (aspeed-mmi) /aspeed-mmi[0] (qemu:memory-region) -/mii[1] (aspeed-mmi) -/mii[2] (aspeed-mmi) -/mii[3] (aspeed-mmi) /rtc (aspeed.rtc) I'm not sure creating @nb_nics devices makes sense. How many does the physical chip provide? Cc: "Cédric Le Goater" Cc: Peter Maydell Cc: Andrew Jeffery Cc: Joel Stanley Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- hw/arm/aspeed_ast2600.c | 2 +- hw/arm/aspeed_soc.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index 71a0acfe26..0a6a77dd54 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -188,7 +188,7 @@ static void aspeed_soc_ast2600_init(Object *obj) sizeof(s->wdt[i]), typename); } -for (i = 0; i < sc->macs_num; i++) { +for (i = 0; i < nb_nics && i < sc->macs_num; i++) { sysbus_init_child_obj(obj, "ftgmac100[*]", OBJECT(>ftgmac100[i]), sizeof(s->ftgmac100[i]), TYPE_FTGMAC100); diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index cf6b6dd116..7ca860392a 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -203,7 +203,7 @@ static void aspeed_soc_init(Object *obj) sizeof(s->wdt[i]), typename); } -for (i = 0; i < sc->macs_num; i++) { +for (i = 0; i < nb_nics && i < sc->macs_num; i++) { sysbus_init_child_obj(obj, "ftgmac100[*]", OBJECT(>ftgmac100[i]), sizeof(s->ftgmac100[i]), TYPE_FTGMAC100); } -- 2.21.1
[PATCH 15/24] macio: Fix macio-bus to be a subtype of System bus
The devices we plug into the macio-bus are all sysbus devices (DeviceClass member bus_type is TYPE_SYSTEM_BUS), but macio-bus does not derive from TYPE_SYSTEM_BUS. Fix that. "info qtree" now shows the devices' mmio ranges, as it should Cc: Mark Cave-Ayland Cc: David Gibson Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- hw/misc/macio/macio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c index ebc96cc8f6..53a9fd5696 100644 --- a/hw/misc/macio/macio.c +++ b/hw/misc/macio/macio.c @@ -492,7 +492,7 @@ static void macio_class_init(ObjectClass *klass, void *data) static const TypeInfo macio_bus_info = { .name = TYPE_MACIO_BUS, -.parent = TYPE_BUS, +.parent = TYPE_SYSTEM_BUS, .instance_size = sizeof(MacIOBusState), }; -- 2.21.1
[PATCH 14/24] macio: Put "macio-nvram" device on the macio bus
macio_oldworld_init() creates a "macio-nvram", sysbus device, but neglects to but it on a bus. Put it on the macio bus. Affects machine g3beige. Visible in "info qtree": bus: macio.0 type macio-bus [...] + dev: macio-nvram, id "" +size = 8192 (0x2000) +it_shift = 4 (0x4) This also makes it a QOM child of macio-oldworld. Visible in "info qom-tree": /machine (g3beige-machine) [...] /unattached (container) [...] /device[6] (macio-oldworld) [...] -/device[7] (macio-nvram) - /macio-nvram[0] (qemu:memory-region) + /nvram (macio-nvram) +/macio-nvram[0] (qemu:memory-region) [rest of device[*] renumbered...] Cc: Mark Cave-Ayland Cc: David Gibson Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- hw/misc/macio/macio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c index b3dddf8be7..ebc96cc8f6 100644 --- a/hw/misc/macio/macio.c +++ b/hw/misc/macio/macio.c @@ -245,7 +245,8 @@ static void macio_oldworld_init(Object *obj) macio_init_child_obj(s, "cuda", >cuda, sizeof(s->cuda), TYPE_CUDA); -object_initialize(>nvram, sizeof(os->nvram), TYPE_MACIO_NVRAM); +macio_init_child_obj(s, "nvram", >nvram, sizeof(os->nvram), + TYPE_MACIO_NVRAM); dev = DEVICE(>nvram); qdev_prop_set_uint32(dev, "size", 0x2000); qdev_prop_set_uint32(dev, "it_shift", 4); -- 2.21.1
[PATCH 23/24] sd: Hide the qdev-but-not-quite thing created by sd_init()
Commit 260bc9d8aa "hw/sd/sd.c: QOMify" QOMified only the device itself, not its users. It kept sd_init() around for non-QOMified users. More than four years later, three such users remain: omap1 (machines cheetah, sx1, sx1-v1) and omap2 (machines n800, n810) are not QOMified, and pl181 (machines integratorcp, realview-eb, realview-eb-mpcore, realview-pb-a8 realview-pbx-a9, versatileab, versatilepb, vexpress-a15, vexpress-a9) is not QOMified properly. The issue I presently have with this: an "sd-card" device should plug into an "sd-bus" (its DeviceClass member bus_type says so), but sd_init() leaves it unplugged. This is normally a bug (I just fixed some instances), and I'd like to assert proper pluggedness to prevent regressions. However, the qdev-but-not-quite thing returned by sd_init() would fail the assertion. Meh. Make sd_init() hide it from QOM/qdev. Visible in "info qom-tree", here's the change for cheetah: /machine (cheetah-machine) [...] /unattached (container) [...] /device[5] (serial-mm) /serial (serial) /serial[0] (qemu:memory-region) -/device[6] (sd-card) -/device[7] (omap-gpio) +/device[6] (omap-gpio) [rest of device[*] renumbered...] Cc: "Philippe Mathieu-Daudé" Signed-off-by: Markus Armbruster --- hw/sd/sd.c | 40 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 71a9af09ab..d7d8b82dfd 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -83,6 +83,10 @@ enum SDCardStates { struct SDState { DeviceState parent_obj; +/* If true, created by sd_init() for a non-qdevified caller */ +/* TODO purge them with fire */ +bool me_no_qdev_me_kill_mammoth_with_rocks; + /* SD Memory Card Registers */ uint32_t ocr; uint8_t scr[8]; @@ -129,6 +133,8 @@ struct SDState { bool cmd_line; }; +static void sd_realize(DeviceState *dev, Error **errp); + static const char *sd_state_name(enum SDCardStates state) { static const char *state_name[] = { @@ -590,7 +596,7 @@ static void sd_cardchange(void *opaque, bool load, Error **errp) { SDState *sd = opaque; DeviceState *dev = DEVICE(sd); -SDBus *sdbus = SD_BUS(qdev_get_parent_bus(dev)); +SDBus *sdbus; bool inserted = sd_get_inserted(sd); bool readonly = sd_get_readonly(sd); @@ -601,19 +607,17 @@ static void sd_cardchange(void *opaque, bool load, Error **errp) trace_sdcard_ejected(); } -/* The IRQ notification is for legacy non-QOM SD controller devices; - * QOMified controllers use the SDBus APIs. - */ -if (sdbus) { -sdbus_set_inserted(sdbus, inserted); -if (inserted) { -sdbus_set_readonly(sdbus, readonly); -} -} else { +if (sd->me_no_qdev_me_kill_mammoth_with_rocks) { qemu_set_irq(sd->inserted_cb, inserted); if (inserted) { qemu_set_irq(sd->readonly_cb, readonly); } +} else { +sdbus = SD_BUS(qdev_get_parent_bus(dev)); +sdbus_set_inserted(sdbus, inserted); +if (inserted) { +sdbus_set_readonly(sdbus, readonly); +} } } @@ -697,6 +701,7 @@ SDState *sd_init(BlockBackend *blk, bool is_spi) { Object *obj; DeviceState *dev; +SDState *sd; Error *err = NULL; obj = object_new(TYPE_SD_CARD); @@ -707,13 +712,24 @@ SDState *sd_init(BlockBackend *blk, bool is_spi) return NULL; } qdev_prop_set_bit(dev, "spi", is_spi); -object_property_set_bool(obj, true, "realized", ); + +/* + * Realizing the device properly would put it into the QOM + * composition tree even though it is not plugged into an + * appropriate bus. That's a no-no. Hide the device from + * QOM/qdev, and call its qdev realize callback directly. + */ +object_ref(obj); +object_unparent(obj); +sd_realize(dev, ); if (err) { error_report("sd_init failed: %s", error_get_pretty(err)); return NULL; } -return SD_CARD(dev); +sd = SD_CARD(dev); +sd->me_no_qdev_me_kill_mammoth_with_rocks = true; +return sd; } void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert) -- 2.21.1
[PATCH 20/24] riscv: Fix type of SiFive[EU]SocState, member parent_obj
Device "riscv.sifive.e.soc" is a direct subtype of TYPE_DEVICE, but its instance struct SiFiveESoCState's member @parent_obj is SysBusDevice instead of DeviceState. Correct that. Same for "riscv.sifive.u.soc"'s instance struct SiFiveUSoCState. Cc: Palmer Dabbelt Cc: Alistair Francis Cc: Sagar Karandikar Cc: Bastian Koppelmann Cc: qemu-ri...@nongnu.org Signed-off-by: Markus Armbruster --- include/hw/riscv/sifive_e.h | 2 +- include/hw/riscv/sifive_u.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h index 25ce7aa9d5..f05644df7c 100644 --- a/include/hw/riscv/sifive_e.h +++ b/include/hw/riscv/sifive_e.h @@ -29,7 +29,7 @@ typedef struct SiFiveESoCState { /*< private >*/ -SysBusDevice parent_obj; +DeviceState parent_obj; /*< public >*/ RISCVHartArrayState cpus; diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h index 16c297ec5f..5f62cf5f85 100644 --- a/include/hw/riscv/sifive_u.h +++ b/include/hw/riscv/sifive_u.h @@ -31,7 +31,7 @@ typedef struct SiFiveUSoCState { /*< private >*/ -SysBusDevice parent_obj; +DeviceState parent_obj; /*< public >*/ CPUClusterState e_cluster; -- 2.21.1
[PATCH 17/24] pnv/psi: Correct the pnv-psi* devices not to be sysbus devices
pnv_chip_power8_instance_init() creates a "pnv-psi-POWER8" sysbus device in a way that leaves it unplugged. pnv_chip_power9_instance_init() and pnv_chip_power10_instance_init() do the same for "pnv-psi-POWER9" and "pnv-psi-POWER10", respectively. These devices aren't actually sysbus devices. Correct that. Cc: "Cédric Le Goater" Cc: David Gibson Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- include/hw/ppc/pnv_psi.h | 2 +- hw/ppc/pnv_psi.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/hw/ppc/pnv_psi.h b/include/hw/ppc/pnv_psi.h index f0f5b55197..979fc59f33 100644 --- a/include/hw/ppc/pnv_psi.h +++ b/include/hw/ppc/pnv_psi.h @@ -31,7 +31,7 @@ #define PSIHB_XSCOM_MAX 0x20 typedef struct PnvPsi { -SysBusDevice parent; +DeviceState parent; MemoryRegion regs_mr; uint64_t bar; diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c index cfd5b7bc25..82f0769465 100644 --- a/hw/ppc/pnv_psi.c +++ b/hw/ppc/pnv_psi.c @@ -943,7 +943,7 @@ static void pnv_psi_class_init(ObjectClass *klass, void *data) static const TypeInfo pnv_psi_info = { .name = TYPE_PNV_PSI, -.parent= TYPE_SYS_BUS_DEVICE, +.parent= TYPE_DEVICE, .instance_size = sizeof(PnvPsi), .class_init= pnv_psi_class_init, .class_size= sizeof(PnvPsiClass), -- 2.21.1
[PATCH 12/24] MAINTAINERS: Make section PowerNV cover pci-host/pnv* as well
Cc: Cédric Le Goater Cc: David Gibson Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 47ef3139e6..074dc7f023 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1207,7 +1207,9 @@ S: Maintained F: hw/ppc/pnv* F: hw/intc/pnv* F: hw/intc/xics_pnv.c +F: hw/pci-host/pnv* F: include/hw/ppc/pnv* +F: include/hw/pci-host/pnv* F: pc-bios/skiboot.lid F: tests/qtest/pnv* -- 2.21.1
[PATCH 16/24] ppc/pnv: Put "*-pnv-chip" and "pnv-xive" on the main system bus
pnv_init() creates "power10_v1.0-pnv-chip", "power8_v2.0-pnv-chip", "power8e_v2.1-pnv-chip", "power8nvl_v1.0-pnv-chip", or "power9_v2.0-pnv-chip" sysbus devices in a way that leaves them unplugged. pnv_chip_power9_instance_init() creates a "pnv-xive" sysbus device in a way that leaves it unplugged. Create them the common way that puts them into the main system bus. Affects machines powernv8, powernv9, and powernv10. Visible in "info qtree". Here's the change for powernv9: bus: main-system-bus type System + dev: power9_v2.0-pnv-chip, id "" +chip-id = 0 (0x0) +ram-start = 0 (0x0) +ram-size = 1879048192 (0x7000) +nr-cores = 1 (0x1) +cores-mask = 72057594037927935 (0xff) +nr-threads = 1 (0x1) +num-phbs = 6 (0x6) +mmio 000603fc/0004 [...] + dev: pnv-xive, id "" +ic-bar = 1692157036462080 (0x603020310) +vc-bar = 1689949371891712 (0x60100) +pc-bar = 1690499127705600 (0x60180) +tm-bar = 1692157036986368 (0x603020318) Cc: "Cédric Le Goater" Cc: David Gibson Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- hw/ppc/pnv.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index da637822f9..8d4fc8109a 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -818,7 +818,7 @@ static void pnv_init(MachineState *machine) pnv->chips = g_new0(PnvChip *, pnv->num_chips); for (i = 0; i < pnv->num_chips; i++) { char chip_name[32]; -Object *chip = object_new(chip_typename); +Object *chip = OBJECT(qdev_create(NULL, chip_typename)); pnv->chips[i] = PNV_CHIP(chip); @@ -1317,8 +1317,8 @@ static void pnv_chip_power9_instance_init(Object *obj) PnvChipClass *pcc = PNV_CHIP_GET_CLASS(obj); int i; -object_initialize_child(obj, "xive", >xive, sizeof(chip9->xive), -TYPE_PNV_XIVE, _abort, NULL); +sysbus_init_child_obj(obj, "xive", >xive, sizeof(chip9->xive), + TYPE_PNV_XIVE); object_property_add_alias(obj, "xive-fabric", OBJECT(>xive), "xive-fabric"); -- 2.21.1
[PATCH 01/24] arm/stm32f405: Fix realization of "stm32f2xx-adc" devices
stm32f405_soc_initfn() creates six such devices, but stm32f405_soc_realize() realizes only one. Affects machine netduinoplus2. I wonder how this ever worked. If the "device becomes real only on realize" thing actually works, then we've always been missing five of six such devices, yet nobody noticed. Fix stm32f405_soc_realize() to realize all six. Visible in "info qtree": bus: main-system-bus type System dev: stm32f405-soc, id "" cpu-type = "cortex-m4-arm-cpu" dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 -mmio /00ff +mmio 40012000/00ff dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 -mmio /00ff +mmio 40012000/00ff dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 -mmio /00ff +mmio 40012000/00ff dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 -mmio /00ff +mmio 40012000/00ff dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 mmio 40012000/00ff dev: stm32f2xx-adc, id "" gpio-out "sysbus-irq" 1 -mmio /00ff +mmio 40012000/00ff dev: armv7m, id "" The mmio addresses look suspicious. Fixes: 529fc5fd3e18ace8f739afd02dc0953354f39442 Cc: Alistair Francis Cc: Peter Maydell Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- hw/arm/stm32f405_soc.c | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c index 4f10ce6176..4649502711 100644 --- a/hw/arm/stm32f405_soc.c +++ b/hw/arm/stm32f405_soc.c @@ -185,16 +185,18 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp) qdev_connect_gpio_out(DEVICE(>adc_irqs), 0, qdev_get_gpio_in(armv7m, ADC_IRQ)); -dev = DEVICE(&(s->adc[i])); -object_property_set_bool(OBJECT(>adc[i]), true, "realized", ); -if (err != NULL) { -error_propagate(errp, err); -return; +for (i = 0; i < STM_NUM_ADCS; i++) { +dev = DEVICE(&(s->adc[i])); +object_property_set_bool(OBJECT(>adc[i]), true, "realized", ); +if (err != NULL) { +error_propagate(errp, err); +return; +} +busdev = SYS_BUS_DEVICE(dev); +sysbus_mmio_map(busdev, 0, ADC_ADDR); +sysbus_connect_irq(busdev, 0, + qdev_get_gpio_in(DEVICE(>adc_irqs), i)); } -busdev = SYS_BUS_DEVICE(dev); -sysbus_mmio_map(busdev, 0, ADC_ADDR); -sysbus_connect_irq(busdev, 0, - qdev_get_gpio_in(DEVICE(>adc_irqs), i)); /* SPI devices */ for (i = 0; i < STM_NUM_SPIS; i++) { -- 2.21.1
[PATCH 07/24] auxbus: Fix aux-to-i2c-bridge to be a subtype of aux-slave
We plug aux-to-i2c-bridge into the aux-bus, even though its DeviceClass member bus_type is null, not TYPE_AUX_BUS. Fix that by deriving it from TYPE_AUX_SLAVE instead of TYPE_DEVICE. Cc: KONRAD Frederic Signed-off-by: Markus Armbruster --- hw/misc/auxbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c index f8e7b97971..5e4794f0ac 100644 --- a/hw/misc/auxbus.c +++ b/hw/misc/auxbus.c @@ -244,7 +244,7 @@ static inline I2CBus *aux_bridge_get_i2c_bus(AUXTOI2CState *bridge) static const TypeInfo aux_to_i2c_type_info = { .name = TYPE_AUXTOI2C, -.parent = TYPE_DEVICE, +.parent = TYPE_AUX_SLAVE, .class_init = aux_bridge_class_init, .instance_size = sizeof(AUXTOI2CState), .instance_init = aux_bridge_init -- 2.21.1
[PATCH 03/24] sd/pxa2xx_mmci: Fix to realize "pxa2xx-mmci" device
pxa2xx_mmci_init() creates a "pxa2xx-mmci" device, but neglects to realize it. Affects machines akita, borzoi, connex, mainstone, spitz, terrier, tosa, verdex, and z2. I wonder how this ever worked. If the "device becomes real only on realize" thing actually works, then we've always been missing the device, yet nobody noticed. Fix by realizing it right away. Visible in "info qom-tree"; here's the change for akita: /machine (akita-machine) [...] /unattached (container) [...] +/device[5] (pxa2xx-mmci) + /pxa2xx-mmci[0] (qemu:memory-region) + /sd-bus (pxa2xx-mmci-bus) [rest of device[*] renumbered...] Fixes: 7a9468c92517e19037bfe2272f64f5dadaf9db15 Cc: Andrzej Zaborowski Cc: Peter Maydell Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster --- hw/sd/pxa2xx_mmci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c index 8f9ab0ec16..6a80181bc9 100644 --- a/hw/sd/pxa2xx_mmci.c +++ b/hw/sd/pxa2xx_mmci.c @@ -492,6 +492,7 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem, sysbus_connect_irq(sbd, 0, irq); qdev_connect_gpio_out_named(dev, "rx-dma", 0, rx_dma); qdev_connect_gpio_out_named(dev, "tx-dma", 0, tx_dma); +qdev_init_nofail(dev); /* Create and plug in the sd card */ carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD); -- 2.21.1
[PATCH 00/24] Fixes around device realization
This fixes a bunch of bugs I ran into while reworking how qdevs plug into buses. I instrumented the code a bit to flush out instances of bug patterns. I'll post these hacks separately. Impact is less than clear in places. Help with that is appreciated. Markus Armbruster (24): arm/stm32f405: Fix realization of "stm32f2xx-adc" devices display/xlnx_dp: Fix to realize "i2c-ddc" and "aux-to-i2c-bridge" sd/pxa2xx_mmci: Fix to realize "pxa2xx-mmci" device aspeed: Don't create unwanted "ftgmac100", "aspeed-mmi" devices aspeed: Don't create unwanted "cortex-a7-arm-cpu" devices armv7m: Bury unwanted "ARM,bitband-memory" devices auxbus: Fix aux-to-i2c-bridge to be a subtype of aux-slave mac_via: Fix to realize "mos6522-q800-via*" devices macio: Fix to realize "mos6522-cuda" and "mos6522-pmu" devices macio: Bury unwanted "macio-gpio" devices pnv/phb4: Bury unwanted "pnv-phb4-pec-stack" devices MAINTAINERS: Make section PowerNV cover pci-host/pnv* as well ppc4xx: Drop redundant device realization macio: Put "macio-nvram" device on the macio bus macio: Fix macio-bus to be a subtype of System bus ppc/pnv: Put "*-pnv-chip" and "pnv-xive" on the main system bus pnv/psi: Correct the pnv-psi* devices not to be sysbus devices display/sm501 display/ati: Fix to realize "i2c-ddc" riscv: Fix to put "riscv.hart_array" devices on sysbus riscv: Fix type of SiFive[EU]SocState, member parent_obj sparc/leon3: Fix to put grlib,* devices on sysbus qdev: Assert devices are plugged into a bus that can take them sd: Hide the qdev-but-not-quite thing created by sd_init() qdev: Assert onboard devices all get realized properly include/hw/ppc/pnv_psi.h| 2 +- include/hw/riscv/sifive_e.h | 2 +- include/hw/riscv/sifive_u.h | 2 +- hw/arm/armv7m.c | 6 -- hw/arm/aspeed_ast2600.c | 5 - hw/arm/aspeed_soc.c | 2 +- hw/arm/stm32f405_soc.c | 20 ++- hw/core/qdev.c | 21 +++ hw/display/ati.c| 1 + hw/display/sm501.c | 1 + hw/display/xlnx_dp.c| 4 hw/misc/auxbus.c| 2 +- hw/misc/mac_via.c | 3 +++ hw/misc/macio/cuda.c| 8 +++- hw/misc/macio/macio.c | 7 +-- hw/misc/macio/pmu.c | 8 +++- hw/pci-host/pnv_phb4_pec.c | 3 +++ hw/ppc/pnv.c| 6 +++--- hw/ppc/pnv_psi.c| 2 +- hw/ppc/ppc440_uc.c | 2 -- hw/riscv/sifive_e.c | 5 ++--- hw/riscv/sifive_u.c | 14 ++--- hw/riscv/spike.c| 12 +-- hw/riscv/virt.c | 4 ++-- hw/sd/pxa2xx_mmci.c | 1 + hw/sd/sd.c | 40 ++--- hw/sparc/leon3.c| 4 ++-- MAINTAINERS | 2 ++ 28 files changed, 121 insertions(+), 68 deletions(-) -- 2.21.1
Re: [PATCH v6 07/20] hw/block/nvme: fix pin-based interrupt behavior
On May 14 06:45, Klaus Jensen wrote: > From: Klaus Jensen > > First, since the device only supports MSI-X or pin-based interrupt, if > MSI-X is not enabled, it should not accept interrupt vectors different > from 0 when creating completion queues. > > Secondly, the irq_status NvmeCtrl member is meant to be compared to the > INTMS register, so it should only be 32 bits wide. And it is really only > useful when used with multi-message MSI. > > Third, since we do not force a 1-to-1 correspondence between cqid and > interrupt vector, the irq_status register should not have bits set > according to cqid, but according to the associated interrupt vector. > > Fix these issues, but keep irq_status available so we can easily support > multi-message MSI down the line. > > Fixes: 5e9aa92eb1a5 ("hw/block: Fix pin-based interrupt behaviour of NVMe") > Cc: "Michael S. Tsirkin" > Cc: Marcel Apfelbaum > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 12 > hw/block/nvme.h | 2 +- > 2 files changed, 9 insertions(+), 5 deletions(-) > Gentle ping on this and [PATCH v6 08/20]. Thanks, Klaus
Re: Onboard audio devices and -audiodev none
Gerd Hoffmann writes: > On Fri, May 15, 2020 at 09:06:20AM +0200, Markus Armbruster wrote: >> Watch this: >> >> $ aarch64-softmmu/qemu-system-aarch64 -S -nodefaults -accel qtest >> -display none -M vexpress-a15 -audiodev none,id=foo >> audio: Device lm4549: audiodev default parameter is deprecated, please >> specify audiodev=foo >> >> I did, didn't I? > > https://patchwork.ozlabs.org/project/qemu-devel/patch/20200429110214.29037-13-kra...@redhat.com/ I don't immediately understand how that addresses my issue, but I trust it does :)
Re: [PATCH] trace/simple: Fix unauthorized enable
Stefan Hajnoczi writes: > On Fri, May 15, 2020 at 09:00:21AM +0200, Markus Armbruster wrote: >> diff --git a/trace/simple.c b/trace/simple.c >> index fc7106ec49..906391538f 100644 >> --- a/trace/simple.c >> +++ b/trace/simple.c >> @@ -302,10 +302,10 @@ static int st_write_event_mapping(void) >> return 0; >> } >> >> -void st_set_trace_file_enabled(bool enable) >> +bool st_set_trace_file_enabled(bool enable) >> { >> if (enable == !!trace_fp) { >> -return; /* no change */ >> +return enable; /* no change */ >> } >> >> /* Halt trace writeout */ >> @@ -323,14 +323,14 @@ void st_set_trace_file_enabled(bool enable) >> >> trace_fp = fopen(trace_file_name, "wb"); >> if (!trace_fp) { >> -return; >> +return !enable; >> } >> >> if (fwrite(, sizeof header, 1, trace_fp) != 1 || >> st_write_event_mapping() < 0) { >> fclose(trace_fp); >> trace_fp = NULL; >> -return; >> +return !enable; >> } >> >> /* Resume trace writeout */ >> @@ -340,6 +340,7 @@ void st_set_trace_file_enabled(bool enable) >> fclose(trace_fp); >> trace_fp = NULL; >> } >> +return !enable; >> } > > The meaning of the return value confuses me. Is it the previous value > (even when the function fails)? Please document the meaning. > > The code might be easier to understand like this: > > bool st_set_trace_file_enabled(bool enable) > { > bool was_enabled = trace_fp; > > if (enable == was_enabled) { > return was_enabled; /* no change */ > } > > ... > > return was_enabled; > } > > Now it is not necessary to remember that !enable is the previous value > because we would have already returned if the value was unchanged. I'll send v2. Thanks!
Re: [PATCH v2 1/3] qom/object: Move Object typedef to 'qemu/typedefs.h'
Philippe Mathieu-Daudé writes: > On Fri, May 15, 2020 at 8:07 AM Markus Armbruster wrote: >> >> Philippe Mathieu-Daudé writes: >> >> > We use the Object type all over the place. >> > Forward declare it in "qemu/typedefs.h". >> > >> > Signed-off-by: Philippe Mathieu-Daudé >> > --- >> > include/qemu/typedefs.h | 1 + >> > include/qom/object.h | 2 -- >> > include/qom/qom-qobject.h | 2 -- >> > include/sysemu/sysemu.h | 1 - >> > 4 files changed, 1 insertion(+), 5 deletions(-) >> > >> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h >> > index 375770a80f..b03ec9f40a 100644 >> > --- a/include/qemu/typedefs.h >> > +++ b/include/qemu/typedefs.h >> > @@ -75,6 +75,7 @@ typedef struct NetFilterState NetFilterState; >> > typedef struct NICInfo NICInfo; >> > typedef struct NodeInfo NodeInfo; >> > typedef struct NumaNodeMem NumaNodeMem; >> > +typedef struct Object Object; >> > typedef struct ObjectClass ObjectClass; >> > typedef struct PCIBridge PCIBridge; >> > typedef struct PCIBus PCIBus; >> > diff --git a/include/qom/object.h b/include/qom/object.h >> > index 784c97c0e1..1edc12e64c 100644 >> > --- a/include/qom/object.h >> > +++ b/include/qom/object.h >> > @@ -20,8 +20,6 @@ >> > struct TypeImpl; >> > typedef struct TypeImpl *Type; >> > >> > -typedef struct Object Object; >> > - >> > typedef struct TypeInfo TypeInfo; >> > >> > typedef struct InterfaceClass InterfaceClass; >> > diff --git a/include/qom/qom-qobject.h b/include/qom/qom-qobject.h >> > index 77cd717e3f..82136e6e80 100644 >> > --- a/include/qom/qom-qobject.h >> > +++ b/include/qom/qom-qobject.h >> > @@ -13,8 +13,6 @@ >> > #ifndef QEMU_QOM_QOBJECT_H >> > #define QEMU_QOM_QOBJECT_H >> > >> > -#include "qom/object.h" >> > - >> > /* >> > * object_property_get_qobject: >> > * @obj: the object >> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h >> > index ef81302e1a..ca4458e451 100644 >> > --- a/include/sysemu/sysemu.h >> > +++ b/include/sysemu/sysemu.h >> > @@ -5,7 +5,6 @@ >> > #include "qemu/timer.h" >> > #include "qemu/notify.h" >> > #include "qemu/uuid.h" >> > -#include "qom/object.h" >> > >> > /* vl.c */ >> >> How did you identify the inclusions to drop? > > Nothing very strict I'm afraid. I suppose I had the files opened in my editor. > > The output of > $ git grep -L -E '(OBJECT_|INTERFACE_)' $(git grep -l qom/object include) Why limit to include, and not check '*.h', or even '*.[ch]'? > suggests I missed those: > > -- >8 -- > diff --git a/include/hw/display/edid.h b/include/hw/display/edid.h > index ff99dc0a05..5b1de57f24 100644 > --- a/include/hw/display/edid.h > +++ b/include/hw/display/edid.h > @@ -1,8 +1,6 @@ > #ifndef EDID_H > #define EDID_H > > -#include "qom/object.h" > - > typedef struct qemu_edid_info { > const char *vendor; /* http://www.uefi.org/pnp_id_list */ > const char *name; > diff --git a/include/io/task.h b/include/io/task.h > index 1abbfb8b65..6818dfedd0 100644 > --- a/include/io/task.h > +++ b/include/io/task.h > @@ -21,8 +21,6 @@ > #ifndef QIO_TASK_H > #define QIO_TASK_H > > -#include "qom/object.h" > - > typedef struct QIOTask QIOTask; > > typedef void (*QIOTaskFunc)(QIOTask *task, > ---
Re: [PATCH Kernel v21 0/8] Add UAPIs to support migration for VFIO devices
On Mon, May 18, 2020 at 10:39:52AM +0800, Xiang Zheng wrote: > Hi Kirti and Yan, > > How can I test this patch series on my SR-IOV devices? > I have looked through Yan's pathes for i40e VF live migration support: > https://patchwork.kernel.org/patch/11375177/ > I just updated the patches to v4. https://patchwork.kernel.org/cover/11554617/. It's based on v17 kernel + v16 qemu with some minor changes in qemu. > However, I cannot find the detailed implementation about device state > saving/restoring and dirty page logging. Has i40e hardware already supported > these two features? > In v4, vendor driver for i40e vf reports dirty pages to vfio container. the detailed implementation of identifying dirty pages and device state is not sent yet for process reason. We use a software way to get dirty pages i.e. dynamically trapping of BAR 0. Thanks Yan > And if once a device supports both features, how to implement live > migration for this device via this series patch? > > On 2020/5/16 5:13, Kirti Wankhede wrote: > > Hi, > > > > This patch set adds: > > * IOCTL VFIO_IOMMU_DIRTY_PAGES to get dirty pages bitmap with > > respect to IOMMU container rather than per device. All pages pinned by > > vendor driver through vfio_pin_pages external API has to be marked as > > dirty during migration. When IOMMU capable device is present in the > > container and all pages are pinned and mapped, then all pages are marked > > dirty. > > When there are CPU writes, CPU dirty page tracking can identify dirtied > > pages, but any page pinned by vendor driver can also be written by > > device. As of now there is no device which has hardware support for > > dirty page tracking. So all pages which are pinned should be considered > > as dirty. > > This ioctl is also used to start/stop dirty pages tracking for pinned and > > unpinned pages while migration is active. > > > > * Updated IOCTL VFIO_IOMMU_UNMAP_DMA to get dirty pages bitmap before > > unmapping IO virtual address range. > > With vIOMMU, during pre-copy phase of migration, while CPUs are still > > running, IO virtual address unmap can happen while device still keeping > > reference of guest pfns. Those pages should be reported as dirty before > > unmap, so that VFIO user space application can copy content of those > > pages from source to destination. > > > > * Patch 8 detect if IOMMU capable device driver is smart to report pages > > to be marked dirty by pinning pages using vfio_pin_pages() API. > > > > > > Yet TODO: > > Since there is no device which has hardware support for system memmory > > dirty bitmap tracking, right now there is no other API from vendor driver > > to VFIO IOMMU module to report dirty pages. In future, when such hardware > > support will be implemented, an API will be required such that vendor > > driver could report dirty pages to VFIO module during migration phases. > > > > Adding revision history from previous QEMU patch set to understand KABI > > changes done till now > > > > v20 -> v21 > > - Added checkin for GET_BITMAP ioctl for vfio_dma boundaries. > > - Updated unmap ioctl function - as suggested by Alex. > > - Updated comments in DIRTY_TRACKING ioctl definition - as suggested by > > Cornelia. > > > > v19 -> v20 > > - Fixed ioctl to get dirty bitmap to get bitmap of multiple vfio_dmas > > - Fixed unmap ioctl to get dirty bitmap of multiple vfio_dmas. > > - Removed flag definition from migration capability. > > > > v18 -> v19 > > - Updated migration capability with supported page sizes bitmap for dirty > > page tracking and maximum bitmap size supported by kernel module. > > - Added patch to calculate and cache pgsize_bitmap when iommu->domain_list > > is updated. > > - Removed extra buffers added in previous version for bitmap manipulation > > and optimised the code. > > > > v17 -> v18 > > - Add migration capability to the capability chain for VFIO_IOMMU_GET_INFO > > ioctl > > - Updated UMAP_DMA ioctl to return bitmap of multiple vfio_dma > > > > v16 -> v17 > > - Fixed errors reported by kbuild test robot on i386 > > > > v15 -> v16 > > - Minor edits and nit picks (Auger Eric) > > - On copying bitmap to user, re-populated bitmap only for pinned pages, > > excluding unmapped pages and CPU dirtied pages. > > - Patches are on tag: next-20200318 and 1-3 patches from Yan's series > > https://lkml.org/lkml/2020/3/12/1255 > > > > v14 -> v15 > > - Minor edits and nit picks. > > - In the verification of user allocated bitmap memory, added check of > >maximum size. > > - Patches are on tag: next-20200318 and 1-3 patches from Yan's series > > https://lkml.org/lkml/2020/3/12/1255 > > > > v13 -> v14 > > - Added struct vfio_bitmap to kabi. updated structure > > vfio_iommu_type1_dirty_bitmap_get and vfio_iommu_type1_dma_unmap. > > - All small changes suggested by Alex. > > - Patches are on tag: next-20200318 and 1-3 patches from Yan's series > >
Re: [PATCH] net: use peer when purging queue in qemu_flush_or_purge_queue_packets()
On 2020/5/11 下午12:21, Alexander Bulekov wrote: On 200511 1204, Jason Wang wrote: The sender of packet will be checked in the qemu_net_queue_purge() but we use NetClientState not its peer when trying to purge the incoming queue in qemu_flush_or_purge_packets(). This will trigger the assert in virtio_net_reset since we can't pass the sender check. Fix by using the peer. Reported-by: "Alexander Bulekov" Fixes: ca77d85e1dbf9 ("net: complete all queued packets on VM stop") Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang Hi Jason, With this patch, I can no longer reproduce the crash Acked-by: Alexander Bulekov Thanks! Applied. Thanks --- net/net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/net.c b/net/net.c index 38778e831d..9e47cf727d 100644 --- a/net/net.c +++ b/net/net.c @@ -610,7 +610,7 @@ void qemu_flush_or_purge_queued_packets(NetClientState *nc, bool purge) qemu_notify_event(); } else if (purge) { /* Unable to empty the queue, purge remaining packets */ -qemu_net_queue_purge(nc->incoming_queue, nc); +qemu_net_queue_purge(nc->incoming_queue, nc->peer); } } -- 2.20.1
Re: [RFC v3 4/6] qmp: add QMP command x-debug-virtio-queue-status
On 2020/5/15 下午11:16, Laurent Vivier wrote: On 08/05/2020 04:57, Jason Wang wrote: On 2020/5/7 下午7:49, Laurent Vivier wrote: This new command shows internal status of a VirtQueue. (vrings and indexes). Signed-off-by: Laurent Vivier It looks to me that packed virtqueue is not supported. It's better to add them in the future. I agree, it's why the series still remains an "RFC". ... diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 59bf6ef651a6..57552bf05014 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -3877,6 +3877,41 @@ static VirtIODevice *virtio_device_find(const char *path) return NULL; } +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path, + uint16_t queue, Error **errp) +{ + VirtIODevice *vdev; + VirtQueueStatus *status; + + vdev = virtio_device_find(path); + if (vdev == NULL) { + error_setg(errp, "Path %s is not a VirtIO device", path); + return NULL; + } + + if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, queue)) { + error_setg(errp, "Invalid virtqueue number %d", queue); + return NULL; + } + + status = g_new0(VirtQueueStatus, 1); + status->queue_index = vdev->vq[queue].queue_index; + status->inuse = vdev->vq[queue].inuse; + status->vring_num = vdev->vq[queue].vring.num; + status->vring_num_default = vdev->vq[queue].vring.num_default; + status->vring_align = vdev->vq[queue].vring.align; + status->vring_desc = vdev->vq[queue].vring.desc; + status->vring_avail = vdev->vq[queue].vring.avail; + status->vring_used = vdev->vq[queue].vring.used; + status->last_avail_idx = vdev->vq[queue].last_avail_idx; This might not be correct when vhost is used. We may consider to sync last_avail_idx from vhost backends here? Yes, but I don't know how to do that. Where can I find the information? It could be synced through vhost ops vhost_get_vring_base(), see vhost_virtqueue_stop(). Thanks Thanks, Laurent
[Bug 1879227] [NEW] Assertion failure in e1000e_write_lgcy_rx_descr
Public bug reported: Hello, While fuzzing, I found an input which triggers an assertion failure in e1000e_write_lgcy_rx_descr: qemu-system-i386: /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1283: void e1000e_write_lgcy_rx_descr(E1000ECore *, uint8_t *, struct NetRxPkt *, const E1000E_RSSInfo *, uint16_t): Assertion `!rss_info->enabled' failed. Aborted #3 0x7684d092 in __GI___assert_fail (assertion=0x583704c0 "!rss_info->enabled", file=0x58361080 "/home/alxndr/Development/qemu/hw/net/e1000e_core.c", line=0x503, function=0x58370500 <__PRETTY_FUNCTION__.e1000e_write_lgcy_rx_descr> "void e1000e_write_lgcy_rx_descr(E1000ECore *, uint8_t *, struct NetRxPkt *, const E1000E_RSSInfo *, uint16_t)") at assert.c:101 #4 0x57209937 in e1000e_write_lgcy_rx_descr (core=0x7fffee0dd4e0, desc=0x7fff8720 "}}\253?", pkt=0x6114b900, rss_info=0x7fff8c50, length=0xcb) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1283 #5 0x57206b0b in e1000e_write_rx_descr (core=0x7fffee0dd4e0, desc=0x7fff8720 "}}\253?", pkt=0x6114b900, rss_info=0x7fff8c50, ps_hdr_len=0x0, written=0x7fff87c0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1360 #6 0x571f8507 in e1000e_write_packet_to_guest (core=0x7fffee0dd4e0, pkt=0x6114b900, rxr=0x7fff8c30, rss_info=0x7fff8c50) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1607 #7 0x571f5670 in e1000e_receive_iov (core=0x7fffee0dd4e0, iov=0x6194e780, iovcnt=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1709 #8 0x571f1afc in e1000e_nc_receive_iov (nc=0x61407460, iov=0x6194e780, iovcnt=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e.c:213 #9 0x571d5977 in net_tx_pkt_sendv (pkt=0x63128800, nc=0x61407460, iov=0x6194e780, iov_cnt=0x4) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:544 #10 0x571d50e4 in net_tx_pkt_send (pkt=0x63128800, nc=0x61407460) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:620 #11 0x571d638f in net_tx_pkt_send_loopback (pkt=0x63128800, nc=0x61407460) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:633 #12 0x5722b600 in e1000e_tx_pkt_send (core=0x7fffee0dd4e0, tx=0x7fffee0fd748, queue_index=0x0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:664 #13 0x57229ca6 in e1000e_process_tx_desc (core=0x7fffee0dd4e0, tx=0x7fffee0fd748, dp=0x7fff9440, queue_index=0x0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:743 #14 0x57228ea5 in e1000e_start_xmit (core=0x7fffee0dd4e0, txr=0x7fff9640) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:934 #15 0x5721c70f in e1000e_set_tdt (core=0x7fffee0dd4e0, index=0xe06, val=0xcb) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:2451 #16 0x571fa436 in e1000e_core_write (core=0x7fffee0dd4e0, addr=0x438, val=0xcb, size=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:3261 #17 0x571ed11c in e1000e_mmio_write (opaque=0x7fffee0da800, addr=0x438, val=0xcb, size=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e.c:109 #18 0x565e78b2 in memory_region_write_accessor (mr=0x7fffee0dd110, addr=0x438, value=0x7fff9cb0, size=0x4, shift=0x0, mask=0x, attrs=...) at /home/alxndr/Development/qemu/memory.c:483 #19 0x565e7212 in access_with_adjusted_size (addr=0x438, value=0x7fff9cb0, size=0x1, access_size_min=0x4, access_size_max=0x4, access_fn=0x565e72e0 , mr=0x7fffee0dd110, attrs=...) at /home/alxndr/Development/qemu/memory.c:544 #20 0x565e5c31 in memory_region_dispatch_write (mr=0x7fffee0dd110, addr=0x438, data=0xcb, op=MO_8, attrs=...) at /home/alxndr/Development/qemu/memory.c:1476 #21 0x563f04b9 in flatview_write_continue (fv=0x60637880, addr=0xe1020438, attrs=..., ptr=0x6199ba80, len=0x1, addr1=0x438, l=0x1, mr=0x7fffee0dd110) at /home/alxndr/Development/qemu/exec.c:3137 #22 0x563df2dd in flatview_write (fv=0x60637880, addr=0xe10200a8, attrs=..., buf=0x6199ba80, len=0x391) at /home/alxndr/Development/qemu/exec.c:3177 I can reproduce this in qemu 5.0 using these qtest commands: cat << EOF | ./qemu-system-i386 \ -qtest stdio -nographic -monitor none -serial none \ -M pc-q35-5.0 outl 0xcf8 0x80001010 outl 0xcfc 0xe102 outl 0xcf8 0x80001014 outl 0xcf8 0x80001004 outw 0xcfc 0x7 outl 0xcf8 0x800010a2 write 0xe1025008 0x4 0xfbffa3fa write 0xed040c 0x3 0x080047 write 0xe1020077 0x3c2
[Bug 1879223] [NEW] Assertion failure in e1000e_write_rx_descr
Public bug reported: Hello, While fuzzing, I found an input which triggers an assertion failure in e1000e_write_rx_descr: qemu-system-i386: /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1359: void e1000e_write_rx_descr(E1000ECore *, uint8_t *, struct NetRxPkt *, const E1000E_RSSInfo *, size_t, uint16_t (*)[4]): Assertion `ps_hdr_len == 0' failed. Aborted #3 0x7684d092 in __GI___assert_fail (assertion=0x583703e0 "ps_hdr_len == 0", file=0x58361080 "/home/alxndr/Development/qemu/hw/net/e1000e_core.c", line=0x54f, function=0x58370420 <__PRETTY_FUNCTION__.e1000e_write_rx_descr> "void e1000e_write_rx_descr(E1000ECore *, uint8_t *, struct NetRxPkt *, const E1000E_RSSInfo *, size_t, uint16_t (*)[4])") at assert.c:101 #4 0x57206a58 in e1000e_write_rx_descr (core=0x7fffee0dd4e0, desc=0x7fff8720 "", pkt=0x0, rss_info=0x7fff8c50, ps_hdr_len=0x2a, written=0x7fff87c0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1359 #5 0x571f8507 in e1000e_write_packet_to_guest (core=0x7fffee0dd4e0, pkt=0x6114b900, rxr=0x7fff8c30, rss_info=0x7fff8c50) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1607 #6 0x571f5670 in e1000e_receive_iov (core=0x7fffee0dd4e0, iov=0x6194e780, iovcnt=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1709 #7 0x571f1afc in e1000e_nc_receive_iov (nc=0x61407460, iov=0x6194e780, iovcnt=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e.c:213 #8 0x571d5977 in net_tx_pkt_sendv (pkt=0x63128800, nc=0x61407460, iov=0x6194e780, iov_cnt=0x4) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:544 #9 0x571d50e4 in net_tx_pkt_send (pkt=0x63128800, nc=0x61407460) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:620 #10 0x571d638f in net_tx_pkt_send_loopback (pkt=0x63128800, nc=0x61407460) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:633 #11 0x5722b600 in e1000e_tx_pkt_send (core=0x7fffee0dd4e0, tx=0x7fffee0fd748, queue_index=0x0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:664 #12 0x57229ca6 in e1000e_process_tx_desc (core=0x7fffee0dd4e0, tx=0x7fffee0fd748, dp=0x7fff9440, queue_index=0x0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:743 #13 0x57228ea5 in e1000e_start_xmit (core=0x7fffee0dd4e0, txr=0x7fff9640) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:934 #14 0x5721c70f in e1000e_set_tdt (core=0x7fffee0dd4e0, index=0xe06, val=0xcb) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:2451 #15 0x571fa436 in e1000e_core_write (core=0x7fffee0dd4e0, addr=0x438, val=0xcb, size=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:3261 #16 0x571ed11c in e1000e_mmio_write (opaque=0x7fffee0da800, addr=0x438, val=0xcb, size=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e.c:109 #17 0x565e78b2 in memory_region_write_accessor (mr=0x7fffee0dd110, addr=0x438, value=0x7fff9cb0, size=0x4, shift=0x0, mask=0x, attrs=...) at /home/alxndr/Development/qemu/memory.c:483 #18 0x565e7212 in access_with_adjusted_size (addr=0x438, value=0x7fff9cb0, size=0x1, access_size_min=0x4, access_size_max=0x4, access_fn=0x565e72e0 , mr=0x7fffee0dd110, attrs=...) at /home/alxndr/Development/qemu/memory.c:544 #19 0x565e5c31 in memory_region_dispatch_write (mr=0x7fffee0dd110, addr=0x438, data=0xcb, op=MO_8, attrs=...) at /home/alxndr/Development/qemu/memory.c:1476 #20 0x563f04b9 in flatview_write_continue (fv=0x60637880, addr=0xe1020438, attrs=..., ptr=0x6199ba80, len=0x1, addr1=0x438, l=0x1, mr=0x7fffee0dd110) at /home/alxndr/Development/qemu/exec.c:3137 #21 0x563df2dd in flatview_write (fv=0x60637880, addr=0xe1020077, attrs=..., buf=0x6199ba80, len=0x3c2) at /home/alxndr/Development/qemu/exec.c:3177 #22 0x563deded in address_space_write (as=0x608027a0, addr=0xe1020077, attrs=..., buf=0x6199ba80, len=0x3c2) at /home/alxndr/Development/qemu/exec.c:3268 I can reproduce this in qemu 5.0 using these qtest commands: cat << EOF | ./qemu-system-i386 \ -qtest stdio -nographic -monitor none -serial none \ -M pc-q35-5.0 outl 0xcf8 0x80001010 outl 0xcfc 0xe102 outl 0xcf8 0x80001014 outl 0xcf8 0x80001004 outw 0xcfc 0x7 outl 0xcf8 0x800010a2 write 0xe1025008 0x4 0xfbffa3fa write 0xed040c 0x3 0x080047 write 0xe1020077 0x3c2
Re: [PATCH Kernel v21 0/8] Add UAPIs to support migration for VFIO devices
On 5/18/2020 8:09 AM, Xiang Zheng wrote: Hi Kirti and Yan, How can I test this patch series on my SR-IOV devices? I have looked through Yan's pathes for i40e VF live migration support: https://patchwork.kernel.org/patch/11375177/ However, I cannot find the detailed implementation about device state saving/restoring and dirty page logging. Details of device state transitions, how user application should use UAPI and how vendor driver should behave are mentioned in the comment of UAPI: https://lore.kernel.org/kvm/1589577203-20640-2-git-send-email-kwankh...@nvidia.com/ Has i40e hardware already supported these two features? And if once a device supports both features, how to implement live migration for this device via this series patch? Here is the patch-set which adds migration support to mtty sample device, you can refer to see how to implement on driver side: https://lore.kernel.org/kvm/1588614860-16330-1-git-send-email-kwankh...@nvidia.com/ I'm working on preparing QEMU patches for v21 and will be sending out soon. Thanks, Kirti On 2020/5/16 5:13, Kirti Wankhede wrote: Hi, This patch set adds: * IOCTL VFIO_IOMMU_DIRTY_PAGES to get dirty pages bitmap with respect to IOMMU container rather than per device. All pages pinned by vendor driver through vfio_pin_pages external API has to be marked as dirty during migration. When IOMMU capable device is present in the container and all pages are pinned and mapped, then all pages are marked dirty. When there are CPU writes, CPU dirty page tracking can identify dirtied pages, but any page pinned by vendor driver can also be written by device. As of now there is no device which has hardware support for dirty page tracking. So all pages which are pinned should be considered as dirty. This ioctl is also used to start/stop dirty pages tracking for pinned and unpinned pages while migration is active. * Updated IOCTL VFIO_IOMMU_UNMAP_DMA to get dirty pages bitmap before unmapping IO virtual address range. With vIOMMU, during pre-copy phase of migration, while CPUs are still running, IO virtual address unmap can happen while device still keeping reference of guest pfns. Those pages should be reported as dirty before unmap, so that VFIO user space application can copy content of those pages from source to destination. * Patch 8 detect if IOMMU capable device driver is smart to report pages to be marked dirty by pinning pages using vfio_pin_pages() API. Yet TODO: Since there is no device which has hardware support for system memmory dirty bitmap tracking, right now there is no other API from vendor driver to VFIO IOMMU module to report dirty pages. In future, when such hardware support will be implemented, an API will be required such that vendor driver could report dirty pages to VFIO module during migration phases. Adding revision history from previous QEMU patch set to understand KABI changes done till now v20 -> v21 - Added checkin for GET_BITMAP ioctl for vfio_dma boundaries. - Updated unmap ioctl function - as suggested by Alex. - Updated comments in DIRTY_TRACKING ioctl definition - as suggested by Cornelia. v19 -> v20 - Fixed ioctl to get dirty bitmap to get bitmap of multiple vfio_dmas - Fixed unmap ioctl to get dirty bitmap of multiple vfio_dmas. - Removed flag definition from migration capability. v18 -> v19 - Updated migration capability with supported page sizes bitmap for dirty page tracking and maximum bitmap size supported by kernel module. - Added patch to calculate and cache pgsize_bitmap when iommu->domain_list is updated. - Removed extra buffers added in previous version for bitmap manipulation and optimised the code. v17 -> v18 - Add migration capability to the capability chain for VFIO_IOMMU_GET_INFO ioctl - Updated UMAP_DMA ioctl to return bitmap of multiple vfio_dma v16 -> v17 - Fixed errors reported by kbuild test robot on i386 v15 -> v16 - Minor edits and nit picks (Auger Eric) - On copying bitmap to user, re-populated bitmap only for pinned pages, excluding unmapped pages and CPU dirtied pages. - Patches are on tag: next-20200318 and 1-3 patches from Yan's series https://lkml.org/lkml/2020/3/12/1255 v14 -> v15 - Minor edits and nit picks. - In the verification of user allocated bitmap memory, added check of maximum size. - Patches are on tag: next-20200318 and 1-3 patches from Yan's series https://lkml.org/lkml/2020/3/12/1255 v13 -> v14 - Added struct vfio_bitmap to kabi. updated structure vfio_iommu_type1_dirty_bitmap_get and vfio_iommu_type1_dma_unmap. - All small changes suggested by Alex. - Patches are on tag: next-20200318 and 1-3 patches from Yan's series https://lkml.org/lkml/2020/3/12/1255 v12 -> v13 - Changed bitmap allocation in vfio_iommu_type1 to per vfio_dma - Changed VFIO_IOMMU_DIRTY_PAGES ioctl behaviour to be per vfio_dma range. - Changed vfio_iommu_type1_dirty_bitmap structure to have
Re: Emulating Solaris 10 on SPARC64 sun4u
> Great progress! Are you planning to contribute your escc2 to the > upstream? I would like to. While it didn't solve the console difficulties on OpenSolaris variants, it's probably still a good idea to increment Sun4u emulation towards being more faithful to hardware. It will take me a few weeks to clean up and test but I'll make an RFC once I have a patch to discuss. It's unlikely that the patch will be feature complete but I'll aim for it to be enough to satisfy the Linux/OpenBSD drivers. > We can use the strategy I used 10 years back for sun4m: boot the > proprietary firmware first to reduce the number of possible emulation > bugs. The proprietary firmware is known to boot Solaris. Using the proprietary firmware for this would be ideal. It would also provide reliable access to the kernel debugger which would be extremely helpful for diagnosing what's going wrong with the console. I'm not sure how I would go about making progress on this though. I know there are binaries of the BIOS for Sun4m machines floating around but I'm not aware of any for Sun4u machines. Thanks, Jasper Lowell. On Sun, 2020-05-17 at 14:37 +0200, Artyom Tarasenko wrote: > On Sun, May 17, 2020 at 9:57 AM wrote: > > I've written up a basic implementation of the SAB 82532 ESCC2 > > device > > and have written a patch for OpenBIOS to add it to the device tree. > > I > > still have the 16550A UART acting as ttya to avoid having to write > > an > > OpenBIOS device driver. > > Great progress! Are you planning to contribute your escc2 to the > upstream? > > > OpenBSD and Solaris both identify the device correctly and attach > > it. > > > > Interestingly, it looks like Solaris entered an interrupt handler > > to > > deal with an event for the SAB 82532 and it probed a few status > > registers. Given that I couldn't encourage Solaris to do anything > > outside of attach the device for the 16550A, I was thinking this > > might > > be a good sign. > > > > There really shouldn't be an interrupt though as the reset state of > > the > > SAB 82532 is to have all interrupts masked. Solaris probes these > > interrupt status registers while "configuring" the sunhme device. > > > > Attempting to configure interface hme0... > > 140070@1589698603.268942:escc2_mem_read channel a addr 0x38 value > > 0x4 > > 140070@1589698603.269011:escc2_irq_update value 0x0 > > 140070@1589698603.269015:escc2_mem_read channel a addr 0x3a value > > 0x80 > > 140070@1589698603.269017:escc2_irq_update value 0x0 > > 140070@1589698603.269018:escc2_mem_read channel a addr 0x3b value > > 0x0 > > 140070@1589698603.269076:escc2_mem_read channel a addr 0x38 value > > 0x0 > > WARNING: Power off requested from power button or SC, powering down > > the > > system! > > 140070@1589698611.270845:escc2_mem_read channel a addr 0x38 value > > 0x0 > > 140070@1589698619.271758:escc2_mem_read channel a addr 0x38 value > > 0x0 > > > > I've noticed that removing the sunhme code for sun4u.c in QEMU > > prevents > > the spurious interrupt from happening for the serial device along > > with > > prevent the unexpected power off request (the power off request was > > not > > introduced by my code). I haven't investigated why the presence of > > sunhme causes these events. > > > > I modified OpenBIOS to use ttyb for stdin/stdout and assigned that > > to > > the 16550A so that ttya could be aliased to the SAB 82532. Outside > > of > > the spurious interrupt handler being called, I'm getting the same > > behaviour where Solaris identifies, attaches, and then ignores the > > device. Doesn't look like my guess was on the mark. > > > > I'll be looking at getting an OpenSolaris-like kernel built with > > prom > > debugging output for console/tty related files like > > usr/src/uts/common/io/consconfig_dacf.c. The illumos kernel is > > probably > > suitable for this because it's still maintained and appears to be > > suffering from the same console problems. I don't have a SPARC64 > > machine immediately available and I'm unfamiliar with the build > > system > > behind these distributions, so it might be a while before this > > happens. > > > > I'm out of ideas. > > We can use the strategy I used 10 years back for sun4m: boot the > proprietary firmware first to reduce the number of possible emulation > bugs. The proprietary firmware is known to boot Solaris. > > Regards, > Artyom
Re: [PATCH v2 5/5] vhost: add device started check in migration set log
On 2020/5/16 上午11:20, Li Feng wrote: Hi, Dima. This abort is what I have mentioned in my previous email. I have triggered this crash without any fix a week ago. And I have written a test patch to let vhost_log_global_start return int and propagate the error to up layer. However, my change is a little large, because the origin callback return void, and don't do some rollback. After test, the migration could migrate to dst successfully, and fio is still running perfectly, but the src vm is still stuck here, no crash. Is it right to return this error to the up layer? That could be a solution or we may ask David for more suggestion. Another thing that might be useful is to block re connection during migration. Thanks Thanks, Feng Li Dima Stepanov 于2020年5月16日周六 上午12:55写道: On Thu, May 14, 2020 at 03:34:24PM +0800, Jason Wang wrote: On 2020/5/13 下午5:47, Dima Stepanov wrote: case CHR_EVENT_CLOSED: /* a close event may happen during a read/write, but vhost * code assumes the vhost_dev remains setup, so delay the * stop & clear to idle. * FIXME: better handle failure in vhost code, remove bh */ if (s->watch) { AioContext *ctx = qemu_get_current_aio_context(); g_source_remove(s->watch); s->watch = 0; qemu_chr_fe_set_handlers(>chr, NULL, NULL, NULL, NULL, NULL, NULL, false); aio_bh_schedule_oneshot(ctx, chr_closed_bh, opaque); } break; I think it's time we dropped the FIXME and moved the handling to common code. Jason? Marc-André? I agree. Just to confirm, do you prefer bh or doing changes like what is done in this series? It looks to me bh can have more easier codes. Could it be a good idea just to make disconnect in the char device but postphone clean up in the vhost-user-blk (or any other vhost-user device) itself? So we are moving the postphone logic and decision from the char device to vhost-user device. One of the idea i have is as follows: - Put ourself in the INITIALIZATION state - Start these vhost-user "handshake" commands - If we got a disconnect error, perform disconnect, but don't clean up device (it will be clean up on the roll back). I can be done by checking the state in vhost_user_..._disconnect routine or smth like it Any issue you saw just using the aio bh as Michael posted above. Then we don't need to deal with the silent vhost_dev_stop() and we will have codes that is much more easier to understand. I've implemented this solution inside hw/block/vhost-user-blk.c:vhost_user_blk_event() in the similar way by using the s->connected field. Looks good and more correct fix ). I have two questions here before i'll rework the fixes: 1. Is it okay to make the similar fix inside vhost_user_blk_event() or we are looking for more generic vhost-user solution? What do you think? 2. For migration we require an additional information that for the vhost-user device it isn't an error, because i'm trigerring the following assert error: Core was generated by `x86_64-softmmu/qemu-system-x86_64 -nodefaults -no-user-config -M q35,sata=false'. Program terminated with signal SIGABRT, Aborted. #0 0x7fb56e729428 in raise () from /lib/x86_64-linux-gnu/libc.so.6 [Current thread is 1 (Thread 0x7fb486ef5700 (LWP 527734))] (gdb) bt #0 0x7fb56e729428 in raise () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x7fb56e72b02a in abort () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x5648ea376ee6 in vhost_log_global_start (listener=0x5648ece4eb08) at ./hw/virtio/vhost.c:857 #3 0x5648ea2dde7e in memory_global_dirty_log_start () at ./memory.c:2611 #4 0x5648ea2e68e7 in ram_init_bitmaps (rs=0x7fb4740008c0) at ./migration/ram.c:2305 #5 0x5648ea2e698b in ram_init_all (rsp=0x5648eb1f0f20 ) at ./migration/ram.c:2323 #6 0x5648ea2e6cc5 in ram_save_setup (f=0x5648ec609e00, opaque=0x5648eb1f0f20 ) at ./migration/ram.c:2436 #7 0x5648ea67b7d3 in qemu_savevm_state_setup (f=0x5648ec609e00) at migration/savevm.c:1176 #8 0x5648ea674511 in migration_thread (opaque=0x5648ec031ff0) at migration/migration.c:3416 #9 0x5648ea85d65d in qemu_thread_start (args=0x5648ec6057f0) at util/qemu-thread-posix.c:519 #10 0x7fb56eac56ba in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 #11 0x7fb56e7fb41d in clone () from /lib/x86_64-linux-gnu/libc.so.6 (gdb) frame 2 #2 0x5648ea376ee6 in vhost_log_global_start (listener=0x5648ece4eb08) at ./hw/virtio/vhost.c:857 857 abort(); (gdb) list 852 { 853 int r; 854 855 r = vhost_migration_log(listener, true); 856 if (r < 0) { 857 abort(); 858 } 859 } 860 861 static void
Re: [PATCH v2 5/5] vhost: add device started check in migration set log
On 2020/5/16 上午12:54, Dima Stepanov wrote: On Thu, May 14, 2020 at 03:34:24PM +0800, Jason Wang wrote: On 2020/5/13 下午5:47, Dima Stepanov wrote: case CHR_EVENT_CLOSED: /* a close event may happen during a read/write, but vhost * code assumes the vhost_dev remains setup, so delay the * stop & clear to idle. * FIXME: better handle failure in vhost code, remove bh */ if (s->watch) { AioContext *ctx = qemu_get_current_aio_context(); g_source_remove(s->watch); s->watch = 0; qemu_chr_fe_set_handlers(>chr, NULL, NULL, NULL, NULL, NULL, NULL, false); aio_bh_schedule_oneshot(ctx, chr_closed_bh, opaque); } break; I think it's time we dropped the FIXME and moved the handling to common code. Jason? Marc-André? I agree. Just to confirm, do you prefer bh or doing changes like what is done in this series? It looks to me bh can have more easier codes. Could it be a good idea just to make disconnect in the char device but postphone clean up in the vhost-user-blk (or any other vhost-user device) itself? So we are moving the postphone logic and decision from the char device to vhost-user device. One of the idea i have is as follows: - Put ourself in the INITIALIZATION state - Start these vhost-user "handshake" commands - If we got a disconnect error, perform disconnect, but don't clean up device (it will be clean up on the roll back). I can be done by checking the state in vhost_user_..._disconnect routine or smth like it Any issue you saw just using the aio bh as Michael posted above. Then we don't need to deal with the silent vhost_dev_stop() and we will have codes that is much more easier to understand. I've implemented this solution inside hw/block/vhost-user-blk.c:vhost_user_blk_event() in the similar way by using the s->connected field. Looks good and more correct fix ). I have two questions here before i'll rework the fixes: 1. Is it okay to make the similar fix inside vhost_user_blk_event() or we are looking for more generic vhost-user solution? What do you think? I think I agree with Michael, it's better to have a generic vhost-user solution. But if it turns out to be not easy, we can start from fixing vhost-user-blk. 2. For migration we require an additional information that for the vhost-user device it isn't an error, because i'm trigerring the following assert error: Core was generated by `x86_64-softmmu/qemu-system-x86_64 -nodefaults -no-user-config -M q35,sata=false'. Program terminated with signal SIGABRT, Aborted. #0 0x7fb56e729428 in raise () from /lib/x86_64-linux-gnu/libc.so.6 [Current thread is 1 (Thread 0x7fb486ef5700 (LWP 527734))] (gdb) bt #0 0x7fb56e729428 in raise () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x7fb56e72b02a in abort () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x5648ea376ee6 in vhost_log_global_start (listener=0x5648ece4eb08) at ./hw/virtio/vhost.c:857 #3 0x5648ea2dde7e in memory_global_dirty_log_start () at ./memory.c:2611 #4 0x5648ea2e68e7 in ram_init_bitmaps (rs=0x7fb4740008c0) at ./migration/ram.c:2305 #5 0x5648ea2e698b in ram_init_all (rsp=0x5648eb1f0f20 ) at ./migration/ram.c:2323 #6 0x5648ea2e6cc5 in ram_save_setup (f=0x5648ec609e00, opaque=0x5648eb1f0f20 ) at ./migration/ram.c:2436 #7 0x5648ea67b7d3 in qemu_savevm_state_setup (f=0x5648ec609e00) at migration/savevm.c:1176 #8 0x5648ea674511 in migration_thread (opaque=0x5648ec031ff0) at migration/migration.c:3416 #9 0x5648ea85d65d in qemu_thread_start (args=0x5648ec6057f0) at util/qemu-thread-posix.c:519 #10 0x7fb56eac56ba in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 #11 0x7fb56e7fb41d in clone () from /lib/x86_64-linux-gnu/libc.so.6 (gdb) frame 2 #2 0x5648ea376ee6 in vhost_log_global_start (listener=0x5648ece4eb08) at ./hw/virtio/vhost.c:857 857 abort(); (gdb) list 852 { 853 int r; 854 855 r = vhost_migration_log(listener, true); 856 if (r < 0) { 857 abort(); 858 } 859 } 860 861 static void vhost_log_global_stop(MemoryListener *listener) Since bh postphone the clean up, we can't use the ->started field. Do we have any mechanism to get the device type/state in the common vhost_migration_log() routine? So for example for the vhost-user/disconnect device we will be able to return 0. Or should we implement it and introduce it in this patch set? This requires more thought, I will reply in Feng's mail. Thanks Thanks, Dima. Thank - vhost-user command returns error back to the _start() routine - Rollback in one place in the start() routine, by calling this postphoned clean
Re: [PATCH Kernel v21 0/8] Add UAPIs to support migration for VFIO devices
Hi Kirti and Yan, How can I test this patch series on my SR-IOV devices? I have looked through Yan's pathes for i40e VF live migration support: https://patchwork.kernel.org/patch/11375177/ However, I cannot find the detailed implementation about device state saving/restoring and dirty page logging. Has i40e hardware already supported these two features? And if once a device supports both features, how to implement live migration for this device via this series patch? On 2020/5/16 5:13, Kirti Wankhede wrote: > Hi, > > This patch set adds: > * IOCTL VFIO_IOMMU_DIRTY_PAGES to get dirty pages bitmap with > respect to IOMMU container rather than per device. All pages pinned by > vendor driver through vfio_pin_pages external API has to be marked as > dirty during migration. When IOMMU capable device is present in the > container and all pages are pinned and mapped, then all pages are marked > dirty. > When there are CPU writes, CPU dirty page tracking can identify dirtied > pages, but any page pinned by vendor driver can also be written by > device. As of now there is no device which has hardware support for > dirty page tracking. So all pages which are pinned should be considered > as dirty. > This ioctl is also used to start/stop dirty pages tracking for pinned and > unpinned pages while migration is active. > > * Updated IOCTL VFIO_IOMMU_UNMAP_DMA to get dirty pages bitmap before > unmapping IO virtual address range. > With vIOMMU, during pre-copy phase of migration, while CPUs are still > running, IO virtual address unmap can happen while device still keeping > reference of guest pfns. Those pages should be reported as dirty before > unmap, so that VFIO user space application can copy content of those > pages from source to destination. > > * Patch 8 detect if IOMMU capable device driver is smart to report pages > to be marked dirty by pinning pages using vfio_pin_pages() API. > > > Yet TODO: > Since there is no device which has hardware support for system memmory > dirty bitmap tracking, right now there is no other API from vendor driver > to VFIO IOMMU module to report dirty pages. In future, when such hardware > support will be implemented, an API will be required such that vendor > driver could report dirty pages to VFIO module during migration phases. > > Adding revision history from previous QEMU patch set to understand KABI > changes done till now > > v20 -> v21 > - Added checkin for GET_BITMAP ioctl for vfio_dma boundaries. > - Updated unmap ioctl function - as suggested by Alex. > - Updated comments in DIRTY_TRACKING ioctl definition - as suggested by > Cornelia. > > v19 -> v20 > - Fixed ioctl to get dirty bitmap to get bitmap of multiple vfio_dmas > - Fixed unmap ioctl to get dirty bitmap of multiple vfio_dmas. > - Removed flag definition from migration capability. > > v18 -> v19 > - Updated migration capability with supported page sizes bitmap for dirty > page tracking and maximum bitmap size supported by kernel module. > - Added patch to calculate and cache pgsize_bitmap when iommu->domain_list > is updated. > - Removed extra buffers added in previous version for bitmap manipulation > and optimised the code. > > v17 -> v18 > - Add migration capability to the capability chain for VFIO_IOMMU_GET_INFO > ioctl > - Updated UMAP_DMA ioctl to return bitmap of multiple vfio_dma > > v16 -> v17 > - Fixed errors reported by kbuild test robot on i386 > > v15 -> v16 > - Minor edits and nit picks (Auger Eric) > - On copying bitmap to user, re-populated bitmap only for pinned pages, > excluding unmapped pages and CPU dirtied pages. > - Patches are on tag: next-20200318 and 1-3 patches from Yan's series > https://lkml.org/lkml/2020/3/12/1255 > > v14 -> v15 > - Minor edits and nit picks. > - In the verification of user allocated bitmap memory, added check of >maximum size. > - Patches are on tag: next-20200318 and 1-3 patches from Yan's series > https://lkml.org/lkml/2020/3/12/1255 > > v13 -> v14 > - Added struct vfio_bitmap to kabi. updated structure > vfio_iommu_type1_dirty_bitmap_get and vfio_iommu_type1_dma_unmap. > - All small changes suggested by Alex. > - Patches are on tag: next-20200318 and 1-3 patches from Yan's series > https://lkml.org/lkml/2020/3/12/1255 > > v12 -> v13 > - Changed bitmap allocation in vfio_iommu_type1 to per vfio_dma > - Changed VFIO_IOMMU_DIRTY_PAGES ioctl behaviour to be per vfio_dma range. > - Changed vfio_iommu_type1_dirty_bitmap structure to have separate data > field. > > v11 -> v12 > - Changed bitmap allocation in vfio_iommu_type1. > - Remove atomicity of ref_count. > - Updated comments for migration device state structure about error > reporting. > - Nit picks from v11 reviews > > v10 -> v11 > - Fix pin pages API to free vpfn if it is marked as unpinned tracking page. > - Added proposal to detect if IOMMU capable device calls external pin pages > API to mark pages dirty. >
checkpatch error checking target arch in libvhost-user
Hey Marc-Andre, I'm working on a patchset with changes to libvhost-user. I'm hitting the following checkpatch error: Checking 0011-Lift-max-ram-slots-limit-in-libvhost-user.patch... WARNING: architecture specific defines should be avoided #117: FILE: contrib/libvhost-user/libvhost-user.h:38: +#if defined(__i386__) || defined(__x86_64__) || \ total: 0 errors, 1 warnings, 120 lines checked 0011-Lift-max-ram-slots-limit-in-libvhost-user.patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. I'm trying to set the maximum number of ram slots to the max supported by the target architecture, and I don't know how to check that other than using these macros. I see other architecture specific macro checks in libvhost-user.h, such as here: https://u16159052.ct.sendgrid.net/ls/click?upn=nHC8zHLUbSCl8801JuFxA5IdcFluFbhkOaN0W6nB6sLdfiznj-2FjAzM5FRqjRFWnMRnCWGbBIBOa9D0WJ4d1Dc3pvHvScsomf772bjiFIvKp8WAltnuQtFL02yD-2FAsRP43foG_E8SO-2FEypfU855L0ybQoiQY4Xaj8Z6NYzBoBK89OH-2BiJs2oVXUe9lHVA11uxF6eAFNBHYjrZ2L2x0rg8pxpJb7k58gV-2F0Pcs9b1FHGfiCxHVycMD52nL6rnjHYR0U2e5u-2Bb-2FDyF1ZIH76E5Zwe4Oe1vSdsfafOUWalRu1CMBhlsPO2TcpqiXnBFj1QaM1IrTe5RuhZMjTr5ZROvgb1i-2B74VaAbgKu-2FcfcA8Sk1-2BYxpqTTxs3x6f-2BYkMHAu-2BqEUJdY Should I ignore this warning? If not, do you have any other suggestions? Thanks, Raphael
[Bug 1295587] Re: Temporal freeze and slowdown while using emulated sb16
After banging my head in a wall for tree or four days, I got the ac97 to work on windows 98se applying something called "Auto-patcher for windows 98se" downloaded from retrosystemsrevival, then using the windows 95 "VXD_A406" driver updated manually by unpacking the executable and picking the .inf file manually. The auto-patcher is mandatory to get everything working. I followed steps from a youtube video for creating a windows 98 VM in Virtualbox, worked on qemu. The installation process was long and boring, but in the end, everything seems to be working without problems (so far). All links can be found in a youtube video by the name "Windows 98 on VirualBox Part 2. AutoPatching, AC97 Sound Drivers, Windows 98 Plus! Gamepad Install." or in the following pastebin: https://pastebin.com/hMvcMzFL -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1295587 Title: Temporal freeze and slowdown while using emulated sb16 Status in QEMU: New Bug description: I have been carrying around this bug since previous versions and on different machines: When I use the -soundhw sb16 option, while playing any sound on the virtual machine it temporally freezes the emulated machine and loops the last bit of such sound effect for 1-2 minutes, then goes back to normal speed (until a new sound is played). Console shows: sb16: warning: command 0xf9,1 is not truly understood yet sb16: warning: command 0xf9,1 is not truly understood yet (...) main-loop: WARNING: I/O thread spun for 1000 iterations -One of my emulated machines is Windows 3.11: I managed to overrun this bug by switching from the local 1.5 version of the sound blaster driver to the 1.0, although since I updated qemu it freezes that machine, so I can't test if it still works. I am using the 1.7.90 version, but I suffered this bug for over one year (confirmed in version 2.0.0-rc0 too) this bug happens anytime I use the -soundhw sb16 switch, but the full command I am using in this specific case is: qemu-system-i386 -localtime -cpu pentium -m 32 -display sdl -vga cirrus -hda c.img -cdrom win95stuff.iso -net nic,model=ne2k_pci -net user -soundhw sb16 This bug appears on all my machines: Pentium III running Slackware 13.0 and freeBSD 10; Dual core T2400, both in Arch, Gentoo and Slackware 14.1 (all 32 bits), and a Dual core T4400 64 bits with Gentoo and Slackware. Same problem in all of those systems after compiling instead of using the distro packages To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1295587/+subscriptions
[RFC] Various questions about TCG implementation, DRM patches dealing with pointers over guest-host barrier.
Hi, I've been patching TCG for my own purposes recently and I was wondering a few things. That being: - Is the TCG backend expected to handle bad cases for instructions? I was wondering as I found a situation where a very large shift constant reaches the backend and causes an illegal instruction to be generated. Is the frontend expected to clean this up, or is the backend supposed to be able to deal with these? I currently patched the bug via clipping the shift constant between 0 and 64. - I've been implementing an instruction scheduler(list scheduler, with priority given to most successors) for TCG and currently if I replace instructions in s->ops(the TCG context) I get a crash later in tcg_reg_alloc_op, even if the instruction stream is identical. Is there anything else I need to move when I do this? - Is insn_start necessary to have in order(and what does it do?)? These currently are serializing instructions in my scheduler and significantly limit my reordering as they create lots of dependencies every few instructions. - Is it "okay" to use g2h and h2g directly in code in syscall.c? Currently it seems like TYPE_PTRVOID doesn't do this conversion, and as such, most of the calls made over the guest-host barrier made by DRM seem to fail spectacularly across bittedness lines. I think a more ideal solution would be implementing types that do this automatically, so I don't have to deal with the difference in struct size using macros, but in the short term I don't really have another option. My last email didn't seem to reach you all, but here's to hoping this one does. Thanks!
[PATCH 0/2] target/xtensa: fix simcall for newer hardware
Hello, this series fixes simcall opcode behavior on the recent xtensa cores making it nop rather than illegal instruction when semihosting is disabled. Max Filippov (2): target/xtensa: fetch HW version from configuration overlay target/xtensa: fix simcall for newer hardware target/xtensa/cpu.h | 1 + target/xtensa/overlay_tool.h | 8 +--- target/xtensa/translate.c| 9 ++--- 3 files changed, 12 insertions(+), 6 deletions(-) -- 2.20.1
[PATCH 1/2] target/xtensa: fetch HW version from configuration overlay
Xtensa architecture has features which behavior depends on hardware version. Provide hardware version information to translators: add XtensaConfig::hw_version and use XCHAL_HW_VERSION from configuration overlay to initialize it. Signed-off-by: Max Filippov --- target/xtensa/cpu.h | 1 + target/xtensa/overlay_tool.h | 8 +--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h index 7a46dccbe11b..32749378bfc7 100644 --- a/target/xtensa/cpu.h +++ b/target/xtensa/cpu.h @@ -464,6 +464,7 @@ struct XtensaConfig { XtensaMemory sysrom; XtensaMemory sysram; +unsigned hw_version; uint32_t configid[2]; void *isa_internal; diff --git a/target/xtensa/overlay_tool.h b/target/xtensa/overlay_tool.h index cab532095c9e..a994e69b6e96 100644 --- a/target/xtensa/overlay_tool.h +++ b/target/xtensa/overlay_tool.h @@ -60,8 +60,9 @@ #define XCHAL_RESET_VECTOR1_VADDR XCHAL_RESET_VECTOR_VADDR #endif -#ifndef XCHAL_HW_MIN_VERSION -#define XCHAL_HW_MIN_VERSION 0 +#ifndef XCHAL_HW_VERSION +#define XCHAL_HW_VERSION (XCHAL_HW_VERSION_MAJOR * 100 \ + + XCHAL_HW_VERSION_MINOR) #endif #ifndef XCHAL_LOOP_BUFFER_SIZE @@ -100,7 +101,7 @@ XCHAL_OPTION(XCHAL_HAVE_FP, XTENSA_OPTION_FP_COPROCESSOR) | \ XCHAL_OPTION(XCHAL_HAVE_RELEASE_SYNC, XTENSA_OPTION_MP_SYNCHRO) | \ XCHAL_OPTION(XCHAL_HAVE_S32C1I, XTENSA_OPTION_CONDITIONAL_STORE) | \ -XCHAL_OPTION(((XCHAL_HAVE_S32C1I && XCHAL_HW_MIN_VERSION >= 23) || \ +XCHAL_OPTION(((XCHAL_HAVE_S32C1I && XCHAL_HW_VERSION >= 23) || \ XCHAL_HAVE_EXCLUSIVE), XTENSA_OPTION_ATOMCTL) | \ XCHAL_OPTION(XCHAL_HAVE_DEPBITS, XTENSA_OPTION_DEPBITS) | \ /* Interrupts and exceptions */ \ @@ -498,6 +499,7 @@ } #define CONFIG_SECTION \ +.hw_version = XCHAL_HW_VERSION, \ .configid = { \ XCHAL_HW_CONFIGID0, \ XCHAL_HW_CONFIGID1, \ -- 2.20.1
[PATCH 2/2] target/xtensa: fix simcall for newer hardware
After Xtensa release RE.2 simcall opcode has become nop for the hardware instead of illegal instruction. Signed-off-by: Max Filippov --- target/xtensa/translate.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c index 546d2fa2facf..4bc15252c8a5 100644 --- a/target/xtensa/translate.c +++ b/target/xtensa/translate.c @@ -2367,9 +2367,10 @@ static bool test_ill_simcall(DisasContext *dc, const OpcodeArg arg[], #ifdef CONFIG_USER_ONLY bool ill = true; #else -bool ill = !semihosting_enabled(); +/* Between RE.2 and RE.3 simcall opcode's become nop for the hardware. */ +bool ill = dc->config->hw_version <= 250002 && !semihosting_enabled(); #endif -if (ill) { +if (ill || !semihosting_enabled()) { qemu_log_mask(LOG_GUEST_ERROR, "SIMCALL but semihosting is disabled\n"); } return ill; @@ -2379,7 +2380,9 @@ static void translate_simcall(DisasContext *dc, const OpcodeArg arg[], const uint32_t par[]) { #ifndef CONFIG_USER_ONLY -gen_helper_simcall(cpu_env); +if (semihosting_enabled()) { +gen_helper_simcall(cpu_env); +} #endif } -- 2.20.1
[Bug 1772165] Re: arm raspi2/raspi3 emulation has no USB support
Have you seen the patch series I have posted on the qemu-devel mailing list? "[PATCH v5 0/7] dwc-hsotg (aka dwc2) USB host controller emulation." If you could test that and give your 'tested-by', it could help get the patch series accepted. That would require you to download the latest Qemu source code, apply the patches, and build it yourself. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1772165 Title: arm raspi2/raspi3 emulation has no USB support Status in QEMU: Confirmed Bug description: Using Qemu 2.12.0 on ArchLinux. Trying to emulate arm device with `qemu-system-arm` and attach usb device for unput using ` -usb -device usb-host,bus=001,vendorid=0x1d6b,productid=0x0002 ` # lsusb returns Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub Bus 001 Device 014: ID 13d3:3487 IMC Networks Bus 001 Device 004: ID 0457:11af Silicon Integrated Systems Corp. Bus 001 Device 003: ID 0bda:57e6 Realtek Semiconductor Corp. Bus 001 Device 002: ID 0bda:0129 Realtek Semiconductor Corp. RTS5129 Card Reader Controller Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub # qemu returns qemu-system-arm: -device usb-host,bus=001,vendorid=0x1d6b,productid=0x0002: Bus '001' not found Tried with connecting external usb keyboard but that didn't seem to work either. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1772165/+subscriptions
[Bug 1502613] Re: [Feature Request] Battery Status / Virtual Battery
Has there been any progress? I'm using KVM for ubuntu 20.04 and would love to have this feature. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1502613 Title: [Feature Request] Battery Status / Virtual Battery Status in QEMU: In Progress Bug description: When using virtualization on notebooks heavily then virtual machines do not realize that they're running on a notebook device causing high power consumption because they're not switching into a optimized "laptop mode". This leads to the circumstance that they are trying to do things like defragmentation / virtus scan / etc. while the host is still running on batteries. So it would be great if QEMU / KVM would have support for emulating "Virtual Batteries" to guests causing them to enable power-saving options like disabling specific services / devices / file operations automatically by OS. Optionally a great feature would be to set virtual battery's status manually. For example: Current charge rate / charging / discharging / ... To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1502613/+subscriptions
Re: [PATCH v6 09/14] iotests: filter few more luks specific create options
On Thu, 2020-05-14 at 16:49 +0200, Max Reitz wrote: > On 10.05.20 15:40, Maxim Levitsky wrote: > > This allows more tests to be able to have same output on both qcow2 luks > > encrypted images > > and raw luks images > > > > Signed-off-by: Maxim Levitsky > > Reviewed-by: Daniel P. Berrangé > > --- > > tests/qemu-iotests/087.out | 6 ++--- > > tests/qemu-iotests/134.out | 2 +- > > tests/qemu-iotests/158.out | 4 +-- > > tests/qemu-iotests/188.out | 2 +- > > tests/qemu-iotests/189.out | 4 +-- > > tests/qemu-iotests/198.out | 4 +-- > > tests/qemu-iotests/263.out | 4 +-- > > tests/qemu-iotests/274.out | 46 > > tests/qemu-iotests/284.out | 6 ++--- > > tests/qemu-iotests/common.filter | 6 +++-- > > 10 files changed, 43 insertions(+), 41 deletions(-) > > [...] > > > diff --git a/tests/qemu-iotests/274.out b/tests/qemu-iotests/274.out > > index 9d6fdeb1f7..59de176b99 100644 > > --- a/tests/qemu-iotests/274.out > > +++ b/tests/qemu-iotests/274.out > > @@ -1,9 +1,9 @@ > > == Commit tests == > > -Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=2097152 cluster_size=65536 > > lazy_refcounts=off refcount_bits=16 > > +Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 size=2097152 > > lazy_refcounts=off refcount_bits=16 > > > > -Formatting 'TEST_DIR/PID-mid', fmt=qcow2 size=1048576 > > backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off > > refcount_bits=16 > > +Formatting 'TEST_DIR/PID-mid', fmt=qcow2 cluster_size=65536 size=1048576 > > backing_file=TEST_DIR/PID-base lazy_refcounts=off refcount_bits=16 > > > > -Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=2097152 > > backing_file=TEST_DIR/PID-mid cluster_size=65536 lazy_refcounts=off > > refcount_bits=16 > > +Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 size=2097152 > > backing_file=TEST_DIR/PID-mid lazy_refcounts=off refcount_bits=16 > > @size and @cluster_size swapping positions doesn’t look right for this > patch. I think all hunks for 274.out should be in patch 5. Fixed > > Max > Best regards, Maxim Levitsky
Re: [PATCH 0/2] exec/memory: Enforce checking MemTxResult values
Patchew URL: https://patchew.org/QEMU/20200517164817.5371-1-f4...@amsat.org/ Hi, This series failed the 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. === TEST SCRIPT BEGIN === #! /bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-mingw@fedora J=14 NETWORK=1 === TEST SCRIPT END === CC hw/display/bcm2835_fb.o In file included from /tmp/qemu-test/src/hw/core/loader.c:327: /tmp/qemu-test/src/include/hw/elf_ops.h: In function 'load_elf64': /tmp/qemu-test/src/include/hw/elf_ops.h:556:21: error: ignoring return value of 'address_space_write', declared with attribute warn_unused_result [-Werror=unused-result] address_space_write(as ? as : _space_memory, ^~~~ addr, MEMTXATTRS_UNSPECIFIED, --- In file included from /tmp/qemu-test/src/hw/core/loader.c:305: /tmp/qemu-test/src/include/hw/elf_ops.h: In function 'load_elf32': /tmp/qemu-test/src/include/hw/elf_ops.h:556:21: error: ignoring return value of 'address_space_write', declared with attribute warn_unused_result [-Werror=unused-result] address_space_write(as ? as : _space_memory, ^~~~ addr, MEMTXATTRS_UNSPECIFIED, --- data, file_size); /tmp/qemu-test/src/hw/core/loader.c: In function 'rom_reset': /tmp/qemu-test/src/hw/core/loader.c:1149:13: error: ignoring return value of 'address_space_write_rom', declared with attribute warn_unused_result [-Werror=unused-result] address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED, ^~~ rom->data, rom->datasize); ~ cc1: all warnings being treated as errors make: *** [/tmp/qemu-test/src/rules.mak:69: hw/core/loader.o] Error 1 make: *** Waiting for unfinished jobs Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=cbc78cc15c714fd48ef89d560cf39ed2', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-3un9zd0v/src/docker-src.2020-05-17-14.15.52.2490:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=cbc78cc15c714fd48ef89d560cf39ed2 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-3un9zd0v/src' make: *** [docker-run-test-mingw@fedora] Error 2 real3m18.237s user0m9.100s The full log is available at http://patchew.org/logs/20200517164817.5371-1-f4...@amsat.org/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH 0/2] exec/memory: Enforce checking MemTxResult values
Patchew URL: https://patchew.org/QEMU/20200517164817.5371-1-f4...@amsat.org/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === CC hw/display/ati_2d.o CC hw/display/ati_dbg.o In file included from /tmp/qemu-test/src/hw/core/loader.c:305: /tmp/qemu-test/src/include/hw/elf_ops.h:556:21: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result] address_space_write(as ? as : _space_memory, ^~~ In file included from /tmp/qemu-test/src/hw/core/loader.c:327: /tmp/qemu-test/src/include/hw/elf_ops.h:556:21: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result] address_space_write(as ? as : _space_memory, ^~~ /tmp/qemu-test/src/hw/core/loader.c:1149:13: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result] address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED, ^~~ ~~~ 3 errors generated. make: *** [/tmp/qemu-test/src/rules.mak:69: hw/core/loader.o] Error 1 make: *** Waiting for unfinished jobs Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=7ba3b942a61f4047b03d411633b03028', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-3dfs_s0u/src/docker-src.2020-05-17-14.08.31.28840:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=7ba3b942a61f4047b03d411633b03028 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-3dfs_s0u/src' make: *** [docker-run-test-debug@fedora] Error 2 real4m49.028s user0m9.839s The full log is available at http://patchew.org/logs/20200517164817.5371-1-f4...@amsat.org/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH 0/2] exec/memory: Enforce checking MemTxResult values
Patchew URL: https://patchew.org/QEMU/20200517164817.5371-1-f4...@amsat.org/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === CC hw/cpu/core.o In file included from /tmp/qemu-test/src/hw/core/loader.c:327:0: /tmp/qemu-test/src/include/hw/elf_ops.h: In function 'load_elf64': /tmp/qemu-test/src/include/hw/elf_ops.h:556:40: error: ignoring return value of 'address_space_write', declared with attribute warn_unused_result [-Werror=unused-result] address_space_write(as ? as : _space_memory, ^ In file included from /tmp/qemu-test/src/hw/core/loader.c:305:0: /tmp/qemu-test/src/include/hw/elf_ops.h: In function 'load_elf32': /tmp/qemu-test/src/include/hw/elf_ops.h:556:40: error: ignoring return value of 'address_space_write', declared with attribute warn_unused_result [-Werror=unused-result] address_space_write(as ? as : _space_memory, ^ /tmp/qemu-test/src/hw/core/loader.c: In function 'rom_reset': /tmp/qemu-test/src/hw/core/loader.c:1149:36: error: ignoring return value of 'address_space_write_rom', declared with attribute warn_unused_result [-Werror=unused-result] address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED, ^ cc1: all warnings being treated as errors make: *** [hw/core/loader.o] Error 1 make: *** Waiting for unfinished jobs Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=f2955b005f4543d8a28b7197af9c37a0', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-ov2cj0l6/src/docker-src.2020-05-17-14.03.09.21320:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=f2955b005f4543d8a28b7197af9c37a0 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-ov2cj0l6/src' make: *** [docker-run-test-quick@centos7] Error 2 real3m37.618s user0m9.689s The full log is available at http://patchew.org/logs/20200517164817.5371-1-f4...@amsat.org/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v6 08/14] block/qcow2: extend qemu-img amend interface with crypto options
On Thu, 2020-05-14 at 16:30 +0200, Max Reitz wrote: > On 10.05.20 15:40, Maxim Levitsky wrote: > > Now that we have all the infrastructure in place, > > wire it in the qcow2 driver and expose this to the user. > > > > Signed-off-by: Maxim Levitsky > > Reviewed-by: Daniel P. Berrangé > > --- > > block/qcow2.c | 72 +- > > tests/qemu-iotests/082.out | 45 > > Again, some rebasing required because of compression_type. Actually for this particular test, the output didn't change, because the compression_type is not amendable (at least yet). > > > 2 files changed, 108 insertions(+), 9 deletions(-) > > > > diff --git a/block/qcow2.c b/block/qcow2.c > > index db86500839..4bb6e3fc8f 100644 > > --- a/block/qcow2.c > > +++ b/block/qcow2.c > > [...] > > > @@ -4744,17 +4757,11 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts > > *opts, BlockDriverState *in_bs, > > g_free(optstr); > > > > if (has_luks) { > > + > > Why? No reason. Fixed! > > With this line dropped, and 082.out fixed up: > > Reviewed-by: Max Reitz > Thank you very much! Best regards, Maxim Levitsky
Re: [PATCH v2 05/10] Makefile: Remove dangerous EOL trailing backslash
On 15/05/2020 19.07, Philippe Mathieu-Daudé wrote: > One might get caught trying to understand unexpected Makefile > behavior. Trailing backslash can help to split very long lines, > but are rather dangerous when nothing follow. Preserve other > developers debugging time by removing this one. > > Signed-off-by: Philippe Mathieu-Daudé > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 34275f57c9..84453789f9 100644 > --- a/Makefile > +++ b/Makefile > @@ -420,7 +420,7 @@ MINIKCONF_ARGS = \ > > MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host $(SRC_PATH)/hw/Kconfig > MINIKCONF_DEPS = $(MINIKCONF_INPUTS) $(wildcard $(SRC_PATH)/hw/*/Kconfig) > -MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py \ > +MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py > > $(SUBDIR_DEVICES_MAK): %/config-devices.mak: default-configs/%.mak > $(MINIKCONF_DEPS) $(BUILD_DIR)/config-host.mak > $(call quiet-command, $(MINIKCONF) $(MINIKCONF_ARGS) > $@.tmp, "GEN", > "$@.tmp") > Reviewed-by: Thomas Huth
Re: [PATCH v6 07/14] block/crypto: implement the encryption key management
On Thu, 2020-05-14 at 16:32 +0200, Max Reitz wrote: > On 14.05.20 16:14, Daniel P. Berrangé wrote: > > On Thu, May 14, 2020 at 04:09:59PM +0200, Max Reitz wrote: > > > On 10.05.20 15:40, Maxim Levitsky wrote: > > > > This implements the encryption key management using the generic code in > > > > qcrypto layer and exposes it to the user via qemu-img > > > > > > > > This code adds another 'write_func' because the initialization > > > > write_func works directly on the underlying file, and amend > > > > works on instance of luks device. > > > > > > > > This commit also adds a 'hack/workaround' I and Kevin Wolf (thanks) > > > > made to make the driver both support write sharing (to avoid breaking > > > > the users), > > > > and be safe against concurrent metadata update (the keyslots) > > > > > > > > Eventually the write sharing for luks driver will be deprecated > > > > and removed together with this hack. > > > > > > > > The hack is that we ask (as a format driver) for > > > > BLK_PERM_CONSISTENT_READ > > > > and then when we want to update the keys, we unshare that permission. > > > > So if someone else has the image open, even readonly, encryption > > > > key update will fail gracefully. > > > > > > > > Also thanks to Daniel Berrange for the idea of > > > > unsharing read, rather that write permission which allows > > > > to avoid cases when the other user had opened the image read-only. > > > > > > > > Signed-off-by: Maxim Levitsky > > > > Reviewed-by: Daniel P. Berrangé > > > > --- > > > > block/crypto.c | 127 +++-- > > > > block/crypto.h | 34 + > > > > 2 files changed, 158 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/block/crypto.c b/block/crypto.c > > > > index 2e16b62bdc..b14cb0ff06 100644 > > > > --- a/block/crypto.c > > > > +++ b/block/crypto.c > > > > > > [...] > > > > > > > +static void > > > > +block_crypto_child_perms(BlockDriverState *bs, BdrvChild *c, > > > > + const BdrvChildRole *role, > > > > + BlockReopenQueue *reopen_queue, > > > > + uint64_t perm, uint64_t shared, > > > > + uint64_t *nperm, uint64_t *nshared) > > > > +{ > > > > + > > > > +BlockCrypto *crypto = bs->opaque; > > > > + > > > > +bdrv_filter_default_perms(bs, c, role, reopen_queue, > > > > +perm, shared, nperm, nshared); > > > > +/* > > > > + * Ask for consistent read permission so that if > > > > + * someone else tries to open this image with this permission > > > > + * neither will be able to edit encryption keys, since > > > > + * we will unshare that permission while trying to > > > > + * update the encryption keys > > > > + */ > > > > +if (!(bs->open_flags & BDRV_O_NO_IO)) { > > > > +*nperm |= BLK_PERM_CONSISTENT_READ; > > > > +} > > > > > > I’m not sure this is important, because this really means we won’t do > > > I/O. Its only relevant use in this case is for qemu-img info. Do we > > > really care if someone edits the key slots while qemu-img info is > > > processing? > > > > FWIW, OpenStack runs qemu-img info in a periodic background job, so > > it can be concurrent with anything else they are running. > > That might actually be a problem then, because this may cause sporadic > failure when trying to change (amend) keyslots; while qemu-img info > holds the CONSISTENT_READ permission, the amend process can’t unshare > it. That might lead to hard-to-track-down bugs. > > > Having said > > that due to previous QEMU bugs, they unconditonally pass the arg to > > qemu-img to explicitly disable locking > > Well, then it doesn’t matter in this case. But still something to > consider, probably. > > Max > So I understand correctly that I should leave the patch as is? Thanks for the review! Best regards, Maxim Levitsky
Re: [PATCH] ati-vga: Do not allow unaligned access via index register
On 5/17/20 4:30 PM, BALATON Zoltan wrote: On Sun, 17 May 2020, Philippe Mathieu-Daudé wrote: On 5/17/20 12:40 PM, Philippe Mathieu-Daudé wrote: On 5/16/20 5:33 PM, BALATON Zoltan wrote: On Sat, 16 May 2020, Alexander Bulekov wrote: On 200516 1513, BALATON Zoltan wrote: Finally, there is a tag documented for bug fixes: https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message If your patch addresses a bug in a public bug tracker, please add a line with "Buglink: " there, too. Buglink: https://bugs.launchpad.net/qemu/+bug/1878134 Does this reply add that tag already or do I need to submit a v2 with it (or the maintainer could add it when merging)? If he doesn't have time he can reply to your patch :) Now, looking at your device implementation, it seems 1/ The device isn't supposed to have 64-bit accesses So this might be a more generic fix to Alexander issue: -- >8 -- @@ -879,6 +879,7 @@ static void ati_mm_write(void *opaque, hwaddr addr, static const MemoryRegionOps ati_mm_ops = { .read = ati_mm_read, .write = ati_mm_write, + .valid.max_access_size = 4, .endianness = DEVICE_LITTLE_ENDIAN, }; --- I've tried that first but it does not work. The reason is that ati_mm_read is recursively called for indexed access via MM_DATA which causes the problem that happens when MM_INDEX is set to a non-aligned value. No 64 bit access, just 32 bit with offset of 2 bytes as can be seen from the stach trace I've attached to the bug. Fortunately indexed access is documented to only support aligned access by not allowing setting low bits of MM_INDEX so unless we find a client needing it my patch should do it. OK, so this is another device affected by the memory.c lacking of unaligned access (Gerd saw another one with USB). 2/ All the registers are 32-bit aligned So you can simplify the implementation by letting access_with_adjusted_size() handle the 8/16-bit accesses by using: @@ -879,6 +879,8 @@ static void ati_mm_write(void *opaque, hwaddr addr, static const MemoryRegionOps ati_mm_ops = { .read = ati_mm_read, .write = ati_mm_write, + .min.min_access_size = 4, I meant '.impl.min_access_size'. I think this would not work either because not all registers are the same, some only can be read all 32 bits, some also 16 or 8 bits and clients do access these with less than 32 bits and accessing parts of the reg may trigger actions so the current way is probably better and necessary to correctly support different valid and invalid unaligned accessses. .valid.xxx_access_size is what the guest are allowed to use, .impl.xxx_access_size is what the developer had in mind when writing the model. .impl.min_access_size = 4 doesn't forbid 8/16-bit guest accesses. Moreover, it overloads you the burden of handling short accesses. Anyway this was just a suggested simplification. Regards, BALATON Zoltan
Re: [PATCH v6 05/14] block/amend: refactor qcow2 amend options
On Thu, 2020-05-14 at 15:36 +0200, Max Reitz wrote: > On 10.05.20 15:40, Maxim Levitsky wrote: > > Some qcow2 create options can't be used for amend. > > Remove them from the qcow2 create options and add generic logic to detect > > such options in qemu-img > > > > Signed-off-by: Maxim Levitsky > > Reviewed-by: Daniel P. Berrangé > > --- > > block/qcow2.c | 108 ++--- > > qemu-img.c | 18 +++- > > tests/qemu-iotests/049.out | 102 ++-- > > tests/qemu-iotests/061.out | 12 ++- > > tests/qemu-iotests/079.out | 18 ++-- > > tests/qemu-iotests/082.out | 149 > > tests/qemu-iotests/085.out | 38 > > tests/qemu-iotests/087.out | 6 +- > > tests/qemu-iotests/115.out | 2 +- > > tests/qemu-iotests/121.out | 4 +- > > tests/qemu-iotests/125.out | 192 ++--- > > tests/qemu-iotests/134.out | 2 +- > > tests/qemu-iotests/144.out | 4 +- > > tests/qemu-iotests/158.out | 4 +- > > tests/qemu-iotests/182.out | 2 +- > > tests/qemu-iotests/185.out | 8 +- > > tests/qemu-iotests/188.out | 2 +- > > tests/qemu-iotests/189.out | 4 +- > > tests/qemu-iotests/198.out | 4 +- > > tests/qemu-iotests/243.out | 16 ++-- > > tests/qemu-iotests/250.out | 2 +- > > tests/qemu-iotests/255.out | 8 +- > > tests/qemu-iotests/259.out | 2 +- > > tests/qemu-iotests/263.out | 4 +- > > tests/qemu-iotests/280.out | 2 +- > > These reference output hunks need some rebasing due to the new > compression_type option. Done. I so hope to get it merged so I won't need to rebase it again :-) > > > 25 files changed, 284 insertions(+), 429 deletions(-) > > > > diff --git a/block/qcow2.c b/block/qcow2.c > > index fc494c7591..db86500839 100644 > > --- a/block/qcow2.c > > +++ b/block/qcow2.c > > [...] > > > @@ -5552,37 +5506,6 @@ void qcow2_signal_corruption(BlockDriverState *bs, > > bool fatal, int64_t offset, > > .help = "The external data file must stay valid " \ > > "as a raw image"\ > > }, \ > > -{ \ > > -.name = BLOCK_OPT_ENCRYPT, \ > > -.type = QEMU_OPT_BOOL, \ > > -.help = "Encrypt the image with format 'aes'. (Deprecated " \ > > -"in favor of " BLOCK_OPT_ENCRYPT_FORMAT "=aes)",\ > > -}, \ > > -{ \ > > -.name = BLOCK_OPT_ENCRYPT_FORMAT, \ > > -.type = QEMU_OPT_STRING,\ > > -.help = "Encrypt the image, format choices: 'aes', 'luks'", \ > > -}, \ > > -BLOCK_CRYPTO_OPT_DEF_KEY_SECRET("encrypt.", \ > > -"ID of secret providing qcow AES key or LUKS passphrase"), \ > > -BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_ALG("encrypt."), \ > > -BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_MODE("encrypt."), \ > > -BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_ALG("encrypt."),\ > > -BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG("encrypt."), \ > > -BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG("encrypt."), \ > > -BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME("encrypt."),\ > > -{ \ > > -.name = BLOCK_OPT_CLUSTER_SIZE, \ > > -.type = QEMU_OPT_SIZE, \ > > -.help = "qcow2 cluster size", \ > > -.def_value_str = stringify(DEFAULT_CLUSTER_SIZE)\ > > -}, \ > > -{ \ > > -.name = BLOCK_OPT_PREALLOC, \ > > -.type = QEMU_OPT_STRING,\ > > -.help = "Preallocation mode (allowed values: off, " \ > > -"metadata, falloc, full)" \ > > -}, \ > > { \ > > .name = BLOCK_OPT_LAZY_REFCOUNTS, \ > > .type = QEMU_OPT_BOOL, \ > > @@ -5600,6 +5523,37 @@ static QemuOptsList qcow2_create_opts = { > > .name = "qcow2-create-opts", > > .head = QTAILQ_HEAD_INITIALIZER(qcow2_create_opts.head), > > .desc = { > > +{
Re: [PATCH v2 3/3] docs/s390x: document vfio-ccw
On 15/05/2020 17.15, Cornelia Huck wrote: > Add a basic example for passing a dasd via vfio-ccw. > > Signed-off-by: Cornelia Huck > --- > docs/system/s390x/vfio-ccw.rst | 72 ++ > docs/system/target-s390x.rst | 1 + > 2 files changed, 73 insertions(+) > create mode 100644 docs/system/s390x/vfio-ccw.rst > > diff --git a/docs/system/s390x/vfio-ccw.rst b/docs/system/s390x/vfio-ccw.rst > new file mode 100644 > index ..4cfd22c3b789 > --- /dev/null > +++ b/docs/system/s390x/vfio-ccw.rst > @@ -0,0 +1,72 @@ > +Subchannel passthrough via vfio-ccw > +=== > + > +vfio-ccw (based upon the mediated vfio device infrastructure) allows to > +make certain I/O subchannels and their devices available to a guest. The > +host will not interact with those subchannels/devices any more. > + > +Note that while vfio-ccw should work with most non-QDIO devices, only ECKD > +DASDs have really been tested. > + > +Example configuration > +- > + > +Step 1: configure the host device > +~ > + > +Note: it is recommended to use the ``mdevctl`` tool for this step. > + > +To define the same device as configured below to be started > +automatically, use > + > +:: > + > + [root@host ~]# driverctl -b css set-override 0.0.0313 vfio_ccw > + [root@host ~]# mdevctl define -u 7e270a25-e163-4922-af60-757fc8ed48c6\ > + -p 0.0.0313 -t vfio-ccw_io -a > + > +If this is not possible or wanted, follow the manual procedure below. > + > +* Locate the subchannel for the device (in this example, ``0.0.2b09``):: > + > +[root@host ~]# lscss | grep 0.0.2b09 | awk '{print $2}' > +0.0.0313 > + > +* Unbind the subchannel (in this example, ``0.0.0313``) from the standard > + I/O subchannel driver and bind it to the vfio-ccw driver:: > + > +[root@host ~]# echo 0.0.0313 > > /sys/bus/css/devices/0.0.0313/driver/unbind > +[root@host ~]# echo 0.0.0313 > /sys/bus/css/drivers/vfio_ccw/bind > + > +* Create the mediated device (identified by a uuid):: > + > +[root@host ~]# uuidgen > +7e270a25-e163-4922-af60-757fc8ed48c6 Maybe the uuidgen lines should now be moved before the mdevctl example already, so that it is already clear there where the uuid comes from? Thomas
Re: [PATCH v2 2/3] docs/s390x: document 3270
On 15/05/2020 17.15, Cornelia Huck wrote: > Add some basic info how to use 3270 devices. > > Signed-off-by: Cornelia Huck > --- > docs/system/s390x/3270.rst | 32 > docs/system/target-s390x.rst | 1 + > 2 files changed, 33 insertions(+) > create mode 100644 docs/system/s390x/3270.rst Reviewed-by: Thomas Huth
Re: [PATCH v2 1/3] docs/s390x: document the virtual css
On 15/05/2020 17.15, Cornelia Huck wrote: > Add some hints about "devno" rules. > > Signed-off-by: Cornelia Huck > --- > docs/system/s390x/css.rst| 86 > docs/system/target-s390x.rst | 1 + > 2 files changed, 87 insertions(+) > create mode 100644 docs/system/s390x/css.rst [...] > +if the guest OS does not enable MCSS-E (which is true of all supported guest Maybe s/true of/true for/ ? Not sure, your English is normally better than mine ;-) Anyway, Reviewed-by: Thomas Huth
Re: [PATCH v2 02/10] MAINTAINERS: Add an 'overall' entry for accelerators
On 15/05/2020 19.07, Philippe Mathieu-Daudé wrote: > Reviewed-by: Richard Henderson > Signed-off-by: Philippe Mathieu-Daudé > --- > Cc: Paolo Bonzini > --- > MAINTAINERS | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index fd88a3de49..659092eb43 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -415,6 +415,15 @@ S: Supported > F: target/i386/kvm.c > F: scripts/kvm/vmxcap > > +Guest CPU Cores (other accelerators) Maybe s/other accelerators/accelerators generic code/ ? > + > +Overall > +M: Richard Henderson > +R: Paolo Bonzini > +S: Maintained > +F: include/sysemu/accel.h > +F: accel/stubs/Makefile.objs What about accel/accel.c and accel/Makefile.objs ? Should they also be in the list here? Thomas
Re: [PATCH v2 01/10] MAINTAINERS: Fix KVM path expansion glob
On 15/05/2020 19.07, Philippe Mathieu-Daudé wrote: > The KVM files has been moved from target-ARCH to the target/ARCH/ > folder in commit fcf5ef2a. Fix the pathname expansion. > > Fixes: fcf5ef2a ("Move target-* CPU file into a target/ folder") Oops, my bad. Sorry for that oversight! > diff --git a/MAINTAINERS b/MAINTAINERS > index 47ef3139e6..fd88a3de49 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -361,7 +361,7 @@ Overall KVM CPUs > M: Paolo Bonzini > L: k...@vger.kernel.org > S: Supported > -F: */kvm.* > +F: */*/kvm* > F: accel/kvm/ > F: accel/stubs/kvm-stub.c > F: include/hw/kvm/ > Reviewed-by: Thomas Huth
[Bug 1879175] Re: GVTd not working (black screen) after upgrade to qemu-5.0.0
** Summary changed: - GVTd not working after upgrade to qemu-5.0.0 + GVTd not working (black screen) after upgrade to qemu-5.0.0 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1879175 Title: GVTd not working (black screen) after upgrade to qemu-5.0.0 Status in QEMU: New Bug description: Hi QEMU team, === Problem Summary === I have recently upgraded from QEMU-3.1.0 to to QEMU-5.0.0 on Debian Unstable. Unfortunately GVTd (legacy passthrough of the integrated intel gpu) stopped working correctly. The guest can still see and loads the driver for the GPU, but the screen stays black. The following is the version used: $ /usr/bin/qemu-system-x86_64 --version QEMU emulator version 5.0.0 (Debian 1:5.0-5) Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers === Investigation/Triage done so far === Running QEMU with trace flags enabled shows the following behavior change for the same VM (left: 3.1.0, right: 5.0.0): vfio_realize (:00:02.0) group 1 vfio_realize (:00:02.0) group 1 vfio_listener_region_add_ram region_add [ram] 0x0 - 0xb [0x7f5b41e0] |vfio_listener_region_add_ram region_add [ram] 0x0 - 0xb [0x7f2bb1e0] vfio_listener_region_add_ram region_add [ram] 0xc - 0xd [0x7f5d1d40] |vfio_listener_region_add_ram region_add [ram] 0xc - 0xd [0x7f2d7c80] vfio_listener_region_add_ram region_add [ram] 0xe - 0xf [0x7f5d1d62] |vfio_listener_region_add_ram region_add [ram] 0xe - 0xf [0x7f2d8422] vfio_listener_region_add_ram region_add [ram] 0x10 - 0xbfff [0x7f5b41f0] |vfio_listener_region_add_ram region_add [ram] 0x10 - 0xbfff [0x7f2bb1f0] vfio_listener_region_add_skip SKIPPING region_add 0xfec0 - 0xfec00fff vfio_listener_region_add_skip SKIPPING region_add 0xfec0 - 0xfec00fff vfio_listener_region_add_skip SKIPPING region_add 0xfee0 - 0xfeef vfio_listener_region_add_skip SKIPPING region_add 0xfee0 - 0xfeef vfio_listener_region_add_ram region_add [ram] 0xfffc - 0x [0x7f5d1d60] |vfio_listener_region_add_ram region_add [ram] 0xfffc - 0x [0x7f2d8420] vfio_listener_region_add_ram region_add [ram] 0x1 - 0x201ff [0x7f5c01e0] |vfio_listener_region_add_ram region_add [ram] 0x1 - 0x201ff [0x7f2c71e0] vfio_mdev (:00:02.0) is_mdev 0 vfio_mdev (:00:02.0) is_mdev 0 vfio_get_device Device :00:02.0 flags: 3, regions: 12, irqs: 5 vfio_get_device Device :00:02.0 flags: 3, regions: 12, irqs: 5 vfio_region_setup Device :00:02.0, region 0 ":00:02.0 BAR 0", flags: 0x7, offset: 0x0, svfio_region_setup Device :00:02.0, region 0 ":00:02.0 BAR 0", flags: 0x7, offset: 0x0, s vfio_region_setup Device :00:02.0, region 1 ":00:02.0 BAR 1", flags: 0x0, offset: 0x1000vfio_region_setup Device :00:02.0, region 1 ":00:02.0 BAR 1", flags: 0x0, offset: 0x1000 vfio_region_setup Device :00:02.0, region 2 ":00:02.0 BAR 2", flags: 0x7, offset: 0x2000vfio_region_setup Device :00:02.0, region 2 ":00:02.0 BAR 2", flags: 0x7, offset: 0x2000 vfio_region_setup Device :00:02.0, region 3 ":00:02.0 BAR 3", flags: 0x0, offset: 0x3000vfio_region_setup Device :00:02.0, region 3 ":00:02.0 BAR 3", flags: 0x0, offset: 0x3000 vfio_region_setup Device :00:02.0, region 4 ":00:02.0 BAR 4", flags: 0x3, offset: 0x4000vfio_region_setup Device :00:02.0, region 4 ":00:02.0 BAR 4", flags: 0x3, offset: 0x4000 vfio_region_setup Device :00:02.0, region 5 ":00:02.0 BAR 5", flags: 0x0, offset: 0x5000vfio_region_setup Device :00:02.0, region 5 ":00:02.0 BAR 5", flags: 0x0, offset: 0x5000 vfio_populate_device_config Device :00:02.0 config: vfio_populate_device_config Device :00:02.0 config: 0x1000, offset: 0x700, flags: 0x3 0x1000, offset: 0x700, flags: 0x3 vfio_region_mmap Region :00:02.0 BAR 0 mmaps[0] [0x0 - 0xff] vfio_region_mmap Region :00:02.0 BAR 0 mmaps[0] [0x0 - 0xff] vfio_region_mmap Region :00:02.0 BAR 2 mmaps[0] [0x0 - 0xfff] vfio_region_mmap Region :00:02.0 BAR 2 mmaps[0] [0x0 - 0xfff] vfio_check_pm_reset :00:02.0 Supports
[Bug 1879175] [NEW] GVTd not working after upgrade to qemu-5.0.0
Public bug reported: Hi QEMU team, === Problem Summary === I have recently upgraded from QEMU-3.1.0 to to QEMU-5.0.0 on Debian Unstable. Unfortunately GVTd (legacy passthrough of the integrated intel gpu) stopped working correctly. The guest can still see and loads the driver for the GPU, but the screen stays black. The following is the version used: $ /usr/bin/qemu-system-x86_64 --version QEMU emulator version 5.0.0 (Debian 1:5.0-5) Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers === Investigation/Triage done so far === Running QEMU with trace flags enabled shows the following behavior change for the same VM (left: 3.1.0, right: 5.0.0): vfio_realize (:00:02.0) group 1 vfio_realize (:00:02.0) group 1 vfio_listener_region_add_ram region_add [ram] 0x0 - 0xb [0x7f5b41e0] |vfio_listener_region_add_ram region_add [ram] 0x0 - 0xb [0x7f2bb1e0] vfio_listener_region_add_ram region_add [ram] 0xc - 0xd [0x7f5d1d40] |vfio_listener_region_add_ram region_add [ram] 0xc - 0xd [0x7f2d7c80] vfio_listener_region_add_ram region_add [ram] 0xe - 0xf [0x7f5d1d62] |vfio_listener_region_add_ram region_add [ram] 0xe - 0xf [0x7f2d8422] vfio_listener_region_add_ram region_add [ram] 0x10 - 0xbfff [0x7f5b41f0] |vfio_listener_region_add_ram region_add [ram] 0x10 - 0xbfff [0x7f2bb1f0] vfio_listener_region_add_skip SKIPPING region_add 0xfec0 - 0xfec00fff vfio_listener_region_add_skip SKIPPING region_add 0xfec0 - 0xfec00fff vfio_listener_region_add_skip SKIPPING region_add 0xfee0 - 0xfeef vfio_listener_region_add_skip SKIPPING region_add 0xfee0 - 0xfeef vfio_listener_region_add_ram region_add [ram] 0xfffc - 0x [0x7f5d1d60] |vfio_listener_region_add_ram region_add [ram] 0xfffc - 0x [0x7f2d8420] vfio_listener_region_add_ram region_add [ram] 0x1 - 0x201ff [0x7f5c01e0] |vfio_listener_region_add_ram region_add [ram] 0x1 - 0x201ff [0x7f2c71e0] vfio_mdev (:00:02.0) is_mdev 0 vfio_mdev (:00:02.0) is_mdev 0 vfio_get_device Device :00:02.0 flags: 3, regions: 12, irqs: 5 vfio_get_device Device :00:02.0 flags: 3, regions: 12, irqs: 5 vfio_region_setup Device :00:02.0, region 0 ":00:02.0 BAR 0", flags: 0x7, offset: 0x0, svfio_region_setup Device :00:02.0, region 0 ":00:02.0 BAR 0", flags: 0x7, offset: 0x0, s vfio_region_setup Device :00:02.0, region 1 ":00:02.0 BAR 1", flags: 0x0, offset: 0x1000vfio_region_setup Device :00:02.0, region 1 ":00:02.0 BAR 1", flags: 0x0, offset: 0x1000 vfio_region_setup Device :00:02.0, region 2 ":00:02.0 BAR 2", flags: 0x7, offset: 0x2000vfio_region_setup Device :00:02.0, region 2 ":00:02.0 BAR 2", flags: 0x7, offset: 0x2000 vfio_region_setup Device :00:02.0, region 3 ":00:02.0 BAR 3", flags: 0x0, offset: 0x3000vfio_region_setup Device :00:02.0, region 3 ":00:02.0 BAR 3", flags: 0x0, offset: 0x3000 vfio_region_setup Device :00:02.0, region 4 ":00:02.0 BAR 4", flags: 0x3, offset: 0x4000vfio_region_setup Device :00:02.0, region 4 ":00:02.0 BAR 4", flags: 0x3, offset: 0x4000 vfio_region_setup Device :00:02.0, region 5 ":00:02.0 BAR 5", flags: 0x0, offset: 0x5000vfio_region_setup Device :00:02.0, region 5 ":00:02.0 BAR 5", flags: 0x0, offset: 0x5000 vfio_populate_device_config Device :00:02.0 config: vfio_populate_device_config Device :00:02.0 config: 0x1000, offset: 0x700, flags: 0x3 0x1000, offset: 0x700, flags: 0x3 vfio_region_mmap Region :00:02.0 BAR 0 mmaps[0] [0x0 - 0xff] vfio_region_mmap Region :00:02.0 BAR 0 mmaps[0] [0x0 - 0xff] vfio_region_mmap Region :00:02.0 BAR 2 mmaps[0] [0x0 - 0xfff] vfio_region_mmap Region :00:02.0 BAR 2 mmaps[0] [0x0 - 0xfff] vfio_check_pm_reset :00:02.0 Supports PM reset vfio_check_pm_reset :00:02.0 Supports PM reset vfio_msi_setup :00:02.0 PCI MSI CAP @0xac vfio_msi_setup :00:02.0 PCI MSI CAP @0xac vfio_check_pcie_flr :00:02.0 Supports FLR via PCIe cap vfio_check_pcie_flr :00:02.0 Supports FLR via PCIe cap
Re: [PATCH v4 19/19] hw/mips: Rename malta/mipssim/r4k/jazz files in hw/mips
нед, 17. мај 2020. у 15:19 Philippe Mathieu-Daudé је написао/ла: > > Hi Aleksandar, > > On 5/17/20 11:23 AM, Aleksandar Markovic wrote: > > Machine file names should not have prefix "mips_". > > > > Folong2 machine source file will be handled in a separate patch, > > Typo: "Fuloong2e" > > > to avoid conflicts. That patch is pending integration into the > > main tree. > > > > Signed-off-by: Aleksandar Markovic > > CC: Philippe Mathieu-Daudé > > --- > > hw/mips/Makefile.objs | 8 > > hw/mips/{mips_jazz.c => jazz.c} | 0 > > hw/mips/{mips_malta.c => malta.c} | 0 > > hw/mips/{mips_mipssim.c => mipssim.c} | 0 > > hw/mips/{mips_r4k.c => r4k.c} | 0 > > 5 files changed, 4 insertions(+), 4 deletions(-) > > rename hw/mips/{mips_jazz.c => jazz.c} (100%) > > rename hw/mips/{mips_malta.c => malta.c} (100%) > > rename hw/mips/{mips_mipssim.c => mipssim.c} (100%) > > rename hw/mips/{mips_r4k.c => r4k.c} (100%) > > Thanks for cleaning this, appreciated! > > You missed MAINTAINERS: > Ouch! You are right. Will be fixed.. Thanks, Aleksandar > -- >8 -- > diff --git a/MAINTAINERS b/MAINTAINERS > index 1f84e3ae2c..3ad904a73c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1045,3 +1045,3 @@ R: Aleksandar Rikalo > S: Maintained > -F: hw/mips/mips_jazz.c > +F: hw/mips/jazz.c > F: hw/display/jazz_led.c > @@ -1056,3 +1056,3 @@ F: hw/isa/piix4.c > F: hw/acpi/piix4.c > -F: hw/mips/mips_malta.c > +F: hw/mips/malta.c > F: hw/mips/gt64xxx_pci.c > @@ -1066,3 +1066,3 @@ R: Aleksandar Rikalo > S: Odd Fixes > -F: hw/mips/mips_mipssim.c > +F: hw/mips/mipssim.c > F: hw/net/mipsnet.c > @@ -1074,3 +1074,3 @@ R: Aleksandar Rikalo > S: Obsolete > -F: hw/mips/mips_r4k.c > +F: hw/mips/r4k.c > > --- > > With this snippet amended: > > Reviewed-by: Philippe Mathieu-Daudé > Tested-by: Philippe Mathieu-Daudé > > Regards, > > Phil. > > > > > diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs > > index 525809af07..1d767ed9a8 100644 > > --- a/hw/mips/Makefile.objs > > +++ b/hw/mips/Makefile.objs > > @@ -1,8 +1,8 @@ > > obj-y += addr.o mips_int.o > > -obj-$(CONFIG_R4K) += mips_r4k.o > > -obj-$(CONFIG_MALTA) += gt64xxx_pci.o mips_malta.o > > -obj-$(CONFIG_MIPSSIM) += mips_mipssim.o > > -obj-$(CONFIG_JAZZ) += mips_jazz.o > > +obj-$(CONFIG_R4K) += r4k.o > > +obj-$(CONFIG_MALTA) += gt64xxx_pci.o malta.o > > +obj-$(CONFIG_MIPSSIM) += mipssim.o > > +obj-$(CONFIG_JAZZ) += jazz.o > > obj-$(CONFIG_FULONG) += mips_fulong2e.o > > obj-$(CONFIG_MIPS_CPS) += cps.o > > obj-$(CONFIG_MIPS_BOSTON) += boston.o > > diff --git a/hw/mips/mips_jazz.c b/hw/mips/jazz.c > > similarity index 100% > > rename from hw/mips/mips_jazz.c > > rename to hw/mips/jazz.c > > diff --git a/hw/mips/mips_malta.c b/hw/mips/malta.c > > similarity index 100% > > rename from hw/mips/mips_malta.c > > rename to hw/mips/malta.c > > diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mipssim.c > > similarity index 100% > > rename from hw/mips/mips_mipssim.c > > rename to hw/mips/mipssim.c > > diff --git a/hw/mips/mips_r4k.c b/hw/mips/r4k.c > > similarity index 100% > > rename from hw/mips/mips_r4k.c > > rename to hw/mips/r4k.c > >
[RFC PATCH 2/2] exec/memory: Emit warning when MemTxResult is ignored
When a function from the memory subsystem return a MemTxResult to indicate that the transaction failed, this return value must not be ignored by the caller. Mark all these functions with the QEMU_WARN_UNUSED_RESULT attribute, to prevent users to ignore possible failed transactions. Signed-off-by: Philippe Mathieu-Daudé --- RFC because it doesn't build. But before going thru each caller, let's talk on the list if this change makes sense. --- include/exec/memory.h | 39 +-- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 5e8c009169..95668d1628 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -161,12 +161,14 @@ struct MemoryRegionOps { hwaddr addr, uint64_t *data, unsigned size, - MemTxAttrs attrs); + MemTxAttrs attrs) + QEMU_WARN_UNUSED_RESULT; MemTxResult (*write_with_attrs)(void *opaque, hwaddr addr, uint64_t data, unsigned size, -MemTxAttrs attrs); +MemTxAttrs attrs) +QEMU_WARN_UNUSED_RESULT; enum device_endian endianness; /* Guest-visible constraints: */ @@ -1989,7 +1991,8 @@ MemTxResult memory_region_dispatch_read(MemoryRegion *mr, hwaddr addr, uint64_t *pval, MemOp op, -MemTxAttrs attrs); +MemTxAttrs attrs) +QEMU_WARN_UNUSED_RESULT; /** * memory_region_dispatch_write: perform a write directly to the specified * MemoryRegion. @@ -2004,7 +2007,8 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr, hwaddr addr, uint64_t data, MemOp op, - MemTxAttrs attrs); + MemTxAttrs attrs) + QEMU_WARN_UNUSED_RESULT; /** * address_space_init: initializes an address space @@ -2053,7 +2057,8 @@ void address_space_remove_listeners(AddressSpace *as); */ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, void *buf, - hwaddr len, bool is_write); + hwaddr len, bool is_write) + QEMU_WARN_UNUSED_RESULT; /** * address_space_write: write to address space. @@ -2070,7 +2075,8 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, */ MemTxResult address_space_write(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, -const void *buf, hwaddr len); +const void *buf, hwaddr len) +QEMU_WARN_UNUSED_RESULT; /** * address_space_write_rom: write to address space, including ROM. @@ -2096,7 +2102,8 @@ MemTxResult address_space_write(AddressSpace *as, hwaddr addr, */ MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, -const void *buf, hwaddr len); +const void *buf, hwaddr len) +QEMU_WARN_UNUSED_RESULT; /* address_space_ld*: load from an address space * address_space_st*: store to an address space @@ -2334,20 +2341,24 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len, /* Internal functions, part of the implementation of address_space_read. */ MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr, -MemTxAttrs attrs, void *buf, hwaddr len); +MemTxAttrs attrs, void *buf, hwaddr len) +QEMU_WARN_UNUSED_RESULT; MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, MemTxAttrs attrs, void *buf, hwaddr len, hwaddr addr1, hwaddr l, - MemoryRegion *mr); + MemoryRegion *mr) + QEMU_WARN_UNUSED_RESULT; void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr); /* Internal functions, part of the implementation of address_space_read_cached * and address_space_write_cached. */ MemTxResult
[PATCH 1/2] exec/memory: Let address_space_read/write_cached() propagate MemTxResult
Both address_space_read_cached_slow() and address_space_write_cached_slow() return a MemTxResult type. Do not discard it, return it to the caller. Signed-off-by: Philippe Mathieu-Daudé --- include/exec/memory.h | 19 +++ exec.c| 16 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index e000bd2f97..5e8c009169 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -2343,10 +2343,11 @@ void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr); /* Internal functions, part of the implementation of address_space_read_cached * and address_space_write_cached. */ -void address_space_read_cached_slow(MemoryRegionCache *cache, -hwaddr addr, void *buf, hwaddr len); -void address_space_write_cached_slow(MemoryRegionCache *cache, - hwaddr addr, const void *buf, hwaddr len); +MemTxResult address_space_read_cached_slow(MemoryRegionCache *cache, + hwaddr addr, void *buf, hwaddr len); +MemTxResult address_space_write_cached_slow(MemoryRegionCache *cache, +hwaddr addr, const void *buf, +hwaddr len); static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write) { @@ -2411,15 +2412,16 @@ MemTxResult address_space_read(AddressSpace *as, hwaddr addr, * @buf: buffer with the data transferred * @len: length of the data transferred */ -static inline void +static inline MemTxResult address_space_read_cached(MemoryRegionCache *cache, hwaddr addr, void *buf, hwaddr len) { assert(addr < cache->len && len <= cache->len - addr); if (likely(cache->ptr)) { memcpy(buf, cache->ptr + addr, len); +return MEMTX_OK; } else { -address_space_read_cached_slow(cache, addr, buf, len); +return address_space_read_cached_slow(cache, addr, buf, len); } } @@ -2431,15 +2433,16 @@ address_space_read_cached(MemoryRegionCache *cache, hwaddr addr, * @buf: buffer with the data transferred * @len: length of the data transferred */ -static inline void +static inline MemTxResult address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, const void *buf, hwaddr len) { assert(addr < cache->len && len <= cache->len - addr); if (likely(cache->ptr)) { memcpy(cache->ptr + addr, buf, len); +return MEMTX_OK; } else { -address_space_write_cached_slow(cache, addr, buf, len); +return address_space_write_cached_slow(cache, addr, buf, len); } } diff --git a/exec.c b/exec.c index 5162f0d12f..877b51cc5c 100644 --- a/exec.c +++ b/exec.c @@ -3716,7 +3716,7 @@ static inline MemoryRegion *address_space_translate_cached( /* Called from RCU critical section. address_space_read_cached uses this * out of line function when the target is an MMIO or IOMMU region. */ -void +MemTxResult address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr, void *buf, hwaddr len) { @@ -3726,15 +3726,15 @@ address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr, l = len; mr = address_space_translate_cached(cache, addr, , , false, MEMTXATTRS_UNSPECIFIED); -flatview_read_continue(cache->fv, - addr, MEMTXATTRS_UNSPECIFIED, buf, len, - addr1, l, mr); +return flatview_read_continue(cache->fv, + addr, MEMTXATTRS_UNSPECIFIED, buf, len, + addr1, l, mr); } /* Called from RCU critical section. address_space_write_cached uses this * out of line function when the target is an MMIO or IOMMU region. */ -void +MemTxResult address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr, const void *buf, hwaddr len) { @@ -3744,9 +3744,9 @@ address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr, l = len; mr = address_space_translate_cached(cache, addr, , , true, MEMTXATTRS_UNSPECIFIED); -flatview_write_continue(cache->fv, -addr, MEMTXATTRS_UNSPECIFIED, buf, len, -addr1, l, mr); +return flatview_write_continue(cache->fv, + addr, MEMTXATTRS_UNSPECIFIED, buf, len, + addr1, l, mr); } #define ARG1_DECLMemoryRegionCache *cache -- 2.21.3
[PATCH 0/2] exec/memory: Enforce checking MemTxResult values
Various places ignore the MemTxResult indicator of transaction failed. Some cases might be justified (DMA?) while other are probably bugs. To avoid ignoring transaction errors, suggestion is to mark functions returning MemTxResult with warn_unused_result attribute. Philippe Mathieu-Daudé (2): exec/memory: Let address_space_read/write_cached() propagate MemTxResult exec/memory: Emit warning when MemTxResult is ignored include/exec/memory.h | 50 +++ exec.c| 16 +++--- 2 files changed, 40 insertions(+), 26 deletions(-) -- 2.21.3
[PATCH v3 8/8] hw/arm/fsl-imx7: Connect watchdog interrupts
i.MX7 supports watchdog pretimeout interupts. With this commit, the watchdog in mcimx7d-sabre is fully operational, including pretimeout support. Reviewed-by: Peter Maydell Signed-off-by: Guenter Roeck --- v3: Added Peter's Reviewed-by: tag v2: No change hw/arm/fsl-imx7.c | 11 +++ include/hw/arm/fsl-imx7.h | 5 + 2 files changed, 16 insertions(+) diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c index d6cf7c48ce..89c3b64c06 100644 --- a/hw/arm/fsl-imx7.c +++ b/hw/arm/fsl-imx7.c @@ -447,11 +447,22 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp) FSL_IMX7_WDOG3_ADDR, FSL_IMX7_WDOG4_ADDR, }; +static const int FSL_IMX7_WDOGn_IRQ[FSL_IMX7_NUM_WDTS] = { +FSL_IMX7_WDOG1_IRQ, +FSL_IMX7_WDOG2_IRQ, +FSL_IMX7_WDOG3_IRQ, +FSL_IMX7_WDOG4_IRQ, +}; +object_property_set_bool(OBJECT(>wdt[i]), true, "pretimeout-support", + _abort); object_property_set_bool(OBJECT(>wdt[i]), true, "realized", _abort); sysbus_mmio_map(SYS_BUS_DEVICE(>wdt[i]), 0, FSL_IMX7_WDOGn_ADDR[i]); +sysbus_connect_irq(SYS_BUS_DEVICE(>wdt[i]), 0, + qdev_get_gpio_in(DEVICE(>a7mpcore), +FSL_IMX7_WDOGn_IRQ[i])); } /* diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h index 47826da2b7..da977f9ffb 100644 --- a/include/hw/arm/fsl-imx7.h +++ b/include/hw/arm/fsl-imx7.h @@ -228,6 +228,11 @@ enum FslIMX7IRQs { FSL_IMX7_USB2_IRQ = 42, FSL_IMX7_USB3_IRQ = 40, +FSL_IMX7_WDOG1_IRQ= 78, +FSL_IMX7_WDOG2_IRQ= 79, +FSL_IMX7_WDOG3_IRQ= 10, +FSL_IMX7_WDOG4_IRQ= 109, + FSL_IMX7_PCI_INTA_IRQ = 125, FSL_IMX7_PCI_INTB_IRQ = 124, FSL_IMX7_PCI_INTC_IRQ = 123, -- 2.17.1
[PATCH v3 6/8] hw/arm/fsl-imx6ul: Connect watchdog interrupts
With this commit, the watchdog on mcimx6ul-evk is fully operational, including pretimeout support. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Guenter Roeck --- v3: Added Philippe's Reviewed-by: tag v2: No change hw/arm/fsl-imx6ul.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c index 56dfd7cecc..3ecb212da6 100644 --- a/hw/arm/fsl-imx6ul.c +++ b/hw/arm/fsl-imx6ul.c @@ -531,12 +531,22 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp) FSL_IMX6UL_WDOG2_ADDR, FSL_IMX6UL_WDOG3_ADDR, }; +static const int FSL_IMX6UL_WDOGn_IRQ[FSL_IMX6UL_NUM_WDTS] = { +FSL_IMX6UL_WDOG1_IRQ, +FSL_IMX6UL_WDOG2_IRQ, +FSL_IMX6UL_WDOG3_IRQ, +}; +object_property_set_bool(OBJECT(>wdt[i]), true, "pretimeout-support", + _abort); object_property_set_bool(OBJECT(>wdt[i]), true, "realized", _abort); sysbus_mmio_map(SYS_BUS_DEVICE(>wdt[i]), 0, FSL_IMX6UL_WDOGn_ADDR[i]); +sysbus_connect_irq(SYS_BUS_DEVICE(>wdt[i]), 0, + qdev_get_gpio_in(DEVICE(>a7mpcore), +FSL_IMX6UL_WDOGn_IRQ[i])); } /* -- 2.17.1
[PATCH v3 4/8] hw/arm/fsl-imx31: Wire up watchdog
With this patch, the watchdog on i.MX31 emulations is fully operational. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Guenter Roeck --- v3: No change v2: Select WDT_IMX2 explicitly Added Philippe's Reviewed-by: tag hw/arm/Kconfig | 1 + hw/arm/fsl-imx31.c | 6 ++ include/hw/arm/fsl-imx31.h | 4 3 files changed, 11 insertions(+) diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 54a49aeabd..9c77f4cbb4 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -366,6 +366,7 @@ config FSL_IMX31 select SERIAL select IMX select IMX_I2C +select WDT_IMX2 select LAN9118 config FSL_IMX6 diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c index 8472d2e911..1e7959863d 100644 --- a/hw/arm/fsl-imx31.c +++ b/hw/arm/fsl-imx31.c @@ -63,6 +63,8 @@ static void fsl_imx31_init(Object *obj) sysbus_init_child_obj(obj, "gpio[*]", >gpio[i], sizeof(s->gpio[i]), TYPE_IMX_GPIO); } + +sysbus_init_child_obj(obj, "wdt", >wdt, sizeof(s->wdt), TYPE_IMX2_WDT); } static void fsl_imx31_realize(DeviceState *dev, Error **errp) @@ -205,6 +207,10 @@ static void fsl_imx31_realize(DeviceState *dev, Error **errp) gpio_table[i].irq)); } +/* Watchdog */ +object_property_set_bool(OBJECT(>wdt), true, "realized", _abort); +sysbus_mmio_map(SYS_BUS_DEVICE(>wdt), 0, FSL_IMX31_WDT_ADDR); + /* On a real system, the first 16k is a `secure boot rom' */ memory_region_init_rom(>secure_rom, OBJECT(dev), "imx31.secure_rom", FSL_IMX31_SECURE_ROM_SIZE, ); diff --git a/include/hw/arm/fsl-imx31.h b/include/hw/arm/fsl-imx31.h index ac5ca9826a..dd8561b309 100644 --- a/include/hw/arm/fsl-imx31.h +++ b/include/hw/arm/fsl-imx31.h @@ -25,6 +25,7 @@ #include "hw/timer/imx_epit.h" #include "hw/i2c/imx_i2c.h" #include "hw/gpio/imx_gpio.h" +#include "hw/watchdog/wdt_imx2.h" #include "exec/memory.h" #include "target/arm/cpu.h" @@ -49,6 +50,7 @@ typedef struct FslIMX31State { IMXEPITState epit[FSL_IMX31_NUM_EPITS]; IMXI2CStatei2c[FSL_IMX31_NUM_I2CS]; IMXGPIOState gpio[FSL_IMX31_NUM_GPIOS]; +IMX2WdtState wdt; MemoryRegion secure_rom; MemoryRegion rom; MemoryRegion iram; @@ -87,6 +89,8 @@ typedef struct FslIMX31State { #define FSL_IMX31_GPIO1_SIZE0x4000 #define FSL_IMX31_GPIO2_ADDR0x53FD #define FSL_IMX31_GPIO2_SIZE0x4000 +#define FSL_IMX31_WDT_ADDR 0x53FDC000 +#define FSL_IMX31_WDT_SIZE 0x4000 #define FSL_IMX31_AVIC_ADDR 0x6800 #define FSL_IMX31_AVIC_SIZE 0x100 #define FSL_IMX31_SDRAM0_ADDR 0x8000 -- 2.17.1
[PATCH v3 5/8] hw/arm/fsl-imx6: Connect watchdog interrupts
With this patch applied, the watchdog in the sabrelite emulation is fully operational, including pretimeout support. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Guenter Roeck --- v3: Added Philippe's Reviewed-by: tag v2: No change hw/arm/fsl-imx6.c | 9 + 1 file changed, 9 insertions(+) diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c index 13f1bf23a6..f58c85aa8c 100644 --- a/hw/arm/fsl-imx6.c +++ b/hw/arm/fsl-imx6.c @@ -433,11 +433,20 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp) FSL_IMX6_WDOG1_ADDR, FSL_IMX6_WDOG2_ADDR, }; +static const int FSL_IMX6_WDOGn_IRQ[FSL_IMX6_NUM_WDTS] = { +FSL_IMX6_WDOG1_IRQ, +FSL_IMX6_WDOG2_IRQ, +}; +object_property_set_bool(OBJECT(>wdt[i]), true, "pretimeout-support", + _abort); object_property_set_bool(OBJECT(>wdt[i]), true, "realized", _abort); sysbus_mmio_map(SYS_BUS_DEVICE(>wdt[i]), 0, FSL_IMX6_WDOGn_ADDR[i]); +sysbus_connect_irq(SYS_BUS_DEVICE(>wdt[i]), 0, + qdev_get_gpio_in(DEVICE(>a9mpcore), +FSL_IMX6_WDOGn_IRQ[i])); } /* ROM memory */ -- 2.17.1
[PATCH v3 2/8] hw/watchdog: Implement full i.MX watchdog support
Implement full support for the watchdog in i.MX systems. Pretimeout support is optional because the watchdog hardware on i.MX31 does not support pretimeouts. Signed-off-by: Guenter Roeck --- v3: Improve handling of write-once registers and bits Stop timers in reset function Use explicit policy for timers v2: Fixup of CONFIG_WDT_IMX -> CONFIG_WDT_IMX2 moved to patch 1/8 hw/watchdog/wdt_imx2.c | 237 +++-- include/hw/watchdog/wdt_imx2.h | 61 - 2 files changed, 284 insertions(+), 14 deletions(-) diff --git a/hw/watchdog/wdt_imx2.c b/hw/watchdog/wdt_imx2.c index ad1ef02e9e..855b51768a 100644 --- a/hw/watchdog/wdt_imx2.c +++ b/hw/watchdog/wdt_imx2.c @@ -13,24 +13,189 @@ #include "qemu/bitops.h" #include "qemu/module.h" #include "sysemu/watchdog.h" +#include "migration/vmstate.h" +#include "hw/qdev-properties.h" #include "hw/watchdog/wdt_imx2.h" -#define IMX2_WDT_WCR_WDABIT(5) /* -> External Reset WDOG_B */ -#define IMX2_WDT_WCR_SRSBIT(4) /* -> Software Reset Signal */ +static void imx2_wdt_interrupt(void *opaque) +{ +IMX2WdtState *s = IMX2_WDT(opaque); + +s->wicr |= IMX2_WDT_WICR_WTIS; +qemu_set_irq(s->irq, 1); +} + +static void imx2_wdt_expired(void *opaque) +{ +IMX2WdtState *s = IMX2_WDT(opaque); + +s->wrsr = IMX2_WDT_WRSR_TOUT; + +/* Perform watchdog action if watchdog is enabled */ +if (s->wcr & IMX2_WDT_WCR_WDE) { +s->wrsr = IMX2_WDT_WRSR_TOUT; +watchdog_perform_action(); +} +} + +static void imx2_wdt_reset(DeviceState *dev) +{ +IMX2WdtState *s = IMX2_WDT(dev); + +ptimer_transaction_begin(s->timer); +ptimer_stop(s->timer); +ptimer_transaction_commit(s->timer); + +if (s->pretimeout_support) { +ptimer_transaction_begin(s->itimer); +ptimer_stop(s->itimer); +ptimer_transaction_commit(s->itimer); +} + +s->wicr_locked = false; +s->wcr_locked = false; +s->wcr_wde_locked = false; + +s->wcr = IMX2_WDT_WCR_WDA | IMX2_WDT_WCR_SRS; +s->wsr = 0; +s->wrsr &= ~(IMX2_WDT_WRSR_TOUT | IMX2_WDT_WRSR_SFTW); +s->wicr = IMX2_WDT_WICR_WICT_DEF; +s->wmcr = IMX2_WDT_WMCR_PDE; +} -static uint64_t imx2_wdt_read(void *opaque, hwaddr addr, - unsigned int size) +static uint64_t imx2_wdt_read(void *opaque, hwaddr addr, unsigned int size) { +IMX2WdtState *s = IMX2_WDT(opaque); + +switch (addr) { +case IMX2_WDT_WCR: +return s->wcr; +case IMX2_WDT_WSR: +return s->wsr; +case IMX2_WDT_WRSR: +return s->wrsr; +case IMX2_WDT_WICR: +return s->wicr; +case IMX2_WDT_WMCR: +return s->wmcr; +} return 0; } +static void imx_wdt2_update_itimer(IMX2WdtState *s, bool start) +{ +bool running = (s->wcr & IMX2_WDT_WCR_WDE) && (s->wcr & IMX2_WDT_WCR_WT); +bool enabled = s->wicr & IMX2_WDT_WICR_WIE; + +ptimer_transaction_begin(s->itimer); +if (start || !enabled) { +ptimer_stop(s->itimer); +} +if (running && enabled) { +int count = ptimer_get_count(s->timer); +int pretimeout = s->wicr & IMX2_WDT_WICR_WICT; + +/* + * Only (re-)start pretimeout timer if its counter value is larger + * than 0. Otherwise it will fire right away and we'll get an + * interrupt loop. + */ +if (count > pretimeout) { +ptimer_set_count(s->itimer, count - pretimeout); +if (start) { +ptimer_run(s->itimer, 1); +} +} +} +ptimer_transaction_commit(s->itimer); +} + +static void imx_wdt2_update_timer(IMX2WdtState *s, bool start) +{ +ptimer_transaction_begin(s->timer); +if (start) { +ptimer_stop(s->timer); +} +if ((s->wcr & IMX2_WDT_WCR_WDE) && (s->wcr & IMX2_WDT_WCR_WT)) { +int count = (s->wcr & IMX2_WDT_WCR_WT) >> 8; + +/* A value of 0 reflects one period (0.5s). */ +ptimer_set_count(s->timer, count + 1); +if (start) { +ptimer_run(s->timer, 1); +} +} +ptimer_transaction_commit(s->timer); +if (s->pretimeout_support) { +imx_wdt2_update_itimer(s, start); +} +} + static void imx2_wdt_write(void *opaque, hwaddr addr, uint64_t value, unsigned int size) { -if (addr == IMX2_WDT_WCR && -(~value & (IMX2_WDT_WCR_WDA | IMX2_WDT_WCR_SRS))) { -watchdog_perform_action(); +IMX2WdtState *s = IMX2_WDT(opaque); + +switch (addr) { +case IMX2_WDT_WCR: +if (s->wcr_locked) { +value &= ~IMX2_WDT_WCR_LOCK_MASK; +value |= (s->wicr & IMX2_WDT_WCR_LOCK_MASK); +} +s->wcr_locked = true; +if (s->wcr_wde_locked) { +value &= ~IMX2_WDT_WCR_WDE; +value |= (s->wicr & ~IMX2_WDT_WCR_WDE); +} else if (value & IMX2_WDT_WCR_WDE) { +s->wcr_wde_locked = true; +} +if
[PATCH v3 7/8] hw/arm/fsl-imx7: Instantiate various unimplemented devices
Instantiating PWM, CAN, CAAM, and OCOTP devices is necessary to avoid crashes when booting mainline Linux. Reviewed-by: Peter Maydell Signed-off-by: Guenter Roeck --- v3: Added Peter's Reviewed-by: tag v2: "octop" -> "ocotp" hw/arm/fsl-imx7.c | 24 include/hw/arm/fsl-imx7.h | 16 2 files changed, 40 insertions(+) diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c index 119b281a50..d6cf7c48ce 100644 --- a/hw/arm/fsl-imx7.c +++ b/hw/arm/fsl-imx7.c @@ -459,6 +459,30 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp) */ create_unimplemented_device("sdma", FSL_IMX7_SDMA_ADDR, FSL_IMX7_SDMA_SIZE); +/* + * CAAM + */ +create_unimplemented_device("caam", FSL_IMX7_CAAM_ADDR, FSL_IMX7_CAAM_SIZE); + +/* + * PWM + */ +create_unimplemented_device("pwm1", FSL_IMX7_PWM1_ADDR, FSL_IMX7_PWMn_SIZE); +create_unimplemented_device("pwm2", FSL_IMX7_PWM2_ADDR, FSL_IMX7_PWMn_SIZE); +create_unimplemented_device("pwm3", FSL_IMX7_PWM3_ADDR, FSL_IMX7_PWMn_SIZE); +create_unimplemented_device("pwm4", FSL_IMX7_PWM4_ADDR, FSL_IMX7_PWMn_SIZE); + +/* + * CAN + */ +create_unimplemented_device("can1", FSL_IMX7_CAN1_ADDR, FSL_IMX7_CANn_SIZE); +create_unimplemented_device("can2", FSL_IMX7_CAN2_ADDR, FSL_IMX7_CANn_SIZE); + +/* + * OCOTP + */ +create_unimplemented_device("ocotp", FSL_IMX7_OCOTP_ADDR, +FSL_IMX7_OCOTP_SIZE); object_property_set_bool(OBJECT(>gpr), true, "realized", _abort); diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h index 3a0041c4c2..47826da2b7 100644 --- a/include/hw/arm/fsl-imx7.h +++ b/include/hw/arm/fsl-imx7.h @@ -113,6 +113,9 @@ enum FslIMX7MemoryMap { FSL_IMX7_IOMUXC_GPR_ADDR = 0x3034, FSL_IMX7_IOMUXCn_SIZE = 0x1000, +FSL_IMX7_OCOTP_ADDR = 0x3035, +FSL_IMX7_OCOTP_SIZE = 0x1, + FSL_IMX7_ANALOG_ADDR = 0x3036, FSL_IMX7_SNVS_ADDR= 0x3037, FSL_IMX7_CCM_ADDR = 0x3038, @@ -124,11 +127,24 @@ enum FslIMX7MemoryMap { FSL_IMX7_ADC2_ADDR= 0x3062, FSL_IMX7_ADCn_SIZE= 0x1000, +FSL_IMX7_PWM1_ADDR= 0x3066, +FSL_IMX7_PWM2_ADDR= 0x3067, +FSL_IMX7_PWM3_ADDR= 0x3068, +FSL_IMX7_PWM4_ADDR= 0x3069, +FSL_IMX7_PWMn_SIZE= 0x1, + FSL_IMX7_PCIE_PHY_ADDR= 0x306D, FSL_IMX7_PCIE_PHY_SIZE= 0x1, FSL_IMX7_GPC_ADDR = 0x303A, +FSL_IMX7_CAAM_ADDR= 0x3090, +FSL_IMX7_CAAM_SIZE= 0x4, + +FSL_IMX7_CAN1_ADDR= 0x30A0, +FSL_IMX7_CAN2_ADDR= 0x30A1, +FSL_IMX7_CANn_SIZE= 0x1, + FSL_IMX7_I2C1_ADDR= 0x30A2, FSL_IMX7_I2C2_ADDR= 0x30A3, FSL_IMX7_I2C3_ADDR= 0x30A4, -- 2.17.1
[PATCH v3 3/8] hw/arm/fsl-imx25: Wire up watchdog
With this commit, the watchdog on imx25-pdk is fully operational, including pretimeout support. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Guenter Roeck --- v3: No change v2: Select WDT_IMX2 explicitly Added Philippe's Reviewed-by: tag hw/arm/Kconfig | 1 + hw/arm/fsl-imx25.c | 10 ++ include/hw/arm/fsl-imx25.h | 5 + 3 files changed, 16 insertions(+) diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index adf401e827..54a49aeabd 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -358,6 +358,7 @@ config FSL_IMX25 select IMX select IMX_FEC select IMX_I2C +select WDT_IMX2 select DS1338 config FSL_IMX31 diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c index 6f1a82ce3d..cdaa79c26b 100644 --- a/hw/arm/fsl-imx25.c +++ b/hw/arm/fsl-imx25.c @@ -87,6 +87,7 @@ static void fsl_imx25_init(Object *obj) TYPE_CHIPIDEA); } +sysbus_init_child_obj(obj, "wdt", >wdt, sizeof(s->wdt), TYPE_IMX2_WDT); } static void fsl_imx25_realize(DeviceState *dev, Error **errp) @@ -302,6 +303,15 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp) usb_table[i].irq)); } +/* Watchdog */ +object_property_set_bool(OBJECT(>wdt), true, "pretimeout-support", + _abort); +object_property_set_bool(OBJECT(>wdt), true, "realized", _abort); +sysbus_mmio_map(SYS_BUS_DEVICE(>wdt), 0, FSL_IMX25_WDT_ADDR); +sysbus_connect_irq(SYS_BUS_DEVICE(>wdt), 0, + qdev_get_gpio_in(DEVICE(>avic), + FSL_IMX25_WDT_IRQ)); + /* initialize 2 x 16 KB ROM */ memory_region_init_rom(>rom[0], OBJECT(dev), "imx25.rom0", FSL_IMX25_ROM0_SIZE, ); diff --git a/include/hw/arm/fsl-imx25.h b/include/hw/arm/fsl-imx25.h index 5e196bbf05..9e228dacea 100644 --- a/include/hw/arm/fsl-imx25.h +++ b/include/hw/arm/fsl-imx25.h @@ -29,6 +29,7 @@ #include "hw/gpio/imx_gpio.h" #include "hw/sd/sdhci.h" #include "hw/usb/chipidea.h" +#include "hw/watchdog/wdt_imx2.h" #include "exec/memory.h" #include "target/arm/cpu.h" @@ -60,6 +61,7 @@ typedef struct FslIMX25State { IMXGPIOState gpio[FSL_IMX25_NUM_GPIOS]; SDHCIState esdhc[FSL_IMX25_NUM_ESDHCS]; ChipideaState usb[FSL_IMX25_NUM_USBS]; +IMX2WdtState wdt; MemoryRegion rom[2]; MemoryRegion iram; MemoryRegion iram_alias; @@ -229,6 +231,8 @@ typedef struct FslIMX25State { #define FSL_IMX25_GPIO1_SIZE0x4000 #define FSL_IMX25_GPIO2_ADDR0x53FD #define FSL_IMX25_GPIO2_SIZE0x4000 +#define FSL_IMX25_WDT_ADDR 0x53FDC000 +#define FSL_IMX25_WDT_SIZE 0x4000 #define FSL_IMX25_USB1_ADDR 0x53FF4000 #define FSL_IMX25_USB1_SIZE 0x0200 #define FSL_IMX25_USB2_ADDR 0x53FF4400 @@ -268,5 +272,6 @@ typedef struct FslIMX25State { #define FSL_IMX25_ESDHC2_IRQ8 #define FSL_IMX25_USB1_IRQ 37 #define FSL_IMX25_USB2_IRQ 35 +#define FSL_IMX25_WDT_IRQ 55 #endif /* FSL_IMX25_H */ -- 2.17.1
[PATCH v3 1/8] hw: Move i.MX watchdog driver to hw/watchdog
In preparation for a full implementation, move i.MX watchdog driver from hw/misc to hw/watchdog. While at it, add the watchdog files to MAINTAINERS. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Guenter Roeck --- v3: No change v2: Instead of auto-selecting WDT_IMX2 if IMX is enabled, select it explicitly for each emulation using it. In Makefile, fix CONFIG_WDT_IMX -> CONFIG_WDT_IMX2 Added Philippe's Reviewed-by: tag MAINTAINERS | 2 ++ hw/arm/Kconfig | 3 +++ hw/misc/Makefile.objs | 1 - hw/watchdog/Kconfig | 3 +++ hw/watchdog/Makefile.objs | 1 + hw/{misc/imx2_wdt.c => watchdog/wdt_imx2.c} | 2 +- include/hw/arm/fsl-imx6.h | 2 +- include/hw/arm/fsl-imx6ul.h | 2 +- include/hw/arm/fsl-imx7.h | 2 +- include/hw/{misc/imx2_wdt.h => watchdog/wdt_imx2.h} | 0 10 files changed, 13 insertions(+), 5 deletions(-) rename hw/{misc/imx2_wdt.c => watchdog/wdt_imx2.c} (98%) rename include/hw/{misc/imx2_wdt.h => watchdog/wdt_imx2.h} (100%) diff --git a/MAINTAINERS b/MAINTAINERS index 8aa8efaf1d..998eb5053c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -631,8 +631,10 @@ S: Odd Fixes F: hw/arm/fsl-imx25.c F: hw/arm/imx25_pdk.c F: hw/misc/imx25_ccm.c +F: hw/watchdog/wdt_imx2.c F: include/hw/arm/fsl-imx25.h F: include/hw/misc/imx25_ccm.h +F: include/hw/watchdog/wdt_imx2.h i.MX31 (kzm) M: Peter Chubb diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 188419dc1e..adf401e827 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -374,6 +374,7 @@ config FSL_IMX6 select IMX_FEC select IMX_I2C select IMX_USBPHY +select WDT_IMX2 select SDHCI config ASPEED_SOC @@ -411,6 +412,7 @@ config FSL_IMX7 select IMX select IMX_FEC select IMX_I2C +select WDT_IMX2 select PCI_EXPRESS_DESIGNWARE select SDHCI select UNIMP @@ -424,6 +426,7 @@ config FSL_IMX6UL select IMX select IMX_FEC select IMX_I2C +select WDT_IMX2 select SDHCI select UNIMP diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs index 68aae2eabb..b25181b711 100644 --- a/hw/misc/Makefile.objs +++ b/hw/misc/Makefile.objs @@ -44,7 +44,6 @@ common-obj-$(CONFIG_IMX) += imx6_ccm.o common-obj-$(CONFIG_IMX) += imx6ul_ccm.o obj-$(CONFIG_IMX) += imx6_src.o common-obj-$(CONFIG_IMX) += imx7_ccm.o -common-obj-$(CONFIG_IMX) += imx2_wdt.o common-obj-$(CONFIG_IMX) += imx7_snvs.o common-obj-$(CONFIG_IMX) += imx7_gpr.o common-obj-$(CONFIG_IMX) += imx_rngc.o diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig index 2118d897c9..293209b291 100644 --- a/hw/watchdog/Kconfig +++ b/hw/watchdog/Kconfig @@ -14,3 +14,6 @@ config WDT_IB700 config WDT_DIAG288 bool + +config WDT_IMX2 +bool diff --git a/hw/watchdog/Makefile.objs b/hw/watchdog/Makefile.objs index 3f536d1cad..631b711d86 100644 --- a/hw/watchdog/Makefile.objs +++ b/hw/watchdog/Makefile.objs @@ -4,3 +4,4 @@ common-obj-$(CONFIG_WDT_IB6300ESB) += wdt_i6300esb.o common-obj-$(CONFIG_WDT_IB700) += wdt_ib700.o common-obj-$(CONFIG_WDT_DIAG288) += wdt_diag288.o common-obj-$(CONFIG_ASPEED_SOC) += wdt_aspeed.o +common-obj-$(CONFIG_WDT_IMX2) += wdt_imx2.o diff --git a/hw/misc/imx2_wdt.c b/hw/watchdog/wdt_imx2.c similarity index 98% rename from hw/misc/imx2_wdt.c rename to hw/watchdog/wdt_imx2.c index 2aedfe803a..ad1ef02e9e 100644 --- a/hw/misc/imx2_wdt.c +++ b/hw/watchdog/wdt_imx2.c @@ -14,7 +14,7 @@ #include "qemu/module.h" #include "sysemu/watchdog.h" -#include "hw/misc/imx2_wdt.h" +#include "hw/watchdog/wdt_imx2.h" #define IMX2_WDT_WCR_WDABIT(5) /* -> External Reset WDOG_B */ #define IMX2_WDT_WCR_SRSBIT(4) /* -> Software Reset Signal */ diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h index 973bcb72f7..1ebd751324 100644 --- a/include/hw/arm/fsl-imx6.h +++ b/include/hw/arm/fsl-imx6.h @@ -21,7 +21,7 @@ #include "hw/cpu/a9mpcore.h" #include "hw/misc/imx6_ccm.h" #include "hw/misc/imx6_src.h" -#include "hw/misc/imx2_wdt.h" +#include "hw/watchdog/wdt_imx2.h" #include "hw/char/imx_serial.h" #include "hw/timer/imx_gpt.h" #include "hw/timer/imx_epit.h" diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h index 1a0bab8daa..37c89cc5f9 100644 --- a/include/hw/arm/fsl-imx6ul.h +++ b/include/hw/arm/fsl-imx6ul.h @@ -24,7 +24,7 @@ #include "hw/misc/imx7_snvs.h" #include "hw/misc/imx7_gpr.h" #include "hw/intc/imx_gpcv2.h" -#include "hw/misc/imx2_wdt.h" +#include "hw/watchdog/wdt_imx2.h" #include "hw/gpio/imx_gpio.h" #include "hw/char/imx_serial.h" #include "hw/timer/imx_gpt.h" diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h index 706aef2e7e..3a0041c4c2 100644 --- a/include/hw/arm/fsl-imx7.h +++ b/include/hw/arm/fsl-imx7.h @@ -26,7 +26,7 @@ #include "hw/misc/imx7_snvs.h" #include
[PATCH v3 0/8] hw/arm: Implement i.MX watchdog support
The current i.MX watchdog implementation only supports resets. This patch series implements the full watchdog, including optional pretimeout support. Notable changes: - The existing i.MX watchdog emulation (which only emulates syste resets) is moved from hw/misc to hw/watchdog and renamed to match the naming convention in hw/watchdog (patch 1/8). - Full watchdog support is implemented in patch 2/8. - The watchdog is wired up for i.MX25 and i.MX31 emulations (patch 3/8 and 4/8). - The watchdog interrupt (for pretimeout support) is connected for i.MX6, i.MX6UL, and i.MX7 emulations (patch 5/8, 6/8, and 8/8). - For i.MX7, various devices are wired up as unimplemented devices (patch 7/8). This was necessary to avoid crashes when booting recent Linux kernels. The code was tested with all available emulations. v3: Improve handling of write-once registers and bits Stop timers in reset function Use explicit policy for timers v2: Select WDT_IMX2 explicitly for supported platforms, not automatically with IMX Rebased to current master (as of 3/22) Fixed typo "octop" -> "ocotp" Added Reviewed-by: tags where given Guenter Roeck (8): hw: Move i.MX watchdog driver to hw/watchdog hw/watchdog: Implement full i.MX watchdog support hw/arm/fsl-imx25: Wire up watchdog hw/arm/fsl-imx31: Wire up watchdog hw/arm/fsl-imx6: Connect watchdog interrupts hw/arm/fsl-imx6ul: Connect watchdog interrupts hw/arm/fsl-imx7: Instantiate various unimplemented devices hw/arm/fsl-imx7: Connect watchdog interrupts MAINTAINERS| 2 + hw/arm/Kconfig | 5 + hw/arm/fsl-imx25.c | 10 ++ hw/arm/fsl-imx31.c | 6 + hw/arm/fsl-imx6.c | 9 ++ hw/arm/fsl-imx6ul.c| 10 ++ hw/arm/fsl-imx7.c | 35 ++ hw/misc/Makefile.objs | 1 - hw/misc/imx2_wdt.c | 90 -- hw/watchdog/Kconfig| 3 + hw/watchdog/Makefile.objs | 1 + hw/watchdog/wdt_imx2.c | 262 + include/hw/arm/fsl-imx25.h | 5 + include/hw/arm/fsl-imx31.h | 4 + include/hw/arm/fsl-imx6.h | 2 +- include/hw/arm/fsl-imx6ul.h| 2 +- include/hw/arm/fsl-imx7.h | 23 +++- include/hw/misc/imx2_wdt.h | 33 -- include/hw/watchdog/wdt_imx2.h | 78 19 files changed, 454 insertions(+), 127 deletions(-) delete mode 100644 hw/misc/imx2_wdt.c create mode 100644 hw/watchdog/wdt_imx2.c delete mode 100644 include/hw/misc/imx2_wdt.h create mode 100644 include/hw/watchdog/wdt_imx2.h
Re: [RFC PATCH 0/2] exec: Fix (too) short device accesses
On 5/17/20 3:51 PM, no-re...@patchew.org wrote: Patchew URL: https://patchew.org/QEMU/20200517113804.9063-1-f4...@amsat.org/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === TESTcheck-qtest-x86_64: tests/qtest/tpm-crb-swtpm-test TESTcheck-qtest-x86_64: tests/qtest/tpm-crb-test ** ERROR:/tmp/qemu-test/src/tests/qtest/tpm-crb-test.c:53:tpm_crb_test: assertion failed (caddr > TPM_CRB_ADDR_BASE): (-1 > 4275306496) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/tpm-crb-test.c:53:tpm_crb_test: assertion failed (caddr > TPM_CRB_ADDR_BASE): (-1 > 4275306496) make: *** [check-qtest-x86_64] Error 1 make: *** Waiting for unfinished jobs qemu-system-aarch64: -accel kvm: invalid accelerator kvm qemu-system-aarch64: falling back to tcg --- TESTcheck-qtest-aarch64: tests/qtest/test-hmp TESTcheck-qtest-aarch64: tests/qtest/qos-test ** ERROR:/tmp/qemu-test/src/tests/qtest/sdhci-test.c:42:check_capab_capareg: assertion failed (capab == expec_capab): (0x == 0x280737ec6481) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/sdhci-test.c:42:check_capab_capareg: assertion failed (capab == expec_capab): (0x == 0x280737ec6481) make: *** [check-qtest-aarch64] Error 1 In both cases we abuse of 64-bit access to do 2x 32-bit ones, and there is no check of MEMTX_ERROR. Actually since the memory transaction attributes are quite recent (2015), in most of the code there is no error check. Quick grep for ignored return value: hw/vfio/pci-quirks.c:1061: memory_region_dispatch_write(>pdev.msix_table_mmio, hw/vfio/pci-quirks.c:1093: memory_region_dispatch_read(>pdev.msix_table_mmio, offset, hw/virtio/virtio-pci.c:556:memory_region_dispatch_write(mr, addr, val, size_memop(len) | MO_LE, hw/virtio/virtio-pci.c:580:memory_region_dispatch_read(mr, addr, , size_memop(len) | MO_LE, address_space_stl*(..., MemTxResult *result) with result = NULL: hw/arm/aspeed.c:166:address_space_stl_notdirty(as, AST_SMP_MBOX_FIELD_GOSIGN, 0, hw/arm/boot.c:282:address_space_stl_notdirty(as, info->smp_bootreg_addr, hw/arm/boot.c:293:address_space_stl_notdirty(as, p, value, \ hw/arm/highbank.c:91: address_space_stl_notdirty(_space_memory, hw/arm/highbank.c:95: address_space_stl_notdirty(_space_memory, hw/arm/highbank.c:99: address_space_stl_notdirty(_space_memory, hw/i386/amd_iommu.c:162: address_space_stl_le(_space_memory, msg.address, msg.data, hw/pci/msi.c:340:address_space_stl_le(>bus_master_as, msg.address, msg.data, hw/s390x/css.c:1539:address_space_stl(_space_memory, sch->curr_status.mba, count, hw/sh4/r2d.c:330:address_space_stl(_space_memory, SH7750_BCR1, 1 << 3, target/i386/helper.c:1141:address_space_stl_notdirty(as, addr, val, attrs, NULL); target/i386/helper.c:1161:address_space_stl(as, addr, val, attrs, NULL); target/i386/misc_helper.c:82:address_space_stl(_space_io, port, data, target/xtensa/op_helper.c:214: address_space_stl(env->address_space_er, addr, data,
Re: [PATCH] ati-vga: Do not allow unaligned access via index register
On Sun, 17 May 2020, Philippe Mathieu-Daudé wrote: On 5/17/20 12:40 PM, Philippe Mathieu-Daudé wrote: On 5/16/20 5:33 PM, BALATON Zoltan wrote: On Sat, 16 May 2020, Alexander Bulekov wrote: On 200516 1513, BALATON Zoltan wrote: Finally, there is a tag documented for bug fixes: https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message If your patch addresses a bug in a public bug tracker, please add a line with "Buglink: " there, too. Buglink: https://bugs.launchpad.net/qemu/+bug/1878134 Does this reply add that tag already or do I need to submit a v2 with it (or the maintainer could add it when merging)? Now, looking at your device implementation, it seems 1/ The device isn't supposed to have 64-bit accesses So this might be a more generic fix to Alexander issue: -- >8 -- @@ -879,6 +879,7 @@ static void ati_mm_write(void *opaque, hwaddr addr, static const MemoryRegionOps ati_mm_ops = { .read = ati_mm_read, .write = ati_mm_write, + .valid.max_access_size = 4, .endianness = DEVICE_LITTLE_ENDIAN, }; --- I've tried that first but it does not work. The reason is that ati_mm_read is recursively called for indexed access via MM_DATA which causes the problem that happens when MM_INDEX is set to a non-aligned value. No 64 bit access, just 32 bit with offset of 2 bytes as can be seen from the stach trace I've attached to the bug. Fortunately indexed access is documented to only support aligned access by not allowing setting low bits of MM_INDEX so unless we find a client needing it my patch should do it. 2/ All the registers are 32-bit aligned So you can simplify the implementation by letting access_with_adjusted_size() handle the 8/16-bit accesses by using: @@ -879,6 +879,8 @@ static void ati_mm_write(void *opaque, hwaddr addr, static const MemoryRegionOps ati_mm_ops = { .read = ati_mm_read, .write = ati_mm_write, + .min.min_access_size = 4, I meant '.impl.min_access_size'. I think this would not work either because not all registers are the same, some only can be read all 32 bits, some also 16 or 8 bits and clients do access these with less than 32 bits and accessing parts of the reg may trigger actions so the current way is probably better and necessary to correctly support different valid and invalid unaligned accessses. Regards, BALATON Zoltan
Re: [RFC PATCH 0/2] exec: Fix (too) short device accesses
Patchew URL: https://patchew.org/QEMU/20200517113804.9063-1-f4...@amsat.org/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === PASS 1 fdc-test /x86_64/fdc/cmos PASS 2 fdc-test /x86_64/fdc/no_media_on_start PASS 3 fdc-test /x86_64/fdc/read_without_media ==6160==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 4 fdc-test /x86_64/fdc/media_change PASS 5 fdc-test /x86_64/fdc/sense_interrupt PASS 6 fdc-test /x86_64/fdc/relative_seek --- PASS 32 test-opts-visitor /visitor/opts/range/beyond PASS 33 test-opts-visitor /visitor/opts/dict/unvisited MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-coroutine" ==6208==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==6208==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe0bfae000; bottom 0x7f19bb52; size: 0x00e450a8e000 (980605788160) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 1 test-coroutine /basic/no-dangling-access --- PASS 12 test-aio /aio/event/flush PASS 13 test-aio /aio/event/wait/no-flush-cb PASS 14 test-aio /aio/timer/schedule ==6223==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 15 test-aio /aio/coroutine/queue-chaining PASS 16 test-aio /aio-gsource/flush PASS 17 test-aio /aio-gsource/bh/schedule --- PASS 12 fdc-test /x86_64/fdc/read_no_dma_19 PASS 13 fdc-test /x86_64/fdc/fuzz-registers MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ide-test" ==6231==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 ide-test /x86_64/ide/identify PASS 28 test-aio /aio-gsource/timer/schedule MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-aio-multithread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-aio-multithread" ==6237==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==6243==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-aio-multithread /aio/multi/lifecycle PASS 2 ide-test /x86_64/ide/flush ==6257==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 3 ide-test /x86_64/ide/bmdma/simple_rw PASS 2 test-aio-multithread /aio/multi/schedule ==6263==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 4 ide-test /x86_64/ide/bmdma/trim PASS 3 test-aio-multithread /aio/multi/mutex/contended ==6274==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 4 test-aio-multithread /aio/multi/mutex/handoff PASS 5 test-aio-multithread /aio/multi/mutex/mcs PASS 6 test-aio-multithread /aio/multi/mutex/pthread MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-throttle" ==6296==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-throttle /throttle/leak_bucket PASS 2 test-throttle /throttle/compute_wait PASS 3 test-throttle /throttle/init --- PASS 14 test-throttle /throttle/config/max PASS 15 test-throttle /throttle/config/iops_size MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-thread-pool" ==6300==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-thread-pool /thread-pool/submit PASS 2 test-thread-pool /thread-pool/submit-aio PASS 3 test-thread-pool /thread-pool/submit-co PASS 4 test-thread-pool /thread-pool/submit-many ==6302==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 5 test-thread-pool /thread-pool/cancel PASS 6 test-thread-pool /thread-pool/cancel-async MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 +
Re: [RFC PATCH 0/2] exec: Fix (too) short device accesses
Patchew URL: https://patchew.org/QEMU/20200517113804.9063-1-f4...@amsat.org/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === TESTcheck-qtest-x86_64: tests/qtest/tpm-crb-swtpm-test TESTcheck-qtest-x86_64: tests/qtest/tpm-crb-test ** ERROR:/tmp/qemu-test/src/tests/qtest/tpm-crb-test.c:53:tpm_crb_test: assertion failed (caddr > TPM_CRB_ADDR_BASE): (-1 > 4275306496) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/tpm-crb-test.c:53:tpm_crb_test: assertion failed (caddr > TPM_CRB_ADDR_BASE): (-1 > 4275306496) make: *** [check-qtest-x86_64] Error 1 make: *** Waiting for unfinished jobs qemu-system-aarch64: -accel kvm: invalid accelerator kvm qemu-system-aarch64: falling back to tcg --- TESTcheck-qtest-aarch64: tests/qtest/test-hmp TESTcheck-qtest-aarch64: tests/qtest/qos-test ** ERROR:/tmp/qemu-test/src/tests/qtest/sdhci-test.c:42:check_capab_capareg: assertion failed (capab == expec_capab): (0x == 0x280737ec6481) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/sdhci-test.c:42:check_capab_capareg: assertion failed (capab == expec_capab): (0x == 0x280737ec6481) make: *** [check-qtest-aarch64] Error 1 Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in sys.exit(main()) --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=8698b64c360548299a7f28563cb6de79', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-363f6c9j/src/docker-src.2020-05-17-09.37.01.12018:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=8698b64c360548299a7f28563cb6de79 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-363f6c9j/src' make: *** [docker-run-test-quick@centos7] Error 2 real14m51.866s user0m8.150s The full log is available at http://patchew.org/logs/20200517113804.9063-1-f4...@amsat.org/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v4 19/19] hw/mips: Rename malta/mipssim/r4k/jazz files in hw/mips
Hi Aleksandar, On 5/17/20 11:23 AM, Aleksandar Markovic wrote: Machine file names should not have prefix "mips_". Folong2 machine source file will be handled in a separate patch, Typo: "Fuloong2e" to avoid conflicts. That patch is pending integration into the main tree. Signed-off-by: Aleksandar Markovic CC: Philippe Mathieu-Daudé --- hw/mips/Makefile.objs | 8 hw/mips/{mips_jazz.c => jazz.c} | 0 hw/mips/{mips_malta.c => malta.c} | 0 hw/mips/{mips_mipssim.c => mipssim.c} | 0 hw/mips/{mips_r4k.c => r4k.c} | 0 5 files changed, 4 insertions(+), 4 deletions(-) rename hw/mips/{mips_jazz.c => jazz.c} (100%) rename hw/mips/{mips_malta.c => malta.c} (100%) rename hw/mips/{mips_mipssim.c => mipssim.c} (100%) rename hw/mips/{mips_r4k.c => r4k.c} (100%) Thanks for cleaning this, appreciated! You missed MAINTAINERS: -- >8 -- diff --git a/MAINTAINERS b/MAINTAINERS index 1f84e3ae2c..3ad904a73c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1045,3 +1045,3 @@ R: Aleksandar Rikalo S: Maintained -F: hw/mips/mips_jazz.c +F: hw/mips/jazz.c F: hw/display/jazz_led.c @@ -1056,3 +1056,3 @@ F: hw/isa/piix4.c F: hw/acpi/piix4.c -F: hw/mips/mips_malta.c +F: hw/mips/malta.c F: hw/mips/gt64xxx_pci.c @@ -1066,3 +1066,3 @@ R: Aleksandar Rikalo S: Odd Fixes -F: hw/mips/mips_mipssim.c +F: hw/mips/mipssim.c F: hw/net/mipsnet.c @@ -1074,3 +1074,3 @@ R: Aleksandar Rikalo S: Obsolete -F: hw/mips/mips_r4k.c +F: hw/mips/r4k.c --- With this snippet amended: Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Regards, Phil. diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs index 525809af07..1d767ed9a8 100644 --- a/hw/mips/Makefile.objs +++ b/hw/mips/Makefile.objs @@ -1,8 +1,8 @@ obj-y += addr.o mips_int.o -obj-$(CONFIG_R4K) += mips_r4k.o -obj-$(CONFIG_MALTA) += gt64xxx_pci.o mips_malta.o -obj-$(CONFIG_MIPSSIM) += mips_mipssim.o -obj-$(CONFIG_JAZZ) += mips_jazz.o +obj-$(CONFIG_R4K) += r4k.o +obj-$(CONFIG_MALTA) += gt64xxx_pci.o malta.o +obj-$(CONFIG_MIPSSIM) += mipssim.o +obj-$(CONFIG_JAZZ) += jazz.o obj-$(CONFIG_FULONG) += mips_fulong2e.o obj-$(CONFIG_MIPS_CPS) += cps.o obj-$(CONFIG_MIPS_BOSTON) += boston.o diff --git a/hw/mips/mips_jazz.c b/hw/mips/jazz.c similarity index 100% rename from hw/mips/mips_jazz.c rename to hw/mips/jazz.c diff --git a/hw/mips/mips_malta.c b/hw/mips/malta.c similarity index 100% rename from hw/mips/mips_malta.c rename to hw/mips/malta.c diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mipssim.c similarity index 100% rename from hw/mips/mips_mipssim.c rename to hw/mips/mipssim.c diff --git a/hw/mips/mips_r4k.c b/hw/mips/r4k.c similarity index 100% rename from hw/mips/mips_r4k.c rename to hw/mips/r4k.c
Re: [PATCH v4 18/19] MAINTAINERS: Change Aleksandar Rikalo's email address
On 5/17/20 11:23 AM, Aleksandar Markovic wrote: Aleksandar Rikalo wants to use a different email address from now on. Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Signed-off-by: Aleksandar Markovic --- .mailmap| 3 ++- MAINTAINERS | 12 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.mailmap b/.mailmap index 6412067bde..e3628c7a66 100644 --- a/.mailmap +++ b/.mailmap @@ -42,7 +42,8 @@ Justin Terry (VM) Justin Terry (VM) via Qemu-devel Aleksandar Markovic Aleksandar Markovic -Aleksandar Rikalo +Aleksandar Rikalo +Aleksandar Rikalo Anthony Liguori Anthony Liguori James Hogan Leif Lindholm diff --git a/MAINTAINERS b/MAINTAINERS index 1f84e3ae2c..8d5562c5c7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -212,7 +212,7 @@ F: disas/microblaze.c MIPS TCG CPUs M: Aleksandar Markovic R: Aurelien Jarno -R: Aleksandar Rikalo +R: Aleksandar Rikalo S: Maintained F: target/mips/ F: default-configs/*mips* @@ -1041,7 +1041,7 @@ MIPS Machines - Jazz M: Hervé Poussineau -R: Aleksandar Rikalo +R: Aleksandar Rikalo S: Maintained F: hw/mips/mips_jazz.c F: hw/display/jazz_led.c @@ -1062,7 +1062,7 @@ F: tests/acceptance/machine_mips_malta.py Mipssim M: Aleksandar Markovic -R: Aleksandar Rikalo +R: Aleksandar Rikalo S: Odd Fixes F: hw/mips/mips_mipssim.c F: hw/net/mipsnet.c @@ -1070,7 +1070,7 @@ F: hw/net/mipsnet.c R4000 M: Aleksandar Markovic R: Aurelien Jarno -R: Aleksandar Rikalo +R: Aleksandar Rikalo S: Obsolete F: hw/mips/mips_r4k.c @@ -1085,7 +1085,7 @@ F: include/hw/isa/vt82c686.h Boston M: Paul Burton -R: Aleksandar Rikalo +R: Aleksandar Rikalo S: Maintained F: hw/core/loader-fit.c F: hw/mips/boston.c @@ -2582,7 +2582,7 @@ F: disas/i386.c MIPS TCG target M: Aleksandar Markovic R: Aurelien Jarno -R: Aleksandar Rikalo +R: Aleksandar Rikalo S: Maintained F: tcg/mips/
Re: [PATCH] ati-vga: Do not allow unaligned access via index register
On 5/17/20 12:40 PM, Philippe Mathieu-Daudé wrote: On 5/16/20 5:33 PM, BALATON Zoltan wrote: On Sat, 16 May 2020, Alexander Bulekov wrote: On 200516 1513, BALATON Zoltan wrote: According to docs bits 1 and 0 of MM_INDEX are hard coded to 0 so unaligned access via this register should not be possible. This also fixes problems reported in bug #1878134. Signed-off-by: BALATON Zoltan --- Hi Zoltan, I applied this patch and confirmed that I cannot reproduce the crash in #1878134 Thanks! Acked-by: Alexander Bulekov Thanks, so that should be Tested-by I think but I don't care much about tags so whatever works for me. 'Acked-by' means as a Fuzzer maintainer, Alexander checked your patch and is happy that another maintainer (usually Gerd for hw/display/, as ati.c doesn't have particular maintainer) takes this patch. You are right, if Alexander tested your patch, he also should add: Tested-by: Alexander Bulekov If a developer review your patch and agree the logic matches the description and doesn't introduce new regressions, he might reply with a 'Reviewed-by' tag. Note than tags are not trophies for the patch author, but are helpful for distributions such Debian/Fedora/NetBSD/... when they backport particular patches fixing bugs, before new QEMU (stable) version is released. Also they are useful in history in case a developer/maintainer goes MIA, there is still others to contact. Finally, there is a tag documented for bug fixes: https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message If your patch addresses a bug in a public bug tracker, please add a line with "Buglink: " there, too. Buglink: https://bugs.launchpad.net/qemu/+bug/1878134 Now, looking at your device implementation, it seems 1/ The device isn't supposed to have 64-bit accesses So this might be a more generic fix to Alexander issue: -- >8 -- @@ -879,6 +879,7 @@ static void ati_mm_write(void *opaque, hwaddr addr, static const MemoryRegionOps ati_mm_ops = { .read = ati_mm_read, .write = ati_mm_write, + .valid.max_access_size = 4, .endianness = DEVICE_LITTLE_ENDIAN, }; --- 2/ All the registers are 32-bit aligned So you can simplify the implementation by letting access_with_adjusted_size() handle the 8/16-bit accesses by using: @@ -879,6 +879,8 @@ static void ati_mm_write(void *opaque, hwaddr addr, static const MemoryRegionOps ati_mm_ops = { .read = ati_mm_read, .write = ati_mm_write, + .min.min_access_size = 4, I meant '.impl.min_access_size'. .endianness = DEVICE_LITTLE_ENDIAN, }; Regards, Phil. Regards, BALATON Zoltan hw/display/ati.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/display/ati.c b/hw/display/ati.c index f4c4542751..2ee23173b2 100644 --- a/hw/display/ati.c +++ b/hw/display/ati.c @@ -531,7 +531,7 @@ static void ati_mm_write(void *opaque, hwaddr addr, } switch (addr) { case MM_INDEX: - s->regs.mm_index = data; + s->regs.mm_index = data & ~3; break; case MM_DATA ... MM_DATA + 3: /* indexed access to regs or memory */ -- 2.21.3
Re: [PATCH v2] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION
On 2020/5/12 上午2:43, Laurent Vivier wrote: >> >> + IOCTL_SPECIAL(DRM_IOCTL_VERSION, IOC_RW, do_ioctl_drm, >> +MK_PTR(MK_STRUCT(STRUCT_drm_version))) > > Add a blank line here. > OK, thanks. >> #ifdef TARGET_TIOCSTART >>IOCTL_IGNORE(TIOCSTART) >>IOCTL_IGNORE(TIOCSTOP) >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index 8d27d10807..2eb7c91ab4 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -112,6 +112,7 @@ >> #include >> #include >> #include >> +#include > > I think you should check in configure that this file is available on the > system. > OK, I'll check in configure in patch v3. >> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h >> index 152ec637cb..3c261cff0e 100644 >> --- a/linux-user/syscall_defs.h >> +++ b/linux-user/syscall_defs.h >> @@ -1167,6 +1167,9 @@ struct target_rtc_pll_info { >> #define TARGET_DM_TARGET_MSG TARGET_IOWRU(0xfd, 0x0e) >> #define TARGET_DM_DEV_SET_GEOMETRYTARGET_IOWRU(0xfd, 0x0f) >> >> +/* drm ioctls */ >> +#define TARGET_DRM_IOCTL_VERSION TARGET_IOWRU('d', 0x00) > > Why do you use the TARGET_IOWRU variant? > > Can't you use TARGET_IOWR('d', 0x00, struct target_drm_version)? > Because qemu will automatically set the size with the target structure size in syscall_init(). It'll be more easier. e.g. usb ioctls and device mapper ioctls also do like this. Thanks.
Re: Emulating Solaris 10 on SPARC64 sun4u
On Sun, May 17, 2020 at 9:57 AM wrote: > > I've written up a basic implementation of the SAB 82532 ESCC2 device > and have written a patch for OpenBIOS to add it to the device tree. I > still have the 16550A UART acting as ttya to avoid having to write an > OpenBIOS device driver. Great progress! Are you planning to contribute your escc2 to the upstream? > OpenBSD and Solaris both identify the device correctly and attach it. > > Interestingly, it looks like Solaris entered an interrupt handler to > deal with an event for the SAB 82532 and it probed a few status > registers. Given that I couldn't encourage Solaris to do anything > outside of attach the device for the 16550A, I was thinking this might > be a good sign. > > There really shouldn't be an interrupt though as the reset state of the > SAB 82532 is to have all interrupts masked. Solaris probes these > interrupt status registers while "configuring" the sunhme device. > > Attempting to configure interface hme0... > 140070@1589698603.268942:escc2_mem_read channel a addr 0x38 value 0x4 > 140070@1589698603.269011:escc2_irq_update value 0x0 > 140070@1589698603.269015:escc2_mem_read channel a addr 0x3a value 0x80 > 140070@1589698603.269017:escc2_irq_update value 0x0 > 140070@1589698603.269018:escc2_mem_read channel a addr 0x3b value 0x0 > 140070@1589698603.269076:escc2_mem_read channel a addr 0x38 value 0x0 > WARNING: Power off requested from power button or SC, powering down the > system! > 140070@1589698611.270845:escc2_mem_read channel a addr 0x38 value 0x0 > 140070@1589698619.271758:escc2_mem_read channel a addr 0x38 value 0x0 > > I've noticed that removing the sunhme code for sun4u.c in QEMU prevents > the spurious interrupt from happening for the serial device along with > prevent the unexpected power off request (the power off request was not > introduced by my code). I haven't investigated why the presence of > sunhme causes these events. > > I modified OpenBIOS to use ttyb for stdin/stdout and assigned that to > the 16550A so that ttya could be aliased to the SAB 82532. Outside of > the spurious interrupt handler being called, I'm getting the same > behaviour where Solaris identifies, attaches, and then ignores the > device. Doesn't look like my guess was on the mark. > > I'll be looking at getting an OpenSolaris-like kernel built with prom > debugging output for console/tty related files like > usr/src/uts/common/io/consconfig_dacf.c. The illumos kernel is probably > suitable for this because it's still maintained and appears to be > suffering from the same console problems. I don't have a SPARC64 > machine immediately available and I'm unfamiliar with the build system > behind these distributions, so it might be a while before this happens. > > I'm out of ideas. We can use the strategy I used 10 years back for sun4m: boot the proprietary firmware first to reduce the number of possible emulation bugs. The proprietary firmware is known to boot Solaris. Regards, Artyom
[RFC PATCH 2/2] exec: Do not let flatview_read/write_continue do (too) short accesses
Instead of accessing a device with an invalid short size, return MEMTX_ERROR to indicate the transaction failed (as the device won't accept the transaction anyway). Reported by libFuzzer. sdhci_sdma_transfer_multi_blocks() ends calling dma_memory_rw() with size < 4, while the DMA MMIO regions are restricted to 32-bit accesses: qemu-fuzz-arm: hw/dma/bcm2835_dma.c:153: uint64_t bcm2835_dma_read(BCM2835DMAState *, hwaddr, unsigned int, unsigned int): Assertion `size == 4' failed. ==27332== ERROR: libFuzzer: deadly signal #8 0x7f0ffa1f5565 in __GI___assert_fail (/lib64/libc.so.6+0x30565) #9 0x562fe3c9c83f in bcm2835_dma_read (qemu-fuzz-arm+0x1d2e83f) #10 0x562fe3c9f81b in bcm2835_dma15_read (qemu-fuzz-arm+0x1d3181b) #11 0x562fe307d265 in memory_region_read_accessor (qemu-fuzz-arm+0x110f265) #12 0x562fe304ecb3 in access_with_adjusted_size (qemu-fuzz-arm+0x10e0cb3) #13 0x562fe304cb37 in memory_region_dispatch_read1 (qemu-fuzz-arm+0x10deb37) #14 0x562fe304c553 in memory_region_dispatch_read (qemu-fuzz-arm+0x10de553) #15 0x562fe2e7fd1d in flatview_read_continue (qemu-fuzz-arm+0xf11d1d) #16 0x562fe2e8147d in flatview_read (qemu-fuzz-arm+0xf1347d) #17 0x562fe2e80fd4 in address_space_read_full (qemu-fuzz-arm+0xf12fd4) #18 0x562fe2e820fa in address_space_rw (qemu-fuzz-arm+0xf140fa) #19 0x562fe411e485 in dma_memory_rw_relaxed (qemu-fuzz-arm+0x21b0485) #20 0x562fe411deb5 in dma_memory_rw (qemu-fuzz-arm+0x21afeb5) #21 0x562fe411d837 in dma_memory_read (qemu-fuzz-arm+0x21af837) #22 0x562fe41190a6 in sdhci_sdma_transfer_multi_blocks (qemu-fuzz-arm+0x21ab0a6) #23 0x562fe41217c1 in sdhci_write (qemu-fuzz-arm+0x21b37c1) #24 0x562fe304f147 in memory_region_write_accessor (qemu-fuzz-arm+0x10e1147) #25 0x562fe304ecb3 in access_with_adjusted_size (qemu-fuzz-arm+0x10e0cb3) #26 0x562fe304d853 in memory_region_dispatch_write (qemu-fuzz-arm+0x10df853) #27 0x562fe2e91e0b in flatview_write_continue (qemu-fuzz-arm+0xf23e0b) #28 0x562fe2e81d02 in flatview_write (qemu-fuzz-arm+0xf13d02) #29 0x562fe2e81834 in address_space_write (qemu-fuzz-arm+0xf13834) qemu-fuzz-arm: hw/dma/bcm2835_dma.c:200: void bcm2835_dma_write(BCM2835DMAState *, hwaddr, uint64_t, unsigned int, unsigned int): Assertion `size == 4' failed. ==16113== ERROR: libFuzzer: deadly signal #8 0x7fd823d3d565 in __GI___assert_fail (/lib64/libc.so.6+0x30565) #9 0x557a62b72ec3 in bcm2835_dma_write (qemu-fuzz-arm+0x1d2eec3) #10 0x557a62b725e8 in bcm2835_dma0_write (qemu-fuzz-arm+0x1d2e5e8) #11 0x557a61f25147 in memory_region_write_accessor (qemu-fuzz-arm+0x10e1147) #12 0x557a61f24cb3 in access_with_adjusted_size (qemu-fuzz-arm+0x10e0cb3) #13 0x557a61f23853 in memory_region_dispatch_write (qemu-fuzz-arm+0x10df853) #14 0x557a61d67e0b in flatview_write_continue (qemu-fuzz-arm+0xf23e0b) #15 0x557a61d57d02 in flatview_write (qemu-fuzz-arm+0xf13d02) #16 0x557a61d57834 in address_space_write (qemu-fuzz-arm+0xf13834) #17 0x557a61d58054 in address_space_rw (qemu-fuzz-arm+0xf14054) #18 0x557a62ff4485 in dma_memory_rw_relaxed (qemu-fuzz-arm+0x21b0485) #19 0x557a62ff3eb5 in dma_memory_rw (qemu-fuzz-arm+0x21afeb5) #20 0x557a62ff379a in dma_memory_write (qemu-fuzz-arm+0x21af79a) #21 0x557a62fee9dc in sdhci_sdma_transfer_multi_blocks (qemu-fuzz-arm+0x21aa9dc) #22 0x557a62ff77c1 in sdhci_write (qemu-fuzz-arm+0x21b37c1) #23 0x557a61f25147 in memory_region_write_accessor (qemu-fuzz-arm+0x10e1147) #24 0x557a61f24cb3 in access_with_adjusted_size (qemu-fuzz-arm+0x10e0cb3) #25 0x557a61f23853 in memory_region_dispatch_write (qemu-fuzz-arm+0x10df853) #26 0x557a61d67e0b in flatview_write_continue (qemu-fuzz-arm+0xf23e0b) #27 0x557a61d57d02 in flatview_write (qemu-fuzz-arm+0xf13d02) #28 0x557a61d57834 in address_space_write (qemu-fuzz-arm+0xf13834) ==5448==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61924380 at pc 0x55aac095e2cc bp 0x7fff9144ead0 sp 0x7fff9144e280 WRITE of size 4 at 0x61924380 thread T0 #0 0x55aac095e2cb in __asan_memcpy (qemu-fuzz-arm+0xeb92cb) #1 0x55aac09de163 in stl_he_p (qemu-fuzz-arm+0xf39163) #2 0x55aac09b796f in stn_he_p (qemu-fuzz-arm+0xf1296f) #3 0x55aac09b6ec5 in flatview_read_continue (qemu-fuzz-arm+0xf11ec5) #4 0x55aac09b86dd in flatview_read (qemu-fuzz-arm+0xf136dd) #5 0x55aac09b8234 in address_space_read_full (qemu-fuzz-arm+0xf13234) #6 0x55aac09b935a in address_space_rw (qemu-fuzz-arm+0xf1435a) #7 0x55aac1c55b35 in dma_memory_rw_relaxed (qemu-fuzz-arm+0x21b0b35) #8 0x55aac1c55565 in dma_memory_rw (qemu-fuzz-arm+0x21b0565) #9 0x55aac1c54ee7 in dma_memory_read (qemu-fuzz-arm+0x21afee7) #10 0x55aac1c5074e in sdhci_sdma_transfer_multi_blocks (qemu-fuzz-arm+0x21ab74e) #11 0x55aac1c58e71 in sdhci_write (qemu-fuzz-arm+0x21b3e71) #12 0x55aac0b86417 in memory_region_write_accessor (qemu-fuzz-arm+0x10e1417) #13
[RFC PATCH 1/2] exec: Let memory_access_size() consider minimum valid access size
As it is illegal to access a device with less that its minimum valid size, also check for access_size_min. Signed-off-by: Philippe Mathieu-Daudé --- exec.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/exec.c b/exec.c index 5162f0d12f..d3ec30f995 100644 --- a/exec.c +++ b/exec.c @@ -3066,10 +3066,14 @@ void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size) static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) { +unsigned access_size_min = mr->ops->valid.min_access_size; unsigned access_size_max = mr->ops->valid.max_access_size; /* Regions are assumed to support 1-4 byte accesses unless otherwise specified. */ +if (access_size_min == 0) { +access_size_min = 1; +} if (access_size_max == 0) { access_size_max = 4; } @@ -3082,11 +3086,14 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) } } -/* Don't attempt accesses larger than the maximum. */ -if (l > access_size_max) { +/* Don't attempt accesses not in the minimum/maximum range. */ +if (l < access_size_min) { +l = access_size_min; +} else if (l > access_size_max) { l = access_size_max; +} else { +l = pow2floor(l); } -l = pow2floor(l); return l; } -- 2.21.3
[RFC PATCH 0/2] exec: Fix (too) short device accesses
Something noticed while debugging Alexander's bug report "Hang with high CPU usage in sdhci_data_transfer": https://bugs.launchpad.net/qemu/+bug/1878054 The flatview ignores the MemoryRegion minimum access size. It seems related to a similar issue Julia had with PCI devices. Not sure it is safe enough, have performance penalties and so on, so RFC. Philippe Mathieu-Daudé (2): exec: Let memory_access_size() consider minimum valid access size exec: Do not let flatview_read/write_continue do (too) short accesses exec.c | 42 +++--- 1 file changed, 31 insertions(+), 11 deletions(-) -- 2.21.3
sharing intention for developing per-target, dynamically loadable accelerator modules
Hello all, my intention would be to develop per-target, dynamically loadable accelerator modules. This would allow to distribute a single QEMU base binary, and then provide accelerators as optional additional binary packages to install, with the first separate optional package being TCG. CONFIG_TCG would become 'm' as a result, but then also CONFIG_KVM, CONFIG_HAX, CONFIG_WHPX, CONFIG_HVF. Here are some elements that seem to be needed: 1 - The module CONFIG_MODULE part of the build system would need some extension to add per-target modules. I have some tentative results that shows that this is possible (but a bit clunky atm). There is some existing instability in the existing Makefile infrastructure of modules that shows up when trying to extend it. 2 - new "accelerator drivers" seems to be needed, either in addition or as additional functionality inside the current AccelState. 3 - for target/i386 in particular, there is some refactoring work needed to split even more different unrelated bits and pieces. dependencies of hw/i386 machine stuff with accelerator-specific stuff are also painful. 4 - CPU Arch Classes could be extended with per-accelerator methods. Initial fooling around shows it should probably work. One alternative would be trying to play with the dynamic linker (weak symbols, creative use of dlsym etc), but I have not sorted out the details of this option. 5 - cputlb, in particular tlb_flush and friends is a separate problem since it is not part of the cpuclass. Should it be? 6 - a painpoint is represented by the fact that in USER mode, the accel class is not applied, which causes a lot of uncleanliness all around (tcg_allowed outside of the AccelClass). 7 - I have not really thought about the KConfig aspects because I am not super-familiar 8 - cpus.c needs some good splitting ... more things to find out and think about ... Overall, I think that the activity has the potential to provide benefits overall beyond the actual goal, in the form of cleanups, leaner configurations, minor fixes, maybe improving the CONFIG_MODULE instabilities if any etc. As an example, the first activity I would plan to submit as RFC is point 8 above, there is the split between cpus.c and cpus-tcg.c that results in lots of TCG-specific code being removed from non-tcg builds (using CONFIG_TCG). One thing that should be kept in check is any performance impact of the changes, in particular for point 4, hot paths should probably avoid going through too many pointer indirections. Does anybody share similar goals? Any major obstacle or blocker that would put the feasibility into question? Any suggestion on any of this? In particular point 4 and 5 come to mind, as well as some better understanding of the reasons behind 6, or even suggestions on how to best to 2. Anyway, I will continue to work on the first RFC for some smaller initial steps and hopefully have something to submit soon. Ciao ciao, Claudio -- Claudio Fontana Engineering Manager Virtualization, SUSE Labs Core SUSE Software Solutions Italy Srl
[Bug 1856335] Re: Cache Layout wrong on many Zen Arch CPUs
Damir: Hm, must be some misconfiguration, then. My config for Linux VMs to utilize 3 out of the 4 CCXs. Important parts of the libvirt domain XML: 24 1 hvm /usr/share/ovmf/x64/OVMF_CODE.fd /var/lib/libvirt/qemu/nvram/ccxtest-clone_VARS.fd . . . The CPUs with cpuset="0,12" are disabled once booted. The host-cache- info=on is the part that makes sure that the cache config is passed to the VM (but unfortunately does not take disabled cores into account, which results in incorrect config). The qemu:commandline is added because I need to add -amd-stibp, otherwise I wouldn't be able to boot. This overrides most parts in the XML part. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1856335 Title: Cache Layout wrong on many Zen Arch CPUs Status in QEMU: New Bug description: AMD CPUs have L3 cache per 2, 3 or 4 cores. Currently, TOPOEXT seems to always map Cache ass if it was an 4-Core per CCX CPU, which is incorrect, and costs upwards 30% performance (more realistically 10%) in L3 Cache Layout aware applications. Example on a 4-CCX CPU (1950X /w 8 Cores and no SMT): EPYC-IBPB AMD In windows, coreinfo reports correctly: Unified Cache 1, Level 3,8 MB, Assoc 16, LineSize 64 Unified Cache 6, Level 3,8 MB, Assoc 16, LineSize 64 On a 3-CCX CPU (3960X /w 6 cores and no SMT): EPYC-IBPB AMD in windows, coreinfo reports incorrectly: -- Unified Cache 1, Level 3,8 MB, Assoc 16, LineSize 64 ** Unified Cache 6, Level 3,8 MB, Assoc 16, LineSize 64 Validated against 3.0, 3.1, 4.1 and 4.2 versions of qemu-kvm. With newer Qemu there is a fix (that does behave correctly) in using the dies parameter: The problem is that the dies are exposed differently than how AMD does it natively, they are exposed to Windows as sockets, which means, that if you are nto a business user, you can't ever have a machine with more than two CCX (6 cores) as consumer versions of Windows only supports two sockets. (Should this be reported as a separate bug?) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1856335/+subscriptions
[Bug 1856335] Re: Cache Layout wrong on many Zen Arch CPUs
No, creating artificial NUMA nodes is, simply put, never a good solution for CPUs that operate as a single NUMA node - which is the case for all Zen2 CPUs (except maybe EPYCs? not sure about those). You may workaround the L3 issue that way, but hit many new bugs/problems by introducing multiple NUMA nodes, _especially_ on Windows VMs, because that OS has crappy NUMA handling and multitude of bugs related to it - which was one of the major reasons why even Zen2 Threadrippers are now single NUMA node (e.g. https://www.servethehome.com/wp- content/uploads/2019/11/AMD-Ryzen-Threadripper-3960X-Topology.png ). The host CPU architecture should be replicated as closely as possible on the VM and for Zen2 CPUs with 4 cores per CCX, _this already works perfectly_ - there are no problems on 3300X/3700(X)/3800X/3950X/3970X/3990X. There is, unfortunately, no way to customize/specify the "disabled" CPU cores in QEMU, and therefore no way to emulate 1 NUMA node + L3 cache per 2/3 cores - only to passthrough the cache config from host, which is unfortunately not done correctly for CPUs with disabled cores (but again, works perfectly for CPUs with all 4 cores enabled per CCX). lscpu: Architecture:x86_64 CPU op-mode(s): 32-bit, 64-bit Byte Order: Little Endian Address sizes: 43 bits physical, 48 bits virtual CPU(s): 24 On-line CPU(s) list: 0-23 Thread(s) per core: 2 Core(s) per socket: 12 Socket(s): 1 NUMA node(s):1 Vendor ID: AuthenticAMD CPU family: 23 Model: 113 Model name: AMD Ryzen 9 3900X 12-Core Processor Stepping:0 Frequency boost: enabled CPU MHz: 2972.127 CPU max MHz: 3800. CPU min MHz: 2200. BogoMIPS:7602.55 Virtualization: AMD-V L1d cache: 384 KiB L1i cache: 384 KiB L2 cache:6 MiB L3 cache:64 MiB NUMA node0 CPU(s): 0-23 Vulnerability Itlb multihit: Not affected Vulnerability L1tf: Not affected Vulnerability Mds: Not affected Vulnerability Meltdown: Not affected Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl and seccomp Vulnerability Spectre v1:Mitigation; usercopy/swapgs barriers and __user pointer sanitization Vulnerability Spectre v2:Mitigation; Full AMD retpoline, IBPB conditional, STIBP conditional, RSB filling Vulnerability Tsx async abort: Not affected Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt pdpe1gb rdtscp lm constant_tsc rep_good nopl nonsto p_tsc cpuid extd_apicid aperfmperf pni pclmulqdq monitor ssse3 fma cx16 sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand lahf_lm cmp_legacy svm extapic cr8_legacy abm sse4a mi salignsse 3dnowprefetch osvw ibs skinit wdt tce topoext perfctr_core perfctr_nb bpext perfctr_llc mwaitx cpb cat_l3 cdp_l3 hw_pstate sme ssbd mba sev ibpb stibp vmmcall fsgsbase b mi1 avx2 smep bmi2 cqm rdt_a rdseed adx smap clflushopt clwb sha_ni xsaveopt xsavec xgetbv1 xsaves cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local clzero irperf xsaveerptr rdpru wbnoinvd arat npt lbrv svm_lock nrip_save tsc_scale vmcb_clean flushbyasid decodeassists pausefilter pfthreshold avic v_vmsave_vmload vgif umip rdpid overflow_recov succor smca But the important thing has already been posted here in previous comments - notice the skipped core ids belonging to the disabled cores: virsh capabilities | grep "cpu id": -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1856335 Title: Cache Layout wrong on many Zen Arch CPUs Status in QEMU: New Bug description: AMD CPUs have L3 cache per 2, 3 or 4 cores. Currently, TOPOEXT seems to always map Cache ass if it was an 4-Core per CCX CPU, which is incorrect, and costs upwards 30% performance (more realistically 10%) in L3 Cache Layout aware applications. Example on a 4-CCX CPU (1950X /w 8 Cores and no SMT): EPYC-IBPB AMD In windows, coreinfo reports correctly: Unified Cache 1, Level 3,8 MB, Assoc 16, LineSize 64 Unified Cache 6, Level 3,8 MB, Assoc 16, LineSize 64 On a 3-CCX CPU (3960X /w 6 cores and no SMT): EPYC-IBPB AMD in windows, coreinfo reports
[PATCH] target/i386: Fix OUTL debug output
Fix OUTL instructions incorrectly displayed as OUTW. Signed-off-by: Philippe Mathieu-Daudé --- target/i386/misc_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/misc_helper.c b/target/i386/misc_helper.c index 7d61221024..b6b1d41b14 100644 --- a/target/i386/misc_helper.c +++ b/target/i386/misc_helper.c @@ -70,7 +70,7 @@ target_ulong helper_inw(CPUX86State *env, uint32_t port) void helper_outl(CPUX86State *env, uint32_t port, uint32_t data) { #ifdef CONFIG_USER_ONLY -fprintf(stderr, "outw: port=0x%04x, data=%08x\n", port, data); +fprintf(stderr, "outl: port=0x%04x, data=%08x\n", port, data); #else address_space_stl(_space_io, port, data, cpu_get_mem_attrs(env), NULL); -- 2.21.3
[Bug 1878915] Re: util/fdmon-io_uring.c:95: get_sqe: Assertion `ret > 1' failed.
** Description changed: qemu 5.0.0, liburing1 0.6-3, Linux 5.6.0-1-686-pae (Debian) Stack trace: - Stack trace of thread 31002: - #0 0xb7faf1cd __kernel_vsyscall (linux-gate.so.1 + 0x11cd) - #1 0xb6c618e2 __libc_signal_restore_set (libc.so.6 + 0x348e2) - #2 0xb6c4a309 __GI_abort (libc.so.6 + 0x1d309) - #3 0xb6c4a1d1 __assert_fail_base (libc.so.6 + 0x1d1d1) - #4 0xb6c59929 __GI___assert_fail (libc.so.6 + 0x2c929) - #5 0x00ba80be get_sqe (qemu-system-i386 + 0x6d00be) - #6 0x00ba80cb add_poll_add_sqe (qemu-system-i386 + 0x6d00cb) - #7 0x00ba820c fill_sq_ring (qemu-system-i386 + 0x6d020c) - #8 0x00ba7145 aio_poll (qemu-system-i386 + 0x6cf145) - #9 0x00aede63 blk_prw (qemu-system-i386 + 0x615e63) - #10 0x00aeef95 blk_pread (qemu-system-i386 + 0x616f95) - #11 0x008abbfa fdctrl_transfer_handler (qemu-system-i386 + 0x3d3bfa) - #12 0x00906c3d i8257_channel_run (qemu-system-i386 + 0x42ec3d) - #13 0x008ac119 fdctrl_start_transfer (qemu-system-i386 + 0x3d4119) - #14 0x008ab233 fdctrl_write_data (qemu-system-i386 + 0x3d3233) - #15 0x00708ae7 memory_region_write_accessor (qemu-system-i386 + 0x230ae7) - #16 0x007059e1 access_with_adjusted_size (qemu-system-i386 + 0x22d9e1) - #17 0x0070b931 memory_region_dispatch_write (qemu-system-i386 + 0x233931) - #18 0x006a87a2 address_space_stb (qemu-system-i386 + 0x1d07a2) - #19 0x00829216 helper_outb (qemu-system-i386 + 0x351216) - #20 0xb06d9fdc n/a (n/a + 0x0) + Stack trace of thread 31002: + #0 0xb7faf1cd __kernel_vsyscall (linux-gate.so.1 + 0x11cd) + #1 0xb6c618e2 __libc_signal_restore_set (libc.so.6 + 0x348e2) + #2 0xb6c4a309 __GI_abort (libc.so.6 + 0x1d309) + #3 0xb6c4a1d1 __assert_fail_base (libc.so.6 + 0x1d1d1) + #4 0xb6c59929 __GI___assert_fail (libc.so.6 + 0x2c929) + #5 0x00ba80be get_sqe (qemu-system-i386 + 0x6d00be) + #6 0x00ba80cb add_poll_add_sqe (qemu-system-i386 + 0x6d00cb) + #7 0x00ba820c fill_sq_ring (qemu-system-i386 + 0x6d020c) + #8 0x00ba7145 aio_poll (qemu-system-i386 + 0x6cf145) + #9 0x00aede63 blk_prw (qemu-system-i386 + 0x615e63) + #10 0x00aeef95 blk_pread (qemu-system-i386 + 0x616f95) + #11 0x008abbfa fdctrl_transfer_handler (qemu-system-i386 + 0x3d3bfa) + #12 0x00906c3d i8257_channel_run (qemu-system-i386 + 0x42ec3d) + #13 0x008ac119 fdctrl_start_transfer (qemu-system-i386 + 0x3d4119) + #14 0x008ab233 fdctrl_write_data (qemu-system-i386 + 0x3d3233) + #15 0x00708ae7 memory_region_write_accessor (qemu-system-i386 + 0x230ae7) + #16 0x007059e1 access_with_adjusted_size (qemu-system-i386 + 0x22d9e1) + #17 0x0070b931 memory_region_dispatch_write (qemu-system-i386 + 0x233931) + #18 0x006a87a2 address_space_stb (qemu-system-i386 + 0x1d07a2) + #19 0x00829216 helper_outb (qemu-system-i386 + 0x351216) + #20 0xb06d9fdc n/a (n/a + 0x0) Steps: 0. qemu-img create -f raw fda.img 3840K 1. mformat -i fda.img -n 48 -t 80 -h 2 2. qemu-system-i386 -fda fda.img -hda freedos.qcow2 3. Attempt to run 'dosfsck a:' in the guest According to hw/block/fdc.c, a 3840K image should result in a virtual floppy with a geometry of 48 sectors/track x 80 tracks x 2 sides. The assert seems bogus either way. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1878915 Title: util/fdmon-io_uring.c:95: get_sqe: Assertion `ret > 1' failed. Status in QEMU: New Bug description: qemu 5.0.0, liburing1 0.6-3, Linux 5.6.0-1-686-pae (Debian) Stack trace: Stack trace of thread 31002: #0 0xb7faf1cd __kernel_vsyscall (linux-gate.so.1 + 0x11cd) #1 0xb6c618e2 __libc_signal_restore_set (libc.so.6 + 0x348e2) #2 0xb6c4a309 __GI_abort (libc.so.6 + 0x1d309) #3 0xb6c4a1d1 __assert_fail_base (libc.so.6 + 0x1d1d1) #4 0xb6c59929 __GI___assert_fail (libc.so.6 + 0x2c929) #5 0x00ba80be get_sqe (qemu-system-i386 + 0x6d00be) #6 0x00ba80cb add_poll_add_sqe (qemu-system-i386 + 0x6d00cb) #7 0x00ba820c fill_sq_ring (qemu-system-i386 + 0x6d020c) #8 0x00ba7145 aio_poll (qemu-system-i386 + 0x6cf145) #9 0x00aede63 blk_prw (qemu-system-i386 + 0x615e63) #10 0x00aeef95 blk_pread (qemu-system-i386 + 0x616f95) #11 0x008abbfa fdctrl_transfer_handler
Re: [PATCH] ati-vga: Do not allow unaligned access via index register
On 5/16/20 5:33 PM, BALATON Zoltan wrote: On Sat, 16 May 2020, Alexander Bulekov wrote: On 200516 1513, BALATON Zoltan wrote: According to docs bits 1 and 0 of MM_INDEX are hard coded to 0 so unaligned access via this register should not be possible. This also fixes problems reported in bug #1878134. Signed-off-by: BALATON Zoltan --- Hi Zoltan, I applied this patch and confirmed that I cannot reproduce the crash in #1878134 Thanks! Acked-by: Alexander Bulekov Thanks, so that should be Tested-by I think but I don't care much about tags so whatever works for me. 'Acked-by' means as a Fuzzer maintainer, Alexander checked your patch and is happy that another maintainer (usually Gerd for hw/display/, as ati.c doesn't have particular maintainer) takes this patch. You are right, if Alexander tested your patch, he also should add: Tested-by: Alexander Bulekov If a developer review your patch and agree the logic matches the description and doesn't introduce new regressions, he might reply with a 'Reviewed-by' tag. Note than tags are not trophies for the patch author, but are helpful for distributions such Debian/Fedora/NetBSD/... when they backport particular patches fixing bugs, before new QEMU (stable) version is released. Also they are useful in history in case a developer/maintainer goes MIA, there is still others to contact. Finally, there is a tag documented for bug fixes: https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message If your patch addresses a bug in a public bug tracker, please add a line with "Buglink: " there, too. Buglink: https://bugs.launchpad.net/qemu/+bug/1878134 Now, looking at your device implementation, it seems 1/ The device isn't supposed to have 64-bit accesses So this might be a more generic fix to Alexander issue: -- >8 -- @@ -879,6 +879,7 @@ static void ati_mm_write(void *opaque, hwaddr addr, static const MemoryRegionOps ati_mm_ops = { .read = ati_mm_read, .write = ati_mm_write, +.valid.max_access_size = 4, .endianness = DEVICE_LITTLE_ENDIAN, }; --- 2/ All the registers are 32-bit aligned So you can simplify the implementation by letting access_with_adjusted_size() handle the 8/16-bit accesses by using: @@ -879,6 +879,8 @@ static void ati_mm_write(void *opaque, hwaddr addr, static const MemoryRegionOps ati_mm_ops = { .read = ati_mm_read, .write = ati_mm_write, +.min.min_access_size = 4, .endianness = DEVICE_LITTLE_ENDIAN, }; Regards, Phil. Regards, BALATON Zoltan hw/display/ati.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/display/ati.c b/hw/display/ati.c index f4c4542751..2ee23173b2 100644 --- a/hw/display/ati.c +++ b/hw/display/ati.c @@ -531,7 +531,7 @@ static void ati_mm_write(void *opaque, hwaddr addr, } switch (addr) { case MM_INDEX: - s->regs.mm_index = data; + s->regs.mm_index = data & ~3; break; case MM_DATA ... MM_DATA + 3: /* indexed access to regs or memory */ -- 2.21.3